Skip to content

fix(theme-classic): render mobile HTML navbar items as list items - #12806

Open
khushalrajput45 wants to merge 1 commit into
meta:mainfrom
khushalrajput45:fix/navbar-html-mobile-li-test
Open

khushalrajput45 wants to merge 1 commit into
meta:mainfrom
khushalrajput45:fix/navbar-html-mobile-li-test

Conversation

@khushalrajput45

@khushalrajput45 khushalrajput45 commented Oct 29, 2026 •

Copy link
Copy Markdown

Description

Fixes #12578.

HTML navbar items currently render as <div> elements on mobile, whereas the mobile navigation list expects <li> elements.

This change renders HTML navbar items as <li> elements on mobile while preserving the existing behavior for desktop navbar items and dropdown items.

Adds regression tests covering mobile, desktop, and dropdown rendering.

Testing

  • Mobile HTML navbar item renders as <li>
  • Desktop HTML navbar item renders as <div>
  • Dropdown HTML navbar item renders as <li>

Ran:

pnpm vitest run packages/docusaurus-theme-classic/src/theme/NavbarItem/__tests__/HtmlNavbarItem.test.tsx

All 3 tests pass locally.

Summary by CodeRabbit

  • Bug Fixes
    • HTML navbar items now render as list items in mobile navigation and when displayed inside dropdowns.
    • On desktop, items outside dropdowns continue to render as regular containers.
    • This keeps the rendered element consistent with the navigation context.

@meta-cla

meta-cla Bot commented Oct 29, 2026

Copy link
Copy Markdown

Hi @khushalrajput45!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks!

@coderabbitai

coderabbitai Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: b48b62b6-3d69-448d-9e6a-b9826e9064ad
Commits

Reviewing files that changed from the base of the PR and between c1546a0 and 14e0a3b.

Files selected for processing: 2
  • packages/docusaurus-theme-classic/src/theme/NavbarItem/HtmlNavbarItem.tsx
  • packages/docusaurus-theme-classic/src/theme/NavbarItem/__tests__/HtmlNavbarItem.test.tsx

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

Walkthrough

HtmlNavbarItem now renders a <li> for mobile and dropdown items, and a <div> for other items. Tests check all three cases.

Changes: HTML Navbar Item Rendering

Layer / File(s) Summary
Root element selection and tests
packages/docusaurus-theme-classic/src/theme/NavbarItem/HtmlNavbarItem.tsx, packages/docusaurus-theme-classic/src/theme/NavbarItem/__tests__/HtmlNavbarItem.test.tsx
HtmlNavbarItem selects <li> when mobile or isDropdownItem is true; otherwise it selects <div>. Tests verify the mobile, dropdown, and default cases.

Priority: Low

Estimated code review effort: 1 (Trivial) | ~3 minutes

Change: Bug fix · Severity of issue fixed: Low

Merge Risk: Minimal · up to 14e0a

Mobile HTML items now use list elements, while desktop and dropdown behavior is preserved. No merge-blocking issue was identified.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title names the main change: mobile HTML navbar items now render as list items (li). The raw summary confirms HtmlNavbarItem now renders li when mobile or isDropdownItem is true, and the objective…
Linked Issues check Passed Issue #12578 requires mobile HTML navbar items to use li elements in the burger-menu list. HtmlNavbarItem.tsx now selects li when mobile is true. The change preserves div for desktop items a…
Out of Scope Changes check Passed The pull request changes only HtmlNavbarItem.tsx and its focused regression test. Both changes directly support issue #12578 and the stated desktop and dropdown compatibility requirements.
Full details: Title check

Explanation

The title names the main change: mobile HTML navbar items now render as list items (li). The raw summary confirms HtmlNavbarItem now renders li when mobile or isDropdownItem is true, and the objectives say desktop div behavior is kept. The title uses the conventional fix(scope) prefix and is specific, short, and free of file lists or vague terms.

Full details: Linked Issues check

Explanation

Issue #12578 requires mobile HTML navbar items to use li elements in the burger-menu list. HtmlNavbarItem.tsx now selects li when mobile is true. The change preserves div for desktop items and li for dropdown items. The added tests cover all three cases.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 2 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@netlify

netlify Bot commented Oct 29, 2026

Copy link
Copy Markdown

✅ [V2]

Built without sensitive environment variables

Name Link
🔨 Latest commit 14e0a3b
🔍 Latest deploy log https://app.netlify.com/projects/docusaurus-2/deploys/6ae3a0129a10f85b0aa946ac
😎 Deploy Preview https://deploy-preview-12806--docusaurus-2.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Navigation bar using div for html items instead of li (small screens)

1 participant