-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Focus the compose field after closing modals #1699
Conversation
@@ -62,6 +70,11 @@ class ReportActionCompose extends React.Component { | |||
if (this.props.comment && prevProps.comment === '' && prevProps.comment !== this.props.comment) { | |||
this.comment = this.props.comment; | |||
} | |||
|
|||
// When any modal goes from visible to hidden, bring focus to the compose field | |||
if (prevProps.modal.isVisible === true && this.props.modal.isVisible === false) { |
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.
NAB. We can just do if (prevProps.modal.isVisible && !this.props.modal.isVisible)
here, right?
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.
Updated!
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.
One minor comment. Looks good otherwise!
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.
Code Looks good
Details
I added a global state for tracking if modals are open or closed, then connected that state to the compose field so it gets focus when modals are closed.
Fixed Issues
Fixes https://github.com//issues/1513 Fixes https://github.com/Expensify/Expensify/issues/154180Tests
Tested On
iOS(n/a because the field does not get focus by design)Android(n/a because the field does not get focus by design)Screenshots
Web
Not really any screenshots necessary
Mobile Web
Not really any screenshots necessary
Desktop
Not really any screenshots necessary
iOS
Not really any screenshots necessary
Android
Not really any screenshots necessary