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

feat(bcd): Sticky headers for BCD table, on desktop #11345

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

Conversation

ferdnyc
Copy link

@ferdnyc ferdnyc commented Jun 21, 2024

Summary

On desktop with sufficient browser height (determined by a media query on (min-height: $screen-height-place-limit)), make the headers of the BCD table sticky against the bottom of the sticky page header.

Fixes #5417

Problem

On pages with (vertically) long compatibility tables (see, for example, https://developer.mozilla.org/en-US/docs/Web/API/Element), the headers scroll off the top of the screen as the user scrolls down, losing useful context.

Solution

This takes a different approach than #8541 did, by making the table headers sticky in the page scroll context rather than giving the table its own local scroll context. Hopefully that will alleviate some of the issues that cropped up in that other PR.

In my testing this seems to work well. In fact, I was inspired to open this PR when, after reading this Discussion, I noticed / reminded myself that I've been running with the "moral equivalent" of these changes as a local userstyle since last December. So, I've been silently testing these changes without issue for the past 7 months. Of course, that's still a very narrow set of browsers, screen sizes, etc. tested against.


Screenshots

Before

Wide:

image

Narrower:

image

After

Wide:

image

At wider breakpoints, the table is allowed to overlap the page TOC when scrolling past it, which I don't personally feel is a problem. But, it's worth mentioning.

Narrower:

image

At narrower breakpoints, this causes the entire page to be horizontally scrollable, instead of just the table, which I actually feel is an improvement. (It doesn't make the rest of the page content any wider, so none of it goes offscreen and the only element that needs horizontal scrolling is still the BCD table.)

Also, as can be seen in the Narrower screenshots, this change has a limitation (shared with the left-hand sidebar) in that its top position is fixed at var(--sticky-header-with-actions-height), which == 98px. So, on narrower screens where the sticky header wraps, it will be taller than 98px, causing part of both sticky elements to be cut off at the top.

I feel that limitation could be better addressed by making --sticky-header-with-actions-height a dynamic variable that adjusts itself to account for wrapping. That would solve it for both sticky elements.


How did you test this change?

Created a local userstyle with the same style modifications.

@ferdnyc ferdnyc requested a review from a team as a code owner June 21, 2024 06:01
@ferdnyc ferdnyc force-pushed the sticky-bcd-headers-desktop branch from bedd709 to b48b0fa Compare July 13, 2024 16:35
@ferdnyc ferdnyc requested a review from mdn-bot as a code owner July 13, 2024 16:35
@github-actions github-actions bot added the merge conflicts 🚧 Please rebase onto or merge the latest main. label Jul 13, 2024
Copy link
Contributor

This pull request has merge conflicts that must be resolved before it can be merged.

@github-actions github-actions bot added dependencies Pull requests that update a dependency file redirects all things related to redirecting flaw-system issues and feature requests related to the flaws system macros tracking issues related to kumascript macros python Pull requests that update Python code plus work around features related to MDN Plus markdown markdown related issues and pull requests deployer Deployment (currently using AWS S3 and AWS Lambda) github-actions copy Copy changes to hard-coded texts on the site metrics placement cloud-function blog plus:playground plus:ai-help labels Jul 13, 2024
@ferdnyc
Copy link
Author

ferdnyc commented Jul 13, 2024

I rebased the PR branch onto main, hopefully that will solve the unsigned commits issue. And when that didn't work, I reset the PR branch to main and re-applied my one commit on top of that. (Unfortunately the main I was using was my local, still-somewhat-out-of-date copy. But I'll do it over again with a fully-current main, if necessary.)

@ferdnyc ferdnyc force-pushed the sticky-bcd-headers-desktop branch from b48b0fa to 9d76a58 Compare July 13, 2024 16:38
@github-actions github-actions bot removed the merge conflicts 🚧 Please rebase onto or merge the latest main. label Jul 13, 2024
On desktop with sufficient browser height (determined by a media
query on `(min-height: $screen-height-place-limit)`), make the
headers of the BCD table sticky against the bottom of the sticky
page header.

Signed-off-by: FeRD (Frank Dana) <[email protected]>
@ferdnyc ferdnyc force-pushed the sticky-bcd-headers-desktop branch from 36d001e to 4f8d2b2 Compare July 13, 2024 16:43
@github-actions github-actions bot added idle and removed idle labels Aug 12, 2024
@github-actions github-actions bot added the idle label Sep 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blog cloud-function copy Copy changes to hard-coded texts on the site dependencies Pull requests that update a dependency file deployer Deployment (currently using AWS S3 and AWS Lambda) flaw-system issues and feature requests related to the flaws system github-actions idle macros tracking issues related to kumascript macros markdown markdown related issues and pull requests metrics placement plus:ai-help plus:playground plus work around features related to MDN Plus python Pull requests that update Python code redirects all things related to redirecting
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Suggestion: Make compatibility table header sticky
1 participant