html
html copied to clipboard
Implement dialog initial focus proposal
This implements the changes proposed here: https://github.com/whatwg/html/wiki/dialog--initial-focus,-a-proposal#dialog-draft-text
Specifically:
- Add a parameter to dialog.show() called preventInitialFocus, which prevents the dialog focusing steps from running.
- Make the dialog focusing steps look at sequentially focusable elements instead of any focusable element.
There are additional open issues around dialog initial focus listed here: https://github.com/whatwg/html/pull/4184#issuecomment-1072536425
TODO This does not implement the change where calling showModal() on <dialog autofocus> will cause the dialog itself to get focus. Probably the best way to spec that is to change the showModal() spec to check for the autofocus attribute and, if present, focus the dialog instead of going into the focus delegate steps.
- [ ] At least two implementers are interested (and none opposed):
- …
- …
- [ ] Tests are written and can be reviewed and commented upon at:
- …
- [ ] Implementation bugs are filed:
- Chrome: …
- Firefox: …
- Safari: …
- Deno (only for timers, structured clone, base64 utils, channel messaging, module resolution, web workers, and web storage): …
- Node.js (only for timers, structured clone, base64 utils, channel messaging, and module resolution): …
(See WHATWG Working Mode: Changes for more details.)
/interaction.html ( diff ) /interactive-elements.html ( diff )
A big part of the proposal was writing up https://github.com/whatwg/html/wiki/dialog--initial-focus,-a-proposal#improving-conformance-requirements-and-examples as spec text, to replace the current single sentence "The dialog element represents a part of an application that a user interacts with to perform a task, for example a dialog box, inspector, or window."
Ah thanks, I totally missed this. It is now in the PR
This does not implement the change where calling showModal() on
<dialog autofocus>will cause the dialog itself to get focus. Probably the best way to spec that is to change the showModal() spec to check for the autofocus attribute and, if present, focus the dialog instead of going into the focus delegate steps.
I added a commit to do this. Instead of doing it all in showModal I changed the dialog focusing steps because I figured that it would still be important to run the other autofocus related steps at the end of the dialog focusing steps
Thanks for the review @domenic! I believe I've addressed all your comments
OK, this is looking good! #8174 got merged and I merged the changes into here, so this is good to go from my perspective.
Can you update the original post with a full description of the changes? It's a bit outdated, still mentioning preventInitialFocus and a TODO that I believe is taken care of. And it doesn't mention all the improvements to the explanatory text, etc.
At this point I believe we're waiting on tests mainly, since there was high-level consensus on these changes from all browsers some time ago. Still, now that we have a clear diff, review appreciated from @emilio @annevk @nt1m for the processing model changes, and if @zcorpan wants to do another review, or @sideshowbarker wants to do his review, on the conformance requirements / explanatory text, that would be appreciated.
I finally got a draft of the implementation in chromium done, and here is a new test and many other test changes to account for the focus ring being rendered on dialogs: (WPT PR export doesn't want to work yet, heres the chromium patch) I tried my best to keep the tests affected tests passing with or without this new behavior, but a couple of them will fail on firefox and safari and are probably tracked in interop22...
This does not implement the change where calling showModal() on
<dialog autofocus>will cause the dialog itself to get focus.
Does this mean that non-modal dialogs should not be focusable?
Implementing this change in chromium has resulted in open modal dialogs preventing the body from being focused when clicking outside of the dialog - that just focuses the dialog. Is that ok...?
Does this mean that non-modal dialogs should not be focusable?
I don't think we've given a lot of thought to non-modal dialogs. Good question. I would tend toward them not being focusable...
Implementing this change in chromium has resulted in open modal dialogs preventing the body from being focused when clicking outside of the dialog - that just focuses the dialog. Is that ok...?
This is surprising, and probably not desired? It might be good to consult the original folks behind the proposal to be sure, but I at least expect that clicking outside should cause the body to be focused.
Probably this is happening because technically the ::backdrop is part of the dialog, and so clicking on it focuses the dialog. Maybe if you add pointer-events: none !important like you did for popover=""??
Maybe if you add pointer-events: none !important like you did for popover=""??
That did the trick! I've added it to this PR
I don't think we've given a lot of thought to non-modal dialogs. Good question. I would tend toward them not being focusable...
I wonder if @scottaohara has any thoughts. I don't think there was any mention of different logic between them in the proposal/wiki he wrote:
Otherwise, if nothing inside the dialog is tabbable, focus the dialog itself, so that at least something gets focused.
Another side effect of my current chromium implementation is that it makes the dialog element keyboard focusable, meaning that I can focus the dialog element by pressing shift+tab when the first focusable element of the dialog is focused. Is this desired behavior...?
I talked to Mason and he mentioned that we could also try to make it only keyboard focusable if it has no keyboard focusable descendants.
Any thoughts? @scottaohara
The intent here was not to have the dialog element be included in the tab-key focus order for the web page. So if there's a focusable element inside of the dialog, someone should not be able to focus/re-focus the dialog, even if the dialog was focused by default. But the use case of a modal dialog being rendered without any focusable element within could be awkward for some people who are trying to enter a web page from the browser chrome by use of the tab key, but then finding that they cannot. Someone 'should', at least on windows, be able to use the F6 key to still move to the web document in that particular case. it's odd though. in this context where there is only a modal dialog without any focusable elements, i could potentially see why in this case alone it could be accessed via tab key from the browser window.
re: body not being focused when a modal dialog is open, body and its contents should be inert, so why would focus be expected to go there? Unless I'm missing something, mouse click shouldn't be able to focus the inert content, so it'd make sense for the backdrop to block that. Otherwise, the content outside of the modal dialog is not fully inert? Again, am I missing something? The pointer events none was added for popover because a popover is not supposed to be modal / cause content outside of it to be inert, and the mouse click to the body was to trigger light dismiss behavior.
re: focusing differences between modal / non-modal dialogs. I'm not sure why they'd be different unless focus was not moved to a non-modal dialog when it was invoked. But if focus moves to it, and there's the use case where there's no focusable element within... then where would focus go? If it remained on the element (likely button) that invoked the non-modal dialog, then that seems reasonable to me. If someone used an autofocus attribute on the non-modal dialog, then i'd still expect it to go to the dialog element itself. Otherwise there'd need to be a stipulation that autofocus can be used on a modal dialog, but not on a non-modal, but the only way to declare a dialog one way or the other is with the show or showModal methods. so outside of the attribute just failing to work, there wouldn't be a good way to surface this to the author through a markup validation error, unless i'm not thinking of something?
The intent here was not to have the dialog element be included in the tab-key focus order for the web page
Say no more, I'll implement it this way!
re: body not being focused when a modal dialog is open, body and its contents should be inert, so why would focus be expected to go there? Unless I'm missing something, mouse click shouldn't be able to focus the inert content, so it'd make sense for the backdrop to block that. Otherwise, the content outside of the modal dialog is not fully inert? Again, am I missing something? The pointer events none was added for popover because a popover is not supposed to be modal / cause content outside of it to be inert, and the mouse click to the body was to trigger light dismiss behavior.
This makes sense, but the current behavior in all browsers is that clicking outside of the dialog element focuses the body element, not the dialog. I can change it but I feel like it's another risk of breaking stuff and doesn't seem like the main point of this change. If you insist though I'll happily go for it.
Implementing this change in chromium has resulted in open modal dialogs preventing the body from being focused when clicking outside of the dialog - that just focuses the dialog. Is that ok...?
This is surprising, and probably not desired? It might be good to consult the original folks behind the proposal to be sure, but I at least expect that clicking outside should cause the body to be focused.
Probably this is happening because technically the ::backdrop is part of the dialog, and so clicking on it focuses the dialog. Maybe if you add pointer-events: none !important like you did for popover=""??
It turns out that this makes a WPT fail: https://github.com/web-platform-tests/wpt/pull/31758
@nt1m I'm guessing that you don't think we should add pointer-events: none to ::backdrop. What was the rationale?
re: body not being focused when a modal dialog is open, body and its contents should be inert, so why would focus be expected to go there? Unless I'm missing something, mouse click shouldn't be able to focus the inert content, so it'd make sense for the backdrop to block that. Otherwise, the content outside of the modal dialog is not fully inert? Again, am I missing something? The pointer events none was added for popover because a popover is not supposed to be modal / cause content outside of it to be inert, and the mouse click to the body was to trigger light dismiss behavior.
I guess that maybe the body never getting focus is OK... I think that made something tricky happen with the implementation or tests but it's been a while, I can give it a shot and see what happens.
@josepharhar That test was added for https://whistlr.info/2021/in-defence-of-dialog/#sidebar-%2F-nav-bar . See the part that says it works in Chrome, but not Safari.
Basically Safari used to consider the ::backdrop not as part of the dialog when querying the event target when clicking the ::backdrop, unlike Chrome & Firefox which considered ::backdrop as part of the element.
I don't have strong opinions on how this should behave fwiw, it was just a test to ensure interop.
There are still a couple of unresolved comment threads. It appears this also needs rebasing.
There are still a couple of unresolved comment threads. It appears this also needs rebasing.
I have resolved the comment threads and merged with a commit from 5 days ago.
re: body not being focused when a modal dialog is open, body and its contents should be inert, so why would focus be expected to go there? Unless I'm missing something, mouse click shouldn't be able to focus the inert content, so it'd make sense for the backdrop to block that. Otherwise, the content outside of the modal dialog is not fully inert? Again, am I missing something? The pointer events none was added for popover because a popover is not supposed to be modal / cause content outside of it to be inert, and the mouse click to the body was to trigger light dismiss behavior.
I guess that maybe the body never getting focus is OK... I think that made something tricky happen with the implementation or tests but it's been a while, I can give it a shot and see what happens.
That test was added for https://whistlr.info/2021/in-defence-of-dialog/#sidebar-%2F-nav-bar . See the part that says it works in Chrome, but not Safari.
I removed the pointer-events change and now the test is passing again.
I am happy with this being merged whenever ready. I am going to ship the changes in chromium soon. It looks like the last build failure is due to some infrastructure rather than my patch, it builds locally on my machine.