[Pending Checklist] [$1000] Description - Cursor half visible when hitting Enter and going to next line of Task Description

[lanitochka17]

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

---

Issue found when executing PR #20775

Action Performed:

1. Login to NewDot
2. Go to any chat
3. Click + and Assign Task
4. Enter first line in Description field
5. Press Enter on the keyboard

Expected Result:

Cursor should be well visible on the second line

Actual Result:

Cursor partially visible until any other character is type after Enter

Workaround:

Unknown

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.3.30.0

Reproducible in staging?: Yes

Reproducible in production?: Yes

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

Bug6101939_video_03__8_.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~014922e794dd66dc4a
• Upwork Job ID: 1677228108574597120
• Last Price Increase: 2023-07-14

[melvin-bot]

Triggered auto assignment to @greg-schroeder (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

[greg-schroeder]

There are a couple issues pointing to this:

#21280
#21205

I think it will just be fixed by #21205.

[chiragxarora]

Proposal

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

Description - Cursor half visible when hitting Enter and going to next line of Task Description

What is the root cause of that problem?

Root cause is the custom autoGrowHeight styles which we are using by passing the autoGrowHeight prop to dynamically update the height of <TextInput/>

App/src/pages/tasks/TaskDescriptionPage.js

Lines 77 to 91 in 82558a1

	<TextInput
	inputID="description"
	name="description"
	label={props.translate('newTaskPage.descriptionOptional')}
	defaultValue={(props.task.report && props.task.report.description) || ''}
	ref={(el) => (inputRef.current = el)}
	autoGrowHeight
	submitOnEnter
	containerStyles={[styles.autoGrowHeightMultilineInput]}
	textAlignVertical="top"
	selection={selection}
	onSelectionChange={(e) => {
	setSelection(e.nativeEvent.selection);
	}}
	/>

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

This is more of an hack but you could see that for first 3-4 lines it partially hides the cursor and post that, it behaves well.
We could either edit the autoGrowHeightMultilineInput style aur create another similar one where we shall increase the minHeight slightly. Currently its 52px and we could increase it to 60px (reason is to provide space for 2nd line beforehand).
And we need a state variable for height to update the styles as the content grows. We can get current height of the input box by using native prop onContentSizeChange.

const [height, setHeight] = useState(60);
...................
<TextInput
 containerStyles={[styles.autoGrowHeightMultilineInputNew(height)]}
 onContentSizeChange={(e) => setHeight(e.nativeEvent.contentSize.height)}
>
</TextInput>

autoGrowHeightMultilineInputNew: (height) => ({
        height: height,
        minHeight: 60
    }),

Results

What alternative solutions did you explore? (Optional)

None

[chiragxarora]

@greg-schroeder this should be opened again

[s-alves10]

Please check this proposal. This will solve the all kind of issues related to auto-grow TextInput

#21905 (comment)

[fabOnReact]

Proposal

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

• TextInput has the wrong height after hitting enter on Safari/Macos/Chrome
• The cursor is cut

What is the root cause of that problem?

The root cause of the issue is the difference in behaviour of the Text Component on different platforms (web, iOS and Android).
The height of the TextInput is calculated with Text onLayout, which has a different behaviour when adding new line between react-native-web and react-native.

React Native Web uses white-space: pre-wrap to render new lines and spaces

react-native-web uses white-space: pre-wrap which adds a new line only when the \n is followed by a character (for ex. Line1\n has one line, Line1\nLine2 has two lines).

https://snack.expo.dev/@fabrizio.bertoglio/grounded-croissant

React Native uses native Layout APIs to calculate the View height and sets the height of the View onMeasure

react-native uses native APIs to calculated the layout from the ShadowNode (on Android using the Layout API). The \n adds a new line (for ex. Line1\n has two lines) on iOS and Android.

snack https://snack.expo.dev/@fabrizio.bertoglio/grounded-croissant

When the Text has some text in the new line:

• Android height 36
• Safari height 34

When the Text has an empty line:

• Android height 36
• Safari height 17

iOS

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

https://reactnative.dev/docs/tutorial#whats-going-on-here

JSX lets you write your markup language inside code. It looks like HTML on the web, except instead of web things like div or span, you use React components.

react-native-community/discussions-and-proposals#496

React Native currently includes many APIs that are modelled on Web APIs but do not conform to the standards of those Web APIs.

React Native is based on Web Standards. For this reason, iOS and Android should uniform to Web.
The native implementation needs to be updated to behave like Web, on Android requires changes to ReactTextShadowNode measure, FontMetricsUtil, ReactTextView onLayout, TextLayoutManager and TextLayoutManagerMapBuffer measureText to avoid adding a new line when \n is not followed by a character.
Similar changes are required on iOS.

What alternative solutions did you explore? (Optional)

Fixing the issue in react-native-web and adding a new line when using \n to uniform the API with iOS and Android.

[sophiepintoraetz]

I can reproduce this, @greg-schroeder (albeit in a different place in the product but there are proposals in here that will address both) - #21205 did not fix this issue.

[Nathan-Mulugeta]

Hey I am coming from #21905 aren't these two issues found on different places of the product? Doesn't that make me eligible for reporting @sophiepintoraetz , thanks.

[sophiepintoraetz]

@Nathan-Mulugeta - they are different areas but the same issue - #21905 has been closed out as a dupe. I see you are prolific in reporting issues, so I think you'll make this one up quickly!.

[Nathan-Mulugeta]

Thanks for your reply and for the compliment @sophiepintoraetz 🙂

[s-alves10]

I think we have to fix the root cause of the problem. Please check my proposal

[Ollyws]

Proposal

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

Autogrow height does not respond to line breaks when pressing enter.

What is the root cause of that problem?

We are using a hidden Text component to calculate the height when using autoGrowHeight:

App/src/components/TextInput/BaseTextInput.js

Lines 387 to 395 in 7e908b1

	<Text
	style={[...props.inputStyle, props.autoGrowHeight && styles.autoGrowHeightHiddenInput(width, maxHeight), styles.hiddenElementOutsideOfWindow, styles.visibilityHidden]}
	onLayout={(e) => {
	setTextInputWidth(e.nativeEvent.layout.width + 2);
	setTextInputHeight(e.nativeEvent.layout.height);
	}}
	>
	{props.value || props.placeholder}
	</Text>

It is a general browser behaviour to remove any trailing newlines, this means the Text component is not updated with a newline unless it is followed by another character. This causes the height of the TextInput to not update until another character is typed.

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

We can add a character such as a zero width space (\u200B) to the end of the string used for calculating the height when there is a newline at the end:

{`${props.value}${props.autoGrowHeight && props.value && (props.value.slice(-1) === '\n') ? '\u200B' : ''}` || props.placeholder}

This won't affect the measurement seeing as it doesn't take up any space, it will just trigger the browser to accept the line break when the user presses enter, and update the textinput height accordingly.

What alternative solutions did you explore? (Optional)

None

[sakluger]

Hey folks, I just closed another duplicate issue (#21449) on yet another page of the product. I think that whatever solution we go with, it should be a global solution that fixes this across all pages.

[melvin-bot]

@greg-schroeder this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~014922e794dd66dc4a

[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.45-7 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• fix: height calculation of BaseTextInput #23286

If no regressions arise, payment will be issued on 2023-08-02. 🎊

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

For reference, here are some details about the assignees on this issue:

• @sobitneupane does not require payment (Eligable for Manual Requests)
• @s-alves10 requires payment

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:

• [@sobitneupane] The PR that introduced the bug has been identified. Link to the PR:
• [@sobitneupane] 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:
• [@sobitneupane] 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:
• [@sobitneupane] Determine if we should create a regression test for this bug.
• [@sobitneupane] 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.
• [@greg-schroeder] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[greg-schroeder]

Assigned July 20 -> Merged July 24th (2 business days)

Payment summary:

Reporter: Applause (no payment)
C: @s-alves10 - $1,500 (bonus applied)
C+: @sobitneupane - $1,500 (bonus applied)

[greg-schroeder]

Payment made to @s-alves10. @sobitneupane will be paid via NewDot. Payments are outlined above.

[greg-schroeder]

@sobitneupane you're up on the checklist!

[greg-schroeder]

bump @sobitneupane

[sobitneupane]

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:

• [@sobitneupane] The PR that introduced the bug has been identified. Link to the PR:

#19377

• [@sobitneupane] 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:

It was a known issue in the PR. #19377 (comment)

• [@sobitneupane] 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:

It was a known issue. Discussion : #19377 (comment)

• [@sobitneupane] Determine if we should create a regression test for this bug.

yes

• [@sobitneupane] 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.

[sobitneupane]

Regression Test Proposal

1. Open ND and log in with any account
2. Click FAB and Assign Task
3. Enter first line in description field
4. Press Enter key
5. Verify that the cursor is fully visible

Do we agree 👍 or 👎

[sobitneupane]

Requested payment on newDot.

#21271 (comment)

[dangrous]

Test looks good to me

[JmillsExpensify]

Reviewed the details for @sobitneupane. $1,500 approved for payment in NewDot.

[greg-schroeder]

Filed regression test issue, closing