Conversation
Holocraft
added
enhancement
Contributor
📝 WalkthroughWalkthroughMigrates the UI from ChangesReact Helmet Async Migration with App Name Context
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
cmd/ui/src/views/Explore/GraphView.tsx (1)
267-269: ⚡ Quick winUse
AppNameContextinstead of hardcoding the app name.The title hardcodes "BloodHound Community Edition" rather than consuming the app name from
AppNameContext. This creates inconsistency with thePageWithTitlecomponent pattern and makes the code less maintainable.Once
AppNameContextor auseAppNamehook is exported fromPageWithTitle.tsx(see previous comment), update this component to use the context dynamically.♻️ Proposed fix (after exporting useAppName hook)
+import { useAppName } from 'bh-shared-ui'; + const GraphView: FC = () => { /* Hooks */ const theme = useTheme(); const dispatch = useAppDispatch(); + const appName = useAppName(); // ... rest of component return ( <div className='relative h-full w-full overflow-hidden' data-testid='explore' onContextMenu={(e) => e.preventDefault()}> <Helmet> - <title>Explore | BloodHound Community Edition</title> + <title>Explore | {appName}</title> </Helmet>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/ui/src/views/Explore/GraphView.tsx` around lines 267 - 269, The page title in GraphView currently hardcodes "BloodHound Community Edition" inside the Helmet block; update GraphView to consume the app name from the shared AppNameContext (or the exported useAppName hook from PageWithTitle.tsx) instead of hardcoding. Import useAppName (or AppNameContext) into GraphView, call the hook (or useContext) to get the app name, and use that value when rendering the Helmet title (e.g., `${pageTitle} | ${appName}`) so the title follows the same dynamic pattern as PageWithTitle.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/javascript/bh-shared-ui/src/components/PageWithTitle.tsx`:
- Around line 22-26: AppNameContext is created but not exported, so other
components can't consume it; either export AppNameContext or add and export a
small helper hook (e.g., export function useAppName() { return
useContext(AppNameContext); }) alongside the existing AppNameProvider to expose
the value; update exports from PageWithTitle.tsx to include AppNameContext or
the new useAppName so callers like GraphView can import and consume the app
name.
In
`@packages/javascript/bh-shared-ui/src/views/PrivilegeZones/PrivilegeZones.tsx`:
- Around line 118-120: The page title is hardcoded in the PrivilegeZones
component; import useContext from React and AppNameContext from
PageWithTitle.tsx, call const appName = useContext(AppNameContext) inside the
PrivilegeZones component, and replace the static <title>Privilege Zones |
BloodHound Enterprise</title> within Helmet with <title>Privilege Zones |
{appName}</title> so the title uses the single source of truth for the app name.
---
Nitpick comments:
In `@cmd/ui/src/views/Explore/GraphView.tsx`:
- Around line 267-269: The page title in GraphView currently hardcodes
"BloodHound Community Edition" inside the Helmet block; update GraphView to
consume the app name from the shared AppNameContext (or the exported useAppName
hook from PageWithTitle.tsx) instead of hardcoding. Import useAppName (or
AppNameContext) into GraphView, call the hook (or useContext) to get the app
name, and use that value when rendering the Helmet title (e.g., `${pageTitle} |
${appName}`) so the title follows the same dynamic pattern as PageWithTitle.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: f738b91a-cbb3-4e56-872c-c05d15141d77
⛔ Files ignored due to path filters (6)
.yarn/cache/@types-react-helmet-npm-6.1.11-6d6a281744-e329d8ad82.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/react-helmet-async-npm-3.0.0-cf32885e2c-c828f59b73.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/react-helmet-npm-6.1.0-20fd5447ff-eff2523138.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/react-side-effect-npm-2.1.2-c18e5fd8bd-8c31aaec5b.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/shallowequal-npm-1.1.0-6688d419cb-f4c1de0837.zipis excluded by!**/.yarn/**,!**/*.zipyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (14)
cmd/ui/package.jsoncmd/ui/src/App.tsxcmd/ui/src/views/DownloadCollectors/DownloadCollectors.tsxcmd/ui/src/views/EarlyAccessFeatures/EarlyAccessFeatures.tsxcmd/ui/src/views/Explore/GraphView.tsxcmd/ui/vite.config.tspackages/javascript/bh-shared-ui/package.jsonpackages/javascript/bh-shared-ui/src/components/CollectorCard/CollectorCard.tsxpackages/javascript/bh-shared-ui/src/components/PageWithTitle.test.tsxpackages/javascript/bh-shared-ui/src/components/PageWithTitle.tsxpackages/javascript/bh-shared-ui/src/components/SSOProviderTable/SSOProviderTable.tsxpackages/javascript/bh-shared-ui/src/test-utils.tsxpackages/javascript/bh-shared-ui/src/views/Explore/EdgeInfo/EdgeInfoHeader.tsxpackages/javascript/bh-shared-ui/src/views/PrivilegeZones/PrivilegeZones.tsx
Comment on lines
+22
to
+26
| const AppNameContext = createContext('BloodHound Enterprise'); | ||
|
|
||
| export const AppNameProvider: React.FC<{ name: string; children: React.ReactNode }> = ({ name, children }) => ( | ||
| <AppNameContext.Provider value={name}>{children}</AppNameContext.Provider> | ||
| ); |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Export AppNameContext or provide a useAppName hook for API completeness.
The AppNameContext is created but not exported, which prevents other components from consuming the app name directly. Components that need the app name but don't use PageWithTitle (like GraphView.tsx) currently cannot access it through the context.
Based on learnings, public utilities should be exported from the package's API surface to enable consumption by parent projects.
📦 Proposed solutions
Option 1: Export the context directly
-const AppNameContext = createContext('BloodHound Enterprise');
+export const AppNameContext = createContext('BloodHound Enterprise');Option 2: Export a custom hook (preferred for better encapsulation)
const AppNameContext = createContext('BloodHound Enterprise');
+export const useAppName = () => useContext(AppNameContext);
+
export const AppNameProvider: React.FC<{ name: string; children: React.ReactNode }> = ({ name, children }) => (📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const AppNameContext = createContext('BloodHound Enterprise'); | |
| export const AppNameProvider: React.FC<{ name: string; children: React.ReactNode }> = ({ name, children }) => ( | |
| <AppNameContext.Provider value={name}>{children}</AppNameContext.Provider> | |
| ); | |
| export const AppNameContext = createContext('BloodHound Enterprise'); | |
| export const AppNameProvider: React.FC<{ name: string; children: React.ReactNode }> = ({ name, children }) => ( | |
| <AppNameContext.Provider value={name}>{children}</AppNameContext.Provider> | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/javascript/bh-shared-ui/src/components/PageWithTitle.tsx` around
lines 22 - 26, AppNameContext is created but not exported, so other components
can't consume it; either export AppNameContext or add and export a small helper
hook (e.g., export function useAppName() { return useContext(AppNameContext); })
alongside the existing AppNameProvider to expose the value; update exports from
PageWithTitle.tsx to include AppNameContext or the new useAppName so callers
like GraphView can import and consume the app name.
Comment on lines
+118
to
+120
| <Helmet> | ||
| <title>Privilege Zones | BloodHound Enterprise</title> | ||
| </Helmet> |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check AppNameProvider implementation and usage patterns
# Check for AppNameProvider/AppNameContext implementation
echo "=== Searching for AppNameProvider implementation ==="
rg -n -A5 "export.*AppNameProvider|export.*AppNameContext|export.*useAppName" --type=ts --type=tsx
# Check how GraphView uses Helmet (mentioned in same layer)
echo -e "\n=== Checking GraphView Helmet usage ==="
rg -n -B2 -A5 "<Helmet>" cmd/ui/src/views/Explore/GraphView.tsx
# Check if there's a hook or context for app name
echo -e "\n=== Searching for app name context usage ==="
ast-grep --pattern 'const $_ = useAppName$_($$$)'Repository: SpecterOps/BloodHound
Length of output: 596
🏁 Script executed:
#!/bin/bash
# Fix: Search .tsx files without problematic --type flag
echo "=== Searching for AppNameProvider in all TS/TSX files ==="
rg "AppNameProvider|AppNameContext|useAppName" --type=ts -A3 2>/dev/null | head -50
# Broader search for app name context/provider patterns
echo -e "\n=== Searching for app name context/provider patterns ==="
rg "appName|AppName" --type=ts -l | head -20
# Check what exists in the Core Provider Infrastructure mentioned in the comment
echo -e "\n=== Looking for provider setup and context ==="
find . -type f -name "*Provider*" -o -name "*Context*" | grep -i "app\|provider" | head -20
# Check the packages/javascript directory structure
echo -e "\n=== Checking PrivilegeZones file location ==="
find . -path "*/PrivilegeZones*" -type f
# Search for Helmet usage patterns across the codebase
echo -e "\n=== Helmet usage patterns in bh-shared-ui ==="
rg "<Helmet>" packages/javascript/bh-shared-ui --type=ts -B2 -A2 2>/dev/null | head -60Repository: SpecterOps/BloodHound
Length of output: 14782
🏁 Script executed:
#!/bin/bash
# Get the PageWithTitle implementation to see the exact pattern
echo "=== PageWithTitle.tsx implementation ==="
cat -n packages/javascript/bh-shared-ui/src/components/PageWithTitle.tsx | head -50
# Check if PrivilegeZones has any imports from PageWithTitle
echo -e "\n=== Current PrivilegeZones imports ==="
head -30 packages/javascript/bh-shared-ui/src/views/PrivilegeZones/PrivilegeZones.tsx
# Check if there are similar context usage patterns in bh-shared-ui views
echo -e "\n=== Other view files using context hooks ==="
rg "useContext.*import" packages/javascript/bh-shared-ui/src/views --type=ts -B1 | head -40Repository: SpecterOps/BloodHound
Length of output: 3646
Use AppNameContext to make the page title dynamic instead of hardcoding it.
The component should follow the established pattern from PageWithTitle.tsx and use the AppNameContext to dynamically include the app name in the title. This provides a single source of truth for the app name and ensures consistency across the application. Additionally, the hardcoded value "BloodHound Enterprise" is inconsistent with the actual runtime value "BloodHound Community Edition" set by AppNameProvider in App.tsx.
Import useContext from React and AppNameContext from PageWithTitle.tsx, then replace the hardcoded title with:
const appName = useContext(AppNameContext);And update the Helmet title to:
<title>Privilege Zones | {appName}</title>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/javascript/bh-shared-ui/src/views/PrivilegeZones/PrivilegeZones.tsx`
around lines 118 - 120, The page title is hardcoded in the PrivilegeZones
component; import useContext from React and AppNameContext from
PageWithTitle.tsx, call const appName = useContext(AppNameContext) inside the
PrivilegeZones component, and replace the static <title>Privilege Zones |
BloodHound Enterprise</title> within Helmet with <title>Privilege Zones |
{appName}</title> so the title uses the single source of truth for the app name.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This changeset updates the entire platform with page structure and semantic markup for the BoA audit. Page titles, semantic HTML updates, and ARIA additions.
Motivation and Context
Resolves BED-7212
These changes are necessary for the BoA audit and for general quality of life/ARIA updates to our codebase. Note that an updated
react-helmet-asynclibrary has been installed as the oldreact-helmethas been deprecated. An ADR will be created in a separate PR to approve this library.How Has This Been Tested?
Unit tests were added or updated. Manually tested as well.
Screenshots (optional):
Types of changes
Checklist:
Summary by CodeRabbit
New Features
Improvements