[HOLD for payment 2023-07-12] [$1000] Assign Task - Description field line is displaced and moves up when typed

[lanitochka17]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Issue found when executing PR #20689

Action Performed:

1. Open http://staging.new.expensify.com/
2. Click on FAB menu / + actions in chat
3. Click on "Assign task"
4. Click on Description field
5. Type a letter

Expected Result:

Description field line remains at the same place all the time

Actual Result:

Description field line moves up when user types something

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

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

Bug6102069_Recording__716.mp4

Expensify/Expensify Issue URL:

Issue reported by: @bernhardoj

Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~014f2dca07ce0d7ce9
• Upwork Job ID: 1673920053781938176
• Last Price Increase: 2023-06-28

[melvin-bot]

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

[jeet-dhandha]

This is a Safari Browser Issue

• As we can see in chrome we are getting height as its already collapsed at initial render.

• But in safari it is set to maxHeight

[jeet-dhandha]

Proposal

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

• The problem is that when we are opening Assign Task modal in Safari browser, the description box collapses as we start typing in it.

What is root cause of that problem?

• The main issue is that the height of Description Input is already set to maxHeight minus some extra height params like padding and when we are typing in it, it collapses as it is recalculatting TextInput height due to autoGrowHeight(multiline) property.

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

Affected File Code:

• We should update StyleUtils.js file's getAutoGrowHeightInputStyle function to set height to textInputHeight by default if the browser is Safari.

Code Changes

    function getAutoGrowHeightInputStyle(textInputHeight, maxHeight) {
    if (textInputHeight > maxHeight) {
        return {
            ...styles.pr0,
            ...styles.overflowAuto,
        };
    }

+   let height = maxHeight - styles.textInputMultilineContainer.paddingTop - styles.textInputContainer.borderBottomWidth;
+   if(Browser.getBrowser() === CONST.BROWSER.SAFARI) {
+       height = textInputHeight;
+   }
+
    return {
        ...styles.pr0,
        ...styles.overflowHidden,
        // maxHeight is not of the input only but the of the whole input container
        // which also includes the top padding and bottom border
-       height: maxHeight - styles.textInputMultilineContainer.paddingTop - styles.textInputContainer.borderBottomWidth,
+       height,
    };
}

Fixed:

Fixed Video

Fix-AssignTask.mp4

What alternative solutions did you explore? (Optional)

• N/A

[bernhardoj]

Hi, I reported this issue 5 days ago here https://expensify.slack.com/archives/C049HHMV9SM/p1686997207642659. Will I get credited as the reporter?

[kushu7]

My proposal will also solve this issue adding link here, RCA is same.
#21433 (comment)

[manjesh-yadav]

Proposal

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

Description field height changed, when typed

What is the root cause of that problem?

When we implement auto-grow text input, we use a hidden <Text> component to calculate the component's height to grow <TextInput> because the <TextInput> component doesn't support auto-grow.

We calculate height and width for input when React fired a NativeEvent called onLayout when our hidden <Text> component rendered

• The problem comes with Safari web browser (also in the Desktop application) when initially component is rendered with no value React is not able to fire NativeEvent
• And we are not able to calculate the height of the hidden <Text> component
• Then our <TextInput> component render with max height CSS only

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

We just need a value in our hidden <Text> component to fire onLayout event

It will work when we set a placeholder or if we have any state value, if we do not have a placeholder or state then just an Empty string will work, and later it will change with the state value

File: App/src/components/TextInput/BaseTextInput.js

-      {this.state.value || this.props.placeholder}
+    {this.state.value || this.props.placeholder || ' '}

[kushu7]

My proposal will also solve this issue adding link here, RCA is same.
#21433 (comment)

Other issue was marked as closed so pasting solution here

Proposal

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

Safari (web and mWeb) - Assign Task Description field shows as multiline

What is the root cause of that problem?

Problem is here TextInput with autoGrowHeight.

App/src/components/TextInput/BaseTextInput.js

Lines 380 to 393 in 66915ac

	{(this.props.autoGrow || this.props.autoGrowHeight) && (
	// Add +2 to width so that the first digit of amount do not cut off on mWeb - https://github.com/Expensify/App/issues/8158.
	<Text
	style={[
	...this.props.inputStyle,
	this.props.autoGrowHeight && styles.autoGrowHeightHiddenInput(this.state.width, maxHeight),
	styles.hiddenElementOutsideOfWindow,
	styles.visibilityHidden,
	]}
	onLayout={(e) => this.setState({textInputWidth: e.nativeEvent.layout.width + 2, textInputHeight: e.nativeEvent.layout.height})}
	>
	{this.state.value || this.props.placeholder}
	</Text>
	)}

when we have empty string, in safari web and mWeb onLayout doesn't get Called(its getting called in chrome), thus we left with textInputHeight as undefined and its set maxHeight as height here.

App/src/components/TextInput/BaseTextInput.js

Line 236 in 66915ac

this.props.autoGrowHeight && styles.autoGrowHeightInputContainer(this.state.textInputHeight, maxHeight),

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

We should define textInputHeight as state variable with default value 0 like we are setting textInputWidth.
In some browser its get called with empty string and in nativeEvent, we get value of height 0. so basically we can define textInputHeight with initial value 0.
it will solve our problem globally with all TextInput with autoGrow.

App/src/components/TextInput/BaseTextInput.js

Line 36 in 66915ac

textInputWidth: 0,

Result

Simulator.Screen.Recording.-.iPhone.14.-.2023-06-24.at.00.56.35.mp4

What alternative solutions did you explore? (Optional)

None

[melvin-bot]

@JmillsExpensify Huh... This is 4 days overdue. Who can take care of this?

[JmillsExpensify]

Agreed that this is a bug and I can reproduce it. I'm opening this one up for the community.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~014f2dca07ce0d7ce9

[melvin-bot]

Current assignee @JmillsExpensify 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)

[narefyev91]

Proposal from @kushu7 makes sense to me - we need to have the same workflow for both growing input vertically and horizontally - based on that adding 0 to initial state variable should fix a problem globally as well #21280 (comment)
🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[thienlnam]

I believe this would have been fixed by #20836. Is that not the case?

[kushu7]

I believe this would have been fixed by #20836. Is that not the case?

Nope, i checked the PR, it was just related to flickering issue

[melvin-bot]

📣 @kushu7 🎉 An offer has been automatically sent to your Upwork account 🎉

Contributor - [$1000] Assign Task - Description field line is displaced and moves up when typed Please accept the offer 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 📖

[thienlnam]

Coolio, thanks!

[melvin-bot]

📣 @bernhardoj You have been assigned to this job!
Please apply to this job in Upwork here and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs!
Keep in mind: Code of Conduct | Contributing 📖

[kushu7]

PR is ready for review.

cc @narefyev91 @thienlnam

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

• Fixed 21280 textinput autogrow height #21850

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

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.
• [@JmillsExpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[JmillsExpensify]

It looks like this PR is ready for payment though @narefyev91 can you please get us kicked off with the BugZero checklist? Additionally, can either you or @thienlnam confirm that this PR is eligible for the 50% bonus. I'm not seeing an automated confirmation above. Checking the PR though, it looks like it is.

[narefyev91]

• The PR that introduced the bug has been identified. Link to the PR - it was not a regression here - it's mostly code improvement after a fully re-writing BaseTextInput component
• 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 - no comments
• 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 - no discussions
• Determine if we should create a regression test for this bug - no needs any additional actions

[narefyev91]

It looks like this PR is ready for payment though @narefyev91 can you please get us kicked off with the BugZero checklist? Additionally, can either you or @thienlnam confirm that this PR is eligible for the 50% bonus. I'm not seeing an automated confirmation above. Checking the PR though, it looks like it is.

Seems so - issue was fixed during 1.5 - 2 days

[JmillsExpensify]

Great thanks!

[JmillsExpensify]

Alright, circling back on payments, I think we are done. Here's where we ended up:

• Contributor: @kushu7 $1,500 (including 50% bonus)
• Contributor+: @narefyev91 $1,500 (including 50% bonus)

@narefyev91 please make sure that this is on our Callstack invoice. Otherwise, I'm closing this one out! 🙌🏼

[bernhardoj]

@JmillsExpensify hi, I just realized I haven't received the payment for the bug report 😅. Sorry for bringing this up late, but can you help me with the payment, please?