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

[Feature] Переписать модальные окна #2449

Open
stoope opened this issue Apr 28, 2022 · 7 comments · May be fixed by #5092
Open

[Feature] Переписать модальные окна #2449

stoope opened this issue Apr 28, 2022 · 7 comments · May be fixed by #5092

Comments

@stoope
Copy link
Contributor

stoope commented Apr 28, 2022

Текущая реализация модальных окон требует много времени на поддержку со стороны разработчиков и содержит много багов. Необходимо подумать как можно упростить модальные окна

UPD
Необходимо задепрекейтить ModalRoot и создать вместо него ModalManager.

@stoope stoope self-assigned this Apr 28, 2022
@k-egor-smirnov
Copy link
Contributor

Хороший пример в AntD есть: https://ant.design/components/modal/. Модалка вызывается из любого места. Конечно, тут нужно придумать, как предотвратить открытие сразу нескольких модалок одновременно, но это точно решаемая проблема

@stoope
Copy link
Contributor Author

stoope commented Apr 28, 2022

Хороший пример в AntD есть: https://ant.design/components/modal/. Модалка вызывается из любого места. Конечно, тут нужно придумать, как предотвратить открытие сразу нескольких модалок одновременно, но это точно решаемая проблема

Ага, что то подобное планирую. А открытие нескольких модалок одновременно стоит контролировать со стороны VKUI или дать возможность разработчику это контролировать? У нас есть какие то кейсы когда это нужно?

@k-egor-smirnov
Copy link
Contributor

Текущая дизайн система не предусматривает открытие сразу нескольких модалок. Так что, как мне кажется, тут есть два варианта:

  1. При открытии второй модалки скрывать предыдущую в стэк и возвращать при закрытии текущей.
  2. Кидать warn в консоль и ничего не делать, пока не закроется предыдущая

@stoope
Copy link
Contributor Author

stoope commented Apr 28, 2022

Текущая дизайн система не предусматривает открытие сразу нескольких модалок. Так что, как мне кажется, тут есть два варианта:

  1. При открытии второй модалки скрывать предыдущую в стэк и возвращать при закрытии текущей.
  2. Кидать warn в консоль и ничего не делать, пока не закроется предыдущая

Я за первый вариант

@inomdzhon
Copy link
Contributor

Текущая дизайн система не предусматривает открытие сразу нескольких модалок. Так что, как мне кажется, тут есть два варианта:

  1. При открытии второй модалки скрывать предыдущую в стэк и возвращать при закрытии текущей.
  2. Кидать warn в консоль и ничего не делать, пока не закроется предыдущая

Возможно стоит с дизайном обсудить этот момент

@thoughtspile
Copy link
Contributor

Напомню что я уже делал подход к этому вопросику: #2182

@SevereCloud SevereCloud linked a pull request May 18, 2023 that will close this issue
@SevereCloud SevereCloud added this to the v6 milestone May 18, 2023
@SevereCloud SevereCloud removed the design Нужно участие команды дизайна label May 18, 2023
@SevereCloud SevereCloud self-assigned this Jun 26, 2023
@inomdzhon inomdzhon modified the milestones: v6, v6.1.0 Jan 16, 2024
@SevereCloud SevereCloud removed this from the v6.1.0 milestone Apr 15, 2024
@inomdzhon inomdzhon mentioned this issue Jun 6, 2024
2 tasks
inomdzhon added a commit that referenced this issue Jun 20, 2024
h2. Описание

В CSS файлах применил медиа-выражение `@media screen and (prefers-reduced-motion: reduce)` там, где необходимо. Попутно рефакторил некоторые из них. Например, примененил токен `--vkui--animation_duration_m` где это возможно.

> [!NOTE]
> 
> Упрощал основываясь на поведении **iOS** и приложений под него. При включении параметра **Уменьшение движения** + **Предпочтение перекрёстным наплывам** (_Настройки_ -> _Универсальный доступ_ -> _Движение_) переходы, зачастую, заменяются на анимацию через смену прозрачности.

Затронутые анимации:

> [!NOTE]
> Длительность и изинги **НЕ** трогал, только подменял изменение размеров и/или передвижения.

- `ActionSheet`
  - параметры анимации вынесены в переменные `--vkui_internal--*`;
  - анимация появления/скрытия упрощается до появление через прозрачность.
  - для анимаций теперь используется `useCSSKeyframesAnimationController()`.
- `Alert`
  - параметры анимации вынесены в переменные `--vkui_internal--*`;
  - анимация появления/скрытия упрощается до появление через прозрачность;
  - добавил анимацию для десктоп (#2250);
  - для анимаций теперь используется `useCSSKeyframesAnimationController()`.
- `Button`
  - удалён `@media screen and (prefers-reduced-motion: no-preference)`, т.к. допустимая анимация.
- `PanelHeaderContext`
  - параметры анимации вынесены в переменные `--vkui_internal--*`;
  - для анимаций теперь используется `useCSSKeyframesAnimationController()`.
  - `useGlobalEventListener()` заменён на `useGlobalOnClickOutside()`. И теперь, если `visible === false`, то компонент не остаётся в DOM;
  - теперь при `visible === false` возвращаем `null`.
- `Removable`
  - удалён `@media screen and (prefers-reduced-motion: no-preference)`, т.к. допустимая анимация.
- `Root`
  - анимация появления/скрытия упрощается до появление через прозрачность.
- `Skeleton`
  - анимации отключаются.
- `Snackbar`
  - анимация появления/скрытия упрощается до появление через прозрачность;
  - остаётся возможность закрыть через свайп – вызывает закрытие через прозрачность.
- `Switch`
  - удалён `@media screen and (prefers-reduced-motion: no-preference)`, т.к. допустимая анимация.
- `View`
  - анимация появления/скрытия упрощается до появление через прозрачность;
  - остаётся возможность свайпать;
  - в примеры документации добавил кнопки "Назад" в `PanelHeader`.
- `animationFades.module.css`
  - удалён `@media screen and (prefers-reduced-motion: no-preference)`, т.к. допустимая анимация.
- `focusVisible.module.css`
  - заменил `@media screen and (prefers-reduced-motion: no-preference)` на `@media screen and (prefers-reduced-motion: reduce)`

h3. Что не затронул?

Не трогал мобильный `ModalRoot` дабы не сломать его. Лучше сделать в рамках #2449.

h2. Остальные изменения

- в `docs/CONTRIBUTING.md` добавил информацию про a11y (заодно немного освежил документ);
- удалён хук `useTimeout()` и везде заменён на ручной запуск и очистку – этот хук больше усложняет код, чем упрощает;
- удалён `lib/supportEvents.ts`, т.к. все браузеры из текущего `.browserslistrc` поддерживают событие [transitionend](https://developer.mozilla.org/ru/docs/Web/API/Element/transitionend_event#%D1%81%D0%BE%D0%B2%D0%BC%D0%B5%D1%81%D1%82%D0%B8%D0%BC%D0%BE%D1%81%D1%82%D1%8C_%D1%81_%D0%B1%D1%80%D0%B0%D1%83%D0%B7%D0%B5%D1%80%D0%B0%D0%BC%D0%B8);
~- удалён хук `useReducedMotion()` – на текущий момент больше не используется, т.к. оказывается `(prefers-reduced-motion: reduce)` не предполагает полное отключение всех анимаций, а лишь говорит, что надо их упростить.~ (**UPD** вернул в рамках ⚡ c89d7d1).

Эти изменения привели к следующему:

- `useWaitTransitionFinish()` – теперь и возвращает сразу функцию не оборачивая его в объект;
- `PopoutWrapper` – теперь компонент неконтролируемый – показывается/скрывается в зависимости от параметра `closing`.

h3. Типы

- добавлен `TimeoutId`, чтобы не приходилось каждый раз писать `ReturnType<typeof setTimeout> | null;`.

h3. Тесты

- применён `waitCSSKeyframeAnimationEnd()` там, где раньше была завязка на `setTimeout`;
- добавлена утилита `waitCSSTransitionEnd()`;
- добавлен мок для `TransitionEvent`.
@SevereCloud SevereCloud removed their assignment Aug 12, 2024
@k-egor-smirnov
Copy link
Contributor

Вот чего очень не хватает в текущих модалках:

  1. Динамического добавления. Это самая важная хотелка. Если оставлять в текущем виде ModalRoot, то хочется научить его работать с модалками, которые динамически в него подключаются. С этим хотя бы можно будет как-то работать через контексты.
  2. Декларативного рендера. Хочется прямо внутри JSX панели иметь возможность рендерить модалку в любом месте внутри структуры компонента. Это можно подвязать к a11y: можно будет обернуть кнопку в компонент модалки, и кнопка будет знать, что по её клику будет открыт какой-то попап.
  3. Возможности использовать FixedLayout внутри модалки. Юзкейс: панель нового комментария.

@inomdzhon inomdzhon self-assigned this Sep 5, 2024
@inomdzhon inomdzhon added this to the v7.0.0 milestone Sep 5, 2024
@inomdzhon inomdzhon modified the milestones: v7.0.0-beta.0, v7.0.0-beta.1 Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 🔧 In progress
Development

Successfully merging a pull request may close this issue.

5 participants