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

feat(PanelHeaderButton): refactor pressets #7874

Merged

Conversation

EldarMuhamethanov
Copy link
Contributor

@EldarMuhamethanov EldarMuhamethanov commented Oct 31, 2024

  • Unit-тесты
  • e2e-тесты
  • Документация фичи
  • Release notes
  • Codemods

Описание

В настоящее время пресеты компонента PanelHeaderButton демонстрируют несогласованное поведение при использовании пропсов label и children на разных платформах.

Изменения

Унифицировано поведение пресетов для обеспечения предсказуемого отображения на всех платформах.

Основные изменения

Поведение label:

  • iOS: текст отображается
  • Android/Desktop: текст визуально скрыт, но доступен для скринридеров

Удаление children:

  • Удален проп children у всех пресетов
  • Иконки теперь встроены в пресеты
  • Текстовый контент передается только через label

Особый случай: PanelHeaderBack

  • Сохранено специальное поведение для отображения label на VKCOM
  • Добавлены новые пропсы для гибкой настройки в разных кейсах:
    • hideLabelOnVKCom - скрывает визуально label на платформе vkcom
    • hideLabelOnIOS - скрывает визуально label на платформе iOS

Документация

  • Обновил примеры в документации и текст описывающие пресеты

Дополнительные изменения

  • Обновлена документация с описанием поведения на разных платформах
  • Написаны кодмоды для автоматической миграции
  • Обновлены скриншоты тестов

UPD

Пришлось оставить исключение PanelHeaderButton, так как у него есть особенная логика - label иногда должен отображаться не только на IOS, но и на VKCOM. Также было бы неплохо, чтобы можно было управлять тем, нужно ли скрывать label на разных платформах, поэтому добавил свойства hideLabelOnVKCom и hideLabelOnIOS для более точно найстройки в зависимости от кейса.

Release notes

BREAKING CHANGE

  • PanelHeaderButton:
    • у пресета PanelHeaderClose удалено свойство children. Теперь для прокидывания текста для a11y нужно прокидывать его в свойствоlabel
    • у пресета PanelHeaderSubmit удалено свойство children. Теперь для прокидывания текста для a11y нужно прокидывать его в свойствоlabel
    • у пресета PanelHeaderEdit удалены свойства children и label. Вместо label можно использовать свойства doneLabel и editLabel. Свойство children не использовалось.
    • у пресета PanelHeaderBack удалено свойство children. Теперь для прокидывания текста для a11y нужно прокидывать его в свойство label. Логика отображения label осталась как была, в отличие от других пресетов. Также для более точно настройки label были добавлены свойства hideLabelOnVKCom и hideLabelOnIOS, чтобы можно было скрывать label на соответствующей платформе.

Copy link
Contributor

github-actions bot commented Oct 31, 2024

size-limit report 📦

Path Size
JS 385.53 KB (+0.02% 🔺)
JS (gzip) 116.91 KB (+0.06% 🔺)
JS (brotli) 96.19 KB (+0.16% 🔺)
JS import Div (tree shaking) 1.46 KB (0%)
CSS 334.79 KB (0%)
CSS (gzip) 42.35 KB (0%)
CSS (brotli) 33.39 KB (0%)

Copy link

codesandbox-ci bot commented Oct 31, 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 31, 2024

e2e tests

Playwright Report

Copy link
Contributor

github-actions bot commented Oct 31, 2024

👀 Docs deployed

Commit 907e50d

Copy link

codecov bot commented Oct 31, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.30%. Comparing base (50960ea) to head (907e50d).
Report is 63 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7874      +/-   ##
==========================================
+ Coverage   95.16%   95.30%   +0.13%     
==========================================
  Files         376      375       -1     
  Lines       11008    11002       -6     
  Branches     3653     3681      +28     
==========================================
+ Hits        10476    10485       +9     
+ Misses        532      517      -15     
Flag Coverage Δ
unittests 95.30% <100.00%> (+0.13%) ⬆️

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.


🚨 Try these New Features:

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.

👏 💅

@inomdzhon inomdzhon self-requested a review November 5, 2024 09:29
@EldarMuhamethanov EldarMuhamethanov marked this pull request as draft November 6, 2024 07:10
@EldarMuhamethanov EldarMuhamethanov requested a review from a team November 13, 2024 09:40
@EldarMuhamethanov EldarMuhamethanov marked this pull request as ready for review November 13, 2024 09:41
@qurle
Copy link
Contributor

qurle commented Nov 18, 2024

Сохранено специальное поведение для отображения label на VKCOM

Кажется, платформа vkcom не умрёт 🌝

qurle
qurle previously approved these changes Nov 18, 2024
Copy link
Contributor

@qurle qurle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Есть микроконсёрн, что у нас в дизайне при отсутствии текста ширина кнопки немного меньше: 30×44 на iOS, 44×44 на Android.

Было бы классно засинкать, чтобы не приходилось потом объяснять, но в целом не критикал именно для этой задачи.
image

В остальном feels good.

inomdzhon
inomdzhon previously approved these changes Nov 20, 2024
packages/vkui/src/components/PanelHeaderButton/Readme.md Outdated Show resolved Hide resolved
Co-authored-by: Inomdzhon Mirdzhamolov <[email protected]>
@EldarMuhamethanov EldarMuhamethanov dismissed stale reviews from inomdzhon and qurle via 907e50d November 20, 2024 12:54
@EldarMuhamethanov EldarMuhamethanov merged commit d35dac3 into master Nov 21, 2024
17 of 19 checks passed
@EldarMuhamethanov EldarMuhamethanov deleted the e.muhamethanov/panel-header-button-pressets-refactor branch November 21, 2024 13:45
@vkcom-publisher
Copy link
Contributor

v7.0.0-beta.2 🎉

@inomdzhon inomdzhon modified the milestones: v7.0.0-beta.2, v7.0.0 Dec 2, 2024
@vkcom-publisher
Copy link
Contributor

v7.0.0 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

6 participants