[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 contributors@expensify.com 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 1when 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

[Santhosh-Sellavel]

Reviewing this now!

[Santhosh-Sellavel]

@christianwen This is what we want,

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

I've tested your proposal on iOS but it always opens keyboard without any interaction.

https://user-images.githubusercontent.com/85645967/193144833-dfb69981-7462-4d0e-a74b-01299b41496d.mp4

[syedsaroshfarrukhdot]

@Santhosh-Sellavel

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

To achieve this we need to introduce a new state such as inputFocus: false, our this.state will become

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,
        inputFocus: false,
    };

We need to write function to clear the inital focus so we can set focus again on basis of our state

blur() {

     if (!this.textInput) {
      return;
     }

    this.textInput.blur();
}

We need to write a state setter function such as

setInputFocus(inputFocus) {

            this.setState({inputFocus});

}

And in Composer we need to pass a props show showSoftInputOnFocus When false, it will prevent the soft keyboard from showing when the field is focused. then we can toggle it onTouchStart. autoFocus will be always true now.

<Composer

     autoFocus
     showSoftInputOnFocus={this.state.inputFocus}
     onTouchStart={() => {
          if (!this.state.inputFocus) {
                this.setInputFocus(true);
                this.blur();
                this.focus();
             }
          }}
      multiline

After Fixing :-

https://user-images.githubusercontent.com/81307212/193192692-f443de76-cf6c-4c06-950e-e9cec45f5db0.mov

PS:- Difference b/w my Proposal and above propsal is that they are not passing showSoftInputOnFocus={this.state.inputFocus} which When false, it will prevent the soft keyboard from showing when the field is focused.

[christianwen]

@Santhosh-Sellavel can you help to add this line? showSoftInputOnFocus={this.state.showSoftInputOnFocus}, in here

<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();
            }
        }}

it should work fine.

I might have missed it out when composing the diff change, but my intention was to add it since as you can see in the original proposal I introduced the new showSoftInputOnFocus prop in the Composer component definition.

Alternative, you can pull my branch here https://github.com/christianwen/expensify-app/tree/fix/10731-chat-composer-behavior that has the fix and run it for testing, will save you some time, also using the branch will eliminate cases of change diffs missing something or are applied incorrectly.

In order to test the react-native-web patch, you'll need to modify the built code in node_modules/@expensify/react-native-web/dist/exports/TextInput/index.js manually.

the other proposal mentioned in https://github.com/Expensify/App/issues/10731#issuecomment-1263101744 will not work on mWeb because the showSoftInputOnFocus is not currently supported in react-native-web so we need to patch react-native-web as in my proposal.

[Santhosh-Sellavel]

I agree with you @christianwen that's easy to miss!

[Santhosh-Sellavel]

@christianwen I verified this seems to work but I noticed some lag in setting focus.

Can you share the screen recording from mWeb, Android, and iOS?

[melvin-bot[bot]]

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

[melvin-bot[bot]]

Current assignee @Santhosh-Sellavel is eligible for the External assigner, not assigning anyone new.

[melvin-bot[bot]]

Current assignee @iwiznia is eligible for the External assigner, not assigning anyone new.

[mallenexpensify]

@puneetlath auto-assigned to you, I'm going to be OOO for a month. Thx

[christianwen]

@Santhosh-Sellavel the lagging is due to the onFocus is triggered on the Composer only after the screen finished mounting, so the style of the border is updated later. I believe this is already an existing behavior and not due to the above changes in this issue, in order to fix this we can further modify the following in https://github.com/Expensify/App/blob/9ff60c3a220c9d05001007b2c0dfe0d954623edb/src/pages/home/report/ReportActionCompose.js#L135

this.state = {
-      isFocused: this.shouldFocusInputOnScreenFocus,
+      isFocused: true,

This is ok since the component is always focused when enter the screen for all platforms so we can make true the default state to show the correct initial border highlight.

Screen recording after applying the above change: iOS https://user-images.githubusercontent.com/21312517/193393980-122451a0-a1da-456e-92fe-4906da879905.mp4

mWeb https://user-images.githubusercontent.com/21312517/193395820-d02aad41-7192-46dd-a431-91973e56dc4d.mp4

Android Android-chat-compose.webm

[Santhosh-Sellavel]

Not overdue

[mallenexpensify]

@puneetlath assigned myself and unassigned you cuz I'm NOT going to be OOO the next month ¯_(ツ)_/¯

Doubled job price to $1000 https://www.upwork.com/jobs/~0171651b178a420568

[Santhosh-Sellavel]

@christianwen proposal mostly looks good. Also need to check this again!

[iwiznia]

Also need to check this again!

@Santhosh-Sellavel so should I review the proposal or not? Not sure what you meant by check this again...

[Santhosh-Sellavel]

@iwiznia didn't get a chance to test the proposal after this https://github.com/Expensify/App/issues/10731#issuecomment-1264264720 that's why commented " need to check this again."

@christianwen Based the docs here if showSoftInputOnFocus set true keyboard should be shown right?

Not sure why this is required, this feels kind of hacky.

// after enable keyboard on focus, we need to retrigger focus again for the keyboard to apply       
this.blur();
this.focus();

[christianwen]

@Santhosh-Sellavel if showSoftInputOnFocus set true keyboard should be shown right -> this is correct if the input is currently not in focus and then it's focused, the keyboard will be shown. But in this case, since the input is already focused (it's focused by default when entering the chat screen), when we change showSoftInputOnFocus to true it will not pop the keyboard up but will require refocusing in order for the keyboard to open. You can think of the showSoftInputOnFocus as show soft input on/when focus, meaning this property will only take effect onFocus (when the focus event takes place) and not when the TextInput is already focused before.

[melvin-bot[bot]]

@iwiznia, @mallenexpensify, @Santhosh-Sellavel Whoops! This issue is 2 days overdue. Let's get this updated quick!

[mallenexpensify]

@Santhosh-Sellavel , can you review @christianwen's comment above plz?

[Santhosh-Sellavel]

if showSoftInputOnFocus is set to true keyboard should be shown right -> this is correct if the input is currently not in focus and then it's focused, the keyboard will be shown.

I disagree as per this doc here.

When false, it will prevent the soft keyboard from showing when the field is focused. The default value is true.

I tried it out without the below lines.

this.blur();
this.focus();

It works fine on android, but it doesn't work on iOS.

@christianwen