chore(Modal, MiniModal, SidePage): update mobile version and fix some bugs #3632

SchwJ · 2025-04-01T10:43:57Z

Проблема

Дизайн Modal, MiniModal, SidePage на мобилке устарел. Необходимо обновить дизайн до состояния макетов.

Решение

Обновила дизайн согласно макетам.
По пути всплывали мелкие баги в модалке, попутно фиксила их.

Ссылки

IF-2125
Макеты адаптивности для модалки и сайдпейжда
Тред с обсуждением задачи

Чек-лист перед запросом ревью

Добавлены тесты на все изменения
⬜ unit-тесты для логики
✅ скриншоты для верстки и кросс-браузерности
⬜ нерелевантно
Добавлена (обновлена) документация
✅ styleguidist для пропов и примеров использования компонентов
⬜ jsdoc для утилит и хелперов
⬜ комментарии для неочевидных мест в коде
⬜ прочие инструкции (README.md, contributing.md и др.)
⬜ нерелевантно
Изменения корректно типизированы
✅ без использования any (см. PR 2856)
⬜ нерелевантно
Прочее
⬜ все тесты и линтеры на CI проходят
✅ в коде нет лишних изменений
✅ заголовок PR кратко и доступно отражает суть изменений (он попадет в changelog)

SchwJ · 2025-04-01T10:56:08Z

packages/react-ui/components/MiniModal/getMiniModalTheme.ts

@@ -14,7 +14,6 @@ export const getMiniModalTheme = (contextTheme: Theme, propsTheme: ThemeIn = {})
      mobileModalFooterPadding: theme.miniModalFooterPaddingMobile,
      mobileModalHeaderPadding: theme.miniModalHeaderPaddingMobile,
      mobileModalBodyPadding: theme.miniModalBodyPaddingMobile,
-      mobileModalContainerHeight: theme.miniModalHeightMobile,


Эта переменная нигде не использовалась

А разве не в классе containerMobile у Modal?

SchwJ · 2025-04-01T10:57:53Z

packages/react-ui/components/Modal/Modal.styles.ts


+  containerDesktop() {


Вынесла в отдельный стиль, так как &::before нужен только в старой версии ИЛИ в текущей версии на десктопе

SchwJ · 2025-04-01T11:09:46Z

packages/react-ui/components/Modal/Modal.styles.ts

@@ -111,7 +178,6 @@ export const styles = memoizeStyle({
      display: flex;
      right: ${padding}px;
      top: ${padding}px;
-      background: none;


Сразу после идет присвоение background: transparent;, так что эту строку можно убрать

SchwJ · 2025-04-01T11:18:45Z

packages/react-ui/components/SidePage/SidePage.styles.ts

@@ -11,12 +11,6 @@ export const styles = memoizeStyle({
    `;
  },

-  mobileRoot() {


Убрала, так как на мобилке ширина должна быть либо передана пропом mobileWidth или быть равной 100%. Вынесла 100% из стилей в SidePage.tsx, чтобы логика выбора ширины была в одном месте

SchwJ · 2025-04-01T12:19:42Z

packages/react-ui/components/SidePage/SidePage.tsx

@@ -177,7 +182,7 @@ export class SidePage extends React.Component<SidePageProps, SidePageState> {
                wrapperRef={this.rootRef}
                style={{ position: 'absolute' }}
              >
-                {blockBackground && this.renderShadow(isMobile)}
+                {blockBackground && (versionGTE5_2 || !isMobile) && this.renderShadow()}


Теперь согласно дизайну вуалька должна быть не только на десктопе, но и на мобилках

SchwJ · 2025-04-02T04:21:25Z

packages/react-ui/components/Sticky/Sticky.tsx

Добавила Math.Ceil, потому что иногда разделитель между хедером и контентом рисовался, когда его быть не должно. Оказалось, что это из-за того, что top меньше offset на 0,001

Например, в этом примере разделитель не должен отображаться, но он отображается

sashachabin · 2025-04-03T08:09:38Z

packages/react-ui/components/Modal/__docs__/Modal.docs.stories.tsx

+        <br />
+        <br />
+        <br />
+        <br />А


Блин, да, надо в этом месте улучшить историю

sashachabin · 2025-04-03T08:12:31Z

packages/react-ui/components/SidePage/SidePage.tsx

    const { offset, role } = this.getProps();

+    const getWidth = isMobile ? mobileWidth || '100%' : width || (blockBackground ? 800 : 500);


А давай запишем без тернарника, очень сложно читать 2 тернарника подряд + ||

Попробуй сделать 1 тернарник и вынести значения с || в переменные, либо конструкцию из if. Переносы строк тоже бы помогли

А ещё это не функция, давай уберем действие get из названия

Как думаешь, так ок? Есть сложности с наименованием)

Мб ещё подумаем, но так точно лучше!

sashachabin · 2025-04-03T08:22:07Z

packages/react-ui/components/Modal/Modal.styles.ts

+      position: absolute;
+      padding: ${t.mobileModalWithoutHeaderCloseButtonPadding};
+      margin: -${t.mobileModalWithoutHeaderCloseButtonPadding};
+      vertical-align: top;


Хм, кажется, что это не сработает, если мы убрали этот элемент из потока

sashachabin · 2025-04-03T08:24:37Z

packages/react-ui/components/Modal/Modal.tsx

+                        versionGTE5_2 &&
+                        (mobileAppearance === 'fullscreen-spacing' || (mobileAppearance === 'auto' && hasFooter)),
+                      [styles.mobileCenterContainerFullscreen5_2()]:
+                        isMobile && versionGTE5_2 && mobileAppearance === 'fullscreen',


Небольшое пожелание: у тебя выше хороший порядок — сначала versionGTE5_2, а потом isMobile. Давай тут и в других местах так же сделаем, чтобы дальше было проще читать/рефакторить

Везде вынесла versionGTE5_2 вперед

sashachabin · 2025-04-03T09:10:23Z

packages/react-ui/components/Modal/__stories__/MobileModal.stories.tsx

+interface ModalWithVariableHeightState {
+  isOpened: boolean;
+}
+class MobileModalWithSettings extends React.Component<SuperModalProps> {


Давай немного приберёмся

Было Стало

render(() => { const [opened, setOpened] = React.useState(false); const [position, setPosition] = React.useState("auto"); const [hasHeader, setHasHeader] = React.useState(false); const [stickyHeader, setStickyHeader] = React.useState(false); const header = <Modal.Header sticky={stickyHeader}>Title</Modal.Header>; const [hasFooter, setHasFooter] = React.useState(false); const [stickyFooter, setStickyFooter] = React.useState(false); const [showSecondButton, setShowSecondButton] = React.useState(false); const footer = ( <ResponsiveLayout> {({ isMobile }) => { return ( <Modal.Footer sticky={stickyFooter}> <Button style={ isMobile ? { width: "100%", } : undefined } onClick={() => setShowSecondButton(!showSecondButton)} > show/hide second button </Button> {showSecondButton && ( <Button style={ isMobile ? { width: "100%", } : undefined } > i'm second button </Button> )} </Modal.Footer> ); }} </ResponsiveLayout> ); const [isLongContent, setIsLongContent] = React.useState(false); const body = isLongContent ? ( <Modal.Body> <div> * Навигация – используйте верхнее меню для быстрого перехода к основным разделам. <br /> * Фильтрация и поиск – применяйте фильтры и строку поиска, чтобы быстрее находить нужные данные. <br /> * Настройки – персонализируйте интерфейс под свои нужды, изменяя темы, уведомления и другие параметры. <br /> ❓ Часто задаваемые вопросы: <br /> 🔹 Как сохранить изменения? – После внесения правок нажмите кнопку «Сохранить». Все данные обновятся автоматически. <br /> 🔹 Можно ли отменить действие? – Да! Используйте кнопку «Отмена» или сочетание клавиш Ctrl + Z (для ПК). <br /> 🔗 Дополнительные ресурсы: Если у вас возникли вопросы или сложности, вы можете: <br /> * Ознакомиться с подробной документацией (доступна в разделе «Помощь»).{" "} <br /> * Обратиться в техническую поддержку через чат или по email. <br />* Посетить форум сообщества, где пользователи делятся советами и решениями. Спасибо, что пользуетесь нашим сервисом! 🚀 <br /> <br /> <br /> <br /> <br /> <br /> <br /> <br /> <br /> <br /> <br /> <br /> <br /> <br /> <br /> <br /> <br /> <br /> <br /> <br /> <br /> <br />А </div> </Modal.Body> ) : ( <Modal.Body> Текст сообщения в лайтбоксе, достаточно длинный, чтобы занять несколько строк. Если в лайтбоксе только это сообщение и нет других контролов, то лайтбокс не нуждается в терминальной плашке и может даже не содержать кнопок. </Modal.Body> ); function renderModal() { return ( <ThemeContext.Provider value={ThemeFactory.create({ mobileMediaQuery: "(max-width: 576px)", })} > <Modal mobileAppearance={position} onClose={() => setOpened(false)}> {hasHeader && header} {body} {hasFooter && footer} </Modal> </ThemeContext.Provider> ); } return ( <Gapped gap={16} vertical> {opened && renderModal()} <Gapped verticalAlign="top" gap={48}> <div> <b>Отображение mobileAppearance</b> <br /> <br /> <RadioGroup items={[ "auto", "top", "center", "bottom", "fullscreen-spacing", "fullscreen", ]} onValueChange={setPosition} /> </div> <div> <b>Настройки</b> <br /> <br /> <Gapped vertical gap={0}> <Checkbox checked={hasHeader} onValueChange={setHasHeader} children={"Шапка"} /> <Checkbox checked={hasFooter} onValueChange={setHasFooter} children={"Повал"} /> <Checkbox checked={stickyHeader} onValueChange={setStickyHeader} children={"Залипаюшая шапка"} /> <Checkbox checked={stickyFooter} onValueChange={setStickyFooter} children={"Залипающий подвал"} /> <Checkbox checked={isLongContent} onValueChange={setIsLongContent} children={"Длинный текст для появления скролла"} /> </Gapped> </div> </Gapped> <Button onClick={() => setOpened(true)}>Открыть</Button> </Gapped> ); });

А я правильно понимаю, что доку нужно открыть в мобилке, чтобы проверить?

Если да, то нам бы что-нибудь придумать — хотя бы задизаблить и предупредить :(

Чтобы увидеть мобильную версию можно сделать экран поуже на десктопе. Для этого я написала в примере:

<ThemeContext.Provider value={ThemeFactory.create({ mobileMediaQuery: "(max-width: 576px)", })} >

Взяла твой код, плюс написала комментарий, получилось вот так (это пример в доке)

Не подумали, что с этой историей будут чаще взаимодействовать на узком экране, и получается там так:

Сделаю отображение второго столбца под первым

Вот так будет

zhzz · 2025-04-04T02:30:17Z

packages/react-ui/components/Modal/Modal.tsx

@@ -57,6 +61,17 @@ export interface ModalProps

  /** Задает объект с переменными темы. Он будет объединён с темой из контекста. */
  theme?: ThemeIn;
+
+  /** Задает внешний вид модалки. Работает с версией темы >= 5_2.
+   *  - `auto` — всегда показывать иконку очистки значения в заполненном поле


Тут текст не тот что нужно

Исправила

zhzz · 2025-04-04T03:14:48Z

packages/react-ui/components/MiniModal/getMiniModalTheme.ts

@@ -14,7 +14,6 @@ export const getMiniModalTheme = (contextTheme: Theme, propsTheme: ThemeIn = {})
      mobileModalFooterPadding: theme.miniModalFooterPaddingMobile,
      mobileModalHeaderPadding: theme.miniModalHeaderPaddingMobile,
      mobileModalBodyPadding: theme.miniModalBodyPaddingMobile,
-      mobileModalContainerHeight: theme.miniModalHeightMobile,


А разве не в классе containerMobile у Modal?

zhzz · 2025-04-04T03:16:50Z

packages/react-ui/components/Modal/Modal.tsx

+                      [styles.mobileCenterContainer()]: !versionGTE5_2 && isMobile,
+                      [styles.mobileCenterContainer5_2(this.theme)]: versionGTE5_2 && isMobile,
+                      [styles.mobileCenterContainerBig5_2(this.theme)]:
+                        versionGTE5_2 &&
+                        isMobile &&
+                        (mobileAppearance === 'fullscreen-spacing' || (mobileAppearance === 'auto' && hasFooter)),
+                      [styles.mobileCenterContainerFullscreen5_2()]:
+                        versionGTE5_2 && isMobile && mobileAppearance === 'fullscreen',


Тут может тоже уже пора вынести логику в отдельную функцию?

Согласна, вынесла

zhzz · 2025-04-04T03:17:08Z

packages/react-ui/components/Modal/Modal.tsx

@@ -247,14 +308,18 @@ export class Modal extends React.Component<ModalProps, ModalState> {
                        >
                          {!hasHeader && !noClose && (
                            <ZIndex
+                              priority={1}


А зачем это понадобилось?

Приоритет в ZIndex'е для крестика я добавила для того, чтобы он был "выше" чем отступ у текста заголовка. Если priority={1} убрать, то ZIndex у крестика становится меньше, чем у текста заголовка, и тогда текст заголовка отображается поверх крестика и на него нельзя нажать

О каком заголовке речь? Это же сценарий без хедера. Или ты про контент модалки? А чем тогда вызвано это изменение в поведении?

Ой, я оговорилась. Вместо "заголовка" должно быть "тело модалки (body)". Оно и выделено на скрине выше.

Приоритет в ZIndex'е для крестика я добавила для того, чтобы он был "выше" чем отступ у текста в body. Если priority={1} убрать, то ZIndex у крестика становится меньше, чем у текста тела модалки, и тогда отступ справа отображается поверх крестика и на крестик нельзя нажать

SchwJ added 5 commits February 14, 2025 16:26

chore(SidePage): improve width

547ec4e

chore(SidePage): enable background on mobile

3283792

Merge branch 'next' into mobile-modal-and-sidepage

24fe8cc

Merge branch 'next' into mobile-modal-and-sidepage

b98594b

chore(): mobile Modal, MiniModal, SidePage

f07f9b0

SchwJ changed the title ~~Mobile modal and sidepage~~ chore(Modal, MiniModal, SidePage): update mobile version and fix some bugs Apr 1, 2025

SchwJ commented Apr 1, 2025

View reviewed changes

SchwJ added 2 commits April 1, 2025 16:53

chore(): refactor

d415252

chore(): fix PropsForwardingCommonProps ModalFooter ModalHeader tests

7332266

SchwJ commented Apr 1, 2025

View reviewed changes

SchwJ added 2 commits April 1, 2025 17:21

chore(): cancel removing

7632219

chore(): fix error in style

d322be2

SchwJ commented Apr 2, 2025

View reviewed changes

sashachabin self-requested a review April 2, 2025 09:23

sashachabin reviewed Apr 3, 2025

View reviewed changes

chore(): fix review comments

f9ad0bb

zhzz reviewed Apr 4, 2025

View reviewed changes

chore(): fix Egor review comments

5994167

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(Modal, MiniModal, SidePage): update mobile version and fix some bugs #3632

chore(Modal, MiniModal, SidePage): update mobile version and fix some bugs #3632

SchwJ commented Apr 1, 2025 •

edited

Loading

SchwJ Apr 1, 2025

zhzz Apr 4, 2025

SchwJ Apr 1, 2025 •

edited

Loading

SchwJ Apr 1, 2025

SchwJ Apr 1, 2025 •

edited

Loading

SchwJ Apr 1, 2025

SchwJ Apr 2, 2025 •

edited

Loading

sashachabin Apr 3, 2025

SchwJ Apr 3, 2025

sashachabin Apr 3, 2025

sashachabin Apr 3, 2025

sashachabin Apr 3, 2025

SchwJ Apr 3, 2025

sashachabin Apr 3, 2025

sashachabin Apr 3, 2025

sashachabin Apr 3, 2025 •

edited

Loading

SchwJ Apr 3, 2025

sashachabin Apr 3, 2025 •

edited

Loading

sashachabin Apr 3, 2025

SchwJ Apr 3, 2025

SchwJ Apr 3, 2025 •

edited

Loading

SchwJ Apr 3, 2025

SchwJ Apr 3, 2025

zhzz Apr 4, 2025

SchwJ Apr 4, 2025

zhzz Apr 4, 2025

zhzz Apr 4, 2025

SchwJ Apr 4, 2025

zhzz Apr 4, 2025

SchwJ Apr 4, 2025

zhzz Apr 4, 2025

SchwJ Apr 6, 2025

		const { offset, role } = this.getProps();

		const getWidth = isMobile ? mobileWidth \|\| '100%' : width \|\| (blockBackground ? 800 : 500);

chore(Modal, MiniModal, SidePage): update mobile version and fix some bugs #3632

Are you sure you want to change the base?

chore(Modal, MiniModal, SidePage): update mobile version and fix some bugs #3632

Conversation

SchwJ commented Apr 1, 2025 • edited Loading

Проблема

Решение

Ссылки

Чек-лист перед запросом ревью

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SchwJ Apr 1, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SchwJ Apr 1, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SchwJ Apr 2, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sashachabin Apr 3, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sashachabin Apr 3, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SchwJ Apr 3, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SchwJ commented Apr 1, 2025 •

edited

Loading

SchwJ Apr 1, 2025 •

edited

Loading

SchwJ Apr 1, 2025 •

edited

Loading

SchwJ Apr 2, 2025 •

edited

Loading

sashachabin Apr 3, 2025 •

edited

Loading

sashachabin Apr 3, 2025 •

edited

Loading

SchwJ Apr 3, 2025 •

edited

Loading