Skip to content
This repository was archived by the owner on Sep 6, 2021. It is now read-only.

Dialog returns focus to previous element after closing#12824

Merged
petetnt merged 5 commits intoadobe:masterfrom
Jerhamre:master
Oct 16, 2016
Merged

Dialog returns focus to previous element after closing#12824
petetnt merged 5 commits intoadobe:masterfrom
Jerhamre:master

Conversation

@Jerhamre
Copy link
Copy Markdown
Contributor

Fixed issue #7511, #4954, #8535.

When creating a new dialog, save the currently focused element.
When closing the dialog, restore focus to previous element.

Jerhamre and others added 2 commits October 10, 2016 13:16
…ting a dialog and set current focus to the previous one when closing
Comment thread src/widgets/Dialogs.js
KeyBindingManager.removeGlobalKeydownHook(keydownHook);


// Restore previous focus
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nit: Remove extra newline

Comment thread src/widgets/Dialogs.js Outdated


// Restore previous focus
lastFocus.focus();
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

document.activeElement could be null instead of something else some rare cases, so it might be good to wrap this inside a null check

if (lastFocus) {
  lastFocus.focus();
}

@petetnt
Copy link
Copy Markdown
Collaborator

petetnt commented Oct 12, 2016

Thanks for this awesome contribution @Jerhamre! Tested and it works great.

I left couple of tiny review comments, if you could fix those this is ready to be merged in 👍

@Jerhamre
Copy link
Copy Markdown
Contributor Author

Added a null check for when restoring focus and removed the extra newline

@petetnt
Copy link
Copy Markdown
Collaborator

petetnt commented Oct 16, 2016

Thanks for this contribution!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants