BP-2210: Glossary navigation component #146
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughAdds a new glossary navigation UI that scans H2 headings to build an alphabetic filter, integrates that component into the glossary overview page, and introduces CSS rules and variables to support filtering and theming. Changes
Sequence DiagramsequenceDiagram
autonumber
participant User
participant GlossaryNav as GlossaryNav (React)
participant Main as MainContent (DOM)
participant Observer as MutationObserver
Note over GlossaryNav,Main: Mount / Initialization
User->>GlossaryNav: page load / mount
GlossaryNav->>Main: querySelectorAll(h2)
Main-->>GlossaryNav: list of H2 elements & text
GlossaryNav->>GlossaryNav: compute letters, add data/classes
GlossaryNav->>Observer: attach debounced observer to main content
alt User selects a letter
User->>GlossaryNav: click letter button
GlossaryNav->>Main: set `.glossary-filtered` and `data-active-letter`
Main-->>User: DOM updates to show matching sections
GlossaryNav->>GlossaryNav: scroll to top
end
alt DOM changes (content added/edited)
Observer->>GlossaryNav: mutation observed (debounced)
GlossaryNav->>Main: rescan H2s and recompute letters
GlossaryNav->>GlossaryNav: update UI state
end
User->>GlossaryNav: component unmount
GlossaryNav->>Observer: disconnect & clear timers
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
docs/snippets/components/glossary-nav.jsx (3)
40-50: Consider whether MutationObserver is necessary.If the glossary content is static after initial page load, the MutationObserver may be unnecessary and could be removed to simplify the component. If dynamic content updates are expected, the current implementation with debouncing is reasonable.
114-114: Reduce the z-index value.A z-index of 9999 is unnecessarily high and could cause stacking context issues. Consider using a more reasonable value like 10 or 100, which should be sufficient for a sticky navigation component.
🔎 Suggested change
- zIndex: 9999, + zIndex: 100,
127-148: Add explicittype="button"to button elements.For better accessibility and to prevent unintended form submission behavior, explicitly set
type="button"on all button elements.🔎 Suggested change
<button + type="button" onClick={() => setActiveFilter(null)} style={{<button + type="button" key={letter} onClick={() => handleLetterClick(letter)} style={{Also applies to: 150-174
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
docs/resources/glossary/overview.mdxdocs/snippets/components/glossary-nav.jsxdocs/style.css
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-02T18:01:39.059Z
Learnt from: StephenHinck
Repo: SpecterOps/bloodhound-docs PR: 67
File: docs/collect-data/enterprise-collection/privileged-collection.mdx:7-7
Timestamp: 2025-10-02T18:01:39.059Z
Learning: In the BloodHound documentation repository, "BloodHound" as a standalone name refers to the entire product family and is appropriate to use when content applies to all products in the family (Enterprise and Community Edition). "BloodHound Enterprise" should be used only when referring specifically to Enterprise-only features or capabilities.
Applied to files:
docs/resources/glossary/overview.mdx
📚 Learning: 2025-08-22T22:40:24.105Z
Learnt from: StephenHinck
Repo: SpecterOps/bloodhound-docs PR: 53
File: docs/analyze-data/posture-page.mdx:33-41
Timestamp: 2025-08-22T22:40:24.105Z
Learning: BloodHound Enterprise has exactly four severity levels for attack paths: CRITICAL (95%–100%), HIGH (80%–94%), MODERATE (40%–79%), and LOW (0%–39%). There is no "NONE" or similar severity level for 0% exposure.
Applied to files:
docs/resources/glossary/overview.mdx
🔇 Additional comments (3)
docs/style.css (1)
69-69: LGTM! CSS variables for theming are well-defined.The new
--glossary-nav-bgcustom properties provide appropriate background colors for both light and dark themes, supporting the glossary navigation component.Also applies to: 74-74
docs/resources/glossary/overview.mdx (2)
7-7: LGTM! Clean integration of the navigation component.The import path and component usage are straightforward. The component is appropriately placed at the top of the glossary content.
Also applies to: 11-11
4-4: Verifymode: wideis the intended layout.The
mode: widefrontmatter property has been added. Ensure this is the desired layout mode for the glossary page and that it provides adequate space for the navigation component.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
docs/style.css (1)
82-113: Consider using attribute selectors to reduce repetition.The 26 explicit class selectors (one per letter) are functional but highly repetitive. Consider using attribute selectors for more maintainable CSS:
/* Instead of 26 separate rules, use: */ .glossary-filtered .glossary-term[data-glossary-letter] { display: block !important; }However, if the explicit class approach is preferred for specificity or performance reasons, the current implementation works correctly.
docs/snippets/components/glossary-nav.jsx (3)
50-50: Extract magic numbers into named constants.The timing values (100ms initial delay, 300ms debounce) would be clearer as named constants at the top of the component.
const SETUP_DELAY_MS = 100; const DEBOUNCE_DELAY_MS = 300;Also applies to: 55-55
93-93: Consider scrolling to the filtered content instead of page top.Scrolling to the top on every filter change might be jarring. Consider scrolling to the first H2 matching the selected letter for better UX.
Alternative approach
const firstTerm = document.querySelector(`.glossary-letter-${letter}`); if (firstTerm) { firstTerm.scrollIntoView({ behavior: 'smooth', block: 'start' }); }
129-134: Inconsistent ternary operator formatting.The ternary operators for
fontWeightandtextDecorationare formatted differently than thecolorternary above them. Consider aligning the formatting for consistency.Also applies to: 153-158
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/snippets/components/glossary-nav.jsxdocs/style.css
🔇 Additional comments (3)
docs/style.css (1)
69-69: LGTM! Theme-aware custom properties added correctly.The new
--glossary-nav-bgvariables provide appropriate background colors for both light and dark modes, maintaining visual consistency with the theming system.Also applies to: 74-74
docs/snippets/components/glossary-nav.jsx (2)
20-20: Regex only matches ASCII letters.The pattern
/[A-Z]/iexcludes glossary terms starting with accented or international characters (e.g., "École," "Überblick"). Consider whether this is intentional for your content.If international terms should be supported, consider using Unicode property escapes:
const match = termName.match(/\p{L}/u);
112-112: The rgb() syntax errors from the previous review have been fixed.All
rgb(var(...))function calls now have proper closing parentheses. The color declarations are syntactically correct.Also applies to: 124-125, 148-149
|
After polling the team, I'm going to close this for now. There's a preference for the existing navigation experience on the right that shows all terms. If the glossary grows larger over time, letter-based navigation/filtering may be useful and we can revisit this approach. |
Purpose
This pull request (PR) adds a custom React component to enhance the glossary navigation experience.
Note
I used GitHub Copilot to create this and went through a few refactors to simplify the implementation as much as possible and optimize performance. CodeRabbit also suggested a refactor to align with React best practices. It would be great if a frontend developer could take a look.
This required glossary-specific CSS to support the new navigation and filtered views in both light and dark mode.
Staging
https://specterops-bp-2210-glossary-nav.mintlify.app/resources/glossary/overview
Testing
docs/resources/glossary/overview.mdxfile