Page MenuHomePhabricator

[lib] add what reaction was used in reaction message title/notif text
ClosedPublic

Authored by ginsu on Feb 6 2023, 7:14 AM.
Tags
None
Referenced Files
F3500909: D6629.diff
Fri, Dec 20, 3:07 AM
Unknown Object (File)
Nov 15 2024, 11:44 PM
Unknown Object (File)
Nov 15 2024, 11:44 PM
Unknown Object (File)
Nov 15 2024, 11:44 PM
Unknown Object (File)
Nov 15 2024, 11:44 PM
Unknown Object (File)
Oct 29 2024, 2:56 PM
Unknown Object (File)
Oct 29 2024, 2:56 PM
Unknown Object (File)
Oct 29 2024, 2:56 PM
Subscribers

Details

Summary

add what reaction was used in reaction message title/notif text


Linear Task: ENG-2908

Test Plan

Please see the screenshots to see the changes I made

Before:

Screenshot 2023-02-05 at 10.34.37 PM.png (286×934 px, 34 KB)

After:

Screenshot 2023-02-05 at 10.34.50 PM.png (310×972 px, 42 KB)

Diff Detail

Repository
rCOMM Comm
Branch
eng-2908
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

ginsu edited the test plan for this revision. (Show Details)
ginsu added reviewers: atul, tomek, rohan.
ginsu requested review of this revision.Feb 6 2023, 7:31 AM
This revision is now accepted and ready to land.Feb 6 2023, 8:10 AM
lib/shared/messages/reaction-message-spec.js
171–175

I wonder if we could reduce the copy-paste here, seems like this code is shared with messageTitle... you could introduce a new function in this file that handles lines 171-177 (which are shared with lines 55-60)

lib/shared/messages/reaction-message-spec.js
171–175

The notif text uses your message while the title text uses a message, but if we still want to make this update, I can update this diff later in the day

lib/shared/messages/reaction-message-spec.js
171–175

Ah nvm, good catch