Skip to content

Replace sidebar HTML element aside with nav - #660

Open
aatuvai wants to merge 4 commits into
mainfrom
aatuvai/2026-05-13-sidebar-nav
Open

Replace sidebar HTML element aside with nav#660
aatuvai wants to merge 4 commits into
mainfrom
aatuvai/2026-05-13-sidebar-nav

Conversation

@aatuvai

@aatuvai aatuvai commented May 13, 2026

Copy link
Copy Markdown
Contributor

Changes

This PR replaces the aside element found in the Docusaurus DocRootLayoutSidebar component with nav. Also the nav element which was previously in DocSidebarDesktopContent is changed to div so that only one nav is present in the sidebar.

Note

The update requires swizzling the internal Docusaurus components (The components are swizzled with the --eject flag). Swizzling the components might cause problems in the future if Docusaurus introduces breaking changes that are related to these components.

@aatuvai
aatuvai requested review from ptgott and roraback May 13, 2026 12:30
@github-actions

github-actions Bot commented May 13, 2026

Copy link
Copy Markdown

Amplify deployment status

Branch Commit Job ID Status Preview Updated (UTC)
aatuvai/2026-05-13-sidebar-nav 9238073 4 ✅SUCCEED aatuvai-2026-05-13-sidebar-nav 2026-06-09 13:58:04

@ptgott

ptgott commented May 19, 2026

Copy link
Copy Markdown
Contributor

@aatuvai Would you mind providing some context for why we are making this change? Are there any intended UX differences here, or just a tag swap? If this is addressing accessibility concerns, would it be worth working with the Docusaurus maintainers here instead of making changes downstream? Thanks!

@aatuvai

aatuvai commented May 20, 2026

Copy link
Copy Markdown
Contributor Author

@ptgott This was one of the docs team Q2 items.

Only the tag swap is intended. The reasoning was that agents reading the page as HTML would more quickly detect the sidebar as a navigation area in order to not waste tokens reading through it. @roraback

Personally I'm not certain about the effectiveness of this though, as it is not straightforward to prove. Also, now that the Markdown pages are served alongside the HTML ones, the concerns of the original comment (in the spreadsheet) are less prominent I think.

And yes, this sort of a change would preferably be done upstream in the Docusaurus repo.

@ptgott

ptgott commented May 20, 2026

Copy link
Copy Markdown
Contributor

I asked Claude for alternatives and it suggested modifying the DocSidebar/Desktop component. Would that work as an alternative to ejecting the two components?


🤖 Prepared with Claude Code

Thanks for the PR! A few thoughts.

This repo already has a full eject of DocSidebar/Desktop at src/theme/DocSidebar/Desktop/index.tsx, which puts it right between the two components you're adding:

DocRoot/Layout/Sidebar    ← <aside>  [Docusaurus default — ejected by this PR]
  └─ DocSidebar/Desktop   ← <div>    [already ejected in this repo]
       └─ Desktop/Content ← <nav>    [Docusaurus default — ejected by this PR]

You could avoid both new ejections by modifying the file that's already there. Adding role="navigation" and aria-label="Docs sidebar" to the existing outer <div> would give it the exact same landmark semantics as <nav> for any ARIA-reading parser, and Desktop/Content's inner <nav> would stay untouched. Alternatively, swapping that <div> for <nav aria-label="Docs sidebar"> directly would also work — nested <nav> elements are valid HTML as long as both carry distinct aria-labels, and it's a smaller trade-off than two new ejected components.

As you note, the right long-term home for this is Docusaurus core, where the maintenance burden is zero for downstream consumers.

One thing worth checking before landing either approach: most LLM web scrapers identify skip regions by ARIA landmark role, and <aside> already maps to the complementary landmark, which is a recognized non-main-content signal. The gain from switching to navigation would be real only for an agent that skips navigation but not complementary — worth verifying against whatever agent motivated this.

…t, add role="navigation" and aria-label="Docs sidebar" to DocSidebar/Desktop

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 946132c830

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/theme/DocSidebar/Desktop/index.tsx Outdated
@aatuvai

aatuvai commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

@ptgott The change suggested by Claude doesn't actually make a whole lot of sense since it just suggests adding a role="navigation" and aria-label="Docs sidebar" to the existing nav's parent div (The nav already has a aria-label="Docs sidebar", so it's unnecessarily duplicating the same accessibility attribute)

If we want to avoid ejecting the components to prevent the additional maintenance burden, I'm leaning towards closing this PR and putting the task from the Q2 list on hold. Alternatively we could look into adding an issue on the Docusaurus repository to switch the tags. What do you think?

@ptgott

ptgott commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

@ptgott The change suggested by Claude doesn't actually make a whole lot of sense since it just suggests adding a role="navigation" and aria-label="Docs sidebar" to the existing nav's parent div (The nav already has a aria-label="Docs sidebar", so it's unnecessarily duplicating the same accessibility attribute)

If we want to avoid ejecting the components to prevent the additional maintenance burden, I'm leaning towards closing this PR and putting the task from the Q2 list on hold. Alternatively we could look into adding an issue on the Docusaurus repository to switch the tags. What do you think?

Are we able to demonstrate that this change has the effect we want it to? If so, I think we can go ahead with it. We have already swizzled a number of components that are low in the component tree, so this isn't going to introduce a new class of issue, just increment the number of now-custom components we'll need to maintain.

Going forward, it might make sense to implement a workflow in which we can merge upstream Docusaurus changes into our swizzled components to make sure we're up to date with bug fixes etc.

@aatuvai

aatuvai commented Jun 12, 2026

Copy link
Copy Markdown
Contributor Author

Are we able to demonstrate that this change has the effect we want it to?

The task description in the Q2 planning sheet reads:

"Consider re-architecting the navigation system on Docs. LLMs have token limits when they fetch a page. Nav HTML burns through that budget before the crawler reaches the actual documentation. This hurts our chances of being cited in AI-generated answers."
Scope to a) replacing top nav and b) changing side nav from an aside to a nav element

@ptgott So the assumption in the task is that when an AI chatbot fetches a page, it would filter content based on the HTML structure of a web page. My understanding is that when e.g. Claude "reads" a web page, its internal fetch tool processes the whole page. These tools probably have some internal logic to only consider the main content of the page for further evaluation.

Thus I can't tell how we could reliably demonstrate that the change has the desired effect.

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.

2 participants