-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Only restore popover focus if focus is in the popover #9018
Conversation
Popovers will restore focus to the element which was focued before opening the popover when the popover is closed. However, if a user opens a popover and then tab-navigates away from the popover, focus should not be changed when the popover is closed. More context: https://chromium-review.googlesource.com/c/chromium/src/+/4322607
source
Outdated
@@ -82072,7 +82072,9 @@ dictionary <dfn dictionary>DragEventInit</dfn> : <span>MouseEventInit</span> { | |||
<ol> | |||
<li><p>Set <var>element</var>'s <span>previously focused element</span> to null.</p></li> | |||
|
|||
<li><p>If <var>focusPreviousElement</var> is true, then run the <span>focusing steps</span> for | |||
<li><p>If <var>focusPreviousElement</var> is true and <var>document</var>'s <span>focused area |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't quite tell from the spec if <span>focused area of the document</span>
is retargeted to the document, or if it might be contained in a descendent shadow root. If the former, then this looks good. If the latter, then <span>descendant</span>
below should be "shadow including inclusive descendant" I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whether its retargeted or not, shadow-including inclusive descendant should give us the behavior we want I think so it seems like a safer option to me. I changed it to shadow-including inclusive descendant.
WebKit is onboard with this change. |
Is this consistent with dialog? |
It probably makes sense to do the same change for non-modal dialogs too. |
Modal dialogs however make everything else inert, so it makes sense to restore focus unconditionally. |
On the question of non-model dialogs, #8904 is related. It would be ideal if we fixed both at once. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Editorially LGTM, and it looks like we have two-implementer interest and tests, but I'm not 100% sure discussion is settled yet (including discussion in #9178) so I'll give it another day or two before merging?
Popovers will restore focus to the element which was focued before opening the popover when the popover is closed. However, if a user opens a popover and then tab-navigates away from the popover, focus should not be changed when the popover is closed.
More context:
https://chromium-review.googlesource.com/c/chromium/src/+/4322607
Fixes #9169
(See WHATWG Working Mode: Changes for more details.)
/popover.html ( diff )