Skip to content

Fix for smooth scrolling#1997

Merged
RobbieTheWagner merged 6 commits intoshipshapecode:masterfrom
henrikr:fix-chrome-scroll
Jul 24, 2022
Merged

Fix for smooth scrolling#1997
RobbieTheWagner merged 6 commits intoshipshapecode:masterfrom
henrikr:fix-chrome-scroll

Conversation

@hrypkema-amplify
Copy link
Copy Markdown
Contributor

Description

Whenever a Tour's default step options is configured with scrollTo: { behavior: 'smooth', block: 'center' } the step's target may not always scroll into the center of the viewport when stepping through the tour.

An example of this behavior can be seen on Shepherd's welcome page

What causes this? Shepherd’s popover receives focus while the smooth scroll to the target is in mid-transition.

The sequence of events when a tour step is first shown:

  1. Smooth-scrolling to the target begins transitioning
  2. After a brief delay, focus is set on the popover and by default will also trigger a scroll to the element
  3. Smooth-scrolling to the target was interrupted by this shift in focus

Changes

  • Configure Shepherd’s focus with preventScroll: true so that it does not interrupt scrolling whenever focus shifts to the popover element
  • Add supporting tests

Demo

fix-smooth-scroll.mp4

When scrollTo is configured with an scrollIntoView object that has behavior set to
'smooth', the element's focus can hijack the scroll and interrupt the target from
scrolling fully into view.

This is particularly noticeable in browsers such as Chrome/Chromium, that implement a
slower transition for the smooth scroll or when the distance to scroll is far.

Configuring the element focus with the preventScroll option will ensure the smooth
scroll transition will not be interrupted.
Copy link
Copy Markdown
Member

@RobbieTheWagner RobbieTheWagner left a comment

Choose a reason for hiding this comment

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

Thank you so much for the incredibly detailed PR with description and tests. This is awesome!

@hrypkema-amplify
Copy link
Copy Markdown
Contributor Author

@rwwagner90 Thank you for the approval! Taking a look at the test runs and will try get the build green shortly

.then((element) => calculateCenteredScrollTop(element[0]))
.then((scrollTop) => {
const plusOrMinusPx = 10;
cy.window().its('scrollY').should('be.closeTo', scrollTop, plusOrMinusPx)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

^ Had to adjust this test to include some margin of difference, since Firefox is capable of scrolling very precisely to 431.616...

Shepherd Acceptance Tests -- Step options -- scrolling -- scrollToscrollIntoViewOptions scrolls with options (failed)

@RobbieTheWagner RobbieTheWagner merged commit b6fc02a into shipshapecode:master Jul 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants