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(Modal): focus to modal container if controllable element doesn't focused #6054

Merged

Conversation

inomdzhon
Copy link
Contributor

@inomdzhon inomdzhon commented Oct 26, 2023


Описание

При открытии, фокусируемся на контейнер ModalPage/ModalCard если внутри модалки нет элемента, на котором уже есть фокус (проверяем через document.activeElement).

Изменения

  • Для контейнера ModalPage/ModalCard добавлены tabIndex={-1}, чтобы была возможность фокусироваться на него, но при этом по Tab'у на него пользователь больше не попадал.
  • Добавил параметр noFocusToDialog, чтобы была возможность отключить фичу (понадобилось в e2e тестах ModalCard, см. коммент в тестах).
  • Добавил отключение outline на фокус в CSS для ModalPage и ModalCard.
  • Для ModalCard добавил недостающие для a11y атрибуты.

@inomdzhon inomdzhon requested a review from a team as a code owner October 26, 2023 13:25
@inomdzhon inomdzhon force-pushed the imirdzhamolov/issue5646/fix/a11y-Modal-first-open-state branch from b3cb306 to 8beff6f Compare October 26, 2023 13:27
@inomdzhon inomdzhon added the v5 Автоматизация: PR продублируется в ветку v5 label Oct 26, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Oct 26, 2023

size-limit report 📦

Path Size
JS 332.24 KB (+0.12% 🔺)
JS (gzip) 101.47 KB (+0.08% 🔺)
JS (brotli) 83.73 KB (+0.07% 🔺)
JS import Div (tree shaking) 2.71 KB (0%)
CSS 255.21 KB (+0.03% 🔺)
CSS (gzip) 33.56 KB (+0.05% 🔺)
CSS (brotli) 27.23 KB (+0.08% 🔺)

@codesandbox-ci
Copy link

codesandbox-ci bot commented Oct 26, 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 a7feb79:

Sandbox Source
VKUI TypeScript Configuration

@github-actions
Copy link
Contributor

github-actions bot commented Oct 26, 2023

e2e tests

Playwright Report

@github-actions
Copy link
Contributor

github-actions bot commented Oct 26, 2023

👀 Docs deployed

Commit a7feb79

@codecov
Copy link

codecov bot commented Oct 26, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (2a84771) 79.23% compared to head (a7feb79) 79.27%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6054      +/-   ##
==========================================
+ Coverage   79.23%   79.27%   +0.04%     
==========================================
  Files         305      305              
  Lines        9598     9609      +11     
  Branches     3247     3252       +5     
==========================================
+ Hits         7605     7618      +13     
+ Misses       1993     1991       -2     
Flag Coverage Δ
unittests 79.27% <100.00%> (+0.04%) ⬆️

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

Files Coverage Δ
...ckages/vkui/src/components/ModalCard/ModalCard.tsx 96.15% <100.00%> (+0.50%) ⬆️
...ckages/vkui/src/components/ModalPage/ModalPage.tsx 97.82% <100.00%> (+0.09%) ⬆️
...ckages/vkui/src/components/ModalRoot/ModalRoot.tsx 54.10% <100.00%> (+0.61%) ⬆️
...vkui/src/components/ModalRoot/ModalRootDesktop.tsx 95.19% <100.00%> (+0.14%) ⬆️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@inomdzhon inomdzhon force-pushed the imirdzhamolov/issue5646/fix/a11y-Modal-first-open-state branch from 8beff6f to a7e5315 Compare October 26, 2023 13:37
@BlackySoul
Copy link
Contributor

Кажется, что-то отломалось:

Было:

2023-10-27.13.30.41.mov

Стало:

2023-10-27.13.32.14.mov

@inomdzhon
Copy link
Contributor Author

Кажется, что-то отломалось:

Было:

2023-10-27.13.30.41.mov
Стало:

2023-10-27.13.32.14.mov

Сейчас фокус на само диалоговое окно идёт, куда навешан role="dialog"

Фокус на кнопку закрытия при открытии не должно быть

… focused

h2. Описание

При открытии, фокусируемся на контейнер `ModalPage`/`ModalCard` если
внутри модалки нет элемента, на котором уже есть фокус (проверяем через
`document.activeElement`).

h2. Изменения

- Для контейнера `ModalPage`/`ModalCard` добавлены `tabIndex={-1}`,
  чтобы была возможность фокусироваться на него, но при этом по Tab'у на
  него пользователь больше не попадал.
- Добавил параметр `noFocusToDialog`, чтобы была возможность отключить
  фичу (понадобилось в e2e тестах `ModalCard`, см. коммент в тестах).
- Добавил отключение `outline` на фокус в CSS для `ModalPage`
  и `ModalCard`.
- Для `ModalCard` добавил недостающие для a11y атрибуты.
@inomdzhon inomdzhon force-pushed the imirdzhamolov/issue5646/fix/a11y-Modal-first-open-state branch from a7e5315 to a7feb79 Compare October 27, 2023 11:12
Copy link
Contributor

@mendrew mendrew left a comment

Choose a reason for hiding this comment

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

Looks great!

@mendrew mendrew added the patch Автоматизация: PR продублируется в ветку последнего минорного релиза для выпуска патча label Nov 8, 2023
@mendrew mendrew merged commit a0bf794 into master Nov 8, 2023
24 checks passed
@mendrew mendrew deleted the imirdzhamolov/issue5646/fix/a11y-Modal-first-open-state branch November 8, 2023 14:45
@vkcom-publisher
Copy link
Contributor

❌ Patch

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

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

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

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

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

vkcom-publisher pushed a commit that referenced this pull request Nov 8, 2023
… focused (#6054)

h2. Описание

При открытии, фокусируемся на контейнер `ModalPage`/`ModalCard` если
внутри модалки нет элемента, на котором уже есть фокус (проверяем через
`document.activeElement`).

h2. Изменения

- Для контейнера `ModalPage`/`ModalCard` добавлены `tabIndex={-1}`,
  чтобы была возможность фокусироваться на него, но при этом по Tab'у на
  него пользователь больше не попадал.
- Добавил параметр `noFocusToDialog`, чтобы была возможность отключить
  фичу (понадобилось в e2e тестах `ModalCard`, см. коммент в тестах).
- Добавил отключение `outline` на фокус в CSS для `ModalPage`
  и `ModalCard`.
- Для `ModalCard` добавил недостающие для a11y атрибуты.
mendrew added a commit that referenced this pull request Nov 8, 2023
… focused (#6054)

h2. Описание

При открытии, фокусируемся на контейнер `ModalPage`/`ModalCard` если
внутри модалки нет элемента, на котором уже есть фокус (проверяем через
`document.activeElement`).

h2. Изменения

- Для контейнера `ModalPage`/`ModalCard` добавлены `tabIndex={-1}`,
  чтобы была возможность фокусироваться на него, но при этом по Tab'у на
  него пользователь больше не попадал.
- Добавил параметр `noFocusToDialog`, чтобы была возможность отключить
  фичу (понадобилось в e2e тестах `ModalCard`, см. коммент в тестах).
- Добавил отключение `outline` на фокус в CSS для `ModalPage`
  и `ModalCard`.
- Для `ModalCard` добавил недостающие для a11y атрибуты.
@mendrew mendrew mentioned this pull request Nov 8, 2023
mendrew added a commit that referenced this pull request Nov 8, 2023
… focused (#6054) (#6085)

h2. Описание

При открытии, фокусируемся на контейнер `ModalPage`/`ModalCard` если
внутри модалки нет элемента, на котором уже есть фокус (проверяем через
`document.activeElement`).

h2. Изменения

- Для контейнера `ModalPage`/`ModalCard` добавлены `tabIndex={-1}`,
  чтобы была возможность фокусироваться на него, но при этом по Tab'у на
  него пользователь больше не попадал.
- Добавил параметр `noFocusToDialog`, чтобы была возможность отключить
  фичу (понадобилось в e2e тестах `ModalCard`, см. коммент в тестах).
- Добавил отключение `outline` на фокус в CSS для `ModalPage`
  и `ModalCard`.
- Для `ModalCard` добавил недостающие для a11y атрибуты.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch Автоматизация: PR продублируется в ветку последнего минорного релиза для выпуска патча v5 Автоматизация: PR продублируется в ветку v5
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature]: Фокус на ModalCard
5 participants