-
Notifications
You must be signed in to change notification settings - Fork 184
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
a11y(CardScroll): fix a11y, ul>li combination #8140
a11y(CardScroll): fix a11y, ul>li combination #8140
Conversation
size-limit report 📦
|
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. |
e2e tests |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #8140 +/- ##
=======================================
Coverage 95.49% 95.49%
=======================================
Files 403 403
Lines 11503 11505 +2
Branches 3812 3812
=======================================
+ Hits 10985 10987 +2
Misses 518 518
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
👀 Docs deployed
Commit 4cc1e54 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Красота 👏 👏 👏 !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PS: в Realese note опечаточки
packages/vkui/src/components/HorizontalScroll/HorizontalScroll.tsx
Outdated
Show resolved
Hide resolved
# Conflicts: # packages/vkui/src/components/HorizontalScroll/HorizontalScroll.tsx
…/fix-card-scroll-a11y # Conflicts: # packages/vkui/src/components/CardScroll/CardScroll.test.tsx
4cc1e54
ul
тег по умолчанию нарушает семантику ul>li #8135Описание
Сейчас в компоненте
CardScroll
есть проблема с доступностью. Карточкиli
не являются прямыми потомкамиul
. Нужно придумать как решить эту проблемуИзменения
Component
с 'ul' на 'div'(как было раньше). Почему это не breaking change? Потому что по идее это баг, текущее поведение все равно не работало как задумывалосьCardsListComponent
, которое позволяет поменять тег используемый для списка карточек. Значение по умолчанию"ul"
CardsScroll
, чтобы уменьшить количество нод DOM дерева. Сделал падинги по бокам через псевдоэлементыafter/before
.HorizontalScroll
добавил свойстваContentWrapperComponent
,contentWrapperRef
иcontentWrapperClassName
для кастомизации обертки над контентом, прокинутом вchildren
. Это было сделано, чтобы вCardsScroll
и других возможных местах не создавать доп обертку над контентом и уменьшить количество нод.a11y
- теперь они проходятRelease notes
Улучшения
ContentWrapperComponent
,contentWrapperRef
иcontentWrapperClassName
для кастомизации обертки над контентом, прокинутом вchildren
CardsListComponent
, которое позволяет поменять тег используемый для списка карточекИсправления
li
(карточки) являются прямыми потомкамиul
(списка карточек)Component
с"ul"
, на"div"
. Для правильно семантики тегов нужно использовать свойствоCardsListComponent
, которое по умолчанию теперь"ul"