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.

Sign up for GitHub

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

Jump to bottom

[Bug]: проблема пересчёта размеров ModalPage при изменении ориентации мобильного устройства #6242

Closed
vdudenkov opened this issue Dec 8, 2023 · 4 comments · Fixed by #6538 or #6583
Closed

[Bug]: проблема пересчёта размеров ModalPage при изменении ориентации мобильного устройства #6242

vdudenkov opened this issue Dec 8, 2023 · 4 comments · Fixed by #6538 or #6583
Assignees
@mendrew
Labels
cmp:modal-page type:bug v5.10.1
Milestone
v6.0.1

Comments

@vdudenkov
Copy link

vdudenkov commented Dec 8, 2023 •
edited by mendrew
Loading

Описание

при открытии модалки в горизонтальном положении и последующем повороте в вертикальное, модалка растягивается

Версия

5.10.0

В каких браузерах воспроизводится проблема?

Chrome

Шаги воспроизведения

  1. открыть модальное окно (с небольшим содержимым) в горизонтальном положении мобильного устройства (МУ)
  2. повернуть МУ, в вертикальное положение

после перерисовки модальное окно растягивается на большую часть экрана, не взирая на небольшое содержимое модального окна

Ожидаемое поведение

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

Скриншоты

Modal Page landscape issue

Видео с воспроизведением

Screen.Recording.2023-12-08.at.19.53.52.mov

Пример с воспроизведением

https://codesandbox.io/p/sandbox/vkui-v5-10-0-modalpage-landscape-behavior-s7sr5h
@mendrew mendrew added this to VKUI Dec 8, 2023
@github-project-automation github-project-automation bot moved this to 🗃 Backlog in VKUI Dec 8, 2023
@mendrew mendrew moved this from 🗃 Backlog to 🔜 To do in VKUI Dec 8, 2023
@mendrew mendrew self-assigned this Dec 20, 2023
@mendrew mendrew moved this from 🔜 To do to 🔧 In progress in VKUI Dec 20, 2023
@mendrew
Copy link
Contributor

mendrew commented Jan 16, 2024

К сожалению, так и не получилось адекватно поправить эту проблему. Попробуем выделить больше времени в будущем. Успокаивает то, что контент остаётся доступным.

Немного деталей:

  • дело в AppRoot. После переворота логика компонента считывает размеры элементов и они в момент переворота оказываются больше, чем если бы модалка просто открывалась уже в вертикальном положении.
  • к сожалению эта проблема присутствует и в новом компоненте модалок ModalSheet, потому что там также используется AppRoot.

@mendrew mendrew moved this from 🔧 In progress to 🗃 Backlog in VKUI Jan 16, 2024
@mendrew mendrew removed their assignment Jan 16, 2024
@mendrew mendrew moved this from 🗃 Backlog to 🔜 To do in VKUI Jan 18, 2024
@mendrew mendrew self-assigned this Jan 18, 2024
@PoltP
Copy link

PoltP commented Jan 18, 2024 •
edited
Loading

v5.7.2
У нас в Мобильной Почте Octavius я столкнулся со схожей проблемой.

  1. Есть модалка, которая показывается либо на динамическую высоту, либо на весь экран в зависимости от ориентации экрана мобильника:
...(orientation === "landscape"
          ? {
              settlingHeight: 100,
            }
          : {
              dynamicContentHeight: true,
            })
  1. Если из "портрета" повернуть мобильник в "альбом" ('rotate' в браузерном эмуляторе мобилы) и затем обратно, то в "портрете" она начинает показывается на всю высоту, несмотря на то, что я прокидываю dynamicContentHeight: true и зову хук обновления высоты (пробовал разные варианты).

Код и сама демка в сендбокс (окружение конечно отличается от нашего, но у нас примерно так):
https://qxg973.csb.app/
https://codesandbox.io/p/sandbox/objective-maxwell-qxg973?file=%2Fsrc%2Findex.js

Проверял на живой мобиле и наш вариант и демку в сендбоксе - поведение такое, не могу понять, как будто игнорит свойство динамической высоты потом после первого задания.

Вот в коде песочницы видно, что я пробовал играться тут, обновлял высоту на смену ориентации или свойств:

  const { updateModalHeight } = useModalRootContext();
  useEffect(() => {
    setTimeout(updateModalHeight, 0);
  }, [props.settlingHeight, props.dynamicContentHeight]);

но не помогает - проблема остаётся...

Видео поведения демо:
https://github.com/VKCOM/VKUI/assets/41975596/8d9db731-6891-4511-a6ff-4658878ac2d5

Видео поведения из реального приложения:
https://github.com/VKCOM/VKUI/assets/41975596/ca640200-1d5a-4154-b8a6-1ceb12537ab6

@mendrew mendrew moved this from 🔜 To do to 🔧 In progress in VKUI Jan 29, 2024
@mendrew mendrew mentioned this issue Feb 9, 2024
Merged
@mendrew mendrew moved this from 🔧 In progress to 👀 In Review in VKUI Feb 13, 2024
@inomdzhon inomdzhon added this to the v6.0.1 milestone Feb 14, 2024
mendrew added a commit that referenced this issue Feb 14, 2024
@mendrew
fix(ModalPage): Properly calculate content height in expanded mode (#…
52f0aa8 
…6538)

- close #6242

- меняем логику определения высоты контента. 
  Раньше мы просто брали `el.offsetScroll` у контейнера контента `.ModalPage__content-in`, теперь же, учитывая, что в режиме `expandable` размер контента модального окна не получить через `el.offsetScroll` мы высчитываем высоту временно меня `height` на `height: auto`, смотрим размер, и возвращаем стиль назад.
- убрали проверку предыдущего состояния `state.expandable` при определении текущего состояния `expandable`, потому что при изменении ориентации это не верно. Верно всё же ориентироваться на высоту контента.

Проблема лишнего места при изменении ориентации экрана в том, что это происходит когда модальное окно в режиме `expandable`. В этом режиме у `.ModalPage__content-in` `height` задано `100%` (изначально добавлено [тут](#4625))
https://github.com/VKCOM/VKUI/blob/e935a7b8dfd2c9d9dbbb5937bd1decba859d45d1/packages/vkui/src/components/ModalPage/ModalPage.module.css#L121-L126

Из-за этих стилей высота контейнера контента становится равна высоте родительского элемента.
При этом, если высота контента в вертикальной ориентации экрана меньше чем высота экрана, то высота контейнера контента будет больше чем высота контента, что как раз и добавляет лишнее место снизу.

При изменении ориентации мы пересчитываем высоту, но селектор всё ещё работает и `element.scrollHeight`, дающий нам высоту контента продолжает выдавать высоту равную высоте родителя, хотя наша логика определения состояния `expandable` основывается на сравнении высоты родительского элемента и высоты контента.
В результате `ModalRoot` продолжает думать, что `ModalPage` в состоянии `expandablе` и позиционирует модальную страницу максимально высоко, хотя контент позволяет отрисовать модальное окно ниже, как если бы модальное окно было изначально открыто в такой ориентации экрана.
@github-project-automation github-project-automation bot moved this from 👀 In Review to ✅ Done in VKUI Feb 14, 2024
vkcom-publisher pushed a commit that referenced this issue Feb 14, 2024
@mendrew @actions-user
fix(ModalPage): Properly calculate content height in expanded mode (#…
9f190d4 
…6538)

- close #6242

- меняем логику определения высоты контента.
  Раньше мы просто брали `el.offsetScroll` у контейнера контента `.ModalPage__content-in`, теперь же, учитывая, что в режиме `expandable` размер контента модального окна не получить через `el.offsetScroll` мы высчитываем высоту временно меня `height` на `height: auto`, смотрим размер, и возвращаем стиль назад.
- убрали проверку предыдущего состояния `state.expandable` при определении текущего состояния `expandable`, потому что при изменении ориентации это не верно. Верно всё же ориентироваться на высоту контента.

Проблема лишнего места при изменении ориентации экрана в том, что это происходит когда модальное окно в режиме `expandable`. В этом режиме у `.ModalPage__content-in` `height` задано `100%` (изначально добавлено [тут](#4625))
https://github.com/VKCOM/VKUI/blob/e935a7b8dfd2c9d9dbbb5937bd1decba859d45d1/packages/vkui/src/components/ModalPage/ModalPage.module.css#L121-L126

Из-за этих стилей высота контейнера контента становится равна высоте родительского элемента.
При этом, если высота контента в вертикальной ориентации экрана меньше чем высота экрана, то высота контейнера контента будет больше чем высота контента, что как раз и добавляет лишнее место снизу.

При изменении ориентации мы пересчитываем высоту, но селектор всё ещё работает и `element.scrollHeight`, дающий нам высоту контента продолжает выдавать высоту равную высоте родителя, хотя наша логика определения состояния `expandable` основывается на сравнении высоты родительского элемента и высоты контента.
В результате `ModalRoot` продолжает думать, что `ModalPage` в состоянии `expandablе` и позиционирует модальную страницу максимально высоко, хотя контент позволяет отрисовать модальное окно ниже, как если бы модальное окно было изначально открыто в такой ориентации экрана.
mendrew added a commit that referenced this issue Feb 14, 2024
@mendrew
fix(ModalPage): Properly calculate content height in expanded mode (#…
6de0a85 
…6538)

- close #6242

- меняем логику определения высоты контента. 
  Раньше мы просто брали `el.offsetScroll` у контейнера контента `.ModalPage__content-in`, теперь же, учитывая, что в режиме `expandable` размер контента модального окна не получить через `el.offsetScroll` мы высчитываем высоту временно меня `height` на `height: auto`, смотрим размер, и возвращаем стиль назад.
- убрали проверку предыдущего состояния `state.expandable` при определении текущего состояния `expandable`, потому что при изменении ориентации это не верно. Верно всё же ориентироваться на высоту контента.

Проблема лишнего места при изменении ориентации экрана в том, что это происходит когда модальное окно в режиме `expandable`. В этом режиме у `.ModalPage__content-in` `height` задано `100%` (изначально добавлено [тут](#4625))
https://github.com/VKCOM/VKUI/blob/e935a7b8dfd2c9d9dbbb5937bd1decba859d45d1/packages/vkui/src/components/ModalPage/ModalPage.module.css#L121-L126

Из-за этих стилей высота контейнера контента становится равна высоте родительского элемента.
При этом, если высота контента в вертикальной ориентации экрана меньше чем высота экрана, то высота контейнера контента будет больше чем высота контента, что как раз и добавляет лишнее место снизу.

При изменении ориентации мы пересчитываем высоту, но селектор всё ещё работает и `element.scrollHeight`, дающий нам высоту контента продолжает выдавать высоту равную высоте родителя, хотя наша логика определения состояния `expandable` основывается на сравнении высоты родительского элемента и высоты контента.
В результате `ModalRoot` продолжает думать, что `ModalPage` в состоянии `expandablе` и позиционирует модальную страницу максимально высоко, хотя контент позволяет отрисовать модальное окно ниже, как если бы модальное окно было изначально открыто в такой ориентации экрана.
mendrew added a commit that referenced this issue Feb 14, 2024
@mendrew
fix(ModalPage): Properly calculate content height in expanded mode (#…
6d6eaec 
…6538)

- close #6242

- меняем логику определения высоты контента. 
  Раньше мы просто брали `el.offsetScroll` у контейнера контента `.ModalPage__content-in`, теперь же, учитывая, что в режиме `expandable` размер контента модального окна не получить через `el.offsetScroll` мы высчитываем высоту временно меня `height` на `height: auto`, смотрим размер, и возвращаем стиль назад.
- убрали проверку предыдущего состояния `state.expandable` при определении текущего состояния `expandable`, потому что при изменении ориентации это не верно. Верно всё же ориентироваться на высоту контента.

Проблема лишнего места при изменении ориентации экрана в том, что это происходит когда модальное окно в режиме `expandable`. В этом режиме у `.ModalPage__content-in` `height` задано `100%` (изначально добавлено [тут](#4625))
https://github.com/VKCOM/VKUI/blob/e935a7b8dfd2c9d9dbbb5937bd1decba859d45d1/packages/vkui/src/components/ModalPage/ModalPage.module.css#L121-L126

Из-за этих стилей высота контейнера контента становится равна высоте родительского элемента.
При этом, если высота контента в вертикальной ориентации экрана меньше чем высота экрана, то высота контейнера контента будет больше чем высота контента, что как раз и добавляет лишнее место снизу.

При изменении ориентации мы пересчитываем высоту, но селектор всё ещё работает и `element.scrollHeight`, дающий нам высоту контента продолжает выдавать высоту равную высоте родителя, хотя наша логика определения состояния `expandable` основывается на сравнении высоты родительского элемента и высоты контента.
В результате `ModalRoot` продолжает думать, что `ModalPage` в состоянии `expandablе` и позиционирует модальную страницу максимально высоко, хотя контент позволяет отрисовать модальное окно ниже, как если бы модальное окно было изначально открыто в такой ориентации экрана.
mendrew added a commit that referenced this issue Feb 15, 2024
@mendrew
fix(ModalPage): Properly calculate content height in expanded mode (#…
326a81a 
…6538) (#6573)

- close #6242

- меняем логику определения высоты контента. 
  Раньше мы просто брали `el.offsetScroll` у контейнера контента `.ModalPage__content-in`, теперь же, учитывая, что в режиме `expandable` размер контента модального окна не получить через `el.offsetScroll` мы высчитываем высоту временно меня `height` на `height: auto`, смотрим размер, и возвращаем стиль назад.
- убрали проверку предыдущего состояния `state.expandable` при определении текущего состояния `expandable`, потому что при изменении ориентации это не верно. Верно всё же ориентироваться на высоту контента.

Проблема лишнего места при изменении ориентации экрана в том, что это происходит когда модальное окно в режиме `expandable`. В этом режиме у `.ModalPage__content-in` `height` задано `100%` (изначально добавлено [тут](#4625))
https://github.com/VKCOM/VKUI/blob/e935a7b8dfd2c9d9dbbb5937bd1decba859d45d1/packages/vkui/src/components/ModalPage/ModalPage.module.css#L121-L126

Из-за этих стилей высота контейнера контента становится равна высоте родительского элемента.
При этом, если высота контента в вертикальной ориентации экрана меньше чем высота экрана, то высота контейнера контента будет больше чем высота контента, что как раз и добавляет лишнее место снизу.

При изменении ориентации мы пересчитываем высоту, но селектор всё ещё работает и `element.scrollHeight`, дающий нам высоту контента продолжает выдавать высоту равную высоте родителя, хотя наша логика определения состояния `expandable` основывается на сравнении высоты родительского элемента и высоты контента.
В результате `ModalRoot` продолжает думать, что `ModalPage` в состоянии `expandablе` и позиционирует модальную страницу максимально высоко, хотя контент позволяет отрисовать модальное окно ниже, как если бы модальное окно было изначально открыто в такой ориентации экрана.
mendrew added a commit that referenced this issue Feb 16, 2024
@mendrew
fix(ModalPage): Properly calculate content height in expanded mode (#…
56255bf 
…6538) (#6573)

- close #6242

- меняем логику определения высоты контента. 
  Раньше мы просто брали `el.offsetScroll` у контейнера контента `.ModalPage__content-in`, теперь же, учитывая, что в режиме `expandable` размер контента модального окна не получить через `el.offsetScroll` мы высчитываем высоту временно меня `height` на `height: auto`, смотрим размер, и возвращаем стиль назад.
- убрали проверку предыдущего состояния `state.expandable` при определении текущего состояния `expandable`, потому что при изменении ориентации это не верно. Верно всё же ориентироваться на высоту контента.

Проблема лишнего места при изменении ориентации экрана в том, что это происходит когда модальное окно в режиме `expandable`. В этом режиме у `.ModalPage__content-in` `height` задано `100%` (изначально добавлено [тут](#4625))
https://github.com/VKCOM/VKUI/blob/e935a7b8dfd2c9d9dbbb5937bd1decba859d45d1/packages/vkui/src/components/ModalPage/ModalPage.module.css#L121-L126

Из-за этих стилей высота контейнера контента становится равна высоте родительского элемента.
При этом, если высота контента в вертикальной ориентации экрана меньше чем высота экрана, то высота контейнера контента будет больше чем высота контента, что как раз и добавляет лишнее место снизу.

При изменении ориентации мы пересчитываем высоту, но селектор всё ещё работает и `element.scrollHeight`, дающий нам высоту контента продолжает выдавать высоту равную высоте родителя, хотя наша логика определения состояния `expandable` основывается на сравнении высоты родительского элемента и высоты контента.
В результате `ModalRoot` продолжает думать, что `ModalPage` в состоянии `expandablе` и позиционирует модальную страницу максимально высоко, хотя контент позволяет отрисовать модальное окно ниже, как если бы модальное окно было изначально открыто в такой ориентации экрана.
mendrew added a commit that referenced this issue Feb 16, 2024
@mendrew
fix(ModalPage): Properly calculate content height in expanded mode (#…
3c22e88 
…6538) (#6573) (#6583)

- close #6242

- меняем логику определения высоты контента. 
  Раньше мы просто брали `el.offsetScroll` у контейнера контента `.ModalPage__content-in`, теперь же, учитывая, что в режиме `expandable` размер контента модального окна не получить через `el.offsetScroll` мы высчитываем высоту временно меня `height` на `height: auto`, смотрим размер, и возвращаем стиль назад.
- убрали проверку предыдущего состояния `state.expandable` при определении текущего состояния `expandable`, потому что при изменении ориентации это не верно. Верно всё же ориентироваться на высоту контента.

Проблема лишнего места при изменении ориентации экрана в том, что это происходит когда модальное окно в режиме `expandable`. В этом режиме у `.ModalPage__content-in` `height` задано `100%` (изначально добавлено [тут](#4625))
https://github.com/VKCOM/VKUI/blob/e935a7b8dfd2c9d9dbbb5937bd1decba859d45d1/packages/vkui/src/components/ModalPage/ModalPage.module.css#L121-L126

Из-за этих стилей высота контейнера контента становится равна высоте родительского элемента.
При этом, если высота контента в вертикальной ориентации экрана меньше чем высота экрана, то высота контейнера контента будет больше чем высота контента, что как раз и добавляет лишнее место снизу.

При изменении ориентации мы пересчитываем высоту, но селектор всё ещё работает и `element.scrollHeight`, дающий нам высоту контента продолжает выдавать высоту равную высоте родителя, хотя наша логика определения состояния `expandable` основывается на сравнении высоты родительского элемента и высоты контента.
В результате `ModalRoot` продолжает думать, что `ModalPage` в состоянии `expandablе` и позиционирует модальную страницу максимально высоко, хотя контент позволяет отрисовать модальное окно ниже, как если бы модальное окно было изначально открыто в такой ориентации экрана.
@inomdzhon inomdzhon mentioned this issue Feb 28, 2024
Merged
@inomdzhon inomdzhon linked a pull request Feb 28, 2024 that will close this issue
patch(v5.10-stable): pr6538 #6583
Merged
@vkcom-publisher
Copy link
Contributor

✅ v5.10.1 🎉

@vkcom-publisher
Copy link
Contributor

✅ v6.0.1 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mendrew mendrew

Labels
cmp:modal-page type:bug v5.10.1
Projects
VKUI
Archived in project
Milestone
v6.0.1
5 participants
@mendrew @inomdzhon @PoltP @vkcom-publisher @vdudenkov