[$500] mWeb -Selecting text in TextInput also cover the the TextInput label area

[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. Navigate to Settings -> Profile -> Display name
2. Enter Text in Input and select the text

Expected Result:

Only area of text should be selected.

Actual Result:

Selecting text in TextInput also cover the the TextInput label area

Workaround:

Unknown

Platforms:

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

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

Version Number: Dev 1.3.73-1
Reproducible in staging?: n
Reproducible in production?: n
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

Record_2023-09-27-00-56-09.mp4

Expensify/Expensify Issue URL:
Issue reported by: @ishpaul777
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1695757053841779

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~010e8cb7ae22826c96
• Upwork Job ID: 1707094980737019904
• Last Price Increase: 2023-09-27
• Automatic offers:
• ishpaul777 | Reporter | 26975736

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~010e8cb7ae22826c96

[melvin-bot]

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

[GianlucaMina]

It looks good to me on Android, and the code doesn't seem to have anything strange, try restarting the terminal and clearing the browser cache, if the problem persists we will look for a solution.

In any case it seems to me more of a style problem and not that you are selecting the Label, you can check it by copying and then pasting it into the phone notes, if these were copied it is a selection problem, but if not it is a style question, which can break in the terminal compilation or due to a problem with the cache.

Let me know if the problem persists and we will fix it.

[ishpaul777]

I can still reproduce after clearing the cache, restarting terminal and even using a different Android device, this is indeed a styling issue selection works as expected.

[vadymbokatov]

Proposal

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

when selecting a text in input the selected text boundary goes beyond the text input expected boundaries and interferes with label.

What is the root cause of that problem?

the root cause of problem lies in difference between '--active-label-translate-y': ${styleConst.ACTIVE_LABEL_TRANSLATE_Y}px, in src/components/textInput/index.js and the value of input height in src/styles/variable.ts line 102 which calculates inputHeight based on fonScale provided by react native and custom arguments passed to it

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

Custom values passed to getValueUsingPixelRatio functtion must change

[tienifr]

Proposal

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

Text selection height overlaps with text input label.

What is the root cause of that problem?

We set the text input's line-height as its height on mWeb Chrome:

App/src/components/TextInput/BaseTextInput.js

Lines 246 to 256 in 3b2dc32

	const lineHeight = useMemo(() => {
	if (Browser.isSafari() && _.isArray(props.inputStyle)) {
	const lineHeightValue = _.find(props.inputStyle, (f) => f.lineHeight !== undefined);
	if (lineHeightValue) {
	return lineHeightValue.lineHeight;
	}
	} else if (Browser.isSafari() || Browser.isMobileChrome()) {
	return height;
	}
	return undefined;
	}, [props.inputStyle, height]);

Which is calculated by onLayout event. But we did not account for the translate animation of text input label when we press the text input. In other words, this height includes both the label and the input content.

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

We should exclude the traslateY value of the label which is determined by:

App/src/components/TextInput/BaseTextInput.js

Line 37 in 3b2dc32

const labelTranslateY = useRef(new Animated.Value(initialActiveLabel ? styleConst.ACTIVE_LABEL_TRANSLATE_Y : styleConst.INACTIVE_LABEL_TRANSLATE_Y)).current;

  return height - labelTranslateY

[tienifr]

This issue now happens on staging as well but not yet on production.

[hoangzinh]

@kbecciv could you help to add "Bug" label to this issue? I guess it's reason why Melvin hasn't assigned any BZ member yet. Thanks

[hoangzinh]

@vadymbokatov @tienifr Thanks for proposals, everyone. I confirm that after reverting the PR here #28126, this issue can not reproduce anymore. Because it's still in regression period so I will let them know.

[melvin-bot]

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

[hoangzinh]

sent a message here #28126 (comment)

[ayazalavi]

@hoangzinh, thank you for bringing this bug to my attention. I'd like to clarify that this issue specifically affects Android Chrome. In Android Chrome, the line height must match the element's height for the shadow DOM to update its content's height; otherwise, it retains its initial value. If we set a value lower than the element's height, it won't impact shadow DOM calculations in Android Chrome.

The issue with element selection has arisen due to this behavior. To address this bug, we have two options: we can opt to disregard Android Chrome's shadow calculations and remove the fix mentioned in

App/src/components/TextInput/BaseTextInput.js

Lines 252 to 254 in b94c32c

	} else if (Browser.isSafari() || Browser.isMobileChrome()) {
	return height;
	}

This change would introduce a minor fluctuation of approximately 0.3 px in android chrome only, as mentioned in this bug report link to the bug report. Since aforementioned bug has not been reported for Android Chrome, I propose that we remove the above check from the code. What are your thoughts?

[ayazalavi]

@hoangzinh How do you want me to fix this? It is browser bug. I can just remove this

App/src/components/TextInput/BaseTextInput.js

Lines 252 to 254 in b94c32c

	} else if (Browser.isSafari() || Browser.isMobileChrome()) {
	return height;
	}

which will fix this issue.

[hoangzinh]

@hoangzinh How do you want me to fix this? It is browser bug. I can just remove this

App/src/components/TextInput/BaseTextInput.js

Lines 252 to 254 in b94c32c

	} else if (Browser.isSafari() || Browser.isMobileChrome()) {
	return height;
	}

which will fix this issue.

@ayazalavi I haven't read the full details yet, but I think if we remove those LOCs and it still passes those test cases #28126 then I think it's good.
cc @arosiclair do you agree with the above idea ^? Thanks

[tienifr]

@ayazalavi Did you try my proposal above #28339 (comment)? I re-tested the original issue and it worked fine.

[ayazalavi]

@tienifr input height is 52px, if you set line-height to be 51px then it is same as not setting any line-height.

[0xmiros]

This should be handled as regression. So @ayazalavi and @sobitneupane will be fixing this.

[melvin-bot]

📣 @sobitneupane Please request via NewDot manual requests for the Reviewer role ($500)

[melvin-bot]

📣 @ayazalavi You have been assigned to this job!
Please apply to the Upwork job 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 📖

[melvin-bot]

📣 @ishpaul777 🎉 An offer has been automatically sent to your Upwork account for the Reporter role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[arosiclair]

@mallenexpensify this was a regression from #28126 so no payments for this other than the bug reporter

[mallenexpensify]

Thanks @arosiclair
@ishpaul777 , can you please accept the job and reply here once you have?
https://www.upwork.com/jobs/~010e8cb7ae22826c96

[ishpaul777]

Thanks for sending the offer. Accepted!

[mallenexpensify]

Issue reporter: @ishpaul777 paid $50 via Upwork