Skip to content

Fix scrolling bug in Chrome 102+#64

Merged
tkadlec merged 2 commits intocatchpoint:mainfrom
tannerhodges:fix-scroll
Jun 27, 2022
Merged

Fix scrolling bug in Chrome 102+#64
tkadlec merged 2 commits intocatchpoint:mainfrom
tannerhodges:fix-scroll

Conversation

@tannerhodges
Copy link
Copy Markdown
Contributor

@tannerhodges tannerhodges commented Jun 25, 2022

😬 Hey y'all, I ran into an issue where I'm unable to read the Getting Started page: https://docs.webpagetest.org/getting-started/

Problem: scrollIntoView() scrolls the whole document

When I try to scroll down, the IntersectionObserver in in-page-nav.js jumps me back to the top of the page:

2022-06-25.WebPageTest.documentation.scroll.jank.mp4

Solution: Replace scrollIntoView() with scrollTo()

Initially, I quick-fixed the issue by just removing the scrollIntoView() call.

But based on #25, since the original intent was for this to scroll to each link as its heading comes into view, I've replaced scrollIntoView() (which seems to scroll the whole document) with scrollTo() (which only scrolls the TOC element).

2022-06-25.Fix.WebPageTest.scroll.jank.by.replacing.scrollIntoView.with.scrollTo.mp4

I also tested on pages with long TOCs (like Scripting) and short TOCs (like Accounts) or no TOC (like Custom Metrics Examples).

To keep the PR as simple as possible, I used .closest('.toc') instead of a separate toc variable.

🙏 Hopefully this helps.

Let me know if there are any other changes y'all recommend for this update.

Note: Honestly, this bug confuses me, because I'd expect scrollIntoView() to only scroll its parent container. But apparently not? I tried reading the spec on scrolling an element into view, but couldn't quite understand what determines a "scrolling box". Maybe someone else can explain, or suggest another way to implement this fix?

Note: It seems like this may be a side effect of this Chrome bug: https://bugs.chromium.org/p/chromium/issues/detail?id=1318615

@netlify
Copy link
Copy Markdown

netlify Bot commented Jun 25, 2022

👷 Deploy request for webpagetest-documentation accepted.

Name Link
🔨 Latest commit 821a89f
🔍 Latest deploy log https://app.netlify.com/sites/webpagetest-documentation/deploys/62b7650cab50520008357e6a

@tannerhodges
Copy link
Copy Markdown
Contributor Author

🤔 Trying this out in Edge, Safari, and Firefox and I'm only seeing it in Chrome 102…

Starting to think this may actually be a Chrome bug.

Screen Shot 2022-06-25 at 3 49 17 PM

@tannerhodges tannerhodges changed the title Fix IntersectionObserver preventing users from scrolling Fix scrolling bug in Chrome 102 Jun 25, 2022
@tannerhodges tannerhodges changed the title Fix scrolling bug in Chrome 102 Fix scrolling bug in Chrome 102+ Jun 25, 2022
@tannerhodges
Copy link
Copy Markdown
Contributor Author

🤷‍♂️ Not sure but this might be related? https://bugs.chromium.org/p/chromium/issues/detail?id=1318615

Seeing the same issue in Chrome Canary too.

Screen Shot 2022-06-25 at 4 00 30 PM

@tkadlec tkadlec merged commit f113b24 into catchpoint:main Jun 27, 2022
@tkadlec
Copy link
Copy Markdown
Contributor

tkadlec commented Jun 27, 2022

Really nice catch, @tannerhodges! This issue has been a bit of a ghost (I ran into it, but no one else on the team and no one has ever reported it, so I never made it a priority to sort out what was up).

Thanks for doing this!

@tannerhodges
Copy link
Copy Markdown
Contributor Author

🙌 Awesome, anytime!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants