NetSim: Stable sent-by dropdown#9816
Merged
Merged
Conversation
BaseDialog is configured to take focus any time it is rendered to make sure pressing ESC to close the dialog works. However, when the dialog is already open and a full dialog update/re-render occurs, this can steal focus from form fields within the dialog. This makes the dialog check whether one of its descendants already has focus before it steals focus for itself.
| const descendantIsActive = document.activeElement && this.refs.dialog && | ||
| this.refs.dialog.contains(document.activeElement); | ||
| if (this.props.isOpen && !descendantIsActive) { | ||
| this.refs.dialog.focus(); |
Contributor
Author
There was a problem hiding this comment.
This is the actual fix for the reported bug. The BaseDialog component steals focus every time it's rendered, to make sure the Esc key can close the dialog. This produced an undesirable behavior when the dialog was open and contained form fields - the fields would lose focus on every re-render, as the dialog itself claimed it at the end of the render process. Thanks @Bjvanminnen for putting me on the right track here.
The solution is to check whether a descendant of the dialog already has focus, and do nothing in that case.
Contributor
|
one comment, otherwise lgtm |
| return { | ||
| ...newProps, | ||
| originalClassName, | ||
| className: originalClassName, |
Contributor
Author
There was a problem hiding this comment.
This fixes a warning for unknown properties on DOM elements, introduced in React 15.2.0.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
@sfilman noticed another problem with the new Teacher View sent-by dropdown (original report):
This PR contains a fix for that issue, and a good chunk of cleanup I did while stumbling around trying to find the fix. See inline comments to explain what's what, or see individual commits.