Skip to content

Commit

Permalink
fix(HorizontalScroll): remove wheels event from container (#8293) (#8294
Browse files Browse the repository at this point in the history
)

- reverts #7882, #8177

h2. Описание

В **Firefox** событие `onwheel` на контейнере и одновременная прокрутка скроллируемого элемента (`scrollBy` или `scrollLeft`) **тач-падом** негативно влияет на перерисовку контента во время скролла.

Чтобы решить проблему #7774, можно навесить `onwheel` на каждую из стрелок навигации, но у этого решения есть свой баг: если начать крутить колесо с сначала или конца, то появление стрелки навигации заблоркирует прокрутку, то тех пор пока не двинешь мышкой над ней, чтобы началась обработка события `onwheel` на стрелке.

Из-за этих проблем оптимальным будет откатить фикс #7774 и пометить его как **Won't Fix**. Это будет являться ограничением компонента.

Тестировалось на версиях **Firefox** `134` и `135`.

Наблюдается с `v6.7.4` до `v7.1.1` при передаче `scrollOnAnyWheel`, а с `v7.1.2` всегда (связано с #8177).

https://github.com/user-attachments/assets/008907de-e6bd-4807-bbea-b0e7f69a8605

h2. Release notes
h2. Исправления
- HorizontalScroll: отменён фикс #7774 из-за проблем перерисовок в **Firefox** при прокрутке через тач-пад, теперь невозможность прокрутить колесом мыши над стрелкой навигации будет считаться ограничением компонента
  • Loading branch information
inomdzhon authored Feb 19, 2025
1 parent 662fb58 commit 499ac28
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 53 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,6 @@ const setup = (element: HTMLElement, startScrollLeft = 0) => {

jest.spyOn(element.firstElementChild!, 'scrollWidth', 'get').mockImplementation(() => 500);

// @ts-expect-error: TS2322 есть другой тип, но в компоненте он не используется
element.scrollBy = (options?: ScrollToOptions) => {
scrollLeft = scrollLeft + (options?.left || 0);
};

return {
get scrollLeft() {
return scrollLeft;
Expand Down Expand Up @@ -195,39 +190,6 @@ describe('HorizontalScroll', () => {
expect(mockedData.scrollLeft).toBe(200);
});
});

it('scroll by arrow', () => {
const ref: React.RefObject<HTMLDivElement | null> = {
current: null,
};
render(
<HorizontalScroll
getRef={ref}
data-testid="horizontal-scroll"
prevButtonTestId="scroll-arrow-left"
nextButtonTestId="scroll-arrow-right"
>
<div style={{ width: '1800px', height: '50px' }} />
</HorizontalScroll>,
);

const mockedData = setup(ref.current!, 50);

fireEvent.mouseEnter(screen.getByTestId('horizontal-scroll'));

const arrowLeft = screen.getByTestId('scroll-arrow-left');
const arrowRight = screen.getByTestId('scroll-arrow-right');

fireEvent.wheel(arrowRight, {
deltaX: 20,
});
expect(mockedData.scrollLeft).toBe(70);

fireEvent.wheel(arrowLeft, {
deltaX: 20,
});
expect(mockedData.scrollLeft).toBe(90);
});
});

function mockRef(element: HTMLDivElement) {
Expand Down
36 changes: 21 additions & 15 deletions packages/vkui/src/components/HorizontalScroll/HorizontalScroll.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,6 @@ export const HorizontalScroll = ({
scrollOnAnyWheel = false,
prevButtonTestId,
nextButtonTestId,
getRootRef,
...restProps
}: HorizontalScrollProps): React.ReactNode => {
const [canScrollLeft, setCanScrollLeft] = React.useState(false);
Expand All @@ -192,8 +191,6 @@ export const HorizontalScroll = ({

const scrollerRef = useExternRef(getRef, directionRef);

const rootRef = useExternRef(getRootRef);

const animationQueue = React.useRef<VoidFunction[]>([]);

const hasPointer = useAdaptivityHasPointer();
Expand Down Expand Up @@ -250,31 +247,40 @@ export const HorizontalScroll = ({

useIsomorphicLayoutEffect(
function addWheelEventHandler() {
if (!rootRef.current) {
const scrollEl = scrollerRef.current;
if (!scrollEl) {
return noop;
}
/**
* Прокрутка с помощью любого колеса мыши
*/
const onWheel = (e: WheelEvent) => {
const left = e.deltaX + (scrollOnAnyWheel ? e.deltaY : 0);
scrollerRef.current!.scrollBy({ left, behavior: 'auto' });
if (e.deltaY && scrollOnAnyWheel) {
e.preventDefault();
}
scrollerRef.current!.scrollBy({ left: e.deltaX + e.deltaY, behavior: 'auto' });
e.preventDefault();
};

const listenerOptions = { passive: false };
rootRef.current?.addEventListener('wheel', onWheel, listenerOptions);
// @ts-expect-error: TS2769 В интерфейсе EventListenerOptions для wheel нет passive свойства
return () => rootRef.current?.removeEventListener('wheel', onWheel, listenerOptions);

if (scrollOnAnyWheel) {
scrollEl.addEventListener('wheel', onWheel, listenerOptions);
}
scrollEl.addEventListener('scroll', calculateArrowsVisibility, listenerOptions);

return () => {
if (scrollOnAnyWheel) {
// @ts-expect-error: TS2769 В интерфейсе EventListenerOptions для wheel нет passive свойства
scrollEl.removeEventListener('wheel', onWheel, listenerOptions);
}
// @ts-expect-error: TS2769 В интерфейсе EventListenerOptions для scroll нет passive свойства
scrollEl.removeEventListener('scroll', calculateArrowsVisibility, listenerOptions);
};
},
[rootRef, scrollOnAnyWheel, scrollerRef],
[scrollOnAnyWheel, calculateArrowsVisibility, scrollerRef],
);

return (
<RootComponent
{...restProps}
getRootRef={rootRef}
baseClassName={classNames(
styles.host,
'vkuiInternalHorizontalScroll',
Expand Down Expand Up @@ -306,7 +312,7 @@ export const HorizontalScroll = ({
onClick={scrollToRight}
/>
)}
<div className={styles.in} ref={scrollerRef} onScroll={calculateArrowsVisibility}>
<div className={styles.in} ref={scrollerRef}>
<div className={styles.inWrapper}>{children}</div>
</div>
</RootComponent>
Expand Down

0 comments on commit 499ac28

Please sign in to comment.