fix(ModalRoot/ModalPage/PullToRefresh): Disable native pull-to-refresh #6004

mendrew · 2023-10-17T08:34:10Z

close [Bug] На iOS двигается страница при закрытии модалки в браузере #2806
related [Bug]: PullToRefresh на мобильном сафари нестабильно работает #5670 и fix(PullToRefresh): Detect pull to refresh gesture in the document's touchmove #5967

Unit-тесты

Описание

Ещё одна попытка победить нативный pull-to-refresh который периодически появляется при закрытии модалки или в PullToRefresh компоненте.

В целом событие touchmove на window уже игнорируется, но этого недостаточно, потому что для pull-to-refresh в мобильном Safari хватит и одного обработанного браузеромtouchstart.

Используется css свойство overscroll-behavior на html элементе.

Перепробывал кучу других вариантов с touchstart/touchmove событиями, но единственный действенный это превентить touchstart. Но тогда перестаёт работать скролл внутри PullToRefresh, а через JS имитировать скролл браузера неблагодарная задача.

Изменения

добавил глобальный класс, которые в последствии будут вешаться на html элемент при открытии модалки или при взаимодействии с PullToRefresh.
Сейчас уже работает наверняка для PullToRefresh компонента и покрывает баг в модалках если закрывать окно потянув из ModalPageHeader.

Используем классы, а не напрямую меняем style, чтобы не думать и не сохранять предыдущие значения overscroll-behavior, это также позволяет избавиться от чтения/записи стилей, чтобы не заставлять браузер готовить и пересчитывать для нас CSSOM в момент чтения/записи style.

ModalPage

устанавливаем overscroll-behavior-y: none когда модалка открыта

PullToRefresh

класс вешается на html на событии touchstart и убирается в обработчике собитий touchend/touchcancel.

Особенности PullToRefresh.

Так как убирается bounce-эффект, то страница становится менее отзывчивая (без эффекта) если это единственный компонент на странице и пользователь доскролил до конца страницы.
Но зато можно начать наш PullToRefresh даже если страница уже была чуть проскроллена вниз, а пользователь начал тянуть. Раньше можно было вызвать наш PullToRefresh только если страница имеет scrollTop = 0 в момент начала жеста.

Есть вариант добиться bounce эффекта когда пользователь доскроли до конца страницы.
Например, добавить к уже имеющейся логику в touchmove и смотреть в какую сторону в пределах PullToRefresh пользователь скроллит, и если это скролл вниз (пальцем снизу вверх), то убирать класс, если снова скролл вверх, то добавлять класс.
Но это так себе решение, потому что мы всегда в touchstart выставляем класс, и если пользователь сразу скроллит вниз, то получится, что класс добавиться и тут же убереться. Хотя, не сильно хуже чем сейчас, конечно, потому что мы сейчас и так всегда класс добавляем а на touchend/touchcancel убираем.
И ещё момент появится, что если пользователь уже доскроллил до конца страницы и попробует поскролить вниз ещё раз, то bounce-эффекта не будет, ведь на touchmove мы не поймём в какую сторону пользователь скролит. Хотя тут можно поиграться. И посмотреть scrollTop значение. 🤔
Вопрос только стоит ли?

Видео

ModalPage

До	После
2023-10-17.20.09.26.mov	2023-10-17.19.59.02.mov

PullToRefresh

2023-10-17.19.49.44.mov

Use overscrollBehavior: 'none' on html elemeht when modal open

github-actions · 2023-10-17T08:39:03Z

size-limit report 📦

Path	Size
JS	331.33 KB (+0.1% 🔺)
JS (gzip)	101.25 KB (+0.08% 🔺)
JS (brotli)	83.52 KB (-0.02% 🔽)
JS import Div (tree shaking)	2.71 KB (0%)
CSS	255.99 KB (+0.03% 🔺)
CSS (gzip)	33.61 KB (+0.07% 🔺)
CSS (brotli)	27.27 KB (+0.13% 🔺)

codesandbox-ci · 2023-10-17T08:39:36Z

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 92448d1:

Sandbox	Source
VKUI TypeScript	Configuration
peaceful-black-48vf58	Issue #2806

github-actions · 2023-10-17T08:40:57Z

e2e tests

Playwright Report

github-actions · 2023-10-17T08:41:17Z

👀 Docs deployed

✅ Styleguide
✅ Storybook

Commit 92448d1

codecov · 2023-10-17T08:42:00Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (0a17a44) 79.16% compared to head (92448d1) 79.21%.
Report is 6 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6004      +/-   ##
==========================================
+ Coverage   79.16%   79.21%   +0.04%     
==========================================
  Files         306      305       -1     
  Lines        9585     9588       +3     
  Branches     3239     3244       +5     
==========================================
+ Hits         7588     7595       +7     
+ Misses       1997     1993       -4

Flag	Coverage Δ
unittests	`79.21% <100.00%> (+0.04%)`	⬆️

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

Files	Coverage Δ
...ckages/vkui/src/components/ModalRoot/ModalRoot.tsx	`53.48% <100.00%> (+0.76%)`	⬆️
...kui/src/components/PullToRefresh/PullToRefresh.tsx	`96.55% <100.00%> (+0.09%)`	⬆️

... and 11 files with indirect coverage changes

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

Adding class to html when modal is open Adding class to html when PullToRefresh touchstart event fired and remove on touchend/touchcancel

inomdzhon

🔥🔥🔥

добавил пару правок по тексту комментов

стоит иметь ввиду, что overscroll-behavior с Safaru >= 16 версии поддерживается

packages/vkui/src/styles/common.css

mendrew · 2023-10-19T14:12:20Z

@inomdzhon Спасибо за ревью!

стоит иметь ввиду, что overscroll-behavior с Safaru >= 16 версии поддерживается

Точно же, я так зарылся в проблему, что забыл ещё раз проверить поддержку.
Тогда я верну обратно код который улучшил в прошлый раз поведение внутри PullToRefresh (#5967), чтобы он работал когда overscroll-behavior не поддерживается.

Co-authored-by: Inomdzhon Mirdzhamolov <[email protected]>

As it still possible to trigger native pull-to-refresh with overscroll-behavior: none in mobile Safari

mendrew · 2023-10-20T10:28:03Z

@inomdzhon

⚡ 1107cd8 вернул код, улучшающий PullToRefresh (fix(PullToRefresh): Detect pull to refresh gesture in the document's touchmove #5967)
⚡92448d1 в модалке теперь тоже как и в PullToRefresh теперь используется overscroll-behavior: none так как с overscroll-behavior: contain я всё же поймал нативный pull-to-refresh в симуляторе

inomdzhon

🤝

#6004) - close #2806 - related #5670 и #5967 * Описание Ещё одна попытка победить нативный pull-to-refresh который периодически появляется при закрытии модалки или в PullToRefresh компоненте. В целом событие `touchmove` на window уже игнорируется, но этого недостаточно, потому что для pull-to-refresh в мобильном Safari хватит и одного обработанного браузером`touchstart`. Используется css свойство [`overscroll-behavior`](https://developer.mozilla.org/en-US/docs/Web/CSS/overscroll-behavior) на `html` элементе. Перепробывал кучу других вариантов с `touchstart/touchmove` событиями, но единственный действенный это превентить `touchstart`. Но тогда перестаёт работать скролл внутри PullToRefresh, а через JS имитировать скролл браузера неблагодарная задача. * Изменения добавил глобальный класс, которые в последствии будут вешаться на `html` элемент при открытии модалки или при взаимодействии с PullToRefresh. Сейчас уже работает наверняка для `PullToRefresh` компонента и покрывает баг в модалках если закрывать окно потянув из `ModalPageHeader`. Используем классы, а не напрямую меняем style, чтобы не думать и не сохранять предыдущие значения `overscroll-behavior`, это также позволяет избавиться от чтения/записи стилей, чтобы не заставлять браузер готовить и пересчитывать для нас CSSOM в момент чтения/записи style. ** ModalPage - устанавливаем `overscroll-behavior-y: none` когда модалка открыта *** PullToRefresh - класс вешается на `html` на событии `touchstart` и убирается в обработчике собитий `touchend/touchcancel`. * Особенности PullToRefresh. Так как убирается bounce-эффект, то страница становится менее отзывчивая (без эффекта) если это единственный компонент на странице и пользователь доскролил до конца страницы. Но зато можно начать наш PullToRefresh даже если страница уже была чуть проскроллена вниз, а пользователь начал тянуть. Раньше можно было вызвать наш PullToRefresh только если страница имеет `scrollTop = 0` в момент начала жеста. Есть вариант добиться `bounce` эффекта когда пользователь доскроли до конца страницы. Например, добавить к уже имеющейся логику в `touchmove` и смотреть в какую сторону в пределах `PullToRefresh` пользователь скроллит, и если это скролл вниз (пальцем снизу вверх), то убирать класс, если снова скролл вверх, то добавлять класс. Но это так себе решение, потому что мы всегда в `touchstart` выставляем класс, и если пользователь сразу скроллит вниз, то получится, что класс добавиться и тут же убереться. Хотя, не сильно хуже чем сейчас, конечно, потому что мы сейчас и так всегда класс добавляем а на touchend/touchcancel убираем. И ещё момент появится, что если пользователь уже доскроллил до конца страницы и попробует поскролить вниз ещё раз, то bounce-эффекта не будет, ведь на touchmove мы не поймём в какую сторону пользователь скролит. Хотя тут можно поиграться. И посмотреть scrollTop значение. 🤔 Вопрос только стоит ли?

h2. Описание В ущерб проблеме, что при сворачивании `ModalPage` и `ModalCard` может вызываться **pull-to-refresh** (см. #3416), удаляем полифил на `overscroll-behavior-y: none` – метод `preventTouch()`. Удалям по причине не работающих скроллов в интерфейсе – они критичная пробелма, т.к. блокирует действия пользователя. В PR #6004 добавили установку `overscroll-behavior-y: none` на `html`, поэтому блокирование **pull-to-refresh** будет в браузерах, где это CSS свойство поддерживается (см. [overscroll-behavior-y | Browser compatibility](https://developer.mozilla.org/en-US/docs/Web/CSS/overscroll-behavior-y#browser_compatibility)). Добавил сноску в документации `ModalRoot/Readme.md` про это ограничение. h2. Изменения - поправил, чтобы `overscroll-behavior-y: none` устанавливался и при первом рендере если `activeModal !== null`; - удалил свойство `documentScrolling`, т.к. уже не нужен – до этого это условие было полезным, чтобы не плодить события `touchmove`.

Disable native pull-to-refresh when modal open

39cad39

Use overscrollBehavior: 'none' on html elemeht when modal open

github-actions bot added patch Автоматизация: PR продублируется в ветку последнего минорного релиза для выпуска патча v5 Автоматизация: PR продублируется в ветку v5 labels Oct 17, 2023

mendrew added 2 commits October 17, 2023 15:30

Disable native pull-to-refresh using overscroll-behavior

cb6fb99

Adding class to html when modal is open Adding class to html when PullToRefresh touchstart event fired and remove on touchend/touchcancel

Add tests

3117c00

mendrew changed the title ~~fix(ModalRoot/ModalPage): Disable native pull-to-refresh when modal open~~ fix(ModalRoot/ModalPage/PullToRefresh): Disable native pull-to-refresh Oct 17, 2023

mendrew marked this pull request as ready for review October 17, 2023 17:41

mendrew requested a review from a team as a code owner October 17, 2023 17:41

inomdzhon reviewed Oct 19, 2023

View reviewed changes

packages/vkui/src/styles/common.css Outdated Show resolved Hide resolved

packages/vkui/src/styles/common.css Outdated Show resolved Hide resolved

mendrew marked this pull request as draft October 19, 2023 14:12

mendrew and others added 4 commits October 19, 2023 17:41

Improve comment

511551c

Co-authored-by: Inomdzhon Mirdzhamolov <[email protected]>

Improve comment

513b6e8

Co-authored-by: Inomdzhon Mirdzhamolov <[email protected]>

Revert the code from the 0a17a44 fix

1107cd8

Use overscroll-behavior: none in ModalRoot

92448d1

As it still possible to trigger native pull-to-refresh with overscroll-behavior: none in mobile Safari

mendrew marked this pull request as ready for review October 20, 2023 10:28

mendrew requested review from inomdzhon and a team October 20, 2023 12:38

inomdzhon approved these changes Oct 20, 2023

View reviewed changes

SevereCloud approved these changes Oct 23, 2023

View reviewed changes

mendrew merged commit 71b5a0b into master Oct 23, 2023

mendrew deleted the mendrew/fix/2806/ModalRoot/disable-native-pull-to-refresh-when-modal-open branch October 23, 2023 08:58

mendrew mentioned this pull request Feb 12, 2024

fix(PullToRefresh): refactor global touchmove listener #6540

Merged

1 task

inomdzhon mentioned this pull request Aug 6, 2024

fix(ModalRoot): rm preventTouch() #7325

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(ModalRoot/ModalPage/PullToRefresh): Disable native pull-to-refresh #6004

fix(ModalRoot/ModalPage/PullToRefresh): Disable native pull-to-refresh #6004

mendrew commented Oct 17, 2023 •

edited

Loading

github-actions bot commented Oct 17, 2023 •

edited

Loading

codesandbox-ci bot commented Oct 17, 2023 •

edited

Loading

github-actions bot commented Oct 17, 2023 •

edited

Loading

github-actions bot commented Oct 17, 2023 •

edited

Loading

codecov bot commented Oct 17, 2023 •

edited

Loading

inomdzhon left a comment

mendrew commented Oct 19, 2023

mendrew commented Oct 20, 2023

inomdzhon left a comment

fix(ModalRoot/ModalPage/PullToRefresh): Disable native pull-to-refresh #6004

fix(ModalRoot/ModalPage/PullToRefresh): Disable native pull-to-refresh #6004

Conversation

mendrew commented Oct 17, 2023 • edited Loading

Описание

Изменения

ModalPage

PullToRefresh

Особенности PullToRefresh.

Видео

ModalPage

PullToRefresh

github-actions bot commented Oct 17, 2023 • edited Loading

size-limit report 📦

codesandbox-ci bot commented Oct 17, 2023 • edited Loading

github-actions bot commented Oct 17, 2023 • edited Loading

e2e tests

github-actions bot commented Oct 17, 2023 • edited Loading

👀 Docs deployed

codecov bot commented Oct 17, 2023 • edited Loading

Codecov Report

inomdzhon left a comment

Choose a reason for hiding this comment

mendrew commented Oct 19, 2023

mendrew commented Oct 20, 2023

inomdzhon left a comment

Choose a reason for hiding this comment

mendrew commented Oct 17, 2023 •

edited

Loading

github-actions bot commented Oct 17, 2023 •

edited

Loading

codesandbox-ci bot commented Oct 17, 2023 •

edited

Loading

github-actions bot commented Oct 17, 2023 •

edited

Loading

github-actions bot commented Oct 17, 2023 •

edited

Loading

codecov bot commented Oct 17, 2023 •

edited

Loading