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
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
🦋 Changeset detectedLatest commit: de7ce88 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 @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:
Look forward to hearing more thoughts on this and happy to make any adjustments to the PR, thank you! |
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 ( |
Adding a small reminder for myself as I forgot to do that today: I would like to see what is the end result with |
Description
toc
gives no visual indication its scrollable #1746Adds a scrollbar to
right-sidebar
for accessibility and navigation. See details & discussion in right navtoc
gives no visual indication its scrollable #1746.Yes, scrollbar added.
Before:
After:
Additional Information
sl-container
had a width specified but then a max-width set whenmin-width: 72rem
when it only appears that the entire component would ever be visible atmin-width: 72rem
. I needed to ensurewidth: 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 toauto
whenmin-width: 72rem
to be consistent with styles applied on.right-sidebar
. Based on what I'm seeing in this component, the both thesl-container
width andmax-width
could likely be removed completely but decided to play it safe - let me know if you think these can be removed.right-sidebar
is to provide spacing away from the window scrollbar so they don't end up immediately next to each other.