[HOLD for payment 2023-10-18] [$500] I cursor not displayed on profile error

[kavimuru]

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

---

Action Performed:

1. Open the app
2. Open settings->profile->upload invalid format image or more then 6mb image
3. App will display error below profile pic
4. Hover and try to select the error, observe that app displays normal cursor and does not allow to select the error
5. Similarly try on error in new contact method i.e. add a new contact method and insert wrong code
6. Hover and observe that it displays I cursor which is correct and allows to select the error (all other errors throughout app displays I cursor and allows to select the text)

Expected Result:

App should display I cursor over errors and allow to select and copy the text

Actual Result:

Profile pic error displays normal cursor and does not allow to select and copy the text

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.3.76-3
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

I.cursor.not.displayed.on.profile.error.2.mp4

bandicam.2023-09-22.15-29-39-962.mp4

Expensify/Expensify Issue URL:
Issue reported by: @dhanashree-sawant
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1695035097969079

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01e242f576582d3c21
• Upwork Job ID: 1705250624405737472
• Last Price Increase: 2023-09-22
• Automatic offers:
• dukenv0307 | Contributor | 26936356
• Dhanashree-Sawant | Reporter | 26936357

[melvin-bot]

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

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01e242f576582d3c21

[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

[melvin-bot]

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

[kavimuru]

Proposal from @dhanashree-sawant

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

We are solving the problem of normal cursor on profile error and error not selectable

What is the root cause of that problem?

On Main View of profile page, we have explicitly set user-select:none because of which it displays normal cursor on profile error

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

Set user-select:text on DotIndicatorMessage component of error on profile page
We can do that by adding new style here

dotIndicatorMessageSelect: {
    ...userSelect.userSelectText
},

Assign that style to DotIndicatorMessage in AvatarWithImagePicker.js here

style={[styles.mt6, styles.dotIndicatorMessageSelect]}

What alternative solutions did you explore? (Optional)

[melvin-bot]

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

[saranshbalyan-1234]

Proposal

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

On Profile error instead of text cursor it is showing default cursor.

What is the root cause of that problem?

We are not setting cursor style on the error message

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

Just need to pass this style (styles.cursorText)

App/src/components/AvatarWithImagePicker.js

Lines 361 to 368 in f39184e

	</View>
	{this.state.validationError && (
	<DotIndicatorMessage
	style={[styles.mt6]}
	messages={{0: this.props.translate(this.state.validationError, this.state.phraseParam)}}
	type="error"
	/>
	)}

which would look like

   {this.state.validationError && (
                    <DotIndicatorMessage
                        style={[styles.mt6, styles.cursorText]}
                        messages={{0: this.props.translate(this.state.validationError, this.state.phraseParam)}}
                        type="error"
                    />
                )}

What alternative solutions did you explore? (Optional)

N/A

[dukenv0307]

Proposal

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

Profile pic error displays normal cursor and does not allow to select and copy the text

What is the root cause of that problem?

This is because the current AvatarWithImagePicker is used inside this ScrollView from react-native-gesture-handler. This ScrollView has a known issue where the text inside it will not be selectable by default, even in the new web implementation.

Most of the ScrollView in the app are from react-native, only in 4 places they are from react-native-gesture-handler although there's no fancy gesture handling there.

This not only happens for the error text in this case, it also happens to other places that use those ScrollView, like contact method add page, where the text Add more ways for people to find you, and forward receipts is not selectable.

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

There're a few options:
Solution 1. (Recommended) we should update those places that use from react-native-gesture-handler to use ScrollView from react-native only for consistency, since we don't really use the gesture handling feature in these places, and the ScrollView from react-native is more stable. The web implementation of react-native-gesture-handler from has some issues, like here where we have to switch back to the legacy implementation.

Solution 2. If 1 is not possible for any reason, we can add contentContainerStyle={[styles.userSelectText]} to those ScrollViews to fix the issue not only for the error text in the OP but all texts inside the ScrollView.

Solution 3. We can also fix individually in the places we see the issues by adding styles.userSelectText to those components, we need to make sure to find and fix for all occurences inside all the ScrollViews mentioned

What alternative solutions did you explore? (Optional)

We can fix in the react-native-gesture-handler package by adding the user-select: text (similar to Solution 2, but upstream). But I'm not sure the maintainers will agree since it might be by design.

[jliexpensify]

Unassigning TRJ as I'm the first B0 member assigned

[jliexpensify]

Hi @robertKozik can you review this one? We have a few proposals that re 5 days old. Thanks!

[robertKozik]

Hi all! Sorry for keeping you waiting! 🙇🏼
I think the correct root cause is provided by @dukenv0307. Other solutions which constist of passing additional styles are only workarounds. The problem described in this issue can be found across every screen with ScrollView imported from react-native-reanimated. Changing it to react-native import gets rid of this problem, as suggested by @dukenv0307 proposal&solution.

Selected Proposal: #28030 (comment)

🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[melvin-bot]

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

Offer link
Upwork job
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 📖

[melvin-bot]

📣 @dhanashree-sawant 🎉 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

[dukenv0307]

@robertKozik The PR is ready for review

[dukenv0307]

@robertKozik Friendly bump. Please help review when you have time

[melvin-bot]

Based on my calculations, the pull request did not get merged within 3 working days of assignment. Please, check out my computations here:

• when @dukenv0307 got assigned: 2023-09-29 05:33:52 Z
• when the PR got merged: 2023-10-06 10:39:52 UTC
• days elapsed: 5

On to the next one 🚀

[jliexpensify]

Should a payment bonus be paid out to @dukenv0307 here? I looked at the PR and it was submitted on the 29th, but didn't get reviewed until the 5th. If it was reviewed same day, it would have qualified cc @thienlnam for your thoughts

[thienlnam]

I believe we should only allow exceptions if the internal engineer approved late without any changes required - in this case since it was the C+ that was delayed no bonuses would be sent out as the goal is to get both the C and C+ to move quickly for the bonus

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

• Profile pic error should display I cursor and allow to select and copy the text #28443

If no regressions arise, payment will be issued on 2023-10-18. 🎊

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:

• @robertKozik does not require payment (Contractor)
• @dukenv0307 requires payment offer (Contributor)
• @dhanashree-sawant requires payment offer (Reporter)

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:

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

[robertKozik]

• [@robertKozik] The PR that introduced the bug has been identified. Link to the PR: Cannot determine one PR which introduced this. It was rather missed during implementation process (importing ScrollView from reanimated rather then react-native). It occurs in several places.
• [@robertKozik] 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: N/A
• [@robertKozik] 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: N/A
• [@robertKozik] Determine if we should create a regression test for this bug. I don't think regression test is needed here
• [@robertKozik] 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. N/A

[jliexpensify]

Payment Summary:

• Bug Reporter: @dhanashree-sawant $50
• Contributor: @dukenv0307 $500
• C+: @robertKozik invoiced payment

Upwork job

[jliexpensify]

Paid and job closed!