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
Unknown Object (File)
Tue, Oct 29, 2:56 PM
Unknown Object (File)
Tue, Oct 29, 2:56 PM
Unknown Object (File)
Tue, Oct 29, 2:56 PM
Unknown Object (File)
Tue, Oct 29, 2:53 PM
Unknown Object (File)
Oct 7 2024, 11:44 PM
Unknown Object (File)
Sep 29 2024, 5:18 PM
Unknown Object (File)
Sep 26 2024, 10:02 AM
Unknown Object (File)
Sep 12 2024, 2:21 AM
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
Lint
Lint Not Applicable
Unit
Tests Not Applicable

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 ↗(On Diff #22126)

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 ↗(On Diff #22126)

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 ↗(On Diff #22126)

Ah nvm, good catch