Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[$1000] Bug: Triple clicking a DM that contains text and URL, the URL isn’t highlighted #11735

Closed
kavimuru opened this issue Oct 11, 2022 · 44 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Engineering External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2

Comments

@kavimuru
Copy link

kavimuru commented Oct 11, 2022

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. Open app
  2. Go to a chat which has text and an URL
  3. Triple click on the message

Expected Result:

URL also should get highlighted

Actual Result:

URL is not getting highlighted

Workaround:

Unknown

Platform:

Where is this issue occurring?

  • Desktop app
  • Web app

Version Number: 1.2.12-3
Reproducible in staging?: y
Reproducible in production?: y
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:

xI1c5IprVd.mp4
Recording.673.mp4

Expensify/Expensify Issue URL:
Issue reported by: @michaelhaxhiu
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1665524227683689

View all open jobs on GitHub

@kavimuru kavimuru added AutoAssignerTriage Auto assign issues for triage to an available triage team member Daily KSv2 labels Oct 11, 2022
@melvin-bot
Copy link

melvin-bot bot commented Oct 11, 2022

Triggered auto assignment to @Christinadobrzyn (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

@melvin-bot melvin-bot bot removed the AutoAssignerTriage Auto assign issues for triage to an available triage team member label Oct 11, 2022
@Christinadobrzyn
Copy link
Contributor

  • I can also recreate this on the desktop app
  • I don't see any other GHs about this issue so sending it to eng!

@melvin-bot
Copy link

melvin-bot bot commented Oct 12, 2022

Triggered auto assignment to @youssef-lr (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@Christinadobrzyn Christinadobrzyn removed their assignment Oct 12, 2022
@michaelhaxhiu
Copy link
Contributor

@Christinadobrzyn 👋 just chiming in to say that you should feel free to update the GH to include desktop app in the list of affected platforms next time! I added it this time, but that's a valuable addition when triaging :)

@melvin-bot melvin-bot bot added the Overdue label Oct 14, 2022
@strepanier03
Copy link
Contributor

@youssef-lr - just a friendly bump here. Is this something you have the bandwidth to work on or should it go back to the pool?

@michaelhaxhiu
Copy link
Contributor

michaelhaxhiu commented Oct 14, 2022 via email

@melvin-bot
Copy link

melvin-bot bot commented Oct 17, 2022

@youssef-lr Eep! 4 days overdue now. Issues have feelings too...

@youssef-lr
Copy link
Contributor

I agree this is an issue, adding External.

@melvin-bot melvin-bot bot removed the Overdue label Oct 17, 2022
@youssef-lr youssef-lr added the External Added to denote the issue can be worked on by a contributor label Oct 17, 2022
@melvin-bot
Copy link

melvin-bot bot commented Oct 17, 2022

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

@melvin-bot
Copy link

melvin-bot bot commented Oct 17, 2022

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Oct 17, 2022
@melvin-bot
Copy link

melvin-bot bot commented Oct 17, 2022

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

@melvin-bot melvin-bot bot changed the title Bug: Triple clicking a DM that contains text and URL, the URL isn’t highlighted [$250] Bug: Triple clicking a DM that contains text and URL, the URL isn’t highlighted Oct 17, 2022
@youssef-lr youssef-lr removed their assignment Oct 17, 2022
@muttmuure muttmuure added the Weekly KSv2 label Oct 19, 2022
@eVoloshchak
Copy link
Contributor

@Ollyws's proposal is good, resolves the current issue, URL's are highlighted when triple-clicking a DM.

Secondly, userSelect is disabled on small screens rendering the link unselectable

I think that's the expected behavior

However, it introduces a new bug when copying a fully selected message to a clipboard. If you triple-click a DM and then press Cmd+C, it seems to append the concierge welcome message to the selected message

Screen.Recording.2022-11-16.at.19.37.04.mov

@Ollyws
Copy link
Contributor

Ollyws commented Nov 16, 2022

@eVoloshchak Interesting, that bug only started to occur when I pulled the latest changes.
It doesn't seem to occur if you apply the changes from @wildan-m's latest PR

@thesahindia
Copy link
Member

If you triple-click a DM and then press Cmd+C, it seems to append the concierge welcome message to the selected message

That's an already known bug which we are handing at #12730

@eVoloshchak
Copy link
Contributor

@Ollyws, @thesahindia, cool cool, thank you for pointing to that PR

In that case I think we should proceed with @Ollyws's proposal (minus the part with disabled userSelect on small screens)

🎀👀🎀 C+ reviewed!
cc: @chiragsalian

@chiragsalian
Copy link
Contributor

Sounds good, @Ollyws proposal LGTM as well. Feel free to create the PR @Ollyws.

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Nov 16, 2022
@melvin-bot
Copy link

melvin-bot bot commented Nov 16, 2022

📣 @Ollyws You have been assigned to this job by @chiragsalian!
Please apply to this job in Upwork 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 📖

@Ollyws
Copy link
Contributor

Ollyws commented Nov 17, 2022

All yours @eVoloshchak. Let's see if we can get this merged within three days!

@chiragsalian chiragsalian added the Reviewing Has a PR in review label Nov 17, 2022
@muttmuure
Copy link
Contributor

@chiragsalian is this complete?

@muttmuure muttmuure added Weekly KSv2 and removed Daily KSv2 labels Nov 22, 2022
@eVoloshchak
Copy link
Contributor

@muttmuure, this was deployed to staging a couple of hours ago

@chiragsalian
Copy link
Contributor

Yes, its on staging right now but not on production just yet.

@muttmuure
Copy link
Contributor

The job posting had expired (sorry!) but I've created a new one and it's here.

https://www.upwork.com/jobs/~01d70533c5adde99c5

@Ollyws @eVoloshchak please apply and we can get you both paid.

@Ollyws
Copy link
Contributor

Ollyws commented Nov 29, 2022

@muttmuure Applied, thanks!

@muttmuure
Copy link
Contributor

Everyone has been paid, bug is fixed and there are no regressions. Great work everyone!

@Ollyws
Copy link
Contributor

Ollyws commented Nov 29, 2022

@muttmuure I think you may have forgot the bonus, we did get it merged within 24 hours.

@eVoloshchak
Copy link
Contributor

@muttmuure I think you may have forgot the bonus, we did get it merged within 24 hours.

Same
By the way, what is the process with bonuses?
Who should track if C and C+ are eligible for bonuses? Contributor, Contributor+, or CME?

@muttmuure muttmuure reopened this Nov 29, 2022
@muttmuure
Copy link
Contributor

oops! Fixing that

@muttmuure
Copy link
Contributor

It's up to me on the BugZero team to check. I just missed it this time.

@eVoloshchak
Copy link
Contributor

It's up to me on the BugZero team to check. I just missed it this time.

Got it, thanks!

@muttmuure
Copy link
Contributor

50% Bonus applied - again, thanks for the speedy work

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Engineering External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2
Projects
None yet
Development

No branches or pull requests