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.

Sign up for GitHub

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

Jump to bottom

Docs: fix overflow:auto horizontal scrollbars covering last line of code blocks #37694

Merged
merged 21 commits into from
Mar 4, 2023
Merged

Docs: fix overflow:auto horizontal scrollbars covering last line of code blocks #37694

merged 21 commits into from
Mar 4, 2023

Conversation

patrickhlauke
Copy link
Member

@patrickhlauke patrickhlauke commented Dec 21, 2022 •
edited
Loading

Description

Some browsers (Firefox in recent builds on Windows, and browsers on macOS) have changed how they handles scrollbars when a container uses overflow: auto. Scrollbars don't show at all at first. When hovering over the container, a thin sliver of a scrollbar appears. When hovering over the thin scrollbar, it expands to a full fat scrollbar.

Unfortunately, the browsers then don't leave any extra space for the full fat scrollbar to appear. As a result, currently horizontal scrollbars cover the last line of code in our documentation code blocks.

Chrome/macOS

chrome-macos-overflow-auto-scrollbar

Fixefox/Windows

firefox-overflow-auto-scrollbar

With this fix, we just add some extra top/bottom padding to code blocks, leaving enough space for the scrollbars that then appear.

firefox-overflow-auto-scrollbar-fixed

Only downside is that in some browsers (e.g. Chrome/Windows) those code blocks now have slightly more unnecessary padding (since these browsers already reserve space for the always visible scrollbar)

Chrome/Windows

chrome-windows-scrollbar

Type of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Refactoring (non-breaking change)
  • Breaking change (fix or feature that would change existing functionality)

Checklist

  • I have read the contributing guidelines
  • My code follows the code style of the project (using npm run lint)
  • My change introduces changes to the documentation
  • I have updated the documentation accordingly
  • I have added tests to cover my changes
  • All new and existing tests passed

Live previews

https://deploy-preview-37694--twbs-bootstrap.netlify.app/docs/5.2/getting-started/introduction/

@patrickhlauke patrickhlauke requested a review from a team as a code owner December 21, 2022 15:17
@patrickhlauke patrickhlauke marked this pull request as draft December 21, 2022 15:20
@patrickhlauke
Disable stylelint check for function-url-quotes
5b771d0 
to make the hack go through unchallenged
@patrickhlauke
Make the fix across all browsers
0bf6fb8 
It's not just Firefox - same issue can be seen (though slightly less horrible looking) on macOS / Chrome etc
@patrickhlauke patrickhlauke force-pushed the patrickhlauke-docs-firefox-overflow-auto-workaround branch from 0a6420a to 0bf6fb8 Compare December 21, 2022 15:37
@patrickhlauke patrickhlauke changed the title Docs: fix overflow:auto horizontal scrollbars covering last line of code blocks in Firefox Docs: fix overflow:auto horizontal scrollbars covering last line of code blocks Dec 21, 2022
@patrickhlauke patrickhlauke marked this pull request as ready for review December 21, 2022 15:40
mdo
mdo requested changes Dec 21, 2022
View reviewed changes
Copy link
Member

@mdo mdo left a comment

Choose a reason for hiding this comment

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

This will need some changes at the least for the homepage hero which looks broken now. I recall there being another solution to this, but can't remember what it is just yet. For now, going to hold off on this.

@patrickhlauke
Copy link
Member Author

Annoyingly, there's scrollbar-gutter but this does effectively the opposite of what I'd like (it only has an effect when browsers use classic scrollbars, and is ignored when they use overlay scrollbars ... in our case, we'd want extra padding/gutter when overaly scrollbars ARE used) https://developer.mozilla.org/en-US/docs/Web/CSS/scrollbar-gutter

@patrickhlauke patrickhlauke force-pushed the patrickhlauke-docs-firefox-overflow-auto-workaround branch from 7e9186a to fef18c7 Compare December 21, 2022 21:57
@patrickhlauke
Copy link
Member Author

@mdo tweaked the padding values a bit, and nudged the clipboard button slightly ... better?

@patrickhlauke patrickhlauke force-pushed the patrickhlauke-docs-firefox-overflow-auto-workaround branch from fef18c7 to b4b3481 Compare December 21, 2022 22:00
@patrickhlauke patrickhlauke force-pushed the patrickhlauke-docs-firefox-overflow-auto-workaround branch from b4b3481 to cf5c706 Compare December 21, 2022 22:02
@XhmikosR
Copy link
Member

Maybe we need .highlight pre margin-bottom: 0?

@patrickhlauke
Copy link
Member Author

patrickhlauke commented Dec 24, 2022 •
edited
Loading

@XhmikosR that would then make it look a bit odd in browsers that don't do the overlay scrollbar, I'd say

Windows/Chrome

image

and arguably even look a bit unbalanced in browsers that DO do the overlay, once you're over it with the mouse

Windows/Firefox

image

@patrickhlauke
Copy link
Member Author

personally, I think with the last few tweaks I did, this PR doesn't look too bad, even for the "hero" code on the homepage (while not vertically centered, it wasn't = perfectly anyway = even on the current getbootstrap.com)

Windows/Chrome

image

Windows/Firefox

image

@patrickhlauke
Merge branch 'main' into patrickhlauke-docs-firefox-overflow-auto-wor…
8b0b254 
…karound
@patrickhlauke
Further CSS tweaks
0c1c761 
Forcing `overflow: overlay` should normalise behaviour between Chrome/Win and other implementations. While visually the Chrome/Win scrollbar still looks big and ugly, its height/spacing is now taken into account as being part of the content, so styles can be applied consistently with extra padding at the bottom
@patrickhlauke
Merge branch 'main' into patrickhlauke-docs-firefox-overflow-auto-wor…
419f48d 
…karound
@patrickhlauke
Copy link
Member Author

patrickhlauke commented Jan 15, 2023 •
edited
Loading

@mdo made some futher tweaks, particularly by adding overflow: overlay which at least normalises how Chrome/Windows positions the scrollbar - it looks the same, but is not inside the content part, so bottom padding applies the same way to Chrome/Windows and other browsers with the thinner overlay scrollbar. I also special-cased the "hero" version in the masthead.

Before/after comparison in Chrome/Windows

Hero masthead (virtually the same)

screenshot of current chrome/windows

screenshot of PR applied in chrome/windows

Further down on the homepage - install / CDN

screenshot of current chrome/windows

screenshot of PR applied in chrome/windows

Before/after comparison in Firefox/Windows

Hero masthead (virtually the same)

screenshot of current firefox/windows

screenshot of PR applied in firefox/windows

Further down on the homepage - install / CDN

screenshot of current firefox/windows

screenshot of PR applied in firefox/windows

@patrickhlauke
Copy link
Member Author

patrickhlauke commented Jan 15, 2023 •
edited
Loading

Before/after comparison Chrome/macOS

screenshot of current chrome/macOS

screenshot of PR applied in chrome/macOS

Before/after comparison Safari/macOS

screenshot of current safari/macOS

screenshot of PR applied in safari/macOS

@patrickhlauke patrickhlauke mentioned this pull request Jan 18, 2023
Open
3 tasks
@patrickhlauke
Copy link
Member Author

related: this problem should be fixed directly in reboot, but that'd be a breaking change ... added a new issue for v6 #37909

@patrickhlauke
Copy link
Member Author

@mdo any more thoughts on this?

@patrickhlauke
Copy link
Member Author

randomly - https://github.com/orgs/community/discussions/49077

mdo
mdo approved these changes Mar 4, 2023
View reviewed changes
Copy link
Member

@mdo mdo left a comment

Choose a reason for hiding this comment

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

Looks great, couple tweaks that I'll commit and then merge. Thanks for all the detail and follow through here!

site/assets/scss/_component-examples.scss Outdated Show resolved Hide resolved
site/assets/scss/_component-examples.scss Outdated Show resolved Hide resolved
@mdo mdo merged commit d5f4532 into main Mar 4, 2023
@mdo mdo deleted the patrickhlauke-docs-firefox-overflow-auto-workaround branch March 4, 2023 20:05
@patrickhlauke
Copy link
Member Author

Cool cool. With the tweaks applied, here's what it looks like in Chrome/Win

image

and in Firefox/Win

Screenshot 2023-03-04 200516

(note that when zooming out heavily, the scrollbar still ends up covering more and more of the content...but I think this is good enough for now to at least avoid the most common scenario)

@mahilanmjd mahilanmjd mentioned this pull request Apr 16, 2023
Closed
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@romaricpascal romaricpascal romaricpascal left review comments

@mdo mdo mdo approved these changes

Assignees
No one assigned
Labels
docs v5
Projects
No open projects
v5.3.0
Status: Done
v5.3.0-alpha2
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@patrickhlauke @XhmikosR @mdo @romaricpascal