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

feat(HorizontalCell): add custom width support #7807

Merged
merged 3 commits into from
Oct 22, 2024
Merged

feat(HorizontalCell): add custom width support #7807

merged 3 commits into from
Oct 22, 2024

Conversation

BlackySoul
Copy link
Contributor

@BlackySoul BlackySoul commented Oct 22, 2024 •
edited by inomdzhon
Loading

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

Описание

Синхронизируемся с дизайном.

Breaking Change: скорее всего size="l" в HorizontalCell использовали для поддержки динамической ширины - поэтому компонент с данными размерами, скорее всего, поедет в верстке, потому что будет выставлена максимальная ширина в соответствии с ДС

Изменения

HorizontalCell:

Значения size расширены до - 's' | 'm' | 'l' | 'xl' | 'auto' | number:

  • 's' | 'm' | 'l' | 'xl' определяются дизайн-системой.
  • auto позволяет задать динамическую ширину, определяемую контентом.
  • пользовательскую ширину можно задать через числовое значение.

HorizontalCellShowMore:

  • size теперь принимает значения либо s, либо m - поддерживать другие значения как будто не имеет смысла, потому что они будут одинаковы по стилям

ScrollArrow:

  • size теперь принимает значения либо s, либо m

Release notes

BREAKING CHANGE

  • Gallery: значение arrowSize="l" удалено, используйте arrowSize="m", а вместо arrowSize="m" используйте arrowSize="s".

    Пример миграции 
    - <Gallery arrowSize="m">
+ <Gallery arrowSize="s">
- <Gallery arrowSize="l">
+ <Gallery arrowSize="m">

  • HorizontalCell: значение size="l" теперь имеет ограничение по максимальной ширине, воспользуйтесь size="auto" там, где это необходимо

  • HorizontalCellShowMore: значение size="l" удалено, используйте size="m"

    Пример миграции 
    - <HorizontalCellShowMore size="l">
+ <HorizontalCellShowMore size="m">

  • HorizontalScroll: значение arrowSize="l" удалено, используйте arrowSize="m", а вместо arrowSize="m" используйте arrowSize="s".

    Пример миграции 
    - <HorizontalScroll arrowSize="m">
+ <HorizontalScroll arrowSize="s">
- <HorizontalScroll arrowSize="l">
+ <HorizontalScroll arrowSize="m">

  • ScrollArrow: значение size="l" удалено, используйте size="m", а вместо size="m" используйте size="s"

    Пример миграции 
    - <ScrollArrow size="m">
+ <ScrollArrow size="s">
- <ScrollArrow size="l">
+ <ScrollArrow size="m">

Улучшения

  • HorizontalCell: значения size расширены до 's' | 'm' | 'l' | 'xl' | 'auto' | number

@BlackySoul BlackySoul requested a review from a team as a code owner October 22, 2024 09:08
@BlackySoul BlackySoul linked an issue Oct 22, 2024 that may be closed by this pull request
[Feature][HorizontalCell]: Поддержка кастомных значений ширины ячейки #7317
Closed
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 22, 2024 •
edited
Loading

size-limit report 📦

Path Size
JS 380.7 KB (+0.05% 🔺)
JS (gzip) 115.39 KB (+0.05% 🔺)
JS (brotli) 95 KB (+0.02% 🔺)
JS import Div (tree shaking) 1.46 KB (0%)
CSS 331.97 KB (+0.01% 🔺)
CSS (gzip) 42.07 KB (+0.03% 🔺)
CSS (brotli) 33.15 KB (-0.02% 🔽)

@codesandbox-ci CodeSandbox CI
Copy link

codesandbox-ci bot commented Oct 22, 2024 •
edited
Loading

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.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 22, 2024 •
edited
Loading

e2e tests

Playwright Report

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 22, 2024 •
edited
Loading

👀 Docs deployed

Commit b682e0f

@codecov Codecov
Copy link

codecov bot commented Oct 22, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.09%. Comparing base (1d408a5) to head (b682e0f).
Report is 8 commits behind head on master.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #7807      +/-   ##
==========================================
- Coverage   95.14%   95.09%   -0.05%     
==========================================
  Files         384      376       -8     
  Lines       11357    11079     -278     
  Branches     3727     3681      -46     
==========================================
- Hits        10806    10536     -270     
+ Misses        551      543       -8
Flag Coverage Δ
unittests 95.09% <100.00%> (-0.05%) ⬇️

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.

andrey-medvedev-vk
andrey-medvedev-vk previously approved these changes Oct 22, 2024
View reviewed changes
Copy link
Contributor

@andrey-medvedev-vk andrey-medvedev-vk left a comment

Choose a reason for hiding this comment

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

Класс! 🥇

@qurle
Copy link
Contributor

qurle commented Oct 22, 2024 •
edited
Loading

компонент с данными размерами, скорее всего, поедет в верстке

Заменим модом на auto?

@BlackySoul
Copy link
Contributor Author

BlackySoul commented Oct 22, 2024 •
edited by qurle
Loading

Заменим кодмодом на auto?

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

@qurle
Copy link
Contributor

qurle commented Oct 22, 2024 •
edited
Loading

Заменим кодмодом на auto?

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

Как будто звучит надёжнее, но могу ошибаться. Доверяю энивей

@qurle
Copy link
Contributor

qurle commented Oct 22, 2024 •
edited
Loading

HorizontalCellShowMore

size теперь принимает значения либо s, либо l

Как я понял по дизайну, для списка из m-ячеек тоже используется крупная стрелка. Мб тогда логичнее size = 's' | 'm' для стрелки?

@BlackySoul
Copy link
Contributor Author

Как я понял по дизайну, для списка из m-ячеек тоже используется крупная стрелка. Мб тогда логичнее size = 's' | 'm' для стрелки?

Просто в фигме значение small и large - ориентировалась на них. Если вы у себя тоже меняете на m, то давайте заменим в коде

@BlackySoul
feat(HorizontalCellShowMore): change size values
b682e0f 
feat(ScrollArrow): change size values
qurle
qurle approved these changes Oct 22, 2024
View reviewed changes
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.

все класс 10/10 спасибо

andrey-medvedev-vk
andrey-medvedev-vk approved these changes Oct 22, 2024
View reviewed changes
Copy link
Contributor

@andrey-medvedev-vk andrey-medvedev-vk 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 approved these changes Oct 22, 2024
View reviewed changes
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.

👏

@BlackySoul BlackySoul merged commit 90ea9e0 into master Oct 22, 2024
27 of 28 checks passed
@BlackySoul BlackySoul deleted the feat/7317/HorizontalCell_add_custom_width branch October 22, 2024 14:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@inomdzhon inomdzhon inomdzhon approved these changes

@qurle qurle qurle approved these changes

@andrey-medvedev-vk andrey-medvedev-vk andrey-medvedev-vk approved these changes

@EldarMuhamethanov EldarMuhamethanov EldarMuhamethanov left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
5 participants
@BlackySoul @qurle @inomdzhon @EldarMuhamethanov @andrey-medvedev-vk