Page MenuHomePhabricator

[lib] Add 'hasBeenEdited' status to messages
ClosedPublic

Authored by kuba on Mar 7 2023, 1:48 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Mar 28, 1:05 AM
Unknown Object (File)
Thu, Mar 28, 1:05 AM
Unknown Object (File)
Thu, Mar 28, 1:05 AM
Unknown Object (File)
Thu, Mar 28, 1:05 AM
Unknown Object (File)
Thu, Mar 28, 1:05 AM
Unknown Object (File)
Thu, Mar 28, 1:05 AM
Unknown Object (File)
Thu, Mar 28, 1:04 AM
Unknown Object (File)
Thu, Mar 28, 12:55 AM
Subscribers

Details

Summary

Added 'hasBeenEdited' variable to track if the message was edited. Will be used to displaying 'edited' label.

Test Plan

Checked if the app compiles and works. Then tested if the status is set properly by changing labels of tooltip buttons to the edited status (instead of 'Reply' i displayed edited status).

Diff Detail

Repository
rCOMM Comm
Branch
kuba/messageediting
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

lib/selectors/chat-selectors.js
332 ↗(On Diff #23479)
403–404 ↗(On Diff #23479)

To work with a read-only type, you can do this

lib/types/messages/text.js
32 ↗(On Diff #23479)
  1. Please never add a new property that isn't read-only unless you have a good reason for it
  2. When touching a type, it's worth seeing if you can easily convert the whole thing to be read-only
kuba marked 3 inline comments as done.

Responded to comments, made hasBeenEdited property read-only

ashoat requested changes to this revision.Mar 8 2023, 7:14 AM

Can we avoid putting this in TextMessageInfo? I don't think it belongs there... and you're doing something pretty inconsistent with how we approached reactions.

Why didn't you put this in ChatMessageInfoItem? (In the future, when you are doing something different like this, please expect that your reviewers will call you out on it, and explain it ahead of time.)

This revision now requires changes to proceed.Mar 8 2023, 7:14 AM

Moved 'hasBeenEdited' property to 'ChatMessageInfoItem' type

Can we avoid putting this in TextMessageInfo? I don't think it belongs there... and you're doing something pretty inconsistent with how we approached reactions.

Why didn't you put this in ChatMessageInfoItem? (In the future, when you are doing something different like this, please expect that your reviewers will call you out on it, and explain it ahead of time.)

You are right. It should be in ChatMessageInfoItem. Going to update the diff.

I wasn't sure if we want to add it to RobotextChatMessageInfoItem, but reactions are there as well, so I also added it to it.

ashoat requested changes to this revision.Mar 8 2023, 10:16 AM

Can RobotextChatMessageInfoItem be edited?

This revision now requires changes to proceed.Mar 8 2023, 10:16 AM

Add 'hasBeenEdited' to code for native to be able to access the variable

Removed 'hasBeenEdited' variable from 'RobotextChatMessageInfoItem'

This revision is now accepted and ready to land.Mar 9 2023, 9:52 AM