docs: add tabbed navigation (Documentation / Release notes) - #2718
docs: add tabbed navigation (Documentation / Release notes)#2718claudiacodacy wants to merge 3 commits into
Conversation
Introduces navigation.tabs and restructures the nav into four top-level tabs (Product, Management, Developer Tools, Release Notes) with matching sidebar icon and styling changes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Scale the tabs down to two: Documentation (the existing nav, unchanged) and Release notes (release notes plus special thanks, as before). Drops the Product/Management/Developer Tools split and its icon remapping, and restores the sidebar's original collapsed-by-default behavior now that navigation.expand isn't enabled. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 4 |
| Duplication | 0 |
AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.
TIP This summary will be updated as you push new changes.
There was a problem hiding this comment.
Pull Request Overview
The PR successfully implements tabbed navigation for 'Documentation' and 'Release notes' using MkDocs Material. While the overall analysis shows the changes are up to standards, there is a technical risk associated with the manual scroll and focus management added to docs-theme.js. This file is flagged as complex and currently lacks automated test coverage, which could lead to regressions in mobile navigation.
A primary concern is the CSS implementation in responsive.css; certain selectors are not scoped to the tabbed navigation state (.md-nav--lifted), which would cause layout breakage on the standard sidebar if tabs are ever disabled in mkdocs.yml. Additionally, minor inconsistencies in JavaScript syntax and YAML indentation should be addressed to maintain codebase quality.
About this PR
- The new manual scroll and focus management logic in
docs-theme.jslacks automated integration or regression tests. Given that this file is flagged as complex and lacks coverage, these changes introduce a maintenance risk for mobile UI stability.
Test suggestions
- Verify mobile drawer scrollLeft is reset to 0 when opened to ensure active tab content is visible.
- Verify focus management in the mobile drawer prioritizes links within the active tab subtree.
- Test icon rendering logic in
nav-item.htmlcorrectly identifies level 2 items when tabs are active. - Verify desktop CSS hides redundant top-level labels while keeping them visible on mobile.
- Automate unit/integration testing for
docs/assets/javascripts/docs-theme.jsto address complexity and lack of coverage.
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Verify mobile drawer scrollLeft is reset to 0 when opened to ensure active tab content is visible.
2. Verify focus management in the mobile drawer prioritizes links within the active tab subtree.
3. Test icon rendering logic in `nav-item.html` correctly identifies level 2 items when tabs are active.
4. Verify desktop CSS hides redundant top-level labels while keeping them visible on mobile.
5. Automate unit/integration testing for `docs/assets/javascripts/docs-theme.js` to address complexity and lack of coverage.
TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback
| /* The active tab's own name already shows in the .md-tabs bar above, so | ||
| drop the redundant repeated label at the top of its sidebar (mobile | ||
| still needs it — it's the only place a tab name appears there). */ | ||
| .md-nav--primary > .md-nav__list > .md-nav__item > .md-nav__link { display: none; } |
There was a problem hiding this comment.
🟡 MEDIUM RISK
This selector and the one on line 45 should be scoped to .md-nav--lifted (e.g., .md-nav--lifted > .md-nav__list ...). This ensures that tab labels are only hidden and top-level sections are only restyled when tabbed navigation is active. Without this, disabling tabs in the configuration would break the standard sidebar layout.
| /* Material's stock ".md-nav__link svg { height: 1.3em }" outranks a plain | ||
| class selector on specificity, stretching every sidebar icon vertically. | ||
| Qualify with the element type to match and win on source order instead. */ | ||
| svg.docs-nav-section-icon { flex: 0 0 .95rem; width: .95rem; height: .95rem; margin-right: .45rem; color: var(--docs-text-secondary); } |
There was a problem hiding this comment.
⚪ LOW RISK
Suggestion: This element-qualified selector (svg.docs-nav-section-icon) overrides default specificity to fix icon stretching. It is a valid solution, but be aware it will require updates if the icon implementation changes from an SVG to another element.
| - organizations/reporting/organization-overview.md | ||
| - organizations/reporting/issues-metrics.md | ||
| - organizations/reporting/codacy-usage.md |
There was a problem hiding this comment.
⚪ LOW RISK
Nitpick: The indentation for the list items under the 'Reporting' section (lines 681-683) is inconsistent. Use 6 spaces to match the rest of the navigation tree.
| // .md-sidebar__scrollwrap scrolled one panel-width to the right, | ||
| // hiding the active tab's own list off-screen. Force it back to | ||
| // the resting position before moving focus. | ||
| var scrollwrap = drawer.querySelector(".md-sidebar__scrollwrap"); |
There was a problem hiding this comment.
⚪ LOW RISK
Nitpick: Use 'let' or 'const' instead of 'var' to maintain consistency with modern JavaScript practices and the existing code style in this file (specifically line 134).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Summary
navigation.tabs/navigation.tabs.stickyand splits the top-level nav into two tabs: Documentation (unchanged existing nav) and Release notes (release notes + special thanks, as before).navigation.expandis not enabled).docs-theme.js.Closes #2711
Test plan
mkdocs servelocally; verified both tabs render with correct icons🤖 Generated with Claude Code