[Pay May 15][$2000] In a long link that split into several lines, the pointer cursor is shown in the empty space

[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 staging.new.expensify.com
2. Send a long hyperlink in a chat that could be split onto several lines, e.g link : [Lorem ipsum dolor sit amet, consectetur adipiscing elit. Integer ornare dignissim nunc, eleifend molestie dui tempus non. Proin semper eu metus sit amet feugiat. Sed turpis augue, pellentesque sed accumsan ornare, malesuada in ligula. Aliquam porta condimentum varius. Morbi id lorem felis.](https://google.com/)
3. Hover the empty space after the link

Expected Result:

Since the link is non-existent on the empty space, and the link is unclickable in that area, the cursor should be the default one

Actual Result:

The shown cursor is pointer, that could possibly leads to a confusion since it does nothing when clicked

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.2.89-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:

Recording.60.mp4

Pointer.Cursor.on.Long.Link.mp4

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

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0174046855a3032866
• Upwork Job ID: 1641401791662075904
• Last Price Increase: 2023-04-10

[MelvinBot]

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

[MelvinBot]

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

[MelvinBot]

@flaviadefaria Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[MelvinBot]

Job added to Upwork: https://www.upwork.com/jobs/~0174046855a3032866

[MelvinBot]

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

[flaviadefaria]

Added the external label.

[MelvinBot]

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

[MelvinBot]

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

[allroundexperts]

Proposal

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

In a long link that split into several lines, the pointer cursor is shown in the empty space.

What is the root cause of that problem?

The root cause of the issue is that we're wrapping the Text component with the PressableWithSecondaryInteraction component (for displaying the copy to clipboard popover). The PressableWithSecondaryInteraction is a block component that has cursor set as pointer. This can be seen here.

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

We can just pass the style prop to the BaseAnchorForCommentsOnly component (probably if onPress is empty) here having the following value:

cursor: 'default'

We'll also need to forward the style prop from BaseAchorForCommentsOnly component in the PressableWithSecondaryInteraction component here with value:

style={{...this.props.inline && styles.dInline, ...this.props.styles}}

Result

Screen.Recording.2023-04-04.at.3.05.59.PM.mov

What alternative solutions did you explore? (Optional)

None

[alexxxwork]

Actually if you have plain text after the long link it works as intended. Is it worth fixing?

[kerupuksambel]

Actually if you have plain text after the long link it works as intended. Is it worth fixing?

In my opinion, it is. As mentioned in the "Expected Result" section, the cursor being pointer could leads to a confusion since the user could think they clicked the link since pointer usually associated with hovered link, while they actually don't click on that link, hence could possibly caused confusion on the user's side.

[MelvinBot]

@chiragsalian, @parasharrajat, @flaviadefaria Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[flaviadefaria]

Are we close to selecting a proposal here?

[parasharrajat]

@allroundexperts Can you explain the affect of these changes overall to the APP not only to the issue at hand?

[allroundexperts]

@allroundexperts Can you explain the affect of these changes overall to the APP not only to the issue at hand?

@parasharrajat I think that we're using RenderHTML on Report Action Item only. As such, this will impact just the messages in the report chat page.

[parasharrajat]

Ok. What will be the effect? Also, I think the root cause is wrong.

[parasharrajat]

Waiting for proposals..

[allroundexperts]

Ok. What will be the effect?

The effect will be that only links will be Pressable (along with pointer cursor). The enclosing div (which is block and takes the whole width) won't have a pointer cursor.

[mallenexpensify]

Not sure why automation isn't working here, PR is on production, updated title to pay on May 15.

[mallenexpensify]

Just realizing this one took a while. @parasharrajat and @aswin-s before I deduct payment for the contributor and C+ is there any details I should be about to why the delay happened?

Assigned

Merged

[aswin-s]

I had raised Initial PR on 17th April itself. However it took a couple of days for it to get reviewed. All review feedbacks were addressed within a couple of hours of posting.

@parasharrajat @chiragsalian Could you guys help out here?

[aswin-s]

Bump @parasharrajat @chiragsalian @mallenexpensify . There was no review activity during the periods 18-21 & 26-29 and I had to remind the reviewers to proceed. It's unfortunate that there will be a payment deduction when I had no control over the outcome.

[mallenexpensify]

@aswin-s , apologies for the delay, the Reviewing label kept this issue from going overdue.

@parasharrajat can you please chime in on the timeline and provide a recommendation?
@aswin-s , since it's taken a while to issue payment, let me know if you'd like me to pay half now, then we can bonus any additional funds if needed.

[parasharrajat]

Timeline:

1. PR created. April 17.
2. review started on April 17.
3. Solution found to be different from Proposal, explanation asked. April 17.
4. Explanation provider April 18.
5. I got busy with EC3 and missed the follow-up.
6. Review continued on and changes suggested April 21
7. Bug found on April 25.
8. PR C+ approved April 26.
9. No activity.
10. PR approved CME on April 29.

There was definitely a delay from my side for a few days. There were many reasons and one of them was that solution on the PR diverged from the proposal and I had to redo the analysis.

That said, there have been no regressions. If PR had been merged on the same day C+ reviewed elapsed business days would be 8.

cc: @mallenexpensify

[Nikhil-Vats]

@parasharrajat FYI, this has caused regression - #18658. I have confirmed by checking the commits in the main branch and running the merge commit for this and the one before it locally. It also checks out with my RCA.

[parasharrajat]

Thanks for informing.

[mallenexpensify]

ooooooof, even more confusing now. Thanks @parasharrajat and @Nikhil-Vats

tl,dr: I'd like to pay @aswin-s 100% because they weren't responsible for any delay and the regression wasn't reported til way later. I'd like to pay @parasharrajat 50% due to delay and not catching the regression. Sound fair to both you?

Let's address the regression first. from CONTRIBUTING.md

If the PR causes a regression at any point within the regression period (starting when the code is merged and ending 7 days after being deployed to production), contributors are not eligible for the 50% bonus.

It appears that the first mention/link to a regression was today and pay was due on May 15, so we missed the boat there, payment isn't affected by that.

I'm now torn because

• There was definitely a delay from my side for a few days. - Rajat's delay affected payment for @aswin-s
• But... it was only a day so we could not penalize based on one day (and provide reasoning)
• And.... now there's a regression, so finding a reason to not reduce payment based on timeline might not make sense.

There was no review activity during the periods 18-21 & 26-29 and I had to remind the reviewers to proceed.

These are week/business days too

For Rajat and C+, this is what our process doc says (Which we want to make public soon)

If regressions are found that “should have”* been caught after the PR has been approved and merged, the C+’s payment gets cut in half for each regression found (½ the first time, ½ the second, etc (ie. $1000 > $500 > $250)). This includes regressions on staging and production. Idea discussion slack here. Any bonus increases that might have been added for timeliness are rescinded for regressions.
*“Should have” = C+ should have caught the bug by fully following the checklist. If C+ skips a step or completed the checklist incompletely, payment will be cut in half.

[parasharrajat]

@mallenexpensify Can we please wrap this up? This is breaking WAQ.

[melvin-bot]

@aswin-s, @chiragsalian, @mallenexpensify, @parasharrajat Eep! 4 days overdue now. Issues have feelings too...

[chiragsalian]

Friendly bump here @mallenexpensify, whats next?

[mallenexpensify]

Paid @parasharrajat $1000 and @aswin-s $2000. @kerupuksambel was paid $250 for reporting.

TestRail GH is here https://github.com/Expensify/Expensify/issues/287421