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: add right sidebar scrollbar (#1746) #1770

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

techfg
Copy link
Contributor

@techfg techfg commented Apr 19, 2024

Description

Before:

image

After:

image

Additional Information

  • I was unable to decipher why the sl-container had a width specified but then a max-width set when min-width: 72rem when it only appears that the entire component would ever be visible at min-width: 72rem. I needed to ensure width: auto now that the parent (.right-sidebar) has a max-width, otherwise a horizontal scrollbar would display which wasn't necessary so to minimize risk in case I'm overlooking something, I just set width to auto when min-width: 72rem to be consistent with styles applied on .right-sidebar. Based on what I'm seeing in this component, the both the sl-container width and max-width could likely be removed completely but decided to play it safe - let me know if you think these can be removed.
  • The margin-right on right-sidebar is to provide spacing away from the window scrollbar so they don't end up immediately next to each other.
  • Manual testing on Windows Chrome/Edge/Firefox/Brave & Linux Chrome

Copy link

vercel bot commented Apr 19, 2024

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

Name Status Preview Updated (UTC)
starlight ✅ Ready (Inspect) Visit Preview Apr 19, 2024 6:53pm

Copy link

changeset-bot bot commented Apr 19, 2024

🦋 Changeset detected

Latest commit: de7ce88

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 Patch

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

@BryceRussell
Copy link
Member

Awesome stuff! Here is a preview of what the scrollbars look like for me on Windows/Firefox:

image
image

@HiDeoo
Copy link
Member

HiDeoo commented May 9, 2024

Sharing a few more screenshots and context. We took a look at your amazing contribution today in a Talkign & Doc'ing session on Discord. It's a great way to have feedback from more people and it's particularly useful for visual changes as people have different browsers, screen sizes, OS, etc.

macos-mouse
macOS with the default scrollbar settings and a mouse plugged in
macos-trackpad
macOS with the default scrollbar settings and no mouse plugged in
sarah
Linux with an extension to tweak the sidebar appearance

To summarize the feedback we got:

  • It can definitely feels weird to have, depending on your browser window size, a potential floating scrollbar for the table of contents.
  • It maybe could be helpful if "On this page" was always visible, maybe sticked to the top.

Personally, I think this can be pondered with the fact that the comparison is between no sidebar at all at the moment and in the PR preview, having one, so it's definitely a big change. It's also something that would only be visible on pages with a fairly long table of contents, which is not the majority of the pages.

I'll try to research a bit more the topic and see if some other websites have other approaches to this, but so far I did not find any that would be particularly relevant so I think the PR is already an amazing step forward.

@techfg
Copy link
Contributor Author

techfg commented May 9, 2024

@BryceRussell @HiDeoo - Thanks for sharing the screenshots on other devices/browsers and for reviewing the PR in Talking and Docs, seems like a great way to collect feedback!

Regarding @HiDeoo's thoughts:

  1. Agreed on it feeling weird on having a scrollbar on the page. I do think it's a necessary step forward for accessibility and UX in general but, especially on smaller window sizes, it is a bit overwhelming to be honest. This is part of the reason why I was suggesting in right nav toc gives no visual indication its scrollable #1746 that a "thin" or "styled" scrollbar might be beneficial as I think it's a bit "less" overwhelming given the width. If you haven't yet, take a look at the updated POC (details here) I put together to get a sense of what this would look like. The more I thought about this, if we do add "thin" styling, I think adding to just left nav & right nav as opposed to everywhere throughout Starlight anywhere a scrollbar could appear would be most appropriate. Another thought here is possibly to consider adding a frontmatter (or Starlight) configuration for "disable scroll" so users have the choice of whether or not they want scrollbar (could always be overridden via CSS as well).
  2. "On this Page" - I did consider sticking this to the top originally but after reviewing other sites, came to the decision to let it scroll. I landed on that decision for a couple of reasons: 1) Other than React.dev, all the other sites (VueJS, Vercel, nextra, etc.) I reviewed do not have it stick (Docusaurus doesn't even have a header at top of TOC); and 2) The important part of the TOC is the list items themselves as users know what the sidebar is for once they initially see "On this Page" so it seemed more appropriate to show as many of the items as possible since that it why users would be scrolling that area.

Look forward to hearing more thoughts on this and happy to make any adjustments to the PR, thank you!

@techfg
Copy link
Contributor Author

techfg commented May 9, 2024

I'll add that, especially given the feedback shared thus far, my recommendation would be to default to having scrollbars when content overflows (as opposed to a config option since user could override via CSS if they want to disable) and add "thin" styling to the scrollbar, defaulting to standard thin (scrollbar-width: thin) with a fallback to webkit for browsers do not yet support scrollbar-width). It would behave like the following config from the POC.

image

@HiDeoo
Copy link
Member

HiDeoo commented May 9, 2024

Adding a small reminder for myself as I forgot to do that today: I would like to see what is the end result with thin scrollbars when using an extension such as the one rendering the gold scrollbars and if the larger size configured in the extension is respected in such cases.

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

right nav toc gives no visual indication its scrollable
3 participants