DOC-6348 fixed (hopefully) bad scrolling in pages with Mermaid diagrams#2868
DOC-6348 fixed (hopefully) bad scrolling in pages with Mermaid diagrams#2868andy-stark-redis merged 1 commit intomainfrom
Conversation
🛡️ Jit Security Scan Results
✅ No security findings were detected in this PR
Security scan by Jit
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
|
|
||
| const header = document.querySelector('header.sticky') | ||
| const headerHeight = header ? header.offsetHeight : 0 | ||
| const offset = headerHeight + 16 // small extra padding |
There was a problem hiding this comment.
CSS scroll-margin and JS offset calculation mismatch
Medium Severity
The CSS scroll-mt-24 sets a scroll margin of 6rem (96px), but the JS offset calculation uses headerHeight + 16 which equals ~86px (the header is h-[70px]). On every page load with a hash, the browser first scrolls natively using the 96px CSS margin, then the JS overrides to 86px at 50ms/250ms/500ms intervals, causing a visible 10px jump. The hardcoded + 16 padding doesn't account for the CSS scroll-margin value it's competing with.
Additional Locations (1)
dwdougherty
left a comment
dwdougherty
left a comment
There was a problem hiding this comment.
I checked all the pages with Mermaid diagrams and also a bunch of other random pages and I didn't notice anything untoward. Other folks should also do some testing, so I'm just leaving this review as a comment.
|
Thanks for checking @dwdougherty :-) |
|
I did a little bit of testing, and it looks like this change may have affected the "On this page" links. If you click those to navigate, it seems to add extra space compared to latest. I don't think it's a huge problem, but I thought I'd mention it just in case. Examples: |
|
@rrelledge Thanks very much for checking :-) Do you think users will be disoriented if the heading isn't at the top when they follow an "On this page" link? The gap doesn't seem to be proportional to the page length (eg, I tried the last in-page link on the very long Streams page and it still scrolled to within a couple of lines of the ideal position), so hopefully this isn't something that's going to trip us up on specific pages. |
|
@andy-stark-redis I don't think the slight gap should be too confusing, so I'm okay with it if everyone else is. |
dwdougherty
left a comment
dwdougherty
left a comment
There was a problem hiding this comment.
I'll go ahead and approve. Other folks can still take a look if they want to.
|
@dwdougherty @rrelledge Thanks for your input - I'll merge it and I guess I can always fix it up later if something unexpected happens :-) |
3 participants
Augie seems to have fixed the issue where an anchor link jumps slightly off-position when the target page has Mermaid diagrams in it. I've tried a few other pages but I can't swear that this doesn't introduce some other fault, so I'd be happy to hear any feedback about this.
Example: the link in this section to the other Pattern 1: Fail fast section in the target page should now jump to the heading correctly - previously, it didn't.
Note
Medium Risk
Adjusts global heading anchor offsets and adds load-time hash re-scrolling, which can change navigation behavior site-wide and may introduce unexpected scroll positioning on some pages.
Overview
Improves in-page anchor navigation in docs pages that render dynamic content (e.g., Mermaid) by increasing header
scroll-margin-top(scroll-mt-6→scroll-mt-24) so targets land below the sticky header.Adds a
window.loadhandler that, when a hash is present, re-scrolls to the target element multiple times (after short delays) using an explicit offset based onheader.stickyheight, including support for hashes that include query parameters.Written by Cursor Bugbot for commit 16cabe2. This will update automatically on new commits. Configure here.