[HOLD PR #10965][$250] mWeb/Chrome - Workspace - Unable to use Enter key for starting a new line under Personal message box

[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 staging.new.expensify.com
2. Log in with expensifail account
3. Go to Setting - Workspace
4. Manage Members
5. Tap Invite button
6. Go to Personal message box and type anything
7. Use Enter Key on your keyboard to start with new line

Expected Result:

Unable to use Enter key for starting a new line under Personal message box

Actual Result:

Able to use Enter key for starting a new line under Personal message box

Workaround:

Unknown

Platform:

Where is this issue occurring?

• Android

Version Number: 1.1.98.0

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/189198910-40007c13-e4fc-4e3c-a5b2-070b99c4ea50.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Interna Team

Slack conversation:

View all open jobs on GitHub

[melvin-bot[bot]]

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

[melvin-bot[bot]]

@neil-marcellini Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[neil-marcellini]

Looks like a legitimate problem and would be a good external issue.

[melvin-bot[bot]]

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

[mallenexpensify]

Job posted https://www.upwork.com/jobs/~01264199d22485a20a

[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 @MonilBhavsar (Exported), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[0xmiroslav]

Proposal

RCA: preventDefault() is triggered on Enter key of TextInput because of OptionsSelector (to select members) which overrides keyboard shortcuts of Enter, Arrow up/down keys. NOTE: On desktop, Shift+Enter can be workaround but still user doesn't know Shift key on this screen. Also, arrow up/down also doesn't move cursor properly on this multiline text input. So we should fix both Enter key and arrow up/down key behaviors on both desktop/mWeb platforms. (no issue on native iOS/Android)

Solution: When multiline input is focused and press Enter or Arrow up/down keys, we should avoid preventDefault() call and also prevent default behavior of BaseOptionsSelector, ArrowKeyFocusManager components. To implement this, we need to introduce new props in both components:

• disabled: to prevent default behavior of keyboard shortcuts
• keyboardShortcutShouldPreventDefault: not to prevent other components' keyboard behavior (i.e. enter on multiline input) You can check full code proposal here

This solution is when focused input should keep focused regardless of keyboard shortcuts triggers. (reference: https://github.com/Expensify/App/issues/10705#issuecomment-1241687298) If focused input should be blurred whenever arrow up/down keys pressed, similar to tab/shit+tab behavior, we need to find another solution. But I don't think this is expected behavior in our app.

https://user-images.githubusercontent.com/97473779/189843871-7c4f58c5-bced-4b3d-a193-a7435d8e6d4c.mov

https://user-images.githubusercontent.com/97473779/189843903-1e114e48-adeb-4e32-bc35-15752189fa72.mov

[mollfpr]

Proposal

Root cause We have a keyboard listener for the Enter key and it's also captured on input. https://github.com/Expensify/App/blob/d3120758546153d25002866f6cd770d6b2b123c2/src/components/OptionsSelector/BaseOptionsSelector.js#L53-L67

On Desktop/Web can use CMD+Enter to create a new line on input but it's not working on native/mWeb.

Solution Activate the captureOnInputs only on non-touchscreen devices.

diff --git a/src/components/OptionsSelector/BaseOptionsSelector.js 
b/src/components/OptionsSelector/BaseOptionsSelector.js
index cbcb0faa4..dd15a495d 100755
--- a/src/components/OptionsSelector/BaseOptionsSelector.js
+++ b/src/components/OptionsSelector/BaseOptionsSelector.js
@@ -18,6 +18,7 @@ import KeyboardShortcut from '../../libs/Keyboard
Shortcut';
 import ONYXKEYS from '../../ONYXKEYS';
 import FullScreenLoadingIndicator from '../FullscreenLoadingIndica
tor';
 import {propTypes as optionsSelectorPropTypes, defaultProps as opt
ionsSelectorDefaultProps} from './optionsSelectorPropTypes';
+import canUseTouchScreen from '../../libs/canUseTouchscreen';
 
 const propTypes = {
     /** Whether we should wait before focusing the TextInput, usef
ul when using transitions on Android */
@@ -62,7 +63,7 @@ class BaseOptionsSelector extends Component {
             },
             enterConfig.descriptionKey,
             enterConfig.modifiers,
-            true,
+            !canUseTouchScreen(),
             () => !this.state.allOptions[this.state.focusedIndex],
         );
 

Result

https://user-images.githubusercontent.com/25520267/189849800-87a28564-04db-42c9-86f1-6972839bb7ac.mov

https://user-images.githubusercontent.com/25520267/189849871-726563ae-db22-483f-a6bc-152808518582.mov

[Puneet-here]

Proposal

It's a good opportunity to implement push to page, we should show this input on a different page individually. I have seen some other issues related to this field they can be fixed by moving it to a different field. For example one issue was that user can't see the list completely when keyboard is open because this field is so big and when keyboard opens it takes too much space. It looks like this -

[rushatgabhane]

https://github.com/Expensify/App/issues/10906#issuecomment-1245190241

cc @Expensify/design

[shawnborton]

I think for this particular issue, we should fix the enter button not working, and then separately we can create a proposal to rethink this page entirely (with push to page).

[Santhosh-Sellavel]

I am suggesting to put this on hold, as the linked PR #10965 is more related to this page and also part of the audit!

[shawnborton]

That works for me too!

[mallenexpensify]

K, put on HOLD and removed Help Wanted label. Thanks Shawn and Santhosh

[MonilBhavsar]

Still on hold and review. Also bumping it to weekly while it is on hold.

[thesahindia]

Hey @mallenexpensify, I just wanted to know if this will be eligible for the reporting as I had reported it here, It looks like it is getting fixed in another PR so not sure about it.

Little context about the slack thread: We thought the root cause is same for https://expensify.slack.com/archives/C01GTK53T8Q/p1657396762843809 and this one so decided to merge them but the root cause is different so we aren't dealing with this in the other issue.

[Santhosh-Sellavel]

This should be/ it's already part of the audit, this would be found there. The main purpose of the audit is to check the current behavior matches expected and to find inconsistent bugs like this, so I think this doesn't qualify for reporting bonus.

[mallenexpensify]

@thesahindia , @Santhosh-Sellavel is much more knowledgeable about this issue than I so I'm going to agree with his post above. If you disagree, please comment and tag Monil for review.

[melvin-bot[bot]]

⚠️ Looks like this issue was linked to a possible regression on PRODUCTION here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a production regression has occurred a Root Cause Analysis is required. Please follow the instructions here.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[MonilBhavsar]

Still on hold

[MonilBhavsar]

Still on hold

[mallenexpensify]

And.. Still on hold

[MonilBhavsar]

Still on hold

[mallenexpensify]

@MonilBhavsar @Santhosh-Sellavel Still on hold? Looks like the linked PR that was mentioned above is still actively being worked on https://github.com/Expensify/App/pull/10965

[Santhosh-Sellavel]

Yes still being worked on here

[melvin-bot[bot]]

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

[mallenexpensify]

@slafortune Reassigning as I'm heading OOO for a month

[melvin-bot[bot]]

@shawnborton, @slafortune, @MonilBhavsar, @Santhosh-Sellavel Eep! 4 days overdue now. Issues have feelings too...

[Santhosh-Sellavel]

Onhold