Page MenuHomePhabricator
Differential D9486

[web] make user profile accessible from message reactions list item
ClosedPublic
Actions

Authored by ginsu on Oct 12 2023, 11:27 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Jul 1, 1:08 PM
Unknown Object (File)
Sun, Jun 30, 5:37 PM
Unknown Object (File)
Sun, Jun 30, 8:27 AM
Unknown Object (File)
Sun, Jun 30, 5:54 AM
Unknown Object (File)
Sat, Jun 29, 4:07 AM
Unknown Object (File)
Sat, Jun 29, 4:07 AM
Unknown Object (File)
Sat, Jun 29, 4:07 AM
Unknown Object (File)
Sat, Jun 29, 4:04 AM
View All 43 Files
Subscribers
ashoat
• ted
tomek
will

Details

Reviewers
atul
inka
Commits
rCOMMb16cf7080050: [web] make user profile accessible from message reactions list item
Summary

This diff makes the message reactions list item interactive. When a user clicks on this component, the user profile modal will be pushed

Linear Task: https://linear.app/comm/issue/ENG-5244/make-user-profile-accessible-from-message-reactions-modal

Depends on D9485

Test Plan

Please see the demo video below

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

ginsu created this revision.Oct 12 2023, 11:27 PM
Herald added subscribers: will, tomek, ashoat. · View Herald TranscriptOct 12 2023, 11:27 PM
ginsu edited the summary of this revision. (Show Details)Oct 12 2023, 11:29 PM
ginsu edited the test plan for this revision. (Show Details)
ginsu added reviewers: atul, inka.
ginsu added a parent revision: D9485: [web] introduce MessageReactionsListItem.
ginsu attached a referenced file: F812524: Screen Recording 2023-10-13 at 2.28.57 AM.mov. (Show Details)
ginsu added a subscriber: • ted.Oct 12 2023, 11:31 PM
ginsu added inline comments.
web/modals/chat/message-reactions-modal.css
21 ↗(On Diff #31992)

Instead of creating a new color variable, I reused one with an incorrect variable name. I made a comment in the Linear task responsible for this part of the new color design system on web project and @ted and I will address this then.

https://linear.app/comm/issue/ENG-5060/message-reactions-modal#comment-98684abe

Harbormaster completed remote builds in B23236: Diff 31992.Oct 12 2023, 11:45 PM
ginsu requested review of this revision.Oct 12 2023, 11:45 PM
ginsu added a child revision: D9488: [web] introduce MarkdownUserMention component.Oct 13 2023, 12:48 PM
atul accepted this revision.Oct 16 2023, 12:18 PM
atul added inline comments.
web/modals/chat/message-reactions-list-item.react.js
21–27 ↗(On Diff #31992)

Doesn't hurt to memoize

web/modals/chat/message-reactions-modal.css
2 ↗(On Diff #31992)

You could shorten this to padding: 16px 24px 24px;, but I personally prefer what you have

21 ↗(On Diff #31992)

Might be worth adding TODO comment here?

This revision is now accepted and ready to land.Oct 16 2023, 12:18 PM
ginsu updated this revision to Diff 32100.Oct 17 2023, 12:03 PM

address @atul's comments + rebase before landing

Harbormaster completed remote builds in B23305: Diff 32100.Oct 17 2023, 12:22 PM
Closed by commit rCOMMb16cf7080050: [web] make user profile accessible from message reactions list item (authored by ginsu). · Explain WhyOct 17 2023, 2:53 PM
This revision was automatically updated to reflect the committed changes.
ginsu added a commit: rCOMMb16cf7080050: [web] make user profile accessible from message reactions list item.

Revision Contents
Changeset List

Diff 32119

View Options

web/modals/chat/message-reactions-list-item.react.js

Loading...
View Options

web/modals/chat/message-reactions-modal.css

Loading...