Skip to content

Conversation

@martinsohn
Copy link
Contributor

@martinsohn martinsohn commented Jan 3, 2026

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

  • Added a TextField component above the category list that allows users to search for edge types
  • Search field has placeholder text "Search edges..."
  • Search query is managed via local state using useState

Search Filtering Logic

  • Implemented filterCategory function in CategoryList that filters categories based on whether they contain matching edge types
  • Implemented filterSubcategory function in CategoryListItem that filters subcategories similarly
  • In SubcategoryListItem, edge types are filtered to show only those matching the search query
  • All filtering is case-insensitive using .toLowerCase()

Auto-Expand/Collapse Behavior

  • Added forceExpand prop to IndeterminateListItem component
  • When search query exists, forceExpand={!!searchQuery} automatically expands all accordions to show matching results
  • When search is cleared, accordions collapse back to their default state
  • The expand/collapse icon updates based on isExpanded state (which combines forceExpand and showCollapsibleContent)

Search Persistence/Cleanup

  • Added handleExited callback that resets search query to empty string
  • This callback is passed to Dialog's TransitionProps.onExited
  • Ensures search field is cleared when dialog closes (via Cancel or Apply button)
  • Provides clean UX when reopening the dialog

Refactoring

  • Replaced Tailwind CSS classes with Material-UI's sx prop 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' }}
  • Replaced <div className='bg-neutral-3 p-2 rounded-lg'> with <Box bgcolor={theme.palette.neutral.tertiary} p={1} borderRadius={1}>
  • Added useTheme hook to access theme palette
  • Added wrapper functions handleClose and handleApplyClick for better organization

New Imports

  • Box - for styled container
  • TextField - for search input
  • useTheme - for accessing theme palette

EdgeFilteringDialog.test.tsx

New Test Suite: Search Functionality Tests

  • Test that search field renders correctly
  • Test that searching filters edge types and auto-expands accordions
  • Test that clearing search collapses accordions
  • Test that search only filters edge types (not categories/subcategories)
  • Test that search is case-insensitive

New Test Suite: Search Persistence Tests

  • Test that search field clears when dialog closes via Cancel button
  • Test that search field clears when dialog closes via Apply button
  • Both tests verify the cleanup behavior by opening, searching, closing, and reopening the dialog

All tests use @testing-library/user-event for 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:

  • Know which category the edge is in
    • Expand the Active Directory category
    • Expand the Lateral Movement category
    • Uncheck the CanRDP edge
  • Look through all 6 categories and 42 edges
    • Expand the Active Directory category
    • Expand all of the 6 categories
    • Manually look for the node, or CTRL+F and search for the node
    • Uncheck the CanRDP edge

How Has This Been Tested?

  • Built BHCE locally, verified the feature works (see below)

Screenshots (optional):

pathfinding.filter.search.demo.mp4

Types of changes

  • New feature (non-breaking change which adds functionality)

Checklist:

Summary by CodeRabbit

  • New Features

    • Added search to the edge filtering dialog for faster type discovery.
    • Search filters edge types in real time, auto-expands matching sections, is case-insensitive, and only targets edge types (not categories).
    • Search input clears when the dialog is closed (via cancel or apply) and is reset when reopened.
  • Tests

    • Added comprehensive UI tests covering rendering, filtering, auto-expansion, case-insensitivity, and dialog state persistence.

✏️ Tip: You can customize this high-level summary in your review settings.

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.
@martinsohn martinsohn self-assigned this Jan 3, 2026
@martinsohn martinsohn added the user interface A pull request containing changes affecting the UI code. label Jan 3, 2026
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 3, 2026

Walkthrough

Adds a local search input to EdgeFilteringDialog, propagates searchQuery through CategoryList → CategoryListItem → SubcategoryListItem → EdgesView to filter edge types and force-expand matching accordions; clears search state when the dialog exits. Includes new UI tests covering rendering, filtering, case-insensitivity, and reset behavior.

Changes

Cohort / File(s) Summary
Tests
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.test.tsx
Adds UI tests: search field rendering; typing filters edge types and auto-expands accordions; clearing collapses accordions and shows expand controls; search scoped to edge types (not categories/subcategories); case-insensitive matching; persistence/clearing behavior on dialog close via cancel/apply and reopen.
Edge filtering UI & props
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.tsx
Adds searchQuery state and TextField input; passes searchQuery to CategoryList, CategoryListItem, SubcategoryListItem; SubcategoryListItem computes filteredEdgeTypes and passes to EdgesView; introduces forceExpand?: boolean on IndeterminateListItem to force expansion when query active; clears search on dialog exit via handleExited/useEffect; minor layout/styling adjusted (Box/useTheme, dividers/padding).
Type/signature updates
packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.tsx
Updates prop interfaces: adds searchQuery: string to CategoryListProps, CategoryListItemProps, SubcategoryListItemProps; adds forceExpand?: boolean to IndeterminateListItemProps.

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
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested labels

enhancement

Suggested reviewers

  • urangel

Poem

🐰
I hopped through lists and found a clue,
A tiny search that shows what's true.
Accordions bloom where matches hide,
Close the door — the field's cleaned wide.
Hooray for paths and carrots, too! 🥕

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately reflects the main feature: adding search functionality to the edge filtering dialog, with a clear ticket reference.
Description check ✅ Passed The description is comprehensive and well-structured, covering changes, motivation, testing, and includes all required template sections with appropriate details.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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, while handleApply={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.

filteredEdgeTypes is recomputed on every render. For large edge type lists or frequent re-renders during typing, this could be optimized with useMemo.

🔎 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

📥 Commits

Reviewing files that changed from the base of the PR and between baf5a44 and be31c6c.

📒 Files selected for processing (2)
  • packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/EdgeFilteringDialog.test.tsx
  • 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-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 handleExited callback correctly clears searchQuery when 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 searchQuery is empty avoids unnecessary computation.


322-324: Note: forceExpand overrides but doesn't sync with local state.

When forceExpand is true (during search), the accordion expands. However, clicking the chevron toggles showCollapsibleContent which 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. The no-sx-props lint rule specifically disallows the sx prop attribute with the message "Please use tailwind classes instead." However, the code does not use the sx prop—it uses standard MUI component props (bgcolor, p, borderRadius) along with the useTheme() hook to access theme values. Using useTheme() is a valid, widely-used pattern throughout this codebase (SSOConfiguration, UsersTable, etc.) and is not what the lint rule restricts. The rule targets only the sx prop attribute, not MUI theme usage or standard component props.

Likely an incorrect or invalid review comment.

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.
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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: sx props are not allowed per linting rules.

The CI is failing because sx props are disallowed in favor of Tailwind classes. Replace all sx prop 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 with handleExited.

The useEffect clears searchQuery when isOpen becomes false, but handleExited (lines 79-81) already clears it after the dialog transition completes. Since handleExited is passed to TransitionProps.onExited (line 84), it covers both Cancel and Apply flows. Remove this useEffect to avoid duplicate clearing logic.

🔎 Proposed fix
-    // Clear search query when dialog closes
-    useEffect(() => {
-        if (!isOpen) {
-            setSearchQuery('');
-        }
-    }, [isOpen]);
-

71-77: Consider removing unnecessary wrapper functions.

handleClose and handleApplyClick are simple pass-throughs to handleCancel and handleApply. 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

📥 Commits

Reviewing files that changed from the base of the PR and between be31c6c and 55b141b.

📒 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 forceExpand prop 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 Box component with theme-based props (bgcolor={theme.palette.neutral.tertiary}). Given the linting rule that prohibits sx props 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.tertiary represents 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.onExited ensures 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.

- 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
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between 55b141b and 1c810e6.

📒 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() and theme.palette.neutral.tertiary for 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 sx props have been successfully replaced with equivalent Tailwind classes, addressing the previous linting failures.

Also applies to: 83-83, 171-171, 213-214

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.
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between 1c810e6 and 962eda1.

📒 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 useTheme and the theme palette for styling, ensuring consistent appearance across the application. The use of Box with direct props (bgcolor, p, borderRadius) instead of the banned sx prop follows the project's linting rules.

Also applies to: 250-250, 270-270

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

user interface A pull request containing changes affecting the UI code.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants