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 2024-04-15] [$250] HIGH: [Comment linking] Highlight the linked comment with a different color than the normal hover color #38851

Closed
roryabraham opened this issue Mar 23, 2024 · 49 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 Design External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item.

Comments

@roryabraham
Copy link
Contributor

roryabraham commented Mar 23, 2024

https://expensify.slack.com/archives/C035J5C9FAP/p1711152608943449?thread_ts=1705035404.136629&cid=C035J5C9FAP

Action Performed:

Click on a link to a comment in any chat.

Expected Result:

  1. You should be linked straight to that comment and be able to load chats in either direction (already built, this is currently on staging)
  2. The comment you linked to should be highlighted in a different color than the normal hover color (temporarily? or permanently?)

Actual Result:

The linked-to comment is highlighted with the color that a message becomes when you hover over it.

Workaround:

n/a

Platforms:

all platforms

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01d8f674d9f769e10c
  • Upwork Job ID: 1774424743522938880
  • Last Price Increase: 2024-03-31
  • Automatic offers:
    • Pujan92 | Reviewer | 0
Issue OwnerCurrent Issue Owner: @Pujan92
@roryabraham roryabraham added Daily KSv2 Weekly KSv2 NewFeature Something to build that is a new item. Design labels Mar 23, 2024
@roryabraham roryabraham self-assigned this Mar 23, 2024
Copy link

melvin-bot bot commented Mar 23, 2024

Triggered auto assignment to @shawnborton (Design), see these Stack Overflow questions for more details.

Copy link

melvin-bot bot commented Mar 23, 2024

@melvin-bot melvin-bot bot removed the Daily KSv2 label Mar 23, 2024
@roryabraham
Copy link
Contributor Author

hey @shawnborton, any thoughts on what color we could use to highlight the message that you linked to?

@quinthar
Copy link
Contributor

ya the comment linking worked great! However, I initially didn't realize what comment I had linked to, because it just looked like I was hovering over it:
image

@quinthar
Copy link
Contributor

I would think it'd remain permanently highlighted (unlike Slack), until the page changes.

@nkdengineer
Copy link
Contributor

nkdengineer commented Mar 23, 2024

Proposal

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

  • Feature request: Highlight the linked comment with a different color than the normal hover color

What is the root cause of that problem?

  • In here, we apply theme.hoverComponentBG if the report action is linked report action

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

  • We can replace theme.hoverComponentBGwith our expected color.
  • We need to consider both dark and light modes

What alternative solutions did you explore? (Optional)

Result

  • Dark mode:
Screen.Recording.2024-03-30.at.10.50.55.mov
  • Light mode:
Screen.Recording.2024-03-30.at.10.53.16.mov

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Mar 23, 2024
@quinthar quinthar changed the title Highlight the linked comment with a different color than the normal hover color HIGH: Highlight the linked comment with a different color than the normal hover color Mar 25, 2024
@quinthar quinthar changed the title HIGH: Highlight the linked comment with a different color than the normal hover color HIGH: [Comment linking] Highlight the linked comment with a different color than the normal hover color Mar 25, 2024
@shawnborton
Copy link
Contributor

Yeah, I think we should just nudge this one up to use our border color as the highlight color - it's consistent with how we highlight active nav items in the LHN too. And I like the idea to leave it highlighted until you navigate away.
CleanShot 2024-03-25 at 11 56 01@2x

We could try other colors if we wanted to, like a shade of yellow or something. Thoughts? cc @Expensify/design

CleanShot 2024-03-25 at 12 00 30@2x

@dannymcclain
Copy link
Contributor

Oh the yellow is kinda interesting! Can you point me to the Figma file? I'd love to see your border color example with a hovered message as well, just to compare. But I'm definitely intrigued by the yellow...

@shawnborton
Copy link
Contributor

Threw it in the ol' dumpster here

@roryabraham
Copy link
Contributor Author

I'm a fan of the yellow as well 💛

@dubielzyk-expensify
Copy link
Contributor

Yellow feels appropriate if we need to go away from the green hue. All the others seem to harsh except for something like ice.

@quinthar
Copy link
Contributor

What is the next step here, who is doing it, and what is the ETA?

@nkdengineer
Copy link
Contributor

Can the contributor work on it? If can, anyone can review my proposal here. Thanks

@shawnborton
Copy link
Contributor

Any more feedback on the colors @Expensify/design? Feel free to explore some ideas if you have any!

This comment was marked as resolved.

@Pujan92
Copy link
Contributor

Pujan92 commented Apr 1, 2024

should i try to do it or is someone else onto it?

@TafreedAhmad For this @nkdengineer is assigned and they will do the task. You can look out the other issues where Help Wanted label is applied.

@quinthar
Copy link
Contributor

quinthar commented Apr 2, 2024

@nkdengineer what is your ETA for completing this? I assume this is very fast, can you finish it today?

@nkdengineer
Copy link
Contributor

@quinthar Yes, I can finish it today.

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Apr 8, 2024
@melvin-bot melvin-bot bot changed the title [$250] HIGH: [Comment linking] Highlight the linked comment with a different color than the normal hover color [HOLD for payment 2024-04-15] [$250] HIGH: [Comment linking] Highlight the linked comment with a different color than the normal hover color Apr 8, 2024
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Apr 8, 2024
Copy link

melvin-bot bot commented Apr 8, 2024

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

Copy link

melvin-bot bot commented Apr 8, 2024

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.60-13 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 2024-04-15. 🎊

For reference, here are some details about the assignees on this issue:

Copy link

melvin-bot bot commented Apr 8, 2024

BugZero Checklist: The PR adding this new feature has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Apr 14, 2024
@kadiealexander
Copy link
Contributor

kadiealexander commented Apr 15, 2024

Payouts due:

Upwork job is here.

@kadiealexander
Copy link
Contributor

@Pujan92 please propose the regression test steps

Copy link

melvin-bot bot commented Apr 19, 2024

@shawnborton, @Pujan92, @roryabraham, @kadiealexander, @nkdengineer Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@melvin-bot melvin-bot bot added the Overdue label Apr 19, 2024
@Pujan92
Copy link
Contributor

Pujan92 commented Apr 19, 2024

Regression Test Proposal

  1. Copy link for any of the report message with the context menu
  2. Open that link
  3. Verify that the linked message is highlighted

Do we agree 👍 or 👎

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Apr 19, 2024
@melvin-bot melvin-bot bot removed the Overdue label Apr 22, 2024
@github-project-automation github-project-automation bot moved this from In Progress to Done in Comment Linking Apr 22, 2024
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 Daily KSv2 Design External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item.
Projects
Status: Done
Development

No branches or pull requests

9 participants