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

[BREAKING CHANGE] feat: SSR, AppRoot: get rid of body classes mutations #7739

Draft
wants to merge 25 commits into
base: master
Choose a base branch
from

Conversation

andrey-medvedev-vk
Copy link
Contributor

@andrey-medvedev-vk andrey-medvedev-vk commented Oct 11, 2024


  • Unit-тесты
  • e2e-тесты
  • Дизайн-ревью
  • Документация фичи
  • Release notes

Описание

Убираем мутацию body из AppRoot.

В том числе избавляемся от добавления в body portalRoot при первом рендере.

Изменения

  • AppRoot больше не выставляет самостоятельно классы .vkui на html теге и .vkui__root на точке монтирования VKUI.
    Вместо этого предлагается выставлять эти теги пользователю самостоятельно, либо применить стили, подобные которым мы теперь используем в .vkui и .vkui__root. В основном они нужны для того, чтобы у body убрать браузерные отступы и сделать так, чтобы VKUI занимало всё пространство контейнера.
    /* stylelint-disable-next-line selector-max-type */
    /* Оставить и предложить утанавливать на html vkui и на root vkui__root */
    /* и подключать этот css файл */
    /* stylelint-disable-next-line selector-max-type */
    .vkui > body,
    .vkui,
    .vkui__root {
    margin: 0;
    padding: 0;
    block-size: 100%;
    }
    .vkui__root {
    /* чтобы можно было ограничить размеры приложения
    * извне с помощью max-height, max-width */
    max-inline-size: inherit;
    max-block-size: inherit;
  • Все классы и токены, требуемые VKUI, которые раньше вешались на body теперь вешаются на контейнер AppRoot. Так как AppRoot это точка входа в VKUI приложение, то такой подход приемлем.
    Чтобы компоненты, рендерящиеся через порталы вне DOM дерева AppRoot, тоже имели доступ к токенам и стилям AppRoot был создан компонент AppRootStyleContainer. Он используется как в AppRoot, так и как обертка для порталов, в компоненте AppRootPortal.
    AppRootStyleContainer через контекст знает какой сейчас режим (mode), какой appearance, поэтому в модалках и других плавающих элементах всегда будет применена верная цветовая схема и токены.
  • AppRootPortal был переработан.
    • Упрощены проверки cвязанные с usePortal свойством.
    • portalRoot контейнер для модалок в режиме embedded, лежащий как последний дочерний элемент body, больше не создается при первом рендере. Он создаётся в body только если модалку или поповер надо отрендерить, и продолжает жить в DOM пока AppRoot не размонтируется. Используется как точка монтирования всем модалками и поповерами, если надо отредерить что-то вне DOM дерева AppRoot.
    • Доработана логика рендера модалок и попаутов в режиме full. Они теперь всегда рендеряться по умолчанию через портал в специальный контейнер внутри SplitLayout, после основного контента. Рендер через портал позволяет избавится от ещё одно появляющегося при первом рендере DOM элемента, если необходимо использовать модалки.
    • Убраны пропы popout и modal из SplitLayout.
      Так как модалки и попауты (Alert, ScreenSpinner, ActionSheet) рендеряться через портал в то же место что и раньше, то надобность в явной передаче ModalRoot и popout в SplitLayout отпадает.
      Это позволяет избавить пользователей от необходимости обязательно передавать ModalRoot в SplitLayout и держать стейт переключения модалок на уровне SplitLayout.
      Теперь ModalRoot можно объявить в любой части приложения, как и Alert, ScreenSpinner и ActionSheet.
      Также можно иметь несколько ModalRoot если требудется по смыслу разделить модалки.
    • Появилось новое значение свойства usePortal для рендеринга через портал в SplitLayout за контентом.

Release notes

Copy link
Contributor

github-actions bot commented Oct 11, 2024

size-limit report 📦

Path Size
JS 382.19 KB (-0.07% 🔽)
JS (gzip) 122.38 KB (+0.01% 🔺)
JS (brotli) 101.47 KB (+0.13% 🔺)
JS import Div (tree shaking) 1.47 KB (0%)
CSS 342.09 KB (-0.04% 🔽)
CSS (gzip) 49.32 KB (-0.01% 🔽)
CSS (brotli) 40 KB (+0.15% 🔺)

Copy link

codesandbox-ci bot commented Oct 11, 2024

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.

Copy link
Contributor

github-actions bot commented Oct 11, 2024

e2e tests

⚠️ Some screenshots were failed. See Playwright Report.

Playwright Report

Copy link
Contributor

github-actions bot commented Oct 11, 2024

👀 Docs deployed

Commit d7e603c

Copy link

codecov bot commented Oct 11, 2024

Codecov Report

Attention: Patch coverage is 98.46154% with 2 lines in your changes missing coverage. Please review.

Project coverage is 95.13%. Comparing base (ebb46a5) to head (d7e603c).

Files with missing lines Patch % Lines
packages/vkui/src/components/AppRoot/AppRoot.tsx 94.44% 1 Missing ⚠️
packages/vkui/src/components/AppRoot/helpers.ts 80.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7739      +/-   ##
==========================================
- Coverage   95.17%   95.13%   -0.04%     
==========================================
  Files         376      377       +1     
  Lines       11041    11027      -14     
  Branches     3664     3661       -3     
==========================================
- Hits        10508    10491      -17     
- Misses        533      536       +3     
Flag Coverage Δ
unittests 95.13% <98.46%> (-0.04%) ⬇️

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.

@vkcom-publisher vkcom-publisher added pr-needs-work Автоматизация: PR автоматически закроется через 14 дней при отсутствии активности and removed pr-needs-work Автоматизация: PR автоматически закроется через 14 дней при отсутствии активности labels Oct 19, 2024
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.

Как всегда, шикарная работа 👏 👏 👏

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

Также:

  1. нужно ребейзнуть master – там Appearance стал ColorScheme;
  2. нужно в доке Stylegude тоже указать vkui и vkui__root классы в HTML;
  3. нужно будет сразу глянуть почему скриншоты поплыли и не нужно ли из-за ошибок в них всё переделывать

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.

Спасибо за правки 🙏 Зарезолвил комменты, а те, что открыты, можешь сам закрыть, оставил их, чтобы было видно где ответил


В Styleguide нужно стили подправить, правый контент не на всю высоту image

Reuse AppRoot styles in AppRootPortal
to share same tokens
We don't need to use it on portals as modals will disappear
Simplify logic of helper functions like shouldUsePortal
resolvePortalContainer
As it creates unnesessary rerender
and can't work with creation of portalRoot dom element on demand
To avoid rendering modal/popout in SplitLayout during SSR and after
hydration
We keep render ModalRoot in the SplitLayout container right
after content, but now we use portal for it.
It means now, that users can put ModalRoot anywhere in the app,
they are not limited by SplitLayout modal prop

Styles from PopoutRoot about modal fixed layout and
z-index are moved directly to ModalRoot AppPortalWrapper from
SplitLayout
In full mode we render Modal/Popout right after content
In other modes we render Modal/Popout as a child of the body
Components like Alert/ScreenSpinner/ActionSheet and
ModalRootDesktop
Props stays, but unused.
Will remove later
Instead of in-app-after-content
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants