Focus compose box when LHN report clicked #2032
Conversation
|
Sorry @johnmlee101 - unassigning b/c i accidentally requested pullerbear twice 😅 |
| // When any modal goes from visible to hidden, bring focus to the compose field | ||
| if (prevProps.modal.isVisible && !this.props.modal.isVisible) { | ||
| // When any modal goes from visible to hidden or when the report ID changes, bring focus to the compose field | ||
| if ( |
There was a problem hiding this comment.
I don't think we separate lines for ifs something like this is closer to our guidelines I believe
if ((prevProps.modal.isVisible && !this.props.modal.isVisible)
|| (prevProps.reportID !== this.props.reportID)) {Would be otherwise happy :)
There was a problem hiding this comment.
never mind that, clearly I have not worked on the this repo for a while 😅
There was a problem hiding this comment.
I was wondering what y'all like to do in these situations! haha
I got this style from: https://github.com/airbnb/javascript#control-statements
Here, they reference this example:
// good
if (
(foo === 123 || bar === 'abc')
&& doesItLookGoodWhenItBecomesThatLong()
&& isThisReallyHappening()
) {
thing1();
}Anyway, let me know what you think! I'm happy to modify :D
There was a problem hiding this comment.
yeah, you are tottally right, sorry for that, I haven't open this repo in a while, this does seem like the way we do it in this project, keeping consitency is important, lets continue to do that
There was a problem hiding this comment.
Ok great, thanks for asking anyway!
|
This issue was only reproducible in iOS Safari. Android mWeb is working as expected. Title: Expected Result: Actual Result: Action Performed:
Build version : 1.0.5.30 Is it reproducible in production? : Yes Device and OS: iPhone 8/iOS 14.2 / Safari browser Attachment |
isagoico
added
the
DeployBlockerCash
|
Hi @isagoico, since this issue is reproducible in production, I think it should not be a |
roryabraham
removed
the
DeployBlockerCash
|
All done! New issue can be found here #2108 |
<If necessary, assign reviewers that know the area or changes well. Feel free to tag any additional reviewers you see fit.>
Details
When a user clicks on a chat from the LHN, the
ReportViewcomponent updates, showing the latest conversation. At this point, we want the compose box at the bottom to be focused so that users can quickly and efficiently start typing a new message.This change will make the compose box (
ReportActionCompose) automatically get focused whenever thereportIDprop changes, which happens whenever a user clicks on a different chat / report - not just via the LHN.Fixed Issues
Fixes https://github.com/Expensify/Expensify/issues/158106Tests
Tested On
Screenshots
Screen.Recording.2021-03-24.at.5.14.17.PM.mov
Web
See above
Mobile Web
Not needed
Desktop
See above
iOS
Not needed
Android
Not needed