fix(PullToRefresh): refactor global touchmove listener #6540

inomdzhon · 2024-02-09T20:17:51Z

close [Bug]: PullToRefresh ломает другие компоненты, опирающиеся на Touch #6530

Unit-тесты

Описание

Глобальный обработчик на touchmove влиял на поведение элементов с горизонтальным скроллом (например, HorizontalScroll) – нужно было проводить пальцем ровно по оси X, чтобы проскроллить по горизонтали.

Видео 1. Поправленный пример из issue6530

issue6530.MOV

После удаления проблемы, из-за которой был добавлен этот обработчик (#5967), не смог воспроизвести. Даже если такая проблема и есть, то думаю она не критичней, в отличие от блокировки горизонтального скролла. Удалил тест, который проверял глобальный обработчик, т.к. он больше не проходит.

Заодно поправил проблему, когда при работе спиннера можно было бы потянуть сверху вниз и вызвать нативный pull-to-refresh – для исправления, мы сохраняем св-во overscroll-behavior на document.body пока не закончится обновление. Вынес логику в эффект.

Видео 2. Базовый пример после изменений

Есть момент, что при обновлении можно проскроллить вниз. Не стал этот момент трогать, т.к. PullToRefresh работает локально под конкретный контейнер. Ввиду этого, не уверен, что стоит блокировать скролл на document.body.

pr6540.MOV

caused by fix(PullToRefresh): Detect pull to refresh gesture in the document's touchmove #5967

Изменения

Избавился от useTimeout, т.к. он скорее усложняет код, чем облегчает его.
В cancelEvent убрал цикл while. Не понял для чего он. По истории коммитов, такая реализация была сразу PullToRefresh component #181. Возможности, раньше по особенном работал Touch.
Добавил больше данных в примере Storybook, чтобы увеличить высоту контента под скролл.

UPD

см. #6540 (comment)

- Глобальный обработчик touchmove влиял на поведение элементов с горизонтальным скроллом (например, HorizontalScroll) – нужно ровно по оси X проводить пальцем, чтобы заработало. После удаления проблему, из-за которой был добавлен этот обработчик, не смог воспроизвести. Даже если такая проблема есть, то думаю она не критичней, в отличии от блокировки горизонтального скролла. Удалил тест, который проверял глобальный обработчик. - Заодно поправил проблему, когда при работе спиннера можно было бы потянуть сверху вниз и вызвать нативный pull-to-refresh – для исправления, мы сохраняем св-во `overscroll-behavior` на `document.body` пока не закончится обновление. Вынес логику в эффект. - Избавился от `useTimeout`, т.к. он скорее усложняет код, чем облегчает его. - Добавил больше данные в примере Storybook, чтобы увеличить высоту контента под скролл.

github-actions · 2024-02-09T20:20:12Z

size-limit report 📦

Path	Size
JS	349.41 KB (+0.03% 🔺)
JS (gzip)	106.77 KB (-0.02% 🔽)
JS (brotli)	88.27 KB (-0.08% 🔽)
JS import Div (tree shaking)	1.43 KB (0%)
CSS	257.87 KB (0%)
CSS (gzip)	33.8 KB (0%)
CSS (brotli)	27.47 KB (0%)

codesandbox-ci · 2024-02-09T20:22:11Z

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 7612315:

Sandbox	Source
VKUI TypeScript	Configuration

github-actions · 2024-02-09T20:23:05Z

e2e tests

Playwright Report

github-actions · 2024-02-09T20:23:16Z

👀 Docs deployed

✅ Styleguide
✅ Storybook

Commit 7612315

packages/vkui/src/components/PullToRefresh/PullToRefresh.tsx

mendrew

Здорово! Вот бы ещё придумать всё же решение без overscroll-behavior, потому что он поддерживается в Safari только с 16 версии.
Это и была причина оставить код с onTouchStart.
#6004 (comment)

Даже в v6 у нас поддержка iOS 10.3.

inomdzhon · 2024-02-12T18:14:14Z

Здорово! Вот бы ещё придумать всё же решение без overscroll-behavior, потому что он поддерживается в Safari только с 16 версии. Это и была причина оставить код с onTouchStart. #6004 (comment)

Даже в v6 у нас поддержка iOS 10.3.

Вернул глобальный обработчик touchmove, но с некоторыми изменениями ⚡ 7612315

Работает не идеально, т.к. нативный pull-to-refresh всё-равно можно вызвать, но тем не менее уменьшен конфликт между нативной и нашей реализациями.

Видео 3. Пример на iOS 15 (в **Xcode**)

2024-02-12.20.59.17.mov

mendrew

Отлично, спасибо что снова вернул фоллбэк! 🥇

Прошлый фоллбэк тоже был не универсальным, потому и был добавлен вариант с overscroll-behavior, так что этого должно хватить. 🎉

vkcom-publisher · 2024-02-13T12:01:05Z

❌ Patch

Не удалось автоматически применить исправление на ветке 6.0-stable.

Дальнейшие действия выполняют контрибьютеры из группы @VKCOM/vkui-core

Чтобы изменение попало в ветку 6.0-stable, выполните следующие действия:

Создайте новую ветку от 6.0-stable и примените изменения используя cherry-pick

git stash # опционально
git fetch origin 6.0-stable
git checkout -b patch/pr6540 origin/6.0-stable

git cherry-pick --no-commit b96c9cfa84a667baf454dfac044e5fe8e77d26a5
git checkout HEAD **/__image_snapshots__/*.png
git diff --quiet HEAD || git commit --no-verify --no-edit

Исправьте конфликты, следуя инструкциям из терминала
Отправьте ветку на GitHub и создайте новый PR с веткой 6.0-stable (установка лейбла не требуется!)

git push --set-upstream origin patch/pr6540
gh pr create --base 6.0-stable --title "patch: pr6540" --body "- patch #6540"

h2. Описание Глобальный обработчик на `touchmove` влиял на поведение элементов с горизонтальным скроллом (например, `HorizontalScroll`) – нужно было проводить пальцем ровно по оси X, чтобы проскроллить по горизонтали. <details><summary>Видео 1. Поправленный пример из issue6530</summary> <p> https://github.com/VKCOM/VKUI/assets/5850354/d3aa291f-6274-470b-9ccb-3e9e19e544ab </p> </details> После удаления проблемы, из-за которой был добавлен этот обработчик (#5967), не смог воспроизвести. Даже если такая проблема и есть, то думаю она не критичней, в отличие от блокировки горизонтального скролла. Удалил тест, который проверял глобальный обработчик, т.к. он больше не проходит. Заодно поправил проблему, когда при работе спиннера можно было бы потянуть сверху вниз и вызвать нативный **pull-to-refresh** – для исправления, мы сохраняем св-во `overscroll-behavior` на `document.body` пока не закончится обновление. Вынес логику в эффект. <details><summary>Видео 2. Базовый пример после изменений</summary> <p> Есть момент, что при обновлении можно проскроллить вниз. Не стал этот момент трогать, т.к. `PullToRefresh` работает локально под конкретный контейнер. Ввиду этого, не уверен, что стоит блокировать скролл на `document.body`. https://github.com/VKCOM/VKUI/assets/5850354/b2baccc9-3cfa-415a-ab42-7ebd93c09114 </p> </details> - caused by #5967 h2. Изменения - Избавился от `useTimeout`, т.к. он скорее усложняет код, чем облегчает его. - В `cancelEvent` убрал цикл `while`. Не понял для чего он. По истории коммитов, такая реализация была сразу #181. Возможности, раньше по особенном работал `Touch`. - Добавил больше данных в примере Storybook, чтобы увеличить высоту контента под скролл. **UPD** см. #6540 (comment)

inomdzhon self-assigned this Feb 9, 2024

inomdzhon requested a review from a team as a code owner February 9, 2024 20:17

github-actions bot added the patch Автоматизация: PR продублируется в ветку последнего минорного релиза для выпуска патча label Feb 9, 2024

SevereCloud previously approved these changes Feb 12, 2024

View reviewed changes

BlackySoul reviewed Feb 12, 2024

View reviewed changes

packages/vkui/src/components/PullToRefresh/PullToRefresh.tsx Show resolved Hide resolved

shevchux reviewed Feb 12, 2024

View reviewed changes

packages/vkui/src/components/PullToRefresh/PullToRefresh.tsx Show resolved Hide resolved

mendrew reviewed Feb 12, 2024

View reviewed changes

fix(PullToRefresh): add JS fallback for overscroll-behavior CSS property

7612315

inomdzhon dismissed SevereCloud’s stale review via 7612315 February 12, 2024 18:09

mendrew approved these changes Feb 13, 2024

View reviewed changes

SevereCloud approved these changes Feb 13, 2024

View reviewed changes

inomdzhon merged commit b96c9cf into master Feb 13, 2024
23 checks passed

inomdzhon deleted the imirdzhamolov/issue6530/fix/PullToRefresh-global-touchmove-problem branch February 13, 2024 12:00

inomdzhon mentioned this pull request Feb 13, 2024

patch: pr6540 #6562

Merged

inomdzhon changed the title ~~fix(PullToRefresh): rm global touchmove listener~~ fix(PullToRefresh): refactor global touchmove listener Feb 13, 2024

BlackySoul mentioned this pull request Feb 28, 2024

[Bug]: PullToRefresh ломает iOS эластик скролла вниз #6645

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(PullToRefresh): refactor global touchmove listener #6540

fix(PullToRefresh): refactor global touchmove listener #6540

inomdzhon commented Feb 9, 2024 •

edited

Loading

github-actions bot commented Feb 9, 2024 •

edited

Loading

codesandbox-ci bot commented Feb 9, 2024 •

edited

Loading

github-actions bot commented Feb 9, 2024 •

edited

Loading

github-actions bot commented Feb 9, 2024 •

edited

Loading

mendrew left a comment

inomdzhon commented Feb 12, 2024

mendrew left a comment

vkcom-publisher commented Feb 13, 2024

fix(PullToRefresh): refactor global touchmove listener #6540

fix(PullToRefresh): refactor global touchmove listener #6540

Conversation

inomdzhon commented Feb 9, 2024 • edited Loading

Описание

Изменения

github-actions bot commented Feb 9, 2024 • edited Loading

size-limit report 📦

codesandbox-ci bot commented Feb 9, 2024 • edited Loading

github-actions bot commented Feb 9, 2024 • edited Loading

e2e tests

github-actions bot commented Feb 9, 2024 • edited Loading

👀 Docs deployed

mendrew left a comment

Choose a reason for hiding this comment

inomdzhon commented Feb 12, 2024

mendrew left a comment

Choose a reason for hiding this comment

vkcom-publisher commented Feb 13, 2024

❌ Patch

inomdzhon commented Feb 9, 2024 •

edited

Loading

github-actions bot commented Feb 9, 2024 •

edited

Loading

codesandbox-ci bot commented Feb 9, 2024 •

edited

Loading

github-actions bot commented Feb 9, 2024 •

edited

Loading

github-actions bot commented Feb 9, 2024 •

edited

Loading