[HOLD for payment 2023-07-06] [$1000] Inconsistent "Enter" key behavior in Assign task

[kavimuru]

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. Click on FAB > Assign task
2. Click inside the title input field and hit "Enter" key and observe the behavior
3. Go back and click inside the description input field and hit "Enter" key and observe the behavior

Expected Result:

The "Enter" key behavior should be consistent for both input fields.

Actual Result:

"Enter" key behavior is different for both input fields. When user is inside the title input field the enter key triggers the "Next" button but when inside the description input field the enter key creates a new line. (It would be better if creating a new line is triggered with the "Shift + Enter" key)

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.2.30-0
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

2023-06-15.10.06.12.mp4

Recording.1045.mp4

Expensify/Expensify Issue URL:
Issue reported by: @Nathan-Mulugeta
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1686812998789659

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0189b1f49c485f8906
• Upwork Job ID: 1671653984207818752
• Last Price Increase: 2023-06-21

[melvin-bot]

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

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[Pujan92]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Inconsistent "Enter" key behavior in Assign task

What is the root cause of that problem?

we are not providing submitOnEnter for the description textarea which is causing this behavior and not submitting the form on enter.

What changes do you think we should make in order to solve the problem?

We need to pass submitOnEnter prop for the description TextInput which will submit the form by pressing the enter key.

App/src/pages/tasks/TaskDescriptionPage.js

Line 77 in 0b7455f

<TextInput

App/src/pages/tasks/NewTaskDescriptionPage.js

Line 87 in 0b7455f

<TextInput

[dukenv0307]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Inconsistent "Enter" key behavior in Assign task

What is the root cause of that problem?

we don't add submitOnEnter field for description text input

What changes do you think we should make in order to solve the problem?

Add submitOnEnter in 3 below places

App/src/pages/tasks/NewTaskDescriptionPage.js

Line 87 in 0b7455f

<TextInput

App/src/pages/tasks/NewTaskDetailsPage.js

Line 102 in 0b7455f

<TextInput

App/src/pages/tasks/TaskDescriptionPage.js

Line 77 in 0b7455f

<TextInput

What alternative solutions did you explore? (Optional)

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0189b1f49c485f8906

[melvin-bot]

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

[melvin-bot]

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

[hebontes]

This happens because this autoGrowHeight prop which is present for taskDescription prevents submitting a form, if we add autoGrowHeight to taskTitle we will get same behaviour

[hebontes]

Digging deeper into BaseTextInput.js on line 226 I found this code:

const isMultiline = this.props.multiline || this.props.autoGrowHeight;

In the same file on line 342:
dataSet={{submitOnEnter: isMultiline && this.props.submitOnEnter}}

Changing submitOnEnter: true or adding submitOnEnter prop to <TextInput/> component will submit the form but it also simultaneously creates new line and if title is empty it will show error and make a new line like this:

btw I noticed that when I make new line, the cursor is half cut off height, if I start typing it goes to normal, so it also needs fixing right? I will dig deeper and find a good solution for autoGrowHeight.

[hebontes]

So do what we want?

• make new line on Shift+Enter for both Title and Description (or should we leave Title single line?)
• Submit the form on Enter for both Title and Description
• Fix cursor on newline

[hebontes]

Proposal

Please re-state the problem that we are trying to solve in this issue.

description input field the enter key creates a new line. You want to create new line with SHIFT+ENTER and submit/next form just by clicking ENTER

What is the root cause of that problem?

autoGrowHeight prop on TextInput causes input to lose it's default pointer-events functionality which is written in BaseInput.js file

What changes do you think we should make in order to solve the problem?

add submitOnEnter prop to TextInput. But it will make new line if we are focused in taskDescription and taskTitle input is empty. cursor's height is cut off on new line. Solution:

• Listen for shift+enter key
• fix styling for textarea when new line is created

[narefyev91]

Proposal of @Pujan92 #21205 (comment) makes sense to me - proposal indicates a root case of the issue - and suggested changes should fix current issue
🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[dukenv0307]

@narefyev91 Could you think about my proposal? The proposal of @Pujan92 doesn't fix the current issue. We need to add submitOnEnter into

App/src/pages/tasks/NewTaskDetailsPage.js

Line 102 in 0b7455f

<TextInput

to fix the current issue. The proposal of @Pujan92 doesn't mention that

[narefyev91]

@narefyev91 Could you think about my proposal? The proposal of @Pujan92 doesn't fix the current issue. We need to add submitOnEnter into

App/src/pages/tasks/NewTaskDetailsPage.js

Line 102 in 0b7455f

<TextInput

to fix the current issue. The proposal of @Pujan92 doesn't mention that

Hey @dukenv0307 - your proposal is the same - that adding submitOnEnter will fix the issue - you pointed out in which files in should be changed including one more file. But the proposal is about adding code which will fix, and not really pointing in which files. Fixing logic is the same for your proposal and for @Pujan92 proposal - he was a first with it. In any case thanks @dukenv0307 will wait your proposals for other issues!

[dukenv0307]

@narefyev91 Understand your meaning. But I mean that :
Proposal of @Pujan92 suggests adding submitOnEnter in here

App/src/pages/tasks/NewTaskDescriptionPage.js

Line 87 in 0b7455f

<TextInput

App/src/pages/tasks/TaskDescriptionPage.js

Line 77 in 0b7455f

<TextInput

It is not related to this issue (you can check the action performed and the video in the issue's description). It only should be an extra fix. To fix the issue mentioned in this issue we should add submitOnEnter in here

App/src/pages/tasks/NewTaskDetailsPage.js

Line 102 in 0b7455f

<TextInput

This is another issue where the completed and more valuable proposal is selected instead of the first proposal.

[narefyev91]

@dukenv0307 i mentioned above my thoughts - in any case the last decision @neil-marcellini. If he will think that your proposal better - he will assign this to you, no worries

[Pujan92]

Definitely, I was aware of it but somehow I missed the file link in the proposal. But as it is in the same flow it can be understandable and for this, I think root cause with the correct solution matters more.

This is #18909 (comment) where the completed and more valuable proposal is selected instead of the first proposal.

I would prefer to not reference other issues of this type case as we have seen the decision goes in favor of the other way around too for many issues, which depends on the reviewer's perspective.

Anyways, I am fine with whatever the @neil-marcellini decides.

[neil-marcellini]

@narefyev91 I like @dukenv0307's proposal better because it's more complete.

However, I'm not sure if this is actually a problem. Do we really care if hitting enter in the description field of a task makes a new line? I don't really think so. If we really want to force using shift+Enter to make a new line, then I think hitting only Enter should remove focus from the input, because we usually don't make the form submit on enter. Are you guys good with closing this @lschurr @thienlnam?

[dukenv0307]

@neil-marcellini For more information, I see that on the description in the IOU flow, clicking enter will submit the form. So I think we should make it consistency

1. Clicking enter should submit form in all description page
2. Clicking enter should not submit form in all description page

Screen.Recording.2023-06-24.at.17.07.04.mov

[narefyev91]

@narefyev91 I like @dukenv0307's proposal better because it's more complete.

However, I'm not sure if this is actually a problem. Do we really care if hitting enter in the description field of a task makes a new line? I don't really think so. If we really want to force using shift+Enter to make a new line, then I think hitting only Enter should remove focus from the input, because we usually don't make the form submit on enter. Are you guys good with closing this @lschurr @thienlnam?

Everywhere in the app - pressing enter usually submit a form - and i think it should be the same expected behaviour for any form. BTW - i'm fully ok if we will leave as it is :-)

[melvin-bot]

📣 @dukenv0307 You have been assigned to this job by @neil-marcellini!
Please apply to this job in Upwork and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[dukenv0307]

@narefyev91 @neil-marcellini The PR is ready to review.

[melvin-bot]

🎯 ⚡️ Woah @narefyev91 / @dukenv0307, great job pushing this forwards! ⚡️

The pull request got merged within 3 working days of assignment, so this job is eligible for a 50% #urgency bonus 🎉

• when @dukenv0307 got assigned: 2023-06-26 17:05:24 Z
• when the PR got merged: 2023-06-27 16:37:22 UTC

On to the next one 🚀

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.34-1 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Submit form when hitting enter in assign task #21661

If no regressions arise, payment will be issued on 2023-07-06. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@narefyev91] The PR that introduced the bug has been identified. Link to the PR:
• [@narefyev91] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@narefyev91] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@narefyev91] Determine if we should create a regression test for this bug.
• [@narefyev91] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@lschurr] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[lschurr]

@narefyev91 @dukenv0307 @Nathan-Mulugeta - could you apply for the job in Upwork?

https://www.upwork.com/jobs/~0189b1f49c485f8906

[lschurr]

@narefyev91 do we need a regression test for this one?

[Nathan-Mulugeta]

Just applied.

[jeet-dhandha]

@narefyev91 Please check this. #22284 Might be a difference in user experience or a regression.

[narefyev91]

@narefyev91 @dukenv0307 @Nathan-Mulugeta - could you apply for the job in Upwork?

https://www.upwork.com/jobs/~0189b1f49c485f8906

I'm from Callstack - no needs to do anything with Upwork :-)

[narefyev91]

@narefyev91 Please check this. #22284 Might be a difference in user experience or a regression.

@jeet-dhandha i will say that it was a misunderstanding and lack of information based on this comment #21661 (comment)

[narefyev91]

@narefyev91 do we need a regression test for this one?

I think really not needed

[lschurr]

Paid and closed :)

[mananjadhav]

@lschurr @neil-marcellini We added enter on submit here. Due to this, when we submit the return key on Android, we submit the form. Hence users cannot enter multiline description as raised #22284. On desktop and web we can still enter multiline with shift+enter, but not on the mobile devices.

Can we confirm that we do want Enter key behavior for all the platforms? or should we solve the linked issue for small screen devices only.

cc - @muttmuure

[neil-marcellini]

We decided that we want to be consistent and have the enter key submit, as with all other forms. It works well on all the other platforms so I think we should figure out a fix for Android.