[HOLD for payment 2024-04-15] [$250] HIGH: [Comment linking] Highlight the linked comment with a different color than the normal hover color

[roryabraham]

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 Owner

Current Issue Owner: @Pujan92

[melvin-bot]

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

[melvin-bot]

Triggered auto assignment to @kadiealexander (NewFeature), see https://stackoverflowteams.com/c/expensify/questions/14418#:~:text=BugZero%20process%20steps%20for%20feature%20requests for more details.

[roryabraham]

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

[quinthar]

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:

[quinthar]

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

[nkdengineer]

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

[shawnborton]

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.

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

[dannymcclain]

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]

Threw it in the ol' dumpster here

[roryabraham]

I'm a fan of the yellow as well 💛

[dubielzyk-expensify]

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]

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

[nkdengineer]

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

[shawnborton]

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

[dannymcclain]

@shawnborton I explored a few but honestly I'm pretty into the yellow haha. 💛

The yellow is very clearly different from "normal" product colors, so it makes it stand out as a highlight vs. a hover/other UI state. Which I think makes it a lot more clear what's happening. I don't know that I'm 100% sold on the highlight staying "forever" (until the page changes) but I feel like if we wanted to revisit the timing, we could do that as a follow-up.

The only design-y thing to note is that we should define these colors as product variables. (Light mode uses yellow-100 but dark mode uses an opacity of a yellow swatch, so I'd like to have that locked down to a variable for easiness.)

[dubielzyk-expensify]

I'm with y'all on the yellow. The fact that yellow is a highlighter color is too serendipitous as well.

Good shout @dannymcclain . I could see us adding a highlight color in Figma.

[dubielzyk-expensify]

Okay, I used it in context and I do love it on the light mode, but I think the dark mode highlight looks pretty muddy so I shift my vote to the blue.

Maybe it's just my eyes, but color-appbg feels almost brown to me when the yellow is used:

[shawnborton]

Yeah, I totally agree the yellow feels muddy when we use reduced opacity like that.

One thing we can try to do is to use true color values from our system, and don't use any opacity. After a few attemps, I think yellow (800/100) or ice (800/100) could work well for dark/light respectively. Of these two, I am digging ice quite a bit:

I tried other combinations that didn't quite work as well:

[Pujan92]

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]

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

[nkdengineer]

@quinthar Yes, I can finish it today.

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

• fix message highlight color #39346

If no regressions arise, payment will be issued on 2024-04-15. 🎊

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

• @Pujan92 requires payment automatic offer (Reviewer)
• @nkdengineer requires payment (Needs manual offer from BZ)

[melvin-bot]

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:

• [@Pujan92] Please propose regression test steps to ensure the new feature will work correctly on production in further releases.
• [@kadiealexander] Link the GH issue for creating/updating the regression test once above steps have been agreed upon: https://github.com/Expensify/Expensify/issues/389569

[kadiealexander]

Payouts due:

• Contributor: $250 @nkdengineer
• Contributor+: $250 @Pujan92

Upwork job is here.

[kadiealexander]

@Pujan92 please propose the regression test steps

[melvin-bot]

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

[Pujan92]

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 👎