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

Move the focus to the previously focused element for dialog.close() #6531

Merged
merged 5 commits into from
Apr 27, 2021

Conversation

sefeng211
Copy link
Contributor

@sefeng211 sefeng211 commented Mar 25, 2021

Add a new previously focused element to dialog as a pointer
to the current focused element when dialog.showModal() and
dialog.show() are called, such that dialog.close() can
use it to restore the focus.

See #5678 for more details.

cc @josepharhar @domenic @annevk


/interactive-elements.html ( diff )

@domenic domenic added the topic: dialog The <dialog> element label Mar 25, 2021
source Show resolved Hide resolved
source Outdated Show resolved Hide resolved
Add a new `previously focused` element to dialog as a pointer
to the current focused element when `dialog.showModal()` and
`dialog.show()` are called, such that `dialog.close()` can
use it to restore the focus.

See whatwg#5678 for more details.
@sefeng211 sefeng211 force-pushed the dialog-move-focus-upon-close branch from 8959353 to 9e71dbc Compare April 6, 2021 15:42
@sefeng211 sefeng211 requested review from annevk and smaug---- April 6, 2021 15:43
@sefeng211
Copy link
Contributor Author

Updated!

Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

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

This generally looks good to me, modulo nits. Note that we hard wrap lines at a 100 columns (upon a space).

@josepharhar any final thoughts here?

source Show resolved Hide resolved
source Show resolved Hide resolved
source Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
@josepharhar
Copy link
Contributor

@josepharhar any final thoughts here?

I made a patch here (thanks for the WPT). If the tests pass, then I'm happy with this spec change.
As for the spec text, I don't see any problems with it.

@sefeng211 sefeng211 requested a review from annevk April 20, 2021 19:51
@sefeng211
Copy link
Contributor Author

Updated the PR to address the comments.

source Show resolved Hide resolved
source Show resolved Hide resolved
@sefeng211 sefeng211 requested a review from annevk April 22, 2021 20:41
@annevk annevk merged commit 5d50bfa into whatwg:main Apr 27, 2021
@annevk
Copy link
Member

annevk commented Apr 27, 2021

Thanks all!

moz-wptsync-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Apr 29, 2021
This change is based on the latest changes for the <dialog> element in
whatwg/html#6531, such that closing
<dialog> should move the focus to the previously focused element.

Differential Revision: https://phabricator.services.mozilla.com/D109726

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1660271
gecko-commit: 130bfa5327532a2b188e4cc06f5e1de7183327e8
gecko-reviewers: smaug
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Apr 30, 2021
…dialog> is closed r=smaug

This change is based on the latest changes for the <dialog> element in
whatwg/html#6531, such that closing
<dialog> should move the focus to the previously focused element.

Differential Revision: https://phabricator.services.mozilla.com/D109726
moz-wptsync-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Apr 30, 2021
This change is based on the latest changes for the <dialog> element in
whatwg/html#6531, such that closing
<dialog> should move the focus to the previously focused element.

Differential Revision: https://phabricator.services.mozilla.com/D109726

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1660271
gecko-commit: 130bfa5327532a2b188e4cc06f5e1de7183327e8
gecko-reviewers: smaug
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: dialog The <dialog> element
Development

Successfully merging this pull request may close these issues.

5 participants