-
Notifications
You must be signed in to change notification settings - Fork 278
feat: add search functionality to edge filtering dialog BED-6673 #2216
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Add a search field that filters edge types in the pathfinding dialog. The search is case-insensitive and automatically expands/collapses accordions to show matching results. Search state clears when the dialog closes to ensure a clean UX on reopening.
WalkthroughAdds a local search input to EdgeFilteringDialog, propagates Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Dialog as EdgeFilteringDialog
participant Input as Search Input
participant Category as CategoryList
participant SubCat as SubcategoryListItem
participant Edges as EdgesView
User->>Input: type query
Input->>Dialog: update searchQuery
Dialog->>Category: pass searchQuery
rect `#f3f6ff`
Note over Category,SubCat: Filtering & forced expansion driven by searchQuery
Category->>Category: filter categories with matching subcategories
Category->>SubCat: render items with searchQuery & forceExpand
SubCat->>SubCat: compute filteredEdgeTypes by searchQuery
SubCat->>Edges: pass filteredEdgeTypes
Edges->>Edges: render matching edge types
end
User->>Dialog: Click Apply / Cancel / Close
Dialog->>Dialog: close -> handleExited clears searchQuery
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.test.tsx (1)
31-32: Inconsistent mock function usage.
handleCancel={vi.fn}passes the mock constructor reference, whilehandleApply={vi.fn()}correctly invokes it to create a mock function. While both work, this inconsistency is confusing.🔎 Proposed fix
- handleCancel={vi.fn} + handleCancel={vi.fn()}
🧹 Nitpick comments (2)
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.test.tsx (1)
243-346: Search persistence tests are thorough but have duplicated setup.The cancel and apply persistence tests follow the same pattern. Consider extracting a shared helper to reduce duplication.
🔎 Example refactor
const testSearchClearsOnClose = async (closeButtonName: RegExp) => { const user = userEvent.setup(); const handleCancel = vi.fn(); const handleApply = vi.fn(); const { rerender } = render( <EdgeFilteringDialog isOpen={true} selectedFilters={INITIAL_FILTERS} handleCancel={handleCancel} handleApply={handleApply} handleUpdate={vi.fn()} /> ); const searchField = screen.getByPlaceholderText(/search edges/i); await user.type(searchField, 'Contains'); expect(searchField).toHaveValue('Contains'); const closeButton = screen.getByRole('button', { name: closeButtonName }); await user.click(closeButton); // ... rest of rerender logic }; it('search field clears when dialog closes via cancel button', async () => { await testSearchClearsOnClose(/cancel/i); });packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.tsx (1)
208-210: Consider memoizing filtered edge types.
filteredEdgeTypesis recomputed on every render. For large edge type lists or frequent re-renders during typing, this could be optimized withuseMemo.🔎 Proposed improvement
+import { useMemo } from 'react'; const SubcategoryListItem = ({ subcategory, checked, setChecked, searchQuery }: SubcategoryListItemProps) => { const { name, edgeTypes } = subcategory; const subcategoryFilter = (element: EdgeCheckboxType) => element.subcategory === name; - const filteredEdgeTypes = searchQuery - ? edgeTypes.filter((edgeType) => edgeType.toLowerCase().includes(searchQuery.toLowerCase())) - : edgeTypes; + const filteredEdgeTypes = useMemo(() => { + if (!searchQuery) return edgeTypes; + const query = searchQuery.toLowerCase(); + return edgeTypes.filter((edgeType) => edgeType.toLowerCase().includes(query)); + }, [searchQuery, edgeTypes]);This also avoids calling
searchQuery.toLowerCase()once per edge type.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.test.tsxpackages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.tsx
🧰 Additional context used
🧠 Learnings (7)
📚 Learning: 2025-09-08T19:38:54.755Z
Learnt from: jvacca-specterops
Repo: SpecterOps/BloodHound PR: 1823
File: packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/SavedQueries/ConfirmUpdateQueryDialog.tsx:27-40
Timestamp: 2025-09-08T19:38:54.755Z
Learning: In BloodHound's ConfirmUpdateQueryDialog component (packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/SavedQueries/ConfirmUpdateQueryDialog.tsx), the nested Dialog structure with DialogTitle and DialogActions inside DialogContent (rather than as direct siblings of Dialog) is intentional design and should not be changed to follow conventional MUI patterns.
Applied to files:
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.tsx
📚 Learning: 2025-08-27T19:22:50.905Z
Learnt from: jvacca-specterops
Repo: SpecterOps/BloodHound PR: 1823
File: packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/SavedQueries/CommonSearches.tsx:71-74
Timestamp: 2025-08-27T19:22:50.905Z
Learning: In BloodHound's CommonSearches component (packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/SavedQueries/CommonSearches.tsx), the useEffect that resets filteredList to queryList when userQueries.data changes is intentional behavior - filters should be cleared when the underlying saved queries data updates.
Applied to files:
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.tsx
📚 Learning: 2025-11-06T21:23:30.689Z
Learnt from: dcairnsspecterops
Repo: SpecterOps/BloodHound PR: 2010
File: packages/javascript/bh-shared-ui/src/components/ExploreSearchCombobox/ExploreSearchCombobox.tsx:0-0
Timestamp: 2025-11-06T21:23:30.689Z
Learning: In ExploreSearchCombobox component (packages/javascript/bh-shared-ui/src/components/ExploreSearchCombobox/ExploreSearchCombobox.tsx), setting aria-autocomplete={null} on the TextField is intentional to override downshift's default aria-autocomplete attribute, which was flagged by Axe DevTools. This approach satisfies Axe accessibility requirements by removing the conflicting attribute.
Applied to files:
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.tsx
📚 Learning: 2025-11-06T21:35:45.118Z
Learnt from: dcairnsspecterops
Repo: SpecterOps/BloodHound PR: 2010
File: packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/CypherSearch.tsx:86-90
Timestamp: 2025-11-06T21:35:45.118Z
Learning: In CypherSearch.tsx (packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/CypherSearch.tsx), the useLayoutEffect that directly sets aria-label on cypherEditorRef.current.cypherEditor.codemirror.contentDOM is necessary because the aria-label prop passed to the CypherEditor component (neo4j-cypher/react-codemirror) does not properly reach the correct DOM element for accessibility. The direct DOM manipulation is required to satisfy Axe DevTools requirements.
Applied to files:
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.tsx
📚 Learning: 2025-08-27T19:58:12.996Z
Learnt from: jvacca-specterops
Repo: SpecterOps/BloodHound PR: 1823
File: packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/SavedQueries/SaveQueryDialog.tsx:89-97
Timestamp: 2025-08-27T19:58:12.996Z
Learning: In BloodHound's SaveQueryDialog component (packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/SavedQueries/SaveQueryDialog.tsx), the useEffect that sets isNew based on selectedQuery existence (rather than id presence) is intentional behavior - the logic for determining whether a query is new vs existing should not be changed to check for id presence.
Applied to files:
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.tsx
📚 Learning: 2025-09-08T19:01:53.112Z
Learnt from: jvacca-specterops
Repo: SpecterOps/BloodHound PR: 1823
File: packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/CypherSearch.tsx:108-148
Timestamp: 2025-09-08T19:01:53.112Z
Learning: In BloodHound's CypherSearch component (packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/CypherSearch.tsx), the sharing state reset for sharedIds and isPublic after the two-step permissions update is handled elsewhere in the codebase, so additional state reset callbacks in the updateQueryPermissions function are not needed.
Applied to files:
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.tsx
📚 Learning: 2025-09-24T16:36:12.870Z
Learnt from: dcairnsspecterops
Repo: SpecterOps/BloodHound PR: 1923
File: packages/javascript/bh-shared-ui/src/components/FileUploadDialog/FileUploadDialog.tsx:55-58
Timestamp: 2025-09-24T16:36:12.870Z
Learning: In BloodHound's FileUploadDialog component (packages/javascript/bh-shared-ui/src/components/FileUploadDialog/FileUploadDialog.tsx), the useOnClickOutside hook with dialogRef on the Dialog root is specifically intended to detect navbar clicks, not backdrop clicks. Backdrop clicks are already handled by MUI Dialog's built-in onClose behavior, while the hook handles navbar interactions to allow closing the dialog when users interact with navigation elements.
Applied to files:
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.tsx
🧬 Code graph analysis (2)
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.test.tsx (1)
packages/javascript/bh-shared-ui/src/hooks/useExploreGraph/queries/utils.ts (1)
INITIAL_FILTERS(42-42)
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.tsx (2)
packages/javascript/bh-shared-ui/src/edgeTypes.tsx (4)
Category(26-29)AllEdgeTypes(36-223)EdgeCheckboxType(19-24)Subcategory(31-34)packages/javascript/bh-shared-ui/src/hooks/useTheme.tsx (1)
useTheme(91-158)
🪛 GitHub Actions: Build UI
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.tsx
[error] 79-79: The sx prop is not allowed. Please use tailwind classes instead sx-props/no-sx-props
[error] 90-90: The sx prop is not allowed. Please use tailwind classes instead sx-props/no-sx-props
[error] 178-178: The sx prop is not allowed. Please use tailwind classes instead sx-props/no-sx-props
[error] 220-221: The sx prop is not allowed. Please use tailwind classes instead sx-props/no-sx-props
[error] 1-1: CI dependency graph error: The command failed for workspaces that are depended upon by other workspaces; can't satisfy the dependency graph.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build BloodHound Container Image / Build and Package Container
- GitHub Check: run-analysis
- GitHub Check: run-tests
🔇 Additional comments (5)
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.test.tsx (1)
157-240: Good test coverage for search functionality.The tests comprehensively cover the search feature including:
- Search field rendering
- Auto-expansion of accordions on search
- Collapse behavior when clearing search
- Edge-type-only filtering (not categories/subcategories)
- Case-insensitive matching
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.tsx (4)
60-74: Search state management and cleanup logic looks good.The
handleExitedcallback correctly clearssearchQuerywhen the dialog transition completes, ensuring a clean state on reopen. The wrapper functions for close/apply are clear.
115-129: Filtering logic is correct and efficient.The case-insensitive filtering at the category level correctly checks if any edge type in any subcategory matches the query. Early return when
searchQueryis empty avoids unnecessary computation.
322-324: Note:forceExpandoverrides but doesn't sync with local state.When
forceExpandis true (during search), the accordion expands. However, clicking the chevron togglesshowCollapsibleContentwhich has no visible effect while searching. This is acceptable UX since users can still click to toggle after clearing the search, but be aware that the local state may desync from the visual state during search.
237-271: The review comment's justification is incorrect. Theno-sx-propslint rule specifically disallows thesxprop attribute with the message "Please use tailwind classes instead." However, the code does not use thesxprop—it uses standard MUI component props (bgcolor,p,borderRadius) along with theuseTheme()hook to access theme values. UsinguseTheme()is a valid, widely-used pattern throughout this codebase (SSOConfiguration, UsersTable, etc.) and is not what the lint rule restricts. The rule targets only thesxprop attribute, not MUI theme usage or standard component props.Likely an incorrect or invalid review comment.
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.tsx
Outdated
Show resolved
Hide resolved
Add useEffect hook to reset searchQuery state when the dialog's isOpen prop changes to false. This ensures the search field is properly cleared when the dialog closes via either the cancel or apply button.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.tsx (1)
86-86: Pipeline failure:sxprops are not allowed per linting rules.The CI is failing because
sxprops are disallowed in favor of Tailwind classes. Replace allsxprop usages with Tailwind classes.🔎 Proposed fix
- <Divider sx={{ ml: 1, mr: 1 }} /> + <Divider className="mx-1" /><TextField fullWidth placeholder='Search edges...' value={searchQuery} onChange={(e) => setSearchQuery(e.target.value)} - sx={{ mb: 2 }} + className="mb-2" />- <List sx={{ pl: 2 }}> + <List className="pl-2">- <List sx={{ pl: 4 }}> - <ListItem sx={{ display: 'block' }}> + <List className="pl-4"> + <ListItem className="block">Also applies to: Lines 97, 185, 227-228
🧹 Nitpick comments (2)
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.tsx (2)
64-69: Redundant state clearing withhandleExited.The
useEffectclearssearchQuerywhenisOpenbecomes false, buthandleExited(lines 79-81) already clears it after the dialog transition completes. SincehandleExitedis passed toTransitionProps.onExited(line 84), it covers both Cancel and Apply flows. Remove thisuseEffectto avoid duplicate clearing logic.🔎 Proposed fix
- // Clear search query when dialog closes - useEffect(() => { - if (!isOpen) { - setSearchQuery(''); - } - }, [isOpen]); -
71-77: Consider removing unnecessary wrapper functions.
handleCloseandhandleApplyClickare simple pass-throughs tohandleCancelandhandleApply. Unless you plan to add additional logic here, you can pass the original handlers directly to the buttons (lines 107, 110).🔎 Proposed fix
- const handleClose = () => { - handleCancel(); - }; - - const handleApplyClick = () => { - handleApply(); - }; -- <Dialog open={isOpen} fullWidth maxWidth={'md'} onClose={handleClose} TransitionProps={{ onExited: handleExited }}> + <Dialog open={isOpen} fullWidth maxWidth={'md'} onClose={handleCancel} TransitionProps={{ onExited: handleExited }}><DialogActions> - <Button variant='tertiary' onClick={handleClose}> + <Button variant='tertiary' onClick={handleCancel}> Cancel </Button> - <Button onClick={handleApplyClick}>Apply</Button> + <Button onClick={handleApply}>Apply</Button> </DialogActions>
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.tsx
🧰 Additional context used
🧠 Learnings (7)
📚 Learning: 2025-09-08T19:38:54.755Z
Learnt from: jvacca-specterops
Repo: SpecterOps/BloodHound PR: 1823
File: packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/SavedQueries/ConfirmUpdateQueryDialog.tsx:27-40
Timestamp: 2025-09-08T19:38:54.755Z
Learning: In BloodHound's ConfirmUpdateQueryDialog component (packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/SavedQueries/ConfirmUpdateQueryDialog.tsx), the nested Dialog structure with DialogTitle and DialogActions inside DialogContent (rather than as direct siblings of Dialog) is intentional design and should not be changed to follow conventional MUI patterns.
Applied to files:
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.tsx
📚 Learning: 2025-08-27T19:22:50.905Z
Learnt from: jvacca-specterops
Repo: SpecterOps/BloodHound PR: 1823
File: packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/SavedQueries/CommonSearches.tsx:71-74
Timestamp: 2025-08-27T19:22:50.905Z
Learning: In BloodHound's CommonSearches component (packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/SavedQueries/CommonSearches.tsx), the useEffect that resets filteredList to queryList when userQueries.data changes is intentional behavior - filters should be cleared when the underlying saved queries data updates.
Applied to files:
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.tsx
📚 Learning: 2025-11-06T21:23:30.689Z
Learnt from: dcairnsspecterops
Repo: SpecterOps/BloodHound PR: 2010
File: packages/javascript/bh-shared-ui/src/components/ExploreSearchCombobox/ExploreSearchCombobox.tsx:0-0
Timestamp: 2025-11-06T21:23:30.689Z
Learning: In ExploreSearchCombobox component (packages/javascript/bh-shared-ui/src/components/ExploreSearchCombobox/ExploreSearchCombobox.tsx), setting aria-autocomplete={null} on the TextField is intentional to override downshift's default aria-autocomplete attribute, which was flagged by Axe DevTools. This approach satisfies Axe accessibility requirements by removing the conflicting attribute.
Applied to files:
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.tsx
📚 Learning: 2025-09-08T19:01:53.112Z
Learnt from: jvacca-specterops
Repo: SpecterOps/BloodHound PR: 1823
File: packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/CypherSearch.tsx:108-148
Timestamp: 2025-09-08T19:01:53.112Z
Learning: In BloodHound's CypherSearch component (packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/CypherSearch.tsx), the sharing state reset for sharedIds and isPublic after the two-step permissions update is handled elsewhere in the codebase, so additional state reset callbacks in the updateQueryPermissions function are not needed.
Applied to files:
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.tsx
📚 Learning: 2025-08-27T19:58:12.996Z
Learnt from: jvacca-specterops
Repo: SpecterOps/BloodHound PR: 1823
File: packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/SavedQueries/SaveQueryDialog.tsx:89-97
Timestamp: 2025-08-27T19:58:12.996Z
Learning: In BloodHound's SaveQueryDialog component (packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/SavedQueries/SaveQueryDialog.tsx), the useEffect that sets isNew based on selectedQuery existence (rather than id presence) is intentional behavior - the logic for determining whether a query is new vs existing should not be changed to check for id presence.
Applied to files:
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.tsx
📚 Learning: 2025-11-06T21:35:45.118Z
Learnt from: dcairnsspecterops
Repo: SpecterOps/BloodHound PR: 2010
File: packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/CypherSearch.tsx:86-90
Timestamp: 2025-11-06T21:35:45.118Z
Learning: In CypherSearch.tsx (packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/CypherSearch.tsx), the useLayoutEffect that directly sets aria-label on cypherEditorRef.current.cypherEditor.codemirror.contentDOM is necessary because the aria-label prop passed to the CypherEditor component (neo4j-cypher/react-codemirror) does not properly reach the correct DOM element for accessibility. The direct DOM manipulation is required to satisfy Axe DevTools requirements.
Applied to files:
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.tsx
📚 Learning: 2025-09-24T16:36:12.870Z
Learnt from: dcairnsspecterops
Repo: SpecterOps/BloodHound PR: 1923
File: packages/javascript/bh-shared-ui/src/components/FileUploadDialog/FileUploadDialog.tsx:55-58
Timestamp: 2025-09-24T16:36:12.870Z
Learning: In BloodHound's FileUploadDialog component (packages/javascript/bh-shared-ui/src/components/FileUploadDialog/FileUploadDialog.tsx), the useOnClickOutside hook with dialogRef on the Dialog root is specifically intended to detect navbar clicks, not backdrop clicks. Backdrop clicks are already handled by MUI Dialog's built-in onClose behavior, while the hook handles navbar interactions to allow closing the dialog when users interact with navigation elements.
Applied to files:
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.tsx
🪛 GitHub Actions: Build UI
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.tsx
[error] 86-86: The sx prop is not allowed. Please use tailwind classes instead sx-props/no-sx-props
[error] 97-97: The sx prop is not allowed. Please use tailwind classes instead sx-props/no-sx-props
[error] 185-185: The sx prop is not allowed. Please use tailwind classes instead sx-props/no-sx-props
[error] 227-227: The sx prop is not allowed. Please use tailwind classes instead sx-props/no-sx-props
[error] 228-228: The sx prop is not allowed. Please use tailwind classes instead sx-props/no-sx-props
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build BloodHound Container Image / Build and Package Container
- GitHub Check: run-tests
- GitHub Check: run-analysis
🔇 Additional comments (6)
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.tsx (6)
123-136: LGTM! Clean filtering logic.The category filtering correctly matches any subcategory containing edge types that match the search query, with proper case-insensitive handling.
168-175: LGTM! Consistent filtering pattern.The subcategory filtering logic is consistent with the category-level filtering and correctly implements case-insensitive matching.
215-217: LGTM! Edge type filtering is correct.The final filtering layer correctly filters individual edge types with case-insensitive matching, completing the three-tier filtering hierarchy (Category → Subcategory → EdgeType).
289-289: LGTM! Clean auto-expand implementation.The
forceExpandprop elegantly handles the requirement to auto-expand accordions when a search is active, while preserving manual expand/collapse behavior when no search is present. The implementation at line 331 (isExpanded = forceExpand || showCollapsibleContent) correctly combines both behaviors.Also applies to: Lines 183, 225, 298, 331
245-246: Verify styling approach: MUI Box vs Tailwind.The code now uses MUI's
Boxcomponent with theme-based props (bgcolor={theme.palette.neutral.tertiary}). Given the linting rule that prohibitssxprops in favor of Tailwind classes, verify whether MUI Box with style props is acceptable, or if Tailwind classes should be used for all styling.If Tailwind-only is required, consider this alternative:
// Replace useTheme // Remove: const theme = useTheme(); // Replace Box usage at line 256 <div className="bg-neutral-tertiary p-1 rounded"> <Grid container spacing={2}> {/* ... */} </Grid> </div>However, if
theme.palette.neutral.tertiaryrepresents a custom theme color not in standard Tailwind, you may need to extend your Tailwind config or use a different approach.Also applies to: Line 256, 276
84-84: LGTM! Proper cleanup timing with TransitionProps.Using
TransitionProps.onExitedensures the search field is cleared after the dialog fully exits, which is the correct timing for state cleanup and prevents visual glitches during the transition.
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.tsx
Show resolved
Hide resolved
- Remove redundant useEffect that cleared search on isOpen change (handleExited callback already handles this after transition) - Remove unnecessary wrapper functions (handleClose, handleApplyClick) and use handleCancel/handleApply directly - Add aria-label to search TextField for accessibility - Simplify Divider styling from 'ml-1 mr-1' to 'mx-1' - Remove unused useEffect import
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.tsx
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: dcairnsspecterops
Repo: SpecterOps/BloodHound PR: 2010
File: packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/CypherSearch.tsx:86-90
Timestamp: 2025-11-06T21:35:45.118Z
Learning: In CypherSearch.tsx (packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/CypherSearch.tsx), the useLayoutEffect that directly sets aria-label on cypherEditorRef.current.cypherEditor.codemirror.contentDOM is necessary because the aria-label prop passed to the CypherEditor component (neo4j-cypher/react-codemirror) does not properly reach the correct DOM element for accessibility. The direct DOM manipulation is required to satisfy Axe DevTools requirements.
Learnt from: dcairnsspecterops
Repo: SpecterOps/BloodHound PR: 2010
File: packages/javascript/bh-shared-ui/src/components/ExploreSearchCombobox/ExploreSearchCombobox.tsx:0-0
Timestamp: 2025-11-06T21:23:30.689Z
Learning: In ExploreSearchCombobox component (packages/javascript/bh-shared-ui/src/components/ExploreSearchCombobox/ExploreSearchCombobox.tsx), setting aria-autocomplete={null} on the TextField is intentional to override downshift's default aria-autocomplete attribute, which was flagged by Axe DevTools. This approach satisfies Axe accessibility requirements by removing the conflicting attribute.
📚 Learning: 2025-09-08T19:38:54.755Z
Learnt from: jvacca-specterops
Repo: SpecterOps/BloodHound PR: 1823
File: packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/SavedQueries/ConfirmUpdateQueryDialog.tsx:27-40
Timestamp: 2025-09-08T19:38:54.755Z
Learning: In BloodHound's ConfirmUpdateQueryDialog component (packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/SavedQueries/ConfirmUpdateQueryDialog.tsx), the nested Dialog structure with DialogTitle and DialogActions inside DialogContent (rather than as direct siblings of Dialog) is intentional design and should not be changed to follow conventional MUI patterns.
Applied to files:
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.tsx
📚 Learning: 2025-08-27T19:22:50.905Z
Learnt from: jvacca-specterops
Repo: SpecterOps/BloodHound PR: 1823
File: packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/SavedQueries/CommonSearches.tsx:71-74
Timestamp: 2025-08-27T19:22:50.905Z
Learning: In BloodHound's CommonSearches component (packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/SavedQueries/CommonSearches.tsx), the useEffect that resets filteredList to queryList when userQueries.data changes is intentional behavior - filters should be cleared when the underlying saved queries data updates.
Applied to files:
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.tsx
📚 Learning: 2025-09-08T19:01:53.112Z
Learnt from: jvacca-specterops
Repo: SpecterOps/BloodHound PR: 1823
File: packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/CypherSearch.tsx:108-148
Timestamp: 2025-09-08T19:01:53.112Z
Learning: In BloodHound's CypherSearch component (packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/CypherSearch.tsx), the sharing state reset for sharedIds and isPublic after the two-step permissions update is handled elsewhere in the codebase, so additional state reset callbacks in the updateQueryPermissions function are not needed.
Applied to files:
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.tsx
📚 Learning: 2025-11-06T21:35:45.118Z
Learnt from: dcairnsspecterops
Repo: SpecterOps/BloodHound PR: 2010
File: packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/CypherSearch.tsx:86-90
Timestamp: 2025-11-06T21:35:45.118Z
Learning: In CypherSearch.tsx (packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/CypherSearch.tsx), the useLayoutEffect that directly sets aria-label on cypherEditorRef.current.cypherEditor.codemirror.contentDOM is necessary because the aria-label prop passed to the CypherEditor component (neo4j-cypher/react-codemirror) does not properly reach the correct DOM element for accessibility. The direct DOM manipulation is required to satisfy Axe DevTools requirements.
Applied to files:
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.tsx
📚 Learning: 2025-08-27T19:58:12.996Z
Learnt from: jvacca-specterops
Repo: SpecterOps/BloodHound PR: 1823
File: packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/SavedQueries/SaveQueryDialog.tsx:89-97
Timestamp: 2025-08-27T19:58:12.996Z
Learning: In BloodHound's SaveQueryDialog component (packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/SavedQueries/SaveQueryDialog.tsx), the useEffect that sets isNew based on selectedQuery existence (rather than id presence) is intentional behavior - the logic for determining whether a query is new vs existing should not be changed to check for id presence.
Applied to files:
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.tsx
📚 Learning: 2025-11-06T21:23:30.689Z
Learnt from: dcairnsspecterops
Repo: SpecterOps/BloodHound PR: 2010
File: packages/javascript/bh-shared-ui/src/components/ExploreSearchCombobox/ExploreSearchCombobox.tsx:0-0
Timestamp: 2025-11-06T21:23:30.689Z
Learning: In ExploreSearchCombobox component (packages/javascript/bh-shared-ui/src/components/ExploreSearchCombobox/ExploreSearchCombobox.tsx), setting aria-autocomplete={null} on the TextField is intentional to override downshift's default aria-autocomplete attribute, which was flagged by Axe DevTools. This approach satisfies Axe accessibility requirements by removing the conflicting attribute.
Applied to files:
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.tsx
📚 Learning: 2025-11-06T21:35:02.751Z
Learnt from: dcairnsspecterops
Repo: SpecterOps/BloodHound PR: 2010
File: packages/javascript/bh-shared-ui/src/components/Navigation/AppLink.tsx:30-30
Timestamp: 2025-11-06T21:35:02.751Z
Learning: In AppLink component (packages/javascript/bh-shared-ui/src/components/Navigation/AppLink.tsx), the aria-label="Navigate to ${path}" is added as a fallback for cases where children may be non-string content (icons, JSX elements) rather than readable text.
Applied to files:
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.tsx
📚 Learning: 2025-08-28T18:42:47.612Z
Learnt from: benwaples
Repo: SpecterOps/BloodHound PR: 1829
File: packages/javascript/bh-shared-ui/src/components/Navigation/MainNav.tsx:41-56
Timestamp: 2025-08-28T18:42:47.612Z
Learning: The BaseSVG component in packages/javascript/bh-shared-ui/src/components/AppIcon/Icons/utils.tsx already provides accessibility through VisuallyHidden labels, eliminating the need for additional accessible names on wrapper elements.
Applied to files:
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.tsx
🧬 Code graph analysis (1)
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.tsx (1)
packages/javascript/bh-shared-ui/src/edgeTypes.tsx (4)
Category(26-29)AllEdgeTypes(36-223)EdgeCheckboxType(19-24)Subcategory(31-34)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build BloodHound Container Image / Build and Package Container
- GitHub Check: build-ui
- GitHub Check: run-tests
- GitHub Check: run-analysis
🔇 Additional comments (5)
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.tsx (5)
60-69: LGTM: Clean state management for search reset.The searchQuery state and handleExited cleanup pattern correctly ensures the search field is cleared after the dialog's exit transition completes, preventing visual artifacts during the close animation.
77-84: LGTM: TextField properly configured.The search TextField includes proper accessibility labels and follows the codebase's Tailwind styling conventions.
109-122: LGTM: Filtering logic is correct and consistent.The hierarchical filtering correctly propagates search queries through the three-level structure (category → subcategory → edge type) with case-insensitive matching. Categories and subcategories are shown when they contain any matching edge type, and only matching edges are displayed.
Also applies to: 154-161, 201-203
231-232: LGTM: Theme-based styling is appropriate.The use of
useTheme()andtheme.palette.neutral.tertiaryfor the background color follows Material-UI best practices and maintains consistency with the theme system.Also applies to: 242-242
71-71: LGTM: Styling correctly uses Tailwind classes.All
sxprops have been successfully replaced with equivalent Tailwind classes, addressing the previous linting failures.Also applies to: 83-83, 171-171, 213-214
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.tsx
Outdated
Show resolved
Hide resolved
Re-add useEffect to clear search on dialog close. The previous commit removed this based on PR feedback, but it's required for tests to pass. The handleExited callback only fires after MUI Dialog transition completes, which doesn't occur in test environments using rerender(). Also fix accordion state bug: prevent chevron clicks from toggling showCollapsibleContent while search is active (forceExpand=true). This ensures accordions collapse to default state when search is cleared, rather than persisting expanded state from clicks during search.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.tsx (1)
201-201: LGTM! Edge type filtering completes the search implementation.The final filtering layer correctly filters individual edge types with case-insensitive matching and passes the results to EdgesView. The forceExpand prop ensures accordions stay expanded during search.
💡 Optional performance optimization
If performance becomes a concern with very large edge type lists, consider memoizing the filtered results:
+import { useMemo } from 'react'; + const SubcategoryListItem = ({ subcategory, checked, setChecked, searchQuery }: SubcategoryListItemProps) => { const { name, edgeTypes } = subcategory; const subcategoryFilter = (element: EdgeCheckboxType) => element.subcategory === name; - const filteredEdgeTypes = searchQuery - ? edgeTypes.filter((edgeType) => edgeType.toLowerCase().includes(searchQuery.toLowerCase())) - : edgeTypes; + const filteredEdgeTypes = useMemo(() => + searchQuery + ? edgeTypes.filter((edgeType) => edgeType.toLowerCase().includes(searchQuery.toLowerCase())) + : edgeTypes, + [edgeTypes, searchQuery] + );This would skip recomputation when other state changes, though the current inline approach is likely sufficient for expected dataset sizes.
Also applies to: 204-204, 209-223
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.tsx
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: dcairnsspecterops
Repo: SpecterOps/BloodHound PR: 2010
File: packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/CypherSearch.tsx:86-90
Timestamp: 2025-11-06T21:35:45.118Z
Learning: In CypherSearch.tsx (packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/CypherSearch.tsx), the useLayoutEffect that directly sets aria-label on cypherEditorRef.current.cypherEditor.codemirror.contentDOM is necessary because the aria-label prop passed to the CypherEditor component (neo4j-cypher/react-codemirror) does not properly reach the correct DOM element for accessibility. The direct DOM manipulation is required to satisfy Axe DevTools requirements.
📚 Learning: 2025-08-27T19:22:50.905Z
Learnt from: jvacca-specterops
Repo: SpecterOps/BloodHound PR: 1823
File: packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/SavedQueries/CommonSearches.tsx:71-74
Timestamp: 2025-08-27T19:22:50.905Z
Learning: In BloodHound's CommonSearches component (packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/SavedQueries/CommonSearches.tsx), the useEffect that resets filteredList to queryList when userQueries.data changes is intentional behavior - filters should be cleared when the underlying saved queries data updates.
Applied to files:
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.tsx
📚 Learning: 2025-09-08T19:38:54.755Z
Learnt from: jvacca-specterops
Repo: SpecterOps/BloodHound PR: 1823
File: packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/SavedQueries/ConfirmUpdateQueryDialog.tsx:27-40
Timestamp: 2025-09-08T19:38:54.755Z
Learning: In BloodHound's ConfirmUpdateQueryDialog component (packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/SavedQueries/ConfirmUpdateQueryDialog.tsx), the nested Dialog structure with DialogTitle and DialogActions inside DialogContent (rather than as direct siblings of Dialog) is intentional design and should not be changed to follow conventional MUI patterns.
Applied to files:
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.tsx
📚 Learning: 2025-09-08T19:01:53.112Z
Learnt from: jvacca-specterops
Repo: SpecterOps/BloodHound PR: 1823
File: packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/CypherSearch.tsx:108-148
Timestamp: 2025-09-08T19:01:53.112Z
Learning: In BloodHound's CypherSearch component (packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/CypherSearch.tsx), the sharing state reset for sharedIds and isPublic after the two-step permissions update is handled elsewhere in the codebase, so additional state reset callbacks in the updateQueryPermissions function are not needed.
Applied to files:
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.tsx
📚 Learning: 2025-11-06T21:35:45.118Z
Learnt from: dcairnsspecterops
Repo: SpecterOps/BloodHound PR: 2010
File: packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/CypherSearch.tsx:86-90
Timestamp: 2025-11-06T21:35:45.118Z
Learning: In CypherSearch.tsx (packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/CypherSearch.tsx), the useLayoutEffect that directly sets aria-label on cypherEditorRef.current.cypherEditor.codemirror.contentDOM is necessary because the aria-label prop passed to the CypherEditor component (neo4j-cypher/react-codemirror) does not properly reach the correct DOM element for accessibility. The direct DOM manipulation is required to satisfy Axe DevTools requirements.
Applied to files:
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.tsx
📚 Learning: 2025-08-27T19:58:12.996Z
Learnt from: jvacca-specterops
Repo: SpecterOps/BloodHound PR: 1823
File: packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/SavedQueries/SaveQueryDialog.tsx:89-97
Timestamp: 2025-08-27T19:58:12.996Z
Learning: In BloodHound's SaveQueryDialog component (packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/SavedQueries/SaveQueryDialog.tsx), the useEffect that sets isNew based on selectedQuery existence (rather than id presence) is intentional behavior - the logic for determining whether a query is new vs existing should not be changed to check for id presence.
Applied to files:
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.tsx
📚 Learning: 2025-11-06T21:23:30.689Z
Learnt from: dcairnsspecterops
Repo: SpecterOps/BloodHound PR: 2010
File: packages/javascript/bh-shared-ui/src/components/ExploreSearchCombobox/ExploreSearchCombobox.tsx:0-0
Timestamp: 2025-11-06T21:23:30.689Z
Learning: In ExploreSearchCombobox component (packages/javascript/bh-shared-ui/src/components/ExploreSearchCombobox/ExploreSearchCombobox.tsx), setting aria-autocomplete={null} on the TextField is intentional to override downshift's default aria-autocomplete attribute, which was flagged by Axe DevTools. This approach satisfies Axe accessibility requirements by removing the conflicting attribute.
Applied to files:
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.tsx
📚 Learning: 2025-11-06T21:35:02.751Z
Learnt from: dcairnsspecterops
Repo: SpecterOps/BloodHound PR: 2010
File: packages/javascript/bh-shared-ui/src/components/Navigation/AppLink.tsx:30-30
Timestamp: 2025-11-06T21:35:02.751Z
Learning: In AppLink component (packages/javascript/bh-shared-ui/src/components/Navigation/AppLink.tsx), the aria-label="Navigate to ${path}" is added as a fallback for cases where children may be non-string content (icons, JSX elements) rather than readable text.
Applied to files:
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.tsx
📚 Learning: 2025-08-28T18:42:47.612Z
Learnt from: benwaples
Repo: SpecterOps/BloodHound PR: 1829
File: packages/javascript/bh-shared-ui/src/components/Navigation/MainNav.tsx:41-56
Timestamp: 2025-08-28T18:42:47.612Z
Learning: The BaseSVG component in packages/javascript/bh-shared-ui/src/components/AppIcon/Icons/utils.tsx already provides accessibility through VisuallyHidden labels, eliminating the need for additional accessible names on wrapper elements.
Applied to files:
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.tsx
📚 Learning: 2025-09-24T16:36:12.870Z
Learnt from: dcairnsspecterops
Repo: SpecterOps/BloodHound PR: 1923
File: packages/javascript/bh-shared-ui/src/components/FileUploadDialog/FileUploadDialog.tsx:55-58
Timestamp: 2025-09-24T16:36:12.870Z
Learning: In BloodHound's FileUploadDialog component (packages/javascript/bh-shared-ui/src/components/FileUploadDialog/FileUploadDialog.tsx), the useOnClickOutside hook with dialogRef on the Dialog root is specifically intended to detect navbar clicks, not backdrop clicks. Backdrop clicks are already handled by MUI Dialog's built-in onClose behavior, while the hook handles navbar interactions to allow closing the dialog when users interact with navigation elements.
Applied to files:
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build BloodHound Container Image / Build and Package Container
- GitHub Check: run-analysis
- GitHub Check: build-ui
- GitHub Check: run-tests
🔇 Additional comments (7)
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.tsx (7)
21-21: LGTM! Clean imports and state initialization.The new imports (Box, TextField, useTheme, useEffect) are appropriate for the search functionality implementation, and the searchQuery state is properly initialized.
Also applies to: 38-38, 40-40, 42-42, 60-60
64-74: LGTM! Well-documented dual clearing mechanism.The combination of useEffect and TransitionProps.onExited ensures search state is cleared reliably in both test environments and real browser usage. The comments clearly explain the rationale for this approach.
Also applies to: 77-77
85-92: LGTM! Accessible search field implementation.The TextField is properly configured with aria-label for screen readers, clear placeholder text, and correct state binding. The use of Tailwind classes instead of sx props follows the project's linting rules.
113-113: LGTM! Efficient category-level filtering.The filtering logic correctly prunes categories that don't contain matching edge types, using case-insensitive search. This top-level filtering is efficient as it avoids rendering entire category branches that don't match the query.
Also applies to: 116-134
154-154: LGTM! Consistent subcategory filtering with auto-expand.The filtering logic mirrors the category-level approach with case-insensitive matching. The
forceExpand={!!searchQuery}correctly forces accordion expansion when a search is active, providing good UX for users to see all matching results.Also applies to: 157-157, 162-169, 177-177, 180-180
323-330: LGTM! Accordion state bug successfully resolved.The guard in toggleCollapsibleContent correctly prevents state changes during force-expansion (search active). This ensures that when the search is cleared, accordions return to their default collapsed state rather than persisting any expanded state from chevron clicks made during the search.
This addresses the previous review concern about unexpected accordion behavior after clearing search.
239-239: LGTM! Theme-based styling for consistency.The EdgesView component correctly uses
useThemeand the theme palette for styling, ensuring consistent appearance across the application. The use of Box with direct props (bgcolor, p, borderRadius) instead of the bannedsxprop follows the project's linting rules.Also applies to: 250-250, 270-270
Add a search field in the Pathfinding edge filtering dialog. The search is case-insensitive and automatically expands/collapses accordions to show matching results. Search state clears when the dialog closes to ensure a clean UX on reopening.
Description
Detailed Changes
EdgeFilteringDialog.tsx
New Feature: Search Functionality
TextFieldcomponent above the category list that allows users to search for edge typesuseStateSearch Filtering Logic
filterCategoryfunction inCategoryListthat filters categories based on whether they contain matching edge typesfilterSubcategoryfunction inCategoryListItemthat filters subcategories similarlySubcategoryListItem, edge types are filtered to show only those matching the search query.toLowerCase()Auto-Expand/Collapse Behavior
forceExpandprop toIndeterminateListItemcomponentforceExpand={!!searchQuery}automatically expands all accordions to show matching resultsisExpandedstate (which combinesforceExpandandshowCollapsibleContent)Search Persistence/Cleanup
handleExitedcallback that resets search query to empty stringDialog'sTransitionProps.onExitedRefactoring
sxprop for consistency:className='ml-2 mr-2'→sx={{ ml: 1, mr: 1 }}className='pl-4'→sx={{ pl: 2 }}className='pl-8'→sx={{ pl: 4 }}className='block'→sx={{ display: 'block' }}<div className='bg-neutral-3 p-2 rounded-lg'>with<Box bgcolor={theme.palette.neutral.tertiary} p={1} borderRadius={1}>useThemehook to access theme palettehandleCloseandhandleApplyClickfor better organizationNew Imports
Box- for styled containerTextField- for search inputuseTheme- for accessing theme paletteEdgeFilteringDialog.test.tsx
New Test Suite: Search Functionality Tests
New Test Suite: Search Persistence Tests
All tests use
@testing-library/user-eventfor realistic user interactions and verify both the presence of UI elements and their behavior.Motivation and Context
Resolves BED-6673
If a user wants to uncheck one edge type for filtering, for example CanRDP, they need to either:
How Has This Been Tested?
Screenshots (optional):
pathfinding.filter.search.demo.mp4
Types of changes
Checklist:
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.