Page MenuHomePhabricator
Differential D5689

[lib] modified messagePreviewText to handle reaction message types
ClosedPublic
Actions

Authored by ginsu on Nov 18 2022, 3:23 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Nov 24, 11:54 AM
Unknown Object (File)
Sun, Nov 24, 11:54 AM
Unknown Object (File)
Sun, Nov 24, 8:27 AM
Unknown Object (File)
Sun, Nov 24, 7:48 AM
Unknown Object (File)
Fri, Nov 22, 3:25 AM
Unknown Object (File)
Fri, Nov 8, 5:16 PM
Unknown Object (File)
Fri, Nov 8, 5:16 PM
Unknown Object (File)
Fri, Nov 8, 5:16 PM
View All Files
Subscribers
ashoat
atul
tomek

Details

Reviewers
atul
rohan
Commits
rCOMMc2bd2290843f: [lib] modified messagePreviewText to handle reaction message types
Summary

Made changes to messagePreviewText to handle reaction message types. We will need this for the messageTitle function in the reaction message spec. For now, I am going with a generic text format of "someone reacted to a message", but will probably change the content when we get further along with the message liking implementation.

Test Plan

Ran web and mobile app locally and nothing crashes/will be testing more in subsequent diffs

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

ginsu created this revision.Nov 18 2022, 3:23 PM
Herald added subscribers: atul, tomek, ashoat. · View Herald TranscriptNov 18 2022, 3:23 PM
ginsu edited the summary of this revision. (Show Details)Nov 18 2022, 3:26 PM
ginsu added reviewers: atul, rohan.
ginsu edited the summary of this revision. (Show Details)Nov 18 2022, 3:29 PM
Harbormaster completed remote builds in B13586: Diff 18608.Nov 18 2022, 3:34 PM
ginsu requested review of this revision.Nov 18 2022, 3:34 PM
ginsu added a child revision: D5690: [lib] introduced reaction message spec boilerplate.Nov 18 2022, 3:44 PM
ashoat added inline comments.Nov 19 2022, 12:19 PM
lib/shared/message-utils.js
296

Is this function only used from lib/shared/messages/multimedia-message-spec.js? Today? Wondering if the logic should just be moved into the individual message specs... what do you think?

atul accepted this revision.Nov 21 2022, 3:21 PM

Looks reasonable, please make sure to respond to @ashoat's question before landing

This revision is now accepted and ready to land.Nov 21 2022, 3:21 PM
ginsu planned changes to this revision.Nov 28 2022, 3:37 PM

Making changes based on ashaoat's comments to put messagePreviewText into the individual message specs

lib/shared/message-utils.js
296

That is correct, messagePreviewText is only called in lib/shared/messages/multimedia-message-spec.js

Wondering if the logic should just be moved into the individual message specs... what do you think?

I will do this, I feel like it will help keep the message specs more modular

ginsu updated this revision to Diff 19114.Dec 2 2022, 1:59 PM
ginsu edited the summary of this revision. (Show Details)

rebase, and realized it is going to be easier land this diff and then to address ashoat's comments in a followup diff

This revision is now accepted and ready to land.Dec 2 2022, 1:59 PM
ginsu edited the summary of this revision. (Show Details)Dec 2 2022, 1:59 PM
ginsu added a comment.Dec 2 2022, 2:07 PM
In D5689#169587, @atul wrote:

Looks reasonable, please make sure to respond to @ashoat's question before landing

Will create follow-up diff to address the comments. Created a task to track this

Harbormaster completed remote builds in B13948: Diff 19114.Dec 2 2022, 2:11 PM
ginsu updated this revision to Diff 19148.Dec 5 2022, 2:16 PM

updated language from reacted to liked

Harbormaster completed remote builds in B13975: Diff 19148.Dec 5 2022, 2:28 PM
ginsu updated this revision to Diff 19216.Dec 7 2022, 7:13 AM

rebase before landing

Harbormaster completed remote builds in B14040: Diff 19216.Dec 7 2022, 7:23 AM
Closed by commit rCOMMc2bd2290843f: [lib] modified messagePreviewText to handle reaction message types (authored by ginsu). · Explain WhyDec 7 2022, 7:34 AM
This revision was automatically updated to reflect the committed changes.
ginsu added a commit: rCOMMc2bd2290843f: [lib] modified messagePreviewText to handle reaction message types.
ginsu mentioned this in D5886: [lib] put messagePreview function for mutlimedia and reaction message into respecitve message spec.Dec 16 2022, 12:01 PM
ginsu mentioned this in rCOMMe5094444d76e: [lib] put messagePreview function for mutlimedia and reaction message into….Dec 27 2022, 7:10 AM

Revision Contents
Changeset List

PathSize
lib/
shared/
12 lines
types/
4 lines

Diff 18608

View Options

lib/shared/message-utils.js

Loading...
View Options

lib/types/message-types.js

Loading...