fix(web-components): avoid dialog from focusing on non-active tab upon showing#36278
fix(web-components): avoid dialog from focusing on non-active tab upon showing#36278marchbox wants to merge 10 commits into
Conversation
📊 Bundle size report✅ No changes found |
|
Pull request demo site: URL |
| @@ -0,0 +1,7 @@ | |||
| { | |||
There was a problem hiding this comment.
🕵🏾♀️ visual changes to review in the Visual Change Report
vr-tests-web-components/Badge 1 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-web-components/Badge. - Dark Mode.normal.chromium.png | 443 | Changed |
vr-tests-web-components/MenuList 1 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-web-components/MenuList. - RTL.1st selected.chromium_2.png | 38479 | Changed |
davatron5000
left a comment
There was a problem hiding this comment.
Mixed feelings about the empty <div> but makes sense given the situation.
Are we pretty sure this is localized to how the native <dialog> handles focus managenet? Another way to put it, is this an issue with fluent-tablist in other contexts like a dropdown?
@davatron5000 I believe so. The issue is caused by browsers proactively manages the focus for Also this is more of an issue with the focusgroup polyfill than |
Previous Behavior
When having this structure:
When the dialog is open, in Chromium and WebKit,
tab1becomes selected andactiveid’s value becomestab1automatically.The root cause is:
<fluent-dialog>uses an HTML<dialog>in its shadow root, whenFluentDialog::show()is called, it calls<dialog>.show()or<dialog>.showModal()<dialog>is open, browsers will determine which element gets initially focusedtabindexvaluetabindex=-1, receives the initial focus<fluent-tablist>, itsfocusinevent handler changes theactiveidattribute to the ID of the tab that just receives the focusThis won’t be an issue if the
<fluent-dialog-body>, or any slotted content/descendant of the<dialog>in shadow root, contains another focusable element prior to the tablist, e.g. the dialog close button.New Behavior
Added a
<div tabindex=-1></div>element before any slotted content of<fluent-dialog>, this makes sure the<fluent-tab>withtabindex=-1won’t receive the initial focus and cause the issue.Accessibility
This effectively puts the focus on the
<dialog>, in my VoiceOver test, it’d read out the dialog role and its label, if any. HittingTabkey again will move focus to the next focusable element, e.g. a tablist. And if any element in the dialog content hasautofocusattribute, the empty div will not prevent that element from being auto focused.Other solutions considered:
<dialog>’sbeforetoggleevent, if it doesn’t contain any element withautofocusattribute, addautofocusto the<dialog>itself. 2 issues with this:autofocusto the<dialog>itself if it doesn’t contain any focusable element, which isn’t our caseautofocusattribute as long as there’s any focusable element in its content<fluent-tablist>used in<fluent-dialog-body>, and addautofocusattritube to the<fluent-tab>element based onactiveid. 2 issues:<fluent-tablist>, it does nothing to the root cause<fluent-tablist>’sfocusinevent handler, check if theevent.relatedTargetis contained within the tablist itself, if not, don’t updateactiveid. This doesn’t work well with the focusgroup polyfill, since the polyfill would still set the “current” element to the focused tab in itsfocusinevent handler, so this will create an inconsistency between the component and the polyfill. Fixing this may require a somewhat significant update to the polyfill.Related Issue(s)
fluent-tablistinside afluent-dialog-bodyseems to ignore thisactiveidattribute #36248