[HOLD for payment 2024-10-22] [$2000] Android/IOS - Split/request money - Chat composer is focused after IOU request from green plus button

[kbecciv]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Action Performed:

1. Go to App and login with any account
2. Click the green plus button and click Request money Enter an amount and select the Next confirmation button Complete the request money flow

Expected Result:

Always focus the input, but prevent keyboard from opening on mobile & mWeb

Actual Result:

The composer is focused.

Workaround:

Swipe the keyboard down to un-focus the composer

Platform:

Where is this issue occurring?

• iOS
• Android

Version Number: 1.1.95.4

Reproducible in staging?: Yes

Reproducible in production?: Yes

Email or phone of affected tester (no customers): any

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation

https://user-images.githubusercontent.com/93399543/187747715-3711327d-e3d4-4198-9df5-32400f8cb46e.mp4

https://user-images.githubusercontent.com/93399543/187747751-f8f2a162-cb62-4b85-91cc-d9bf024a13ba.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

Issue Owner

Current Issue Owner: @Christinadobrzyn

[melvin-bot[bot]]

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

[neil-marcellini]

Hmm, so if you navigate to any normal chat on mobile, the composer is not focused until you tap into it. That is the desired behavior. I think this is a valid issue if we change it from: "Chat composer is not focused after IOU request" to "Chat composer is focused after IOU request from green plus button"

Also it looks like this only happens when the request is with a brand new chat. I was not able to see the issue when sending a request via the green plus to a chat where there was a good amount of history and zero IOU requests. This is a good external issue with lower priority (weekly).

[neil-marcellini]

I updated the issue title and description to reflect the actual problem.

[melvin-bot[bot]]

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

[bfitzexpensify]

Upwork job here

[melvin-bot[bot]]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @Santhosh-Sellavel (Exported)

[melvin-bot[bot]]

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

[syedsaroshfarrukhdot]

Proposal

https://github.com/Expensify/App/blob/8e07c62fb9780c627106f45eb72614466a850f27/src/pages/home/report/ReportActionCompose.js#L595

This issue is caused by _.size(this.props.reportActions) === 1 in autofocus props of Composer when this.props.reportActions === 1 is true it focuses in the input field to fix the issue we need to remove it.

Solution :- To fix the issue we need to set autofocus props like this autoFocus={this.shouldFocusInputOnScreenFocus}

After Fixing :-

https://user-images.githubusercontent.com/81307212/187970668-d2c6ef18-7ab4-41e9-8e42-a48253a8ec7f.mov

[christianwen]

Proposal

Problem

The issue is in src/pages/home/report/ReportActionCompose.js.

This is due to when we split/request money for the first time for a new group/person, the this.props.reportActions is loaded with only 1 IOU message initially and then it will be populated with other messages like the CREATED message for the group. Thus, initially the _.size(this.props.reportActions) === 1 will be true (before it loads everything) and the composer will be focused incorrectly.

The previous proposal to remove the _.size(this.props.reportActions) === 1 will cause a regression because that code is there so that when user enters a new chat, the keyboard will be focused automatically. We can refer to the requirement for that here https://github.com/Expensify/App/pull/4921#discussion_r718023887.

Solution

We'll implement a new isNewChat method that checks both _.size(this.props.reportActions) === 1 and the only message in the chat is the CREATED message, this will help us avoid the lazy loading problem mentioned.

+isNewChat() {
+        if (_.size(this.props.reportActions) === 1) {
+            const firstReportAction = _.head(_.values(this.props.reportActions));
+            return firstReportAction.actionName === CONST.REPORT.ACTIONS.TYPE.CREATED;
+        }

+        return false;
+    }
....
<Composer
-      autoFocus={this.shouldFocusInputOnScreenFocus || _.size(this.props.reportActions) === 1}
+      autoFocus={this.shouldFocusInputOnScreenFocus || this.isNewChat()}

After the fix, the chat composer is no longer focused incorrectly

https://user-images.githubusercontent.com/21312517/188118275-8c39c491-5048-4b85-8e31-240e0f5b3666.mp4

I also discover another bug where the Chat composer will initially have the Say hello! placeholder (which should only appear if the chat is completely new and there's no message in the chat), only a few moments later it will change to Write something... placeholder. You can see the issue in the video capture attached in the issue description:

The fix for that will use the same isNewChat method I mentioned above, please confirm if I should fix it in the scope of this issue as well.

Please let me know if you have any concerns regarding the above proposal. Thanks!

[melvin-bot[bot]]

@iwiznia, @bfitzexpensify, @Santhosh-Sellavel Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[syedsaroshfarrukhdot]

As mentioned by @christianwen _.size(this.props.reportActions) === 1 will cause a regression because that code is there so that when a user enters a new chat, the keyboard will be focused automatically.

But I tested it thoroughly when was proposing a solution the composer was not focused when a user enters a new chat

https://user-images.githubusercontent.com/81307212/188401528-b592ec1b-18c6-47e4-b1d9-b64099f6bb04.mp4

So I thought there is no need of _.size(this.props.reportActions) === 1 so we will be able to resolve the issue by removing this condition. If we need to focus when a user enters a new chat then we could change condition to _.size(this.props.reportActions) === 0 or we need to find a solution to check why _.size(this.props.reportActions) === 1 is not getting correct size of messages on intial render.

Solution after _.size(this.props.reportActions) === 0 :-

https://user-images.githubusercontent.com/81307212/188406145-0d73f2de-af2d-442a-9921-533abc13a947.mp4

We need to identify first what we want to achieve does the composer needs to be focused on a new chat or not ? After identifying what needs to be done then we can work on a solution. @neil-marcellini

[neil-marcellini]

@syedsaroshfarrukhdot thanks for investigating. I think we want to have the composer focused when creating a new chat but not after creating an IOU request. I'll let @iwiznia handle this going forward since he's the CME on this issue.

[christianwen]

would it be more suitable for the composer not focused when creating a new chat to be a separate bug issue since that's not really included in the scope of this issue, or if it's included, we can update the description of this issue to reflect both.

For this issue, the solution mentioned in https://github.com/Expensify/App/issues/10731#issuecomment-1235329280 should work, for the composer not focused when creating a new chat issue, it will require investigation of why this.props.reportActions is incorrect on new chat creation as also mentioned by @syedsaroshfarrukhdot.

[bfitzexpensify]

@Santhosh-Sellavel some proposals for you to review above

[Santhosh-Sellavel]

@bfitzexpensify Thanks for the bump, But I'm following the discussions and proposals.

Will post my comments by tomorrow.

[syedsaroshfarrukhdot]

After testing this.props.reportActions i figured out it is nothing to do with this.props.reportActions it return always correct value the issue lies with autofocus props of the composer which if true, focuses the input on componentDidMount and didn't update the focus on componentDidUpdate as it is the default behavior of the autofocus props as it takes value always componentDidMount. So, I think as it is going to be when ReportScreen Is loaded with no chat _.size(this.props.reportActions) is always 0 and after few seconds it updates to 1 when first message in it the list renders. And when IOURequest is made first time in the new chat it has a _.size(this.props.reportActions) is always 1.

So to resolve both issue the viable solution i would suggest would be to set

_.size(this.props.reportActions) === 0} is autofocus prop

https://github.com/Expensify/App/blob/8e07c62fb9780c627106f45eb72614466a850f27/src/pages/home/report/ReportActionCompose.js#L595

After fixing it resolves both issues

https://user-images.githubusercontent.com/81307212/188406145-0d73f2de-af2d-442a-9921-533abc13a947.mp4

[Santhosh-Sellavel]

Sorry might need some more time to dig this, will update later today or tomorrow!

[christianwen]

I believe the solution to use _.size(this.props.reportActions) === 0 that relies on the observed behavior of

when ReportScreen Is loaded with no chat _.size(this.props.reportActions) is always 0 and after few seconds it updates to 1 when first message in it the list renders

is flaky and not very future proof. Imagine later when it's updated so that the ReportScreen is loaded with correct messages list from Onyx initially (instead of having 0 then 1 message after a few seconds), that solution will break.

Updated Proposal

Original proposal: https://github.com/Expensify/App/issues/10731#issuecomment-1235329280

Solution

We'll implement a new isNewChat method that checks both _.size(this.props.reportActions) === 1 and the only message in the chat is the CREATED message, this will help us avoid the lazy loading problem mentioned. And the logic to autofocus will be put in the componentDidUpdate instead of relying on the autoFocus property of the Composer which is flaky and does not work as expected when there's rerender.

This will fix both issues:

1. The chat composer will no longer be focused incorrectly after IOU request from green plus button
2. When a new group is created, the chat composer will be focused correctly

this.state = {
      isFocused: this.shouldFocusInputOnScreenFocus,
      isFullComposerAvailable: props.isComposerFullSize,
      textInputShouldClear: false,
      isCommentEmpty: props.comment.length === 0,
      isMenuVisible: false,
      selection: {
          start: props.comment.length,
          end: props.comment.length,
      },
      maxLines: props.isSmallScreenWidth ? CONST.COMPOSER.MAX_LINES_SMALL_SCREEN : CONST.COMPOSER.MAX_LINES,
+   didAutoFocusOnNewChat: false,
  };
  
 componentDidUpdate(prevProps) {
      ...
+      if (!this.state.didAutoFocusOnNewChat && this.isNewChat()) {
+            this.setDidAutoFocusOnNewChat(true);
+            this.focus();
+        }
      ...
 }
        
+setDidAutoFocusOnNewChat(didAutoFocusOnNewChat) {
+      this.setState({didAutoFocusOnNewChat});
+}

+isNewChat() {
+        if (_.size(this.props.reportActions) === 1) {
+            const firstReportAction = _.head(_.values(this.props.reportActions));
+            return firstReportAction.actionName === CONST.REPORT.ACTIONS.TYPE.CREATED;
+        }

+        return false;
+    }
....
<Composer
-      autoFocus={this.shouldFocusInputOnScreenFocus || _.size(this.props.reportActions) === 1}
+      autoFocus={this.shouldFocusInputOnScreenFocus}

1. The chat composer will no longer be focused incorrectly after IOU request from green plus button https://user-images.githubusercontent.com/21312517/188118275-8c39c491-5048-4b85-8e31-240e0f5b3666.mp4

2. When a new group is created, the chat composer will be focused correctly https://user-images.githubusercontent.com/21312517/189062189-5b447c22-e3ad-4fa4-93bb-a7782b873581.MP4

[Santhosh-Sellavel]

@Expensify/design @iwiznia

Need some clarifications

What's the expected behavior here? When should we set focus on the chat composer?

• [ ] On a new chat is created and opened
• [ ] On a new group chat is created and opened
• [ ] On a new chat is created by an IOU request and opened the chat
• [ ] On a new group is created by an IOU Split and opened the chat

Should the focus be set only when the chat is opened for the first time?

Thanks.

[shawnborton]

Hmm maybe just when a new chat or group is created and opened, but not after a new IOU or Split is created?

[shawnborton]

This might be a good one to bring to the open source room for more visibility though!

[iwiznia]

I think it's all of them. If I am left in a chat, I expect to be able to write directly in the chat, if it is not focused on the composer, where is it focused on and is it useful to be focused on that element?

But yeah, maybe bringing the conversation to the slack channel is a good idea.

[Santhosh-Sellavel]

Thread is here for discussion

[bfitzexpensify]

Thanks for raising the discussion @Santhosh-Sellavel. Looks like we're going with this:

Always focus the input, but prevent keyboard from opening on mobile & mWeb

[melvin-bot[bot]]

@iwiznia, @bfitzexpensify, @Santhosh-Sellavel Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[melvin-bot[bot]]

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

[bfitzexpensify]

Doubled to $500. Reassigning as I head ooo for 2 weeks

[iwiznia]

Not overdue, Melvin

[christianwen]

Proposal

Solution

We'll make use of showSoftInputOnFocus of React Native Text Input and inputMode of web input to achieve the intended behavior.

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

this.state = {
            ...
+            showSoftInputOnFocus: false,
        };

+setShowSoftInputOnFocus(showSoftInputOnFocus) {
+        this.setState({showSoftInputOnFocus});
+    }

+blur() {
+        if (!this.textInput) {
+              return;
+       }

+       this.textInput.blur();
+}

....
<Composer
// since it will always autofocus
-      autoFocus={this.shouldFocusInputOnScreenFocus || _.size(this.props.reportActions) === 1}
+      autoFocus
+.     showSoftInputOnFocus={this.state.showSoftInputOnFocus}
+        onTouchStart={() => {
+            if (!this.state.showSoftInputOnFocus) {
+                this.setShowSoftInputOnFocus(true);
                  // after enable keyboard on focus, we need to retrigger focus again for the keyboard to apply
+                this.blur();
+                this.focus();
            }
        }}

In src/components/Composer/index.ios.js, src/components/Composer/index.android.js, src/components/Composer/index.js

const propTypes = {
+      showSoftInputOnFocus: PropTypes.bool,
}

const defaultProps = {
+      showSoftInputOnFocus: true,
}


<RNTextInput
+       showSoftInputOnFocus={this.props.showSoftInputOnFocus}
 />

Since react-native-web does not support showSoftInputOnFocus yet, we need to make an update to support it In https://github.com/Expensify/react-native-web -> packages/react-native-web/src/exports/TextInput/index.js

const {
+    showSoftInputOnFocus = true
  } = props;
...
  switch (keyboardType) {
    case 'email-address':
      type = 'email';
      break;
    case 'number-pad':
    case 'numeric':
      inputMode = 'numeric';
      break;
    case 'decimal-pad':
      inputMode = 'decimal';
      break;
    case 'phone-pad':
      type = 'tel';
      break;
    case 'search':
    case 'web-search':
      type = 'search';
      break;
    case 'url':
      type = 'url';
      break;
    default:
      type = 'text';
  }

+  if (!showSoftInputOnFocus) {
+    inputMode = 'none';
+  }

After the fix, we have the intended behavior, I demo in iOS and mWeb below

iOS https://user-images.githubusercontent.com/21312517/191922173-bb289d97-c967-49ff-b8f3-4a33b5e2ccfd.mp4

mWeb

https://user-images.githubusercontent.com/21312517/191925165-2466ac2b-fa51-43e8-9fa0-be04c36db82c.MP4

Please let me know if you have any concerns regarding the above proposal. Thanks!

[mallenexpensify]

@Santhosh-Sellavel can you please review @christianwen 's proposal above? Bumping back to Daily til then