Skip to content

fix(Footer): Add a11y documentation for the Footer component and use role=contentinfo by default#7378

Merged
mendrew merged 5 commits intomasterfrom
mendrew/Footer/a11y-doc
Aug 23, 2024
Merged

fix(Footer): Add a11y documentation for the Footer component and use role=contentinfo by default#7378
mendrew merged 5 commits intomasterfrom
mendrew/Footer/a11y-doc

Conversation

@mendrew
Copy link
Copy Markdown
Contributor

@mendrew mendrew commented Aug 13, 2024


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

Описание

Действительно, отсутствует роль contentinfo на элемененте, чтобы поддержать правильную работу скринридеров в Safari<v13.
Но также заметил, что, возможно использовать нативный тег footer в этом компоненте Footer было излишне, потому что этот компонент не очень поход на семантическо значение footer.

Обычно в футере находится справочная информация о сайте, копирайтинг, основная навигация, ссылки на социальные сети и другой похожий контент. doka

Поэтому довольно трудно написать рекомендации по доступности, когда пример использования по смыслу нарушает правила использования footer.
В нашем примере, мы кладём Footer вне Group, тем самым как бы устанавливая Footer для всей страницы, что явно не то, чего мы хотели бы добиться семантически в реальности.


В идеале, в v7 я бы убрал использование нативного тега footer, чтобы не ломать семантику страниц неправильным использованием, потому что, подозреваю, его чаще используют и будут использовать с декоративными целями, а не по смыслу, закладываемому в тег footer.

Изменения

  • Добавилено автоматическое выставление role=contentinfo если Component="footer".
  • Добавлен раздел по доступности с пояснениями когда и как использовать компонент с дефолтным тегом footer. Приведены примеры, где стараемся объяснить где есть смысл в footer а где лучше div.

@github-actions github-actions bot added the ci:cherry-pick:patch Автоматизация: PR продублируется в ветку последнего минорного релиза для выпуска патча label Aug 13, 2024
@codesandbox-ci
Copy link
Copy Markdown

codesandbox-ci bot commented Aug 13, 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.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Aug 13, 2024

size-limit report 📦

Path Size
JS 378.46 KB (+0.03% 🔺)
JS (gzip) 114.82 KB (+0.03% 🔺)
JS (brotli) 94.18 KB (-0.11% 🔽)
JS import Div (tree shaking) 1.39 KB (0%)
CSS 306.22 KB (0%)
CSS (gzip) 39.28 KB (0%)
CSS (brotli) 31.49 KB (0%)

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Aug 13, 2024

e2e tests

Playwright Report

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Aug 13, 2024

👀 Docs deployed

Commit 89f3419

@codecov
Copy link
Copy Markdown

codecov bot commented Aug 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.48%. Comparing base (ead90ac) to head (89f3419).
Report is 6 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #7378   +/-   ##
=======================================
  Coverage   94.47%   94.48%           
=======================================
  Files         375      375           
  Lines       11123    11126    +3     
  Branches     3650     3652    +2     
=======================================
+ Hits        10509    10512    +3     
  Misses        614      614           
Flag Coverage Δ
unittests 94.48% <100.00%> (+<0.01%) ⬆️

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.

@mendrew mendrew marked this pull request as ready for review August 13, 2024 12:23
@mendrew mendrew requested a review from a team as a code owner August 13, 2024 12:23
@BlackySoul
Copy link
Copy Markdown
Contributor

А мы сейчас никакое поведение не сломаем тем, что будем выставлять role=contentinfo по умолчанию, хотя оно должно быть одно на страницу? Или оно типа и сейчас уже сломано?)

@mendrew
Copy link
Copy Markdown
Contributor Author

mendrew commented Aug 14, 2024

А мы сейчас никакое поведение не сломаем тем, что будем выставлять role=contentinfo по умолчанию, хотя оно должно быть одно на страницу? Или оно типа и сейчас уже сломано?)

Дополнительно не сломаем. Оно и сейчас так) По умолчанию у footer такая роль. Явное дублирование роли нужно, чтобы все бразуеры точно поняли что это за контент. В частности это нужно для Safari<13.

@mendrew mendrew changed the title fix[Footer]: Add a11y documentation for the Footer component and use role=contentinfo by default fix(Footer): Add a11y documentation for the Footer component and use role=contentinfo by default Aug 16, 2024
Copy link
Copy Markdown
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.

💅

@mendrew mendrew requested a review from inomdzhon August 20, 2024 08:20
inomdzhon
inomdzhon previously approved these changes Aug 20, 2024
Copy link
Copy Markdown
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.

👏

@mendrew mendrew added ci:cherry-pick:patch Автоматизация: PR продублируется в ветку последнего минорного релиза для выпуска патча and removed ci:cherry-pick:patch Автоматизация: PR продублируется в ветку последнего минорного релиза для выпуска патча labels Aug 22, 2024
@mendrew mendrew added this to the v6.5.2 milestone Aug 22, 2024
SevereCloud
SevereCloud previously approved these changes Aug 22, 2024
@mendrew mendrew removed this from the v6.5.2 milestone Aug 22, 2024
Add role="contentinfo" if tag is 'footer'.
Add examples where we should and should not use native tag footer.
@mendrew mendrew dismissed stale reviews from SevereCloud and inomdzhon via 89f3419 August 22, 2024 12:45
@mendrew mendrew force-pushed the mendrew/Footer/a11y-doc branch from 146ebc6 to 89f3419 Compare August 22, 2024 12:45
@mendrew mendrew merged commit 4ba7ed0 into master Aug 23, 2024
@mendrew mendrew deleted the mendrew/Footer/a11y-doc branch August 23, 2024 08:43
@vkcom-publisher
Copy link
Copy Markdown
Contributor

❌ Patch

Не удалось автоматически применить исправление на ветке 6.5-stable.

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

Чтобы изменение попало в ветку 6.5-stable, выполните следующие действия:

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

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

mendrew added a commit that referenced this pull request Aug 23, 2024
…role=contentinfo by default (#7378)

Добавилено автоматическое выставление role=contentinfo если Component="footer".
Добавлен раздел по доступности с пояснениями когда и как использовать компонент с дефолтным тегом footer. Приведены примеры, где стараемся объяснить где есть смысл в footer а где лучше div.

Заметили, что, возможно, использовать нативный тег footer в этом компоненте Footer было излишне, потому что этот компонент не очень похож на семантическо значение footer.

    Обычно в футере находится справочная информация о сайте, копирайтинг, основная навигация, ссылки на социальные сети и другой похожий контент. doka

Поэтому довольно трудно написать рекомендации по доступности, когда пример использования по смыслу нарушает правила использования footer.
В нашем примере, мы кладём Footer вне Group, тем самым как бы устанавливая Footer для всей страницы, что явно не то, чего мы хотели бы добиться семантически в реальности.

В идеале, в v7 я бы убрал использование нативного тега footer, чтобы не ломать семантику страниц неправильным использованием, потому что, подозреваю, его чаще используют и будут использовать с декоративными целями, а не по смыслу, закладываемому в тег footer.
Изменения
@mendrew mendrew mentioned this pull request Aug 23, 2024
mendrew added a commit that referenced this pull request Aug 23, 2024
…role=contentinfo by default (#7378) (#7432)

Добавилено автоматическое выставление role=contentinfo если Component="footer".
Добавлен раздел по доступности с пояснениями когда и как использовать компонент с дефолтным тегом footer. Приведены примеры, где стараемся объяснить где есть смысл в footer а где лучше div.

Заметили, что, возможно, использовать нативный тег footer в этом компоненте Footer было излишне, потому что этот компонент не очень похож на семантическо значение footer.

    Обычно в футере находится справочная информация о сайте, копирайтинг, основная навигация, ссылки на социальные сети и другой похожий контент. doka

Поэтому довольно трудно написать рекомендации по доступности, когда пример использования по смыслу нарушает правила использования footer.
В нашем примере, мы кладём Footer вне Group, тем самым как бы устанавливая Footer для всей страницы, что явно не то, чего мы хотели бы добиться семантически в реальности.

В идеале, в v7 я бы убрал использование нативного тега footer, чтобы не ломать семантику страниц неправильным использованием, потому что, подозреваю, его чаще используют и будут использовать с декоративными целями, а не по смыслу, закладываемому в тег footer.
Изменения
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci:cherry-pick:patch Автоматизация: PR продублируется в ветку последнего минорного релиза для выпуска патча

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Enhancement][a11y][Footer]: Добавить role="contentinfo"

5 participants