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

Sidebar state persistence #2150

Merged
merged 13 commits into from
Aug 16, 2024
Merged

Sidebar state persistence #2150

merged 13 commits into from
Aug 16, 2024

Conversation

delucis
Copy link
Member

@delucis delucis commented Jul 24, 2024

Description

Scope

  • Currently, this PR specifically targets navigations between two pages that share the same sidebar. Navigations between pages with different sidebars are not impacted. Those will continue to behave as they do currently. Navigations across sessions are also not impacted.
  • Currently, restoring state does not happen on small viewports where the menu is collapsed behind a hamburger button. It felt weird to me to restore state in this context, but I’m open to hear feedback on this.

How does it work?

  1. We generate a hash of the sidebar content (labels and link href)
  2. This hash and the scroll position of the sidebar is stored in sessionStorage each page visit
  3. Additionally, when a user expands/collapses a sidebar group, that modified state is also persisted
  4. On a new page load, we check if the stored hash matches the sidebar hash for the new page, and if so, we restore collapsed state and scroll position.
  5. That’s it.

More technical details

  • The persistence script is included in Page.astro so it always runs, even in the absence of a sidebar. This makes sure visiting a page without a sidebar “clears” the stored state. Otherwise, for example, visiting the Starlight docs homepage and then returning to a docs page persists state, which feels wrong, I think. (Again, thinking about this as a UX improvement specific to navigating between pages with a shared sidebar.)
  • The hashing takes place at build time and uses a fast and insecure hash algorithm rather than relying on one of the slower SHA algorithms.
  • The hashing is done inside of the <Sidebar> component. In general, we try to keep logic out of these components to help make overriding simpler, and in an earlier commit the has was added as a sidebarHash property to the route data object. There are pros and cons to both approaches but I think this is the right way based on:
    • The hash is component specific — it is completely tied to our implementation’s behaviour, and should not be needed when a totally custom override is in use.
    • It is becoming a not uncommon pattern for people to filter sidebar entries before rendering the default sidebar component. By hashing in the component we are able to hash those correctly — otherwise in theory someone would need to filter AND generate a unique hash when reusing the default component in this way.
  • The state-restore script is inlined in Sidebar.astro to ensure it runs as soon as the sidebar HTML is complete.
  • This technique will not work automatically for entirely custom sidebars (not sure that’s possible really?) so those will need some extra help probably for people overriding. But open to feedback here too.

Todo

  • Remove nav from getting started page
  • Delete test pages
  • Delete extra sidebar config in astro.config.mjs

Copy link

changeset-bot bot commented Jul 24, 2024

🦋 Changeset detected

Latest commit: 81c2578

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@astrojs/starlight Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added the 🌟 core Changes to Starlight’s main package label Jul 24, 2024
Copy link

netlify bot commented Jul 24, 2024

Deploy Preview for astro-starlight ready!

Name Link
🔨 Latest commit 81c2578
🔍 Latest deploy log https://app.netlify.com/sites/astro-starlight/deploys/66be4dbba1b3d600092d579c
😎 Deploy Preview https://deploy-preview-2150--astro-starlight.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 100 (no change from production)
Accessibility: 100 (no change from production)
Best Practices: 100 (no change from production)
SEO: 92 (no change from production)
PWA: -
View the detailed breakdown and full score reports

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

@astrobot-houston
Copy link
Collaborator

astrobot-houston commented Jul 24, 2024

size-limit report 📦

Path Size
/index.html 5.95 KB (0%)
/_astro/*.js 22.23 KB (+1.36% 🔺)
/_astro/*.css 13.84 KB (+0.17% 🔺)

@delucis
Copy link
Member Author

delucis commented Aug 7, 2024

Made an experimental update in 06dc927 to see if it might handle sidebars with collapsed groups better. Still needs some cleanup probably but the principle here is to only persist sidebar collapse state that has been directly modified by user interaction.

In other words, given a four group sidebar with each group marked with collapsed: true in the sidebar config:

  • On first visit, the current page’s group is expanded, the other three are collapsed (as per Starlight’s current defaults)
  • When navigating to a page in a different group (via a link in body, so no sidebar interaction yet), that new group will expand and the old group will collapse (again, as per Starlight’s current behaviour)
  • If a user expands a group in the sidebar, that gets stored and persisted across navigations, while the other groups continue to collapse/expand depending on if they contain the current page

In theory, this improves the experience for sidebars with lots of collapsed groups. Nevertheless, it may also be somewhat confusing as it introduces two behaviours in parallel:

  • Some groups are “sticky”, remembering user interaction (with no way to “unstick” them without closing and reopening the tab)
  • Other groups continue to toggle back and forth depending on current page

Will need some user testing and feedback to decide whether something like this change makes sense!

@github-actions github-actions bot added the 📚 docs Documentation website changes label Aug 8, 2024
@astrobot-houston

This comment was marked as outdated.

@delucis delucis added the 🌟 minor Change that triggers a minor release label Aug 13, 2024
@delucis delucis marked this pull request as ready for review August 15, 2024 18:38
@github-actions github-actions bot removed the 📚 docs Documentation website changes label Aug 15, 2024
Copy link
Member

@HiDeoo HiDeoo left a comment

Choose a reason for hiding this comment

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

Looks perfect to me, the best of all tested versions 🌟 Amazing work and such a great improvement! 🎉

Small question: I think the Lunaria Status Overview is outdated/invalid as you removed all the test pages in ae66035 ?

@delucis
Copy link
Member Author

delucis commented Aug 16, 2024

Small question: I think the Lunaria Status Overview is outdated/invalid as you removed all the test pages in ae66035 ?

Yes! I asked @yanthomasdev about it — it’s a small bug where the Lunaria action bails early when it finds no content files, which means it couldn’t update that comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌟 core Changes to Starlight’s main package 🌟 minor Change that triggers a minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants