Skip to content

Commit

Permalink
fix(View): edit isSwipeBackTarget predicate (#7392)
Browse files Browse the repository at this point in the history 
h2. Описание

Была проблема, что до #6979:

1. переменная `prevSwipeBackResult` использовалась в `React.useEffect()`, из-за чего она была всегда актуальная, т.к. обращались мы к ней после окончания рендера. В #6979 она переместилась в **render prop**, где мы обращаемся к переменной до окончания рендера, поэтому и получалось так, что свайп бёк срабатывал в первый раз, пока `prevSwipeBackResult` являлся `null`, а во второй раз уже не срабатывал, т.к. `prevSwipeBackResult` имел предыдущее состояние. Изучил, что проверку на `prevSwipeBackResult` в целом можно удалить.

2. Событие `transitionend` навешивалось в обход **React**, а также был фолбек с `setTimeout()`. В #6979 события навешиваются через **React** – `onTransitionEnd`. Иногда обработчик не вызывался. Чтобы исправить это, перенёс навешивания события на текущий элемент (`isSwipeBackPrev`) вместо последующего (`isSwipeBackNext`). Поправил тесты под это изменение.

h2. Изменения

- удалил `React.useCallback()` на обработчике события `onAnimationEnd`, т.к. это обычный `<div />` и мемоизация ни к чему;
- заодно отрефакторил `calcPanelSwipeStyles()` и `calcPanelSwipeBackOverlayStyles()`.
  • Loading branch information
@inomdzhon @actions-user
inomdzhon authored and actions-user committed Aug 19, 2024
1 parent e1c5d25 commit b965342
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 40 deletions.
8 changes: 4 additions & 4 deletions packages/vkui/src/components/View/View.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ describe(View, () => {
fireEvent.mouseUp(view);
expect(events.onSwipeBack).not.toHaveBeenCalled();
expect(events.onSwipeBackCancel).not.toHaveBeenCalled();
await waitCSSTransitionEnd(getViewPanelById('p1'), { propertyName: 'transform' });
await waitCSSTransitionEnd(getViewPanelById('p2'), { propertyName: 'transform' });
expect(events.onSwipeBack).toHaveBeenCalledTimes(1);
});
it('should swipe back by start touch anywhere', async () => {
Expand All @@ -231,7 +231,7 @@ describe(View, () => {
expect(events.onSwipeBackStart).toHaveBeenCalledTimes(1);

fireEvent.mouseUp(view);
await waitCSSTransitionEnd(getViewPanelById('p1'), {
await waitCSSTransitionEnd(getViewPanelById('p2'), {
propertyName: 'transform',
});

Expand Down Expand Up @@ -343,7 +343,7 @@ describe(View, () => {

scrollToLeftAndSwipe(100);
expect(events.onSwipeBackStart).toHaveBeenCalledTimes(1);
await waitCSSTransitionEnd(getViewPanelById('p1'), { propertyName: 'transform' });
await waitCSSTransitionEnd(getViewPanelById('p2'), { propertyName: 'transform' });
expect(events.onSwipeBack).toHaveBeenCalledTimes(1);
expect(events.onSwipeBackCancel).not.toHaveBeenCalled();

Expand Down Expand Up @@ -386,7 +386,7 @@ describe(View, () => {

scrollToLeftAndSwipe(0);
expect(events.onSwipeBackStart).toHaveBeenCalledTimes(1);
await waitCSSTransitionEnd(getViewPanelById('p1'), { propertyName: 'transform' });
await waitCSSTransitionEnd(getViewPanelById('p2'), { propertyName: 'transform' });
expect(events.onSwipeBack).toHaveBeenCalledTimes(1);
expect(events.onSwipeBackCancel).not.toHaveBeenCalled();
});
Expand Down
57 changes: 21 additions & 36 deletions packages/vkui/src/components/View/View.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import * as React from 'react';
import { classNames } from '@vkontakte/vkjs';
import { usePlatform } from '../../hooks/usePlatform';
import { usePrevious } from '../../hooks/usePrevious';
import { blurActiveElement, canUseDOM, useDOM } from '../../lib/dom';
import { blurActiveElement, useDOM } from '../../lib/dom';
import { getNavId, NavIdProps } from '../../lib/getNavId';
import { warnOnce } from '../../lib/warnOnce';
import { HTMLAttributesWithRootRef } from '../../types';
Expand Down Expand Up @@ -156,11 +156,11 @@ export const View = ({
[activePanelProp, layoutEffectCall, onTransition, scroll],
);

const onAnimationEnd = React.useCallback(() => {
const handleAnimatedTargetAnimationEnd = () => {
if (prevPanel !== null) {
flushTransition(prevPanel, Boolean(isBack));
}
}, [flushTransition, isBack, prevPanel]);
};

const onSwipeBackSuccess = React.useCallback(() => {
onSwipeBack && onSwipeBack();
Expand Down Expand Up @@ -275,56 +275,41 @@ export const View = ({
}
};

const calcPanelSwipeStyles = (panelId: string | undefined): React.CSSProperties => {
if (!canUseDOM || !window) {
return {};
}

const isPrev = panelId === swipeBackPrevPanel;
const isNext = panelId === swipeBackNextPanel;

const calcPanelSwipeStyles = (isPrev: boolean, isNext: boolean): React.CSSProperties => {
if ((!isPrev && !isNext) || swipeBackResult) {
return {};
}

let prevPanelTranslate = `${swipeBackShift}px`;
let nextPanelTranslate = `${-50 + (swipeBackShift * 100) / window.innerWidth / 2}%`;

if (isNext) {
return {
transform: `translate3d(${nextPanelTranslate}, 0, 0)`,
};
return window
? {
transform: `translate3d(${-50 + (swipeBackShift * 100) / window.innerWidth / 2}%, 0, 0)`,
}
: {};
}

if (isPrev) {
return {
transform: `translate3d(${prevPanelTranslate}, 0, 0)`,
};
return { transform: `translate3d(${swipeBackShift}px, 0, 0)` };
}

return {};
};

const calcPanelSwipeBackOverlayStyles = (panelId?: string): React.CSSProperties => {
if (!canUseDOM || !window) {
const calcPanelSwipeBackOverlayStyles = (isNext: boolean): React.CSSProperties => {
if (!window || !isNext) {
return {};
}

const isNext = panelId === swipeBackNextPanel;
if (!isNext) {
return {};
}

const calculatedOpacity = 1 - swipeBackShift / window.innerWidth;
const opacityOnSwipeEnd =
swipeBackResult === 'success' ? 0 : swipeBackResult === 'fail' ? 1 : null;

return {
display: 'block',
opacity: opacityOnSwipeEnd === null ? calculatedOpacity : opacityOnSwipeEnd,
opacity:
opacityOnSwipeEnd === null ? 1 - swipeBackShift / window.innerWidth : opacityOnSwipeEnd,
};
};

const onTransitionEnd = (event: React.TransitionEvent<HTMLDivElement>) => {
const handleSwipeBackTargetTransitionEnd = (event: React.TransitionEvent<HTMLDivElement>) => {
if (event.propertyName.includes('transform')) {
swipingBackTransitionEndHandler();
}
Expand Down Expand Up @@ -482,7 +467,7 @@ export const View = ({

const isSwipeBackPrev = panelId === swipeBackPrevPanel;
const isSwipeBackNext = panelId === swipeBackNextPanel;
const isSwipeBackTarget = !prevSwipeBackResult && swipeBackResult && isSwipeBackNext;
const isSwipeBackTarget = swipeBackResult && isSwipeBackPrev;

let scrollCompensateStyle: React.CSSProperties | undefined = undefined;

Expand All @@ -507,16 +492,16 @@ export const View = ({
swipeBackResult === 'success' && styles['View__panel--swipe-back-success'],
swipeBackResult === 'fail' && styles['View__panel--swipe-back-failed'],
)}
onTransitionEnd={isSwipeBackTarget ? onTransitionEnd : undefined}
onAnimationEnd={isAnimatedTarget ? onAnimationEnd : undefined}
onTransitionEnd={isSwipeBackTarget ? handleSwipeBackTargetTransitionEnd : undefined}
onAnimationEnd={isAnimatedTarget ? handleAnimatedTargetAnimationEnd : undefined}
ref={(el) => panelId !== undefined && (panelNodes.current[panelId] = el)}
style={calcPanelSwipeStyles(panelId)}
style={calcPanelSwipeStyles(isSwipeBackPrev, isSwipeBackNext)}
key={panelId}
>
{platform === 'ios' && (
<div
className={styles['View__panel-overlay']}
style={calcPanelSwipeBackOverlayStyles(panelId)}
style={calcPanelSwipeBackOverlayStyles(isSwipeBackNext)}
/>
)}
<div className={styles['View__panel-in']} style={scrollCompensateStyle}>
Expand Down

0 comments on commit b965342

Please sign in to comment.