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

fix(SubnavigationBar): combining buttons into a list [a11y] #5225

Merged
merged 18 commits into from
Jul 17, 2023
Merged

fix(SubnavigationBar): combining buttons into a list [a11y] #5225

merged 18 commits into from
Jul 17, 2023

Conversation

scffs
Copy link
Contributor

@scffs scffs commented Jun 11, 2023

Не уверен насчет правильности решения, в доке SubnavigationButton используется без SubnavigationBar, поэтому возникает это (li внутри div)

image

UPD
В одном из своих проектов у меня есть такой же самописный компонент, в нём такая логика, попробую позже что-то типа такого же реализовтаь. (Children#children-toarray)

image

UPD
Сделал в этом коммите

a11y тесты теперь не падают, ура

UPD
Немного сломались стили, теперь margin-left не срабатывает

image

UPD
Как-то пофиксил стили, но скорее всего там ещё надо будет что-то менять...

@scffs scffs requested a review from a team as a code owner June 11, 2023 10:06
@codesandbox-ci
Copy link

codesandbox-ci bot commented Jun 11, 2023

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.

Latest deployment of this branch, based on commit 24882e8:

Sandbox Source
VKUI TypeScript Configuration

@codecov
Copy link

codecov bot commented Jun 11, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.04 ⚠️

Comparison is base (9e8f40f) 81.70% compared to head (24882e8) 81.66%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5225      +/-   ##
==========================================
- Coverage   81.70%   81.66%   -0.04%     
==========================================
  Files         283      285       +2     
  Lines        9476     9555      +79     
  Branches     3013     3050      +37     
==========================================
+ Hits         7742     7803      +61     
- Misses       1734     1752      +18     
Flag Coverage Δ
a11ytests ?
unittests 81.66% <0.00%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...c/components/SubnavigationBar/SubnavigationBar.tsx 80.95% <0.00%> (-4.77%) ⬇️

... and 33 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@github-actions
Copy link
Contributor

github-actions bot commented Jun 11, 2023

size-limit report 📦

Path Size
JS 311.33 KB (+0.03% 🔺)
JS (gzip) 91.03 KB (+0.02% 🔺)
JS (brotli) 75.53 KB (+0.02% 🔺)
JS import Div (tree shaking) 2.97 KB (0%)
CSS 272.91 KB (+0.01% 🔺)
CSS (gzip) 35.7 KB (+0.02% 🔺)
CSS (brotli) 28.29 KB (+0.07% 🔺)

@github-actions
Copy link
Contributor

github-actions bot commented Jun 11, 2023

👀 Docs deployed

Commit 24882e8

@github-actions
Copy link
Contributor

github-actions bot commented Jun 11, 2023

e2e tests

Playwright Report

scffs added 4 commits June 11, 2023 20:41
…ponent="li"` props in `Tappable`

Signed-off-by: scffs <khidey@inbox.ru>
Signed-off-by: scffs <khidey@inbox.ru>
Signed-off-by: scffs <khidey@inbox.ru>
Signed-off-by: scffs <khidey@inbox.ru>
@scffs scffs changed the title fix(SubnavigationBar): combining buttons into a list [a11y ] fix(SubnavigationBar): combining buttons into a list [a11y] Jun 15, 2023
….module.css

Co-authored-by: Daniil Suvorov <severecloud@gmail.com>
@SevereCloud SevereCloud requested a review from eugpoloz June 22, 2023 07:54
scffs and others added 2 commits June 22, 2023 15:31
….module.css

Co-authored-by: Daniil Suvorov <severecloud@gmail.com>
unset ломает верстку

Co-authored-by: Daniil Suvorov <severecloud@gmail.com>
SevereCloud
SevereCloud previously approved these changes Jun 22, 2023
@github-actions github-actions bot added the pr-needs-work Автоматизация: PR автоматически закроется через 14 дней при отсутствии активности label Jun 29, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jul 7, 2023

PR закрыт из-за отсутствия активности в течение последних 14 дней. Если это произошло по ошибке или изменения все ещё актуальны, откройте PR повторно.

@github-actions github-actions bot closed this Jul 7, 2023
@SevereCloud SevereCloud reopened this Jul 10, 2023
@SevereCloud SevereCloud removed the pr-needs-work Автоматизация: PR автоматически закроется через 14 дней при отсутствии активности label Jul 10, 2023
scffs added 2 commits July 12, 2023 22:15
Signed-off-by: scffs <khidey@inbox.ru>
Signed-off-by: scffs <khidey@inbox.ru>
scffs added 2 commits July 13, 2023 16:30
@scffs
Copy link
Contributor Author

scffs commented Jul 13, 2023

Из-за этого я подумал, что это уже удалить надо сейчас)

image

P.S. Типа красный цвет все дела)

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
Copy link
Contributor

@scffs добавишь плиз в задачу #5023 коммент со своей ссылкой на твой профиль ВКонтакте 🙏 Заодно смогу заасайнить тебя. Отметил задачу как 🔥 fix bounty

@inomdzhon inomdzhon added the patch Автоматизация: PR продублируется в ветку последнего минорного релиза для выпуска патча label Jul 13, 2023
@SevereCloud SevereCloud merged commit ccfb8e4 into VKCOM:master Jul 17, 2023
@vkcom-publisher
Copy link
Contributor

⚠️ Patch (forked repo)

Необходимо вручную перенести исправление в стабильную ветку.

Дальнейшие действия выполняют контрибьютеры из группы @VKCOM/vkui-core

Чтобы исправление попало в стабильную ветку, выполните следующие действия:

  1. Создайте новую ветку от стабильной и примените исправления используя cherry-pick
git stash # опционально
git fetch origin 5.6-stable
git checkout -b patch/pr5225 origin/5.6-stable

git cherry-pick --no-commit ccfb8e4a539784ddb9d62ce0b22c228c3c73e9dd
git checkout HEAD **/__image_snapshots__/*.png
git diff --quiet HEAD || git commit --no-verify --no-edit
  1. Исправьте конфликты, следуя инструкциям из терминала
  2. Отправьте ветку на GitHub и создайте новый PR с последней стабильной веткой (метка patch не нужна)
git push --set-upstream origin patch/pr5225
gh pr create --base 5.6-stable --title "patch: pr5225" --body "- patch #5225"

SevereCloud added a commit that referenced this pull request Jul 17, 2023
* fix(Popover): fix incorrect words ending

* Revert "doc(Popover): incorrect declension of two words"

This reverts commit 8494645.

* fix(SubnavigationBar): the `SubnavigationButton` components are combined into a list

Signed-off-by: scffs <khidey@inbox.ru>

* fix(SubnavigationBar): getting rid of the extra div

Signed-off-by: scffs <khidey@inbox.ru>

* Revert "Revert "doc(Popover): incorrect declension of two words""

This reverts commit 9ff5698.

* fix(SubnavigationBar): using `React.Children.toArray` instead of `Component="li"` props in `Tappable`

Signed-off-by: scffs <khidey@inbox.ru>

* fix(SubnavigationBar): added `key`

Signed-off-by: scffs <khidey@inbox.ru>

* fix(SubnavigationBar): fix styles

Signed-off-by: scffs <khidey@inbox.ru>

* fix(SubnavigationBar): returned a comment

Signed-off-by: scffs <khidey@inbox.ru>

* Update packages/vkui/src/components/SubnavigationBar/SubnavigationBar.module.css

Co-authored-by: Daniil Suvorov <severecloud@gmail.com>

* Update packages/vkui/src/components/SubnavigationBar/SubnavigationBar.module.css

Co-authored-by: Daniil Suvorov <severecloud@gmail.com>

* combine into one padding

* fix invalid prop value

unset ломает верстку

Co-authored-by: Daniil Suvorov <severecloud@gmail.com>

* feat(subnavigation-bar-a11y): del global

Signed-off-by: scffs <khidey@inbox.ru>

* feat(subnavigation-bar-a11y): del `className` and add `TODO`

Signed-off-by: scffs <khidey@inbox.ru>

* Revert "feat(subnavigation-bar-a11y): del global"

This reverts commit 4a5a80a.

* feat(subnavigation-bar-a11y): move TODO

Signed-off-by: scffs <khidey@inbox.ru>

---------

Signed-off-by: scffs <khidey@inbox.ru>
Co-authored-by: Daniil Suvorov <severecloud@gmail.com>
SevereCloud added a commit that referenced this pull request Jul 17, 2023
* feat(update-doc): add htmlFor / bottomId + aria-describedby (#5459)

Signed-off-by: scffs <khidey@inbox.ru>
Co-authored-by: Inomdzhon Mirdzhamolov <inomjon92@gmail.com>
Co-authored-by: GitHub Action <actions@github.com>

* fix(SubnavigationBar): combining buttons into a list [a11y] (#5225)

* fix(Popover): fix incorrect words ending

* Revert "doc(Popover): incorrect declension of two words"

This reverts commit 8494645.

* fix(SubnavigationBar): the `SubnavigationButton` components are combined into a list

Signed-off-by: scffs <khidey@inbox.ru>

* fix(SubnavigationBar): getting rid of the extra div

Signed-off-by: scffs <khidey@inbox.ru>

* Revert "Revert "doc(Popover): incorrect declension of two words""

This reverts commit 9ff5698.

* fix(SubnavigationBar): using `React.Children.toArray` instead of `Component="li"` props in `Tappable`

Signed-off-by: scffs <khidey@inbox.ru>

* fix(SubnavigationBar): added `key`

Signed-off-by: scffs <khidey@inbox.ru>

* fix(SubnavigationBar): fix styles

Signed-off-by: scffs <khidey@inbox.ru>

* fix(SubnavigationBar): returned a comment

Signed-off-by: scffs <khidey@inbox.ru>

* Update packages/vkui/src/components/SubnavigationBar/SubnavigationBar.module.css

Co-authored-by: Daniil Suvorov <severecloud@gmail.com>

* Update packages/vkui/src/components/SubnavigationBar/SubnavigationBar.module.css

Co-authored-by: Daniil Suvorov <severecloud@gmail.com>

* combine into one padding

* fix invalid prop value

unset ломает верстку

Co-authored-by: Daniil Suvorov <severecloud@gmail.com>

* feat(subnavigation-bar-a11y): del global

Signed-off-by: scffs <khidey@inbox.ru>

* feat(subnavigation-bar-a11y): del `className` and add `TODO`

Signed-off-by: scffs <khidey@inbox.ru>

* Revert "feat(subnavigation-bar-a11y): del global"

This reverts commit 4a5a80a.

* feat(subnavigation-bar-a11y): move TODO

Signed-off-by: scffs <khidey@inbox.ru>

---------

Signed-off-by: scffs <khidey@inbox.ru>
Co-authored-by: Daniil Suvorov <severecloud@gmail.com>

---------

Signed-off-by: scffs <khidey@inbox.ru>
Co-authored-by: Inomdzhon Mirdzhamolov <inomjon92@gmail.com>
Co-authored-by: GitHub Action <actions@github.com>
@scffs scffs deleted the subnavigation-bar-a11y branch July 17, 2023 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch Автоматизация: PR продублируется в ветку последнего минорного релиза для выпуска патча
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Enhancement][a11y][SubnavigationBar]: Улучшить доступность SubnavigationBar
4 participants