Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(View): Restore scroll position on swipe back cancel #6325

Merged

Conversation

mendrew
Copy link
Contributor

@mendrew mendrew commented Dec 21, 2023


Описание

Мы очень хитро восстанавливаем позицию скролла, надеясь на то, что это всегда будет работать в useEffect, который сработает перед обработкой события transition (перед завершением анимации), где окончательно сбросятся все состояния.
Но в случае, если пользователь вернул панель назад, мы просто сбрасываем состояние компонента, отвечающее, за свайп. Логика, отвечающая за восстановление скролла не срабатывает, потому что это не считается как failure.

Вот тут мы решаем swipeBack был успешным (success), был отменён (fail), то есть свайп не закончен, либо это вообще не считается свайпом, потому что панель по завершении жеста осталась на той же позиции.

const handleTouchEndForIOSSwipeBackSimulation = (event: TouchEvent) => {
swipeBackPrevented.current = false;
if (swipingBack) {
const speed = (swipeBackShift / event.duration) * 1000;
if (swipeBackShift === 0) {
onSwipeBackCancel();
} else if (swipeBackShift >= (window!.innerWidth ?? 0)) {
onSwipeBackSuccess();
} else if (speed > 250 || swipeBackShift >= window!.innerWidth / 2) {
setSwipeBackResult('success');
} else {
setSwipeBackResult('fail');
}
}
};

Если это не свайп вовсе (пользователь вернул панель на место), то мы просто сбрасываем состояние свайпа с помощью функции onSwipeBackCancel.

Но в такой ситуации не сработает условие для восстановления скролла при отмене свайпа.

if (
prevSwipeBackResult === SwipeBackResults.fail &&
!swipeBackResult &&
activePanel !== null
) {
scroll?.scrollTo(0, scrolls.current[activePanel]);
}

Изменения

Вынес логику по восстановлению скролла при отмене свайпа в отдельный useEffect, потому что изначальный слишком большой.
Смотрю на переменные prevSwipingBack, swipingBack, чтобы понять был ли всё же свайп, потому что swipeBackResult нам ни о чем не скажет, даже если бы мы его устанавливали, то он не был бы тут же очищен в том же рендере из-за вызова onSwipeBackCancel. Также проверяю prevSwipeBackShift, который равен нулю, то есть смещения после жеста у панели нет, что значит, что пользователь жестом свайпа вернул панель туда откуда взял.

Видео

До

Screen.Recording.2023-12-21.at.18.11.29.mov

После

Screen.Recording.2023-12-21.at.18.12.23.mov

When user moves panel back to initial point on swipe back
Copy link
Contributor

github-actions bot commented Dec 21, 2023

size-limit report 📦

Path Size
JS 347.5 KB (+0.03% 🔺)
JS (gzip) 106.2 KB (+0.04% 🔺)
JS (brotli) 87.67 KB (+0.1% 🔺)
JS import Div (tree shaking) 1.43 KB (0%)
CSS 258 KB (0%)
CSS (gzip) 33.66 KB (0%)
CSS (brotli) 27.34 KB (0%)

Copy link

codesandbox-ci bot commented Dec 21, 2023

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 302e040:

Sandbox Source
VKUI TypeScript Configuration

Copy link
Contributor

github-actions bot commented Dec 21, 2023

e2e tests

Playwright Report

Copy link

codecov bot commented Dec 21, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (29679b8) 81.57% compared to head (302e040) 81.55%.
Report is 11 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6325      +/-   ##
==========================================
- Coverage   81.57%   81.55%   -0.02%     
==========================================
  Files         325      324       -1     
  Lines       10095    10092       -3     
  Branches     3382     3384       +2     
==========================================
- Hits         8235     8231       -4     
- Misses       1860     1861       +1     
Flag Coverage Δ
unittests 81.55% <100.00%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

github-actions bot commented Dec 21, 2023

👀 Docs deployed

Commit 302e040

@mendrew mendrew marked this pull request as ready for review December 21, 2023 15:09
@mendrew mendrew requested a review from a team as a code owner December 21, 2023 15:09
Copy link
Contributor

@inomdzhon inomdzhon left a comment

Choose a reason for hiding this comment

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

ШЕДЕВР!!! 💅

@mendrew mendrew merged commit 03d7a58 into master Dec 26, 2023
46 of 47 checks passed
@mendrew mendrew deleted the origin/mendrew/fix/6320/View/swipe-back-scroll-position-restore branch December 26, 2023 08:04
mendrew added a commit that referenced this pull request Jan 17, 2024
Cherry-picked из v6.

Мы очень хитро восстанавливаем позицию скролла, надеясь на то, что это всегда будет работать в useEffect, который сработает перед обработкой события transition (перед завершением анимации), где окончательно сбросятся все состояния.
Но в случае, если пользователь вернул панель назад, мы просто сбрасываем состояние компонента, отвечающее, за свайп. Логика, отвечающая за восстановление скролла не срабатывает, потому что это не считается как `failure`.

Вот тут мы решаем `swipeBack` был успешным (`success`), был отменён (`fail`), то есть свайп не закончен, либо это вообще не считается свайпом, потому что панель по завершении жеста осталась на той же позиции.
https://github.com/VKCOM/VKUI/blob/a4719b49f887c2584eec6655d72e373e62409c59/packages/vkui/src/components/View/View.tsx#L301-L316
Если это не свайп вовсе (пользователь вернул панель на место), то мы просто сбрасываем состояние свайпа с помощью функции `onSwipeBackCancel`.

Но в такой ситуации не сработает условие для восстановления скролла при отмене свайпа.
https://github.com/VKCOM/VKUI/blob/8dbb1de9855af8c772abcb719848175654e39a8a/src/components/View/View.tsx#L494-L500

- caused by #5725

-- Изменения
Вынес логику по восстановлению скролла при отмене свайпа в отдельный useEffect, потому что изначальный слишком большой.
Смотрю на переменные `prevSwipingBack`, `swipingBack`, чтобы понять был ли всё же свайп, потому что `swipeBackResult` нам ни о чем не скажет, даже если бы мы его устанавливали, то он не был бы тут же очищен в том же рендере из-за вызова `onSwipeBackCancel`. Также проверяю `prevSwipeBackShift`, который равен нулю, то есть смещения после жеста у панели нет, что значит, что пользователь жестом свайпа вернул панель туда откуда взял.
@mendrew mendrew mentioned this pull request Jan 17, 2024
mendrew added a commit that referenced this pull request Jan 18, 2024
Cherry-picked из v6.

Мы очень хитро восстанавливаем позицию скролла, надеясь на то, что это всегда будет работать в useEffect, который сработает перед обработкой события transition (перед завершением анимации), где окончательно сбросятся все состояния.
Но в случае, если пользователь вернул панель назад, мы просто сбрасываем состояние компонента, отвечающее, за свайп. Логика, отвечающая за восстановление скролла не срабатывает, потому что это не считается как `failure`.

Вот тут мы решаем `swipeBack` был успешным (`success`), был отменён (`fail`), то есть свайп не закончен, либо это вообще не считается свайпом, потому что панель по завершении жеста осталась на той же позиции.
https://github.com/VKCOM/VKUI/blob/a4719b49f887c2584eec6655d72e373e62409c59/packages/vkui/src/components/View/View.tsx#L301-L316
Если это не свайп вовсе (пользователь вернул панель на место), то мы просто сбрасываем состояние свайпа с помощью функции `onSwipeBackCancel`.

Но в такой ситуации не сработает условие для восстановления скролла при отмене свайпа.
https://github.com/VKCOM/VKUI/blob/8dbb1de9855af8c772abcb719848175654e39a8a/src/components/View/View.tsx#L494-L500

- caused by #5725

-- Изменения
Вынес логику по восстановлению скролла при отмене свайпа в отдельный useEffect, потому что изначальный слишком большой.
Смотрю на переменные `prevSwipingBack`, `swipingBack`, чтобы понять был ли всё же свайп, потому что `swipeBackResult` нам ни о чем не скажет, даже если бы мы его устанавливали, то он не был бы тут же очищен в том же рендере из-за вызова `onSwipeBackCancel`. Также проверяю `prevSwipeBackShift`, который равен нулю, то есть смещения после жеста у панели нет, что значит, что пользователь жестом свайпа вернул панель туда откуда взял.
vkcom-publisher pushed a commit that referenced this pull request Jan 18, 2024
Cherry-picked из v6.

Мы очень хитро восстанавливаем позицию скролла, надеясь на то, что это всегда будет работать в useEffect, который сработает перед обработкой события transition (перед завершением анимации), где окончательно сбросятся все состояния.
Но в случае, если пользователь вернул панель назад, мы просто сбрасываем состояние компонента, отвечающее, за свайп. Логика, отвечающая за восстановление скролла не срабатывает, потому что это не считается как `failure`.

Вот тут мы решаем `swipeBack` был успешным (`success`), был отменён (`fail`), то есть свайп не закончен, либо это вообще не считается свайпом, потому что панель по завершении жеста осталась на той же позиции.
https://github.com/VKCOM/VKUI/blob/a4719b49f887c2584eec6655d72e373e62409c59/packages/vkui/src/components/View/View.tsx#L301-L316
Если это не свайп вовсе (пользователь вернул панель на место), то мы просто сбрасываем состояние свайпа с помощью функции `onSwipeBackCancel`.

Но в такой ситуации не сработает условие для восстановления скролла при отмене свайпа.
https://github.com/VKCOM/VKUI/blob/8dbb1de9855af8c772abcb719848175654e39a8a/src/components/View/View.tsx#L494-L500

- caused by #5725

-- Изменения
Вынес логику по восстановлению скролла при отмене свайпа в отдельный useEffect, потому что изначальный слишком большой.
Смотрю на переменные `prevSwipingBack`, `swipingBack`, чтобы понять был ли всё же свайп, потому что `swipeBackResult` нам ни о чем не скажет, даже если бы мы его устанавливали, то он не был бы тут же очищен в том же рендере из-за вызова `onSwipeBackCancel`. Также проверяю `prevSwipeBackShift`, который равен нулю, то есть смещения после жеста у панели нет, что значит, что пользователь жестом свайпа вернул панель туда откуда взял.
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.

[Bug][View][iOS]: Теряется скролл при отмене swipeback
3 participants