Skip to content

Commit 8e2f818

Browse files
authored
fix(segment): avoid scrolling webkit bug (#28376)
Issue number: resolves #28373 --------- 🚨 Reviewers: Please test this on Chrome, Firefox, and Safari <!-- Please do not submit updates to dependencies unless it fixes an issue. --> <!-- Please try to limit your pull request to one type (bugfix, feature, etc). Submit multiple pull requests if needed. --> ## What is the current behavior? <!-- Please describe the current behavior that you are modifying. --> The fix in 1167a93 uncovered a WebKit bug where scrolling an element using `scrollIntoView` during an accelerated transition causes the segment to jump during the transition. `scrollIntoView` can cause parent elements to also scroll, and given that the entering view begins outside the viewport, it's possible this triggered some sort of WebKit bug where it was trying to scroll the element into view. ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> - Updated the internal implementation to use `scrollBy` instead of `scrollIntoView`. `scrollBy` does not attempt to scroll parent elements which avoids the WebKit issue. - I also updated the initial scroll to be instant rather than smoothly scroll. If a segment is added to the DOM, I'd expect it to be added with the correct scroll position (after the first render that is), so animating on load seemed like a strange behavior. User interaction will continue to use smooth scrolling though. ## Does this introduce a breaking change? - [ ] Yes - [x] No <!-- If this introduces a breaking change, please describe the impact and migration path for existing applications below. --> ## Other information <!-- Any other information that is important to this PR such as screenshots of how the component looks before and after the change. --> Dev build: `7.5.2-dev.11697638908.1f04980a`
1 parent a4b303e commit 8e2f818

File tree

1 file changed

+46
-18
lines changed

1 file changed

+46
-18
lines changed

Diff for: core/src/components/segment/segment.tsx

+46-18
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,13 @@ export class Segment implements ComponentInterface {
145145
* before we can scroll.
146146
*/
147147
raf(() => {
148-
this.scrollActiveButtonIntoView();
148+
/**
149+
* When the segment loads for the first
150+
* time we just want to snap the active button into
151+
* place instead of scroll. Smooth scrolling should only
152+
* happen when the user interacts with the segment.
153+
*/
154+
this.scrollActiveButtonIntoView(false);
149155
});
150156

151157
this.gesture = (await import('../../utils/gesture')).createGesture({
@@ -305,30 +311,52 @@ export class Segment implements ComponentInterface {
305311
}
306312
}
307313

308-
private scrollActiveButtonIntoView() {
309-
const { scrollable, value } = this;
314+
private scrollActiveButtonIntoView(smoothScroll = true) {
315+
const { scrollable, value, el } = this;
310316

311317
if (scrollable) {
312318
const buttons = this.getButtons();
313319
const activeButton = buttons.find((button) => button.value === value);
314320
if (activeButton !== undefined) {
321+
const scrollContainerBox = el.getBoundingClientRect();
322+
const activeButtonBox = activeButton.getBoundingClientRect();
323+
324+
/**
325+
* Subtract the active button x position from the scroll
326+
* container x position. This will give us the x position
327+
* of the active button within the scroll container.
328+
*/
329+
const activeButtonLeft = activeButtonBox.x - scrollContainerBox.x;
330+
331+
/**
332+
* If we just used activeButtonLeft, then the active button
333+
* would be aligned with the left edge of the scroll container.
334+
* Instead, we want the segment button to be centered. As a result,
335+
* we subtract half of the scroll container width. This will position
336+
* the left edge of the active button at the midpoint of the scroll container.
337+
* We then add half of the active button width. This will position the active
338+
* button such that the midpoint of the active button is at the midpoint of the
339+
* scroll container.
340+
*/
341+
const centeredX = activeButtonLeft - scrollContainerBox.width / 2 + activeButtonBox.width / 2;
342+
315343
/**
316-
* Scrollable segment buttons should be
317-
* centered within the view including
318-
* buttons that are partially offscreen.
344+
* We intentionally use scrollBy here instead of scrollIntoView
345+
* to avoid a WebKit bug where accelerated animations break
346+
* when using scrollIntoView. Using scrollIntoView will cause the
347+
* segment container to jump during the transition and then snap into place.
348+
* This is because scrollIntoView can potentially cause parent element
349+
* containers to also scroll. scrollBy does not have this same behavior, so
350+
* we use this API instead.
351+
*
352+
* Note that if there is not enough scrolling space to center the element
353+
* within the scroll container, the browser will attempt
354+
* to center by as much as it can.
319355
*/
320-
activeButton.scrollIntoView({
321-
behavior: 'smooth',
322-
inline: 'center',
323-
324-
/**
325-
* Segment should scroll on the
326-
* horizontal axis. `block: 'nearest'`
327-
* ensures that the vertical axis
328-
* does not scroll if the segment
329-
* as a whole is already in view.
330-
*/
331-
block: 'nearest',
356+
el.scrollBy({
357+
top: 0,
358+
left: centeredX,
359+
behavior: smoothScroll ? 'smooth' : 'instant',
332360
});
333361
}
334362
}

0 commit comments

Comments
 (0)