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

[HOLD for payment 2023-04-10] [$1000] Hovering over again to ‘Copy to clipboard’ seems to split the ‘Copy to clipboard’ into two lines in the second click for long emails #15949

Closed
1 of 6 tasks
kavimuru opened this issue Mar 14, 2023 · 64 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor

Comments

@kavimuru
Copy link

kavimuru commented Mar 14, 2023

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 chrome web > Click on + button > New Chat > Type a Long email and hit enter
  2. Click on the avatar > Hover over the copy to clipboard icon and see that the text ‘Copy to Clipboard’ displays in one line on the tooltip
  3. Next click on the icon and it displays ‘copied’. Again, hover over the icon and see that the ‘Copy to Clipboard’ splits into two lines on the tooltip. It doesn't happens with shorter emails, but only with longer emails.

Expected Result:

Hovering over again to ‘Copy to clipboard’ should not split the ‘Copy to clipboard’ into two lines in the second click for long emails. There should be consistency and it should display in the single line only.

Actual Result:

Hovering over again to ‘Copy to clipboard’ splits the ‘Copy to clipboard’ into two lines in the second click for long emails

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.84-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.1699.mp4
email.2.mp4

Expensify/Expensify Issue URL:
Issue reported by: @avi-shek-jha
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1678781967498269

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0113ef7945a36f778f
  • Upwork Job ID: 1638556398300405760
  • Last Price Increase: 2023-03-29
@kavimuru kavimuru added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Mar 14, 2023
@melvin-bot melvin-bot bot locked and limited conversation to collaborators Mar 14, 2023
@MelvinBot
Copy link

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

@MelvinBot
Copy link

MelvinBot commented Mar 14, 2023

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
Copy link

@michaelhaxhiu Whoops! This issue is 2 days overdue. Let's get this updated quick!

@melvin-bot melvin-bot bot added the Overdue label Mar 17, 2023
@MelvinBot
Copy link

@michaelhaxhiu Still overdue 6 days?! Let's take care of this!

@Expensify Expensify unlocked this conversation Mar 21, 2023
@tienifr

This comment was marked as outdated.

@michaelhaxhiu michaelhaxhiu added the External Added to denote the issue can be worked on by a contributor label Mar 22, 2023
@melvin-bot melvin-bot bot changed the title Hovering over again to ‘Copy to clipboard’ seems to split the ‘Copy to clipboard’ into two lines in the second click for long emails [$1000] Hovering over again to ‘Copy to clipboard’ seems to split the ‘Copy to clipboard’ into two lines in the second click for long emails Mar 22, 2023
@MelvinBot
Copy link

Job added to Upwork: https://www.upwork.com/jobs/~0113ef7945a36f778f

@MelvinBot
Copy link

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

@MelvinBot
Copy link

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 Mar 22, 2023
@MelvinBot
Copy link

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

@michaelhaxhiu
Copy link
Contributor

All good, definitely external eligible. But also a good and easy pick up for an internal engineer who wants to quickly fix

@melvin-bot melvin-bot bot removed the Overdue label Mar 22, 2023
@hungvu193
Copy link
Contributor

@michaelhaxhiu Are external contributors allowed to post Proposal before the Help Wanted label is added? I just wonder.

@michaelhaxhiu
Copy link
Contributor

michaelhaxhiu commented Mar 22, 2023

Edit: No changes have been announced yet. We are no longer locking jobs the way we used to, and so yes, it's acceptable to give a solution before the Help Wanted label is applied. But this comes with risk -- we may not choose to go external depending on what's ideal.

@michaelhaxhiu
Copy link
Contributor

Let me see if this was formally announced yet.

@situchan
Copy link
Contributor

The doc clearly says:
image

Example C+ review:
#16053 (comment)

@hungvu193
Copy link
Contributor

Thanks for your help @situchan 👍

@eVoloshchak
Copy link
Contributor

The doc clearly says:
Example C+ review: #16053 (comment)

Yeah, I was following the contributing guideline. Found the discussion on slack, but can't find the official announcement if there was one.
I think the contributing guidelines need to be updated, first-time contributors won't have access to slack

@michaelhaxhiu
Copy link
Contributor

Hm I made a mistake in my explanation above actually. We haven't announced the unlocked behavior yet (and when we do, I think we'll likely adhere to the Help Wanted label being needed before we allow proposals -- this is a correction from what I posted last time).

So for now we should be adhering to the guidance that's public. Which means proposals should only begin after the Help Wanted label is added.

@MelvinBot
Copy link

Reviewing label has been removed, please complete the "BugZero Checklist".

@MelvinBot
Copy link

MelvinBot commented Apr 3, 2023

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.2.93-4 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

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

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 - @avi-shek-jha $250
  • Contributor that fixed the issue - @s77rt $1500
  • Contributor+ that helped on the issue and/or PR - @eVoloshchak $1500

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

@MelvinBot
Copy link

MelvinBot commented Apr 3, 2023

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:

@s77rt
Copy link
Contributor

s77rt commented Apr 3, 2023

@eVoloshchak Will you take the checklist or should I do it?

@eVoloshchak
Copy link
Contributor

@eVoloshchak Will you take the checklist or should I do it?

I'll do it tomorrow

@michaelhaxhiu
Copy link
Contributor

Hmm I wonder if we need a regression test for this?

Was the original design (splitting the text) done by design or mistake in the code?

@eVoloshchak
Copy link
Contributor

@michaelhaxhiu, It was due to a mistake in the code

@eVoloshchak
Copy link
Contributor

As @s77rt has previously said, this is a wrong calculation bug that has been revealed by this PR

  • 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: [Web] Change tooltip component to accept generic content #15325 (comment)

  • 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: this is a visual bug, we already have the following item in the checklist: If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App

@avi-shek-jha
Copy link

Hi @michaelhaxhiu. I am the external reporter for this bug. Why is the melvin bot asking me to submit a PR?

@melvin-bot melvin-bot bot removed the Overdue label Apr 12, 2023
@Expensify Expensify deleted a comment from MelvinBot Apr 12, 2023
@michaelhaxhiu
Copy link
Contributor

michaelhaxhiu commented Apr 12, 2023

Sorry it's an automated action - it's because I assigned you to the GH. Ignore that please.

@michaelhaxhiu
Copy link
Contributor

michaelhaxhiu commented Apr 12, 2023

Helping move this forward. Here's a regression test suggestion:

  1. Start a DM with that user that has a long email address (e.g. 25+ characters)
  2. Click the other user's profile (from the DM)
  3. Click the long email address, (e.g. hover & click the email a few times)
  4. Ensure the Copy to clipboard tooltip text is one line

cc @eVoloshchak & @s77rt lmk if you agree or suggest a tweak

@s77rt
Copy link
Contributor

s77rt commented Apr 12, 2023

@michaelhaxhiu I don't think that's the correct regression test, we can just use the PR Tests as the regression test.

Regression Test Proposal

  1. Login to App
  2. Click on FAB > New Chat
  3. Create a chat with a user that has a very long email e.g. longlonglongemailaddress@longlonglongdomain.com
  4. Click on user avatar
  5. Hover the copy to clipboard button
  6. Verify the tooltip shows "Copy to clipboard" in one line
  7. Click the copy to clipboard button
  8. Verify the tooltip shows "Copied!" in one line
  9. Wait a few seconds
  10. Verify the tooltip text is changed to "Copy to clipboard" and is still displayed in one line

@michaelhaxhiu
Copy link
Contributor

we can just use the PR Tests as the regression test.

oh... duh.

@michaelhaxhiu
Copy link
Contributor

cool, works for me

@michaelhaxhiu
Copy link
Contributor

Checklist is satisfied. Closing.

@MelvinBot
Copy link

@flodnv @michaelhaxhiu Be sure to fill out the Contact List!

@eVoloshchak
Copy link
Contributor

The payments weren't issued yet, shouldn't this still be open?

@eVoloshchak
Copy link
Contributor

@michaelhaxhiu, I think this got closed out without issuing the payments, I still have the job in progress on Upwork

@michaelhaxhiu
Copy link
Contributor

For some reason payment didn't go through for you the first time, despite me doing it? Weird. I'll do yours now again.

@michaelhaxhiu
Copy link
Contributor

all set, sorry about that @eVoloshchak. I'm blaming Upwork this time because I'm certain I paid all 3 parties last week (yet yours failed).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor
Projects
None yet
Development

No branches or pull requests