Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix sidebar generation and display logic #490

Merged
merged 2 commits into from
Aug 28, 2024

Conversation

ptgott
Copy link
Contributor

@ptgott ptgott commented Aug 7, 2024

This change fixes two issues that prevented auto-generated sidebar entries from appearing as highlighted in the sidebar when a user visits them:

  1. The sidebar generator assumes that the table of contents page for a subsection can either be at the same level as its corresponding subdirectory or within that subdirectory. Currently, placing a page at both locations causes an unexpected reuslt. This change throws an error if a page exists at both locations.

  2. The navigation component currently assumes that a docs page's slug begins with the slug of its parent menu page. However, this is not true if the menu page is within its corresponding directory. E.g., the sidebar generator treats docs/pages/directory/directory/ as a valid slug for the menu page of docs/pages/directory/, but the navigation component would not show docs/pages/directory/page1 as highlighted in that case.

    This change resolves this issue by recursively descending through an entry to determine whether one of its entries is highlighted, rather than using the URL path alone.

This change fixes two issues that prevented auto-generated sidebar
entries from appearing as highlighted in the sidebar when a user visits
them:

1. The sidebar generator assumes that the table of contents page for a
   subsection can *either* be at the same level as its corresponding
   subdirectory *or* within that subdirectory. Currently, placing a page
   at both locations causes an unexpected reuslt. This change throws an
   error if a page exists at both locations.
2. The navigation component currently assumes that a docs page's slug
   begins with the slug of its parent menu page. However, this is not
   true if the menu page is within its corresponding directory. E.g.,
   the sidebar generator treats `docs/pages/directory/directory/` as a
   valid slug for the menu page of `docs/pages/directory/`, but the
   navigation component would not show `docs/pages/directory/page1` as
   highlighted in that case.

   This change resolves this issue by recursively descending through an
   entry to determine whether one of its entries is highlighted, rather
   than using the URL path alone.
@ptgott ptgott requested a review from avatus as a code owner August 7, 2024 22:31
Copy link

vercel bot commented Aug 7, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 28, 2024 9:16pm

avatus
avatus previously approved these changes Aug 7, 2024
@ptgott
Copy link
Contributor Author

ptgott commented Aug 28, 2024

@avatus It turns out that my original changes failed CI, so I've re-requested a review after getting them to pass. Thanks!

@ptgott ptgott merged commit abde851 into main Aug 28, 2024
6 checks passed
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