[HOLD for payment 2022-12-20][$2000] [mWeb] keyboard hides while converting Composer to full size - reported by @gadhiyamanan

[mvtglobally]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Action Performed:

1. go to any chat
2. type multiple lines
3. click on expand button

Expected Result:

keyboard should not hide

Actual Result:

keyboard hides

Workaround:

unknown

Platform:

Where is this issue occurring?

• Mobile Web

Version Number: 1.1.99-0
Reproducible in staging?: Y
Reproducible in production?: Y
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

Screen_Recording_20220830_162757_Chrome.mp4

Expensify/Expensify Issue URL:
Issue reported by: @gadhiyamanan
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1661857234117209

View all open jobs on GitHub

[melvin-bot]

Triggered auto assignment to @sakluger (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

[sakluger]

@neil-marcellini mentioned in Slack that this has been a known issue for a while, but hasn't been reported before. Probably can be worked on by an external contributor.

[melvin-bot]

Triggered auto assignment to @johnmlee101 (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[johnmlee101]

Yep agreed, will open this up.

[melvin-bot]

Triggered auto assignment to @MitchExpensify (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @parasharrajat (External)

[melvin-bot]

Triggered auto assignment to @luacmartins (External), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[MitchExpensify]

Upwork Job

[aimane-chnaif]

Proposal

This happens because on web, when click other element, focus changes to that element as default.

Solution:
We can prevent default on mouse down event so focus doesn't change

Collapse button:
https://github.com/Expensify/App/blob/main/src/pages/home/report/ReportActionCompose.js#L531-L539

                                                        <TouchableOpacity
+                                                            onMouseDown={e => e.preventDefault()}
                                                            onPress={(e) => {
                                                                e.preventDefault();
                                                                Report.setIsComposerFullSize(this.props.reportID, false);
                                                            }}
                                                            style={styles.composerSizeButton}
                                                            underlayColor={themeColors.componentBG}
                                                            disabled={isBlockedFromConcierge}
                                                        >

Expand button:
https://github.com/Expensify/App/blob/main/src/pages/home/report/ReportActionCompose.js#L547-L555

                                                        <TouchableOpacity
+                                                            onMouseDown={e => e.preventDefault()}
                                                            onPress={(e) => {
                                                                e.preventDefault();
                                                                Report.setIsComposerFullSize(this.props.reportID, true);
                                                            }}
                                                            style={styles.composerSizeButton}
                                                            underlayColor={themeColors.componentBG}
                                                            disabled={isBlockedFromConcierge}
                                                        >

onPress is always called when press out even though we prevent onMouseDown default behavior so this is safe.
This approach is already used on Send button here:

App/src/pages/home/report/ReportActionCompose.js

Line 668 in 2b6124e

onMouseDown={e => e.preventDefault()}

[parasharrajat]

Thanks for the proposal @aimane-chnaif. This approach is creating a problem on Mweb safari for spellcheck. we are trying to find a solution to that issue. I don't want to spread the known issue. Do you have any other approach to achieve it? If you can find that, happy to hire you for the other issue as well. #8592.

Also, could please share a video while interacting with that feature on mweb safari and web?

[aimane-chnaif]

I noticed similar issue you are concerned of: #10555 (comment)
Still researching on that highlight issue on mobile safari

BTW demo for this GH:
Mobile safari:

mobile.safari.MP4

Mobile chrome:

mobile.chrome.mp4

Desktop safari:

desktop.safari.mov

Desktop chrome:

desktop.chrome.mov

[melvin-bot]

@luacmartins, @parasharrajat, @MitchExpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[luacmartins]

Still waiting on a solution for this issue - #11093 (comment)

[aimane-chnaif]

@eh2077 That difference is only from mSafari.
I already tested your solution on various browsers at the time you posted proposal.
Each solution has drawback since both are beyond the default behavior of web.
The issue you mentioned is quite minor and also happens only on mSafari, but your solution never works on Firefox (both desktop and mobile) which is critical.

[eh2077]

Hey @aimane-chnaif

That difference is only from mSafari.

I found that the calling focus method has differences on Safari(both desktop and mobile) and Firefox(both desktop and mobile) based on my testing. There's no difference from Chrome(both desktop and mobile)

Each solution has drawback since both are beyond the default behavior of web.

I can't agree more on this.

but your solution never works on Firefox (both desktop and mobile) which is critical.

This should be not true based on my testing.

Firefox mobile

Firefox desktop

11093-FF-Desktop.mov

Safari desktop

11093-Safari-Desktop.mov

Below diff will be helpful if you would like to test what I mentioned.

diff --git a/src/pages/home/report/ReportActionCompose.js b/src/pages/home/report/ReportActionCompose.js
index 2335864f91..0f566031f8 100644
--- a/src/pages/home/report/ReportActionCompose.js
+++ b/src/pages/home/report/ReportActionCompose.js
@@ -128,6 +128,8 @@ class ReportActionCompose extends React.Component {
         this.getInputPlaceholder = this.getInputPlaceholder.bind(this);
         this.getIOUOptions = this.getIOUOptions.bind(this);
         this.addAttachment = this.addAttachment.bind(this);
+        this.composerSizingCollapseID = 'composerSizingCollapseID';
+        this.composerSizingExpandID = 'composerSizingExpandID';
 
         this.comment = props.comment;
         this.shouldFocusInputOnScreenFocus = canFocusInputOnScreenFocus();
@@ -136,6 +138,7 @@ class ReportActionCompose extends React.Component {
             isFocused: this.shouldFocusInputOnScreenFocus,
             isFullComposerAvailable: props.isComposerFullSize,
             textInputShouldClear: false,
+            textInputShouldFocus: false,
             isCommentEmpty: props.comment.length === 0,
             isMenuVisible: false,
             selection: {
@@ -219,6 +222,10 @@ class ReportActionCompose extends React.Component {
         this.setState({textInputShouldClear: shouldClear});
     }
 
+    setTextInputShouldFocus(shouldFocus) {
+        this.setState({textInputShouldFocus: shouldFocus});
+    }
+
     /**
      * Updates the visibility state of the menu
      *
@@ -561,8 +568,13 @@ class ReportActionCompose extends React.Component {
                                                             onPress={(e) => {
                                                                 e.preventDefault();
                                                                 Report.setIsComposerFullSize(this.props.reportID, false);
+                                                                if (this.state.textInputShouldFocus) {
+                                                                    this.textInput.focus();
+                                                                    this.setTextInputShouldFocus(false);
+                                                                }
                                                             }}
                                                             style={styles.composerSizeButton}
+                                                            nativeID={this.composerSizingCollapseID}
                                                             underlayColor={themeColors.componentBG}
                                                             disabled={isBlockedFromConcierge}
                                                         >
@@ -577,8 +589,13 @@ class ReportActionCompose extends React.Component {
                                                             onPress={(e) => {
                                                                 e.preventDefault();
                                                                 Report.setIsComposerFullSize(this.props.reportID, true);
+                                                                if (this.state.textInputShouldFocus) {
+                                                                    this.textInput.focus();
+                                                                    this.setTextInputShouldFocus(false);
+                                                                }
                                                             }}
                                                             style={styles.composerSizeButton}
+                                                            nativeID={this.composerSizingExpandID}
                                                             underlayColor={themeColors.componentBG}
                                                             disabled={isBlockedFromConcierge}
                                                         >
@@ -664,7 +681,14 @@ class ReportActionCompose extends React.Component {
                                         style={[styles.textInputCompose, this.props.isComposerFullSize ? styles.textInputFullCompose : styles.flex4]}
                                         maxLines={this.state.maxLines}
                                         onFocus={() => this.setIsFocused(true)}
-                                        onBlur={() => this.setIsFocused(false)}
+                                        onBlur={(event) => {
+                                            // We want to prevent composer from losing (keyboard)focus after pressing the expand or collapse icon
+                                            // Save a state and the handling of expand/collapse click will set focus back to the Composer
+                                            if (_.contains([this.composerSizingCollapseID, this.composerSizingExpandID], lodashGet(event, 'nativeEvent.relatedTarget.id'))) {
+                                                this.setTextInputShouldFocus(true);
+                                            }
+                                            this.setIsFocused(false);
+                                        }}
                                         onPasteFile={displayFileInModal}
                                         shouldClear={this.state.textInputShouldClear}
                                         onClear={() => this.setTextInputShouldClear(false)}

Edited: resized FF mobile gif

[aimane-chnaif]

@eh2077 I tested your code and seems working, but still not 100% sure if it wouldn't cause any other regression that we haven't found yet.

• Your solution has one more render because of textInputShouldFocus state value update.

• Here's another UI difference (blue outline) between 2 solutions. I think first one is better because user feels composer keeps focused

using preventDefault():

1.mov

using focus():

2.mov

[eh2077]

@aimane-chnaif Thanks for the testing! Your last comment is basically fair while it could be minor too.

[melvin-bot]

@luacmartins, @parasharrajat, @MitchExpensify, @aimane-chnaif Eep! 4 days overdue now. Issues have feelings too...

[luacmartins]

PR is merged. Just waiting for the deploys!

[MitchExpensify]

I'm heading ooo so reassigning the Bug label for a new CM!

[melvin-bot]

Triggered auto assignment to @zanyrenney (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

@luacmartins, @parasharrajat, @zanyrenney, @aimane-chnaif Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[luacmartins]

@zanyrenney I'm not sure why the title wasn't updated, but this PR was deployed to prod on Dec 13 so the regression period ends today!

[zanyrenney]

awesome, paying this one out and closing.

[aimane-chnaif]

#11093 (comment)

New Upwork Job created. Invited everyone for eventual payment

@zanyrenney FYI: we are already hired on this job so hope you can use this one

[zanyrenney]

Hey @aimane-chnaif thanks for that, I had the upwork link handy, we're just having some internal discussions. FYI for Expensifiers: here

[zanyrenney]

I'm heading OOO and we're still discussing this here, so reapplying the Bug0 label in this case.

[melvin-bot]

Triggered auto assignment to @puneetlath (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[puneetlath]

All paid!