Page MenuHomePhabricator

[lib] modified messagePreviewText to handle reaction message types
ClosedPublic

Authored by ginsu on Nov 18 2022, 3:23 PM.
Tags
None
Referenced Files
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
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:15 PM
Unknown Object (File)
Fri, Nov 8, 4:51 PM
Subscribers

Details

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 added reviewers: atul, rohan.
ginsu requested review of this revision.Nov 18 2022, 3:34 PM
lib/shared/message-utils.js
296 ↗(On Diff #18608)

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?

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

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

lib/shared/message-utils.js
296 ↗(On Diff #18608)

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 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
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

updated language from reacted to liked