Paths

Table of Contentst

Differential D6629

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

Authored by ginsu on Feb 6 2023, 7:14 AM.

Tags

None

Referenced Files

	Unknown Object (File)
	Tue, Jun 11, 6:59 PM

	Unknown Object (File)
	Tue, Jun 11, 12:54 PM

	Unknown Object (File)
	Sun, Jun 9, 6:50 AM

	Unknown Object (File)
	Sun, Jun 9, 6:50 AM

	Unknown Object (File)
	Sun, Jun 9, 6:50 AM

	Unknown Object (File)
	Sun, Jun 9, 6:50 AM

	Unknown Object (File)
	May 13 2024, 9:39 PM

	Unknown Object (File)
	May 13 2024, 9:39 PM

View All 55 Files

Subscribers

Details

Reviewers

atul
tomek
rohan

Commits

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

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

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

ginsu created this revision.Feb 6 2023, 7:14 AM

Herald added subscribers: atul, tomek, ashoat. · View Herald TranscriptFeb 6 2023, 7:14 AM

ginsu edited the summary of this revision. (Show Details)Feb 6 2023, 7:15 AM

ginsu edited the test plan for this revision. (Show Details)

ginsu added reviewers: atul, tomek, rohan.

ginsu attached a referenced file: F366817: Screenshot 2023-02-05 at 10.34.50 PM.png. (Show Details)

ginsu attached a referenced file: F366816: Screenshot 2023-02-05 at 10.34.37 PM.png. (Show Details)

Harbormaster completed remote builds in B16176: Diff 22126.Feb 6 2023, 7:31 AM

ginsu requested review of this revision.Feb 6 2023, 7:31 AM

Nice!

This revision is now accepted and ready to land.Feb 6 2023, 8:10 AM

ashoat added inline comments.Feb 6 2023, 8:22 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)

ginsu added inline comments.Feb 6 2023, 9:02 AM

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

ashoat added inline comments.Feb 6 2023, 9:25 AM

lib/shared/messages/reaction-message-spec.js
171–175 ↗	(On Diff #22126)	Ah nvm, good catch

Closed by commit rCOMM2574c87a1b4f: [lib] add what reaction was used in reaction message title/notif text (authored by ginsu). · Explain WhyFeb 6 2023, 9:27 AM

This revision was automatically updated to reflect the committed changes.

ginsu added a commit: rCOMM2574c87a1b4f: [lib] add what reaction was used in reaction message title/notif text.

Revision Contents
Changeset List

Path

Size

lib/

shared/

messages/

reaction-message-spec.js

4 lines

Diff 22136

lib/shared/messages/reaction-message-spec.js

Loading...