Page MenuHomePhabricator
Differential D7742

[native] Show a pin icon next to pinned messages
ClosedPublic
Actions

Authored by rohan on May 8 2023, 6:52 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Nov 14, 7:57 AM
Unknown Object (File)
Wed, Nov 13, 7:32 PM
Unknown Object (File)
Wed, Nov 13, 7:09 PM
Unknown Object (File)
Mon, Oct 28, 6:39 AM
Unknown Object (File)
Oct 23 2024, 3:38 PM
Unknown Object (File)
Oct 2 2024, 9:24 AM
Unknown Object (File)
Sep 28 2024, 8:02 AM
Unknown Object (File)
Sep 28 2024, 8:02 AM
View All 67 Files
Subscribers
ashoat

Details

Reviewers
atul
ginsu
tomek
Commits
rCOMM086aa7bc67da: [native] Show a pin icon next to pinned messages
Summary

Next to each pinned message, we want to display a pin icon to indicate that the message is pinned. We only want to show this icon in chat, and not the pin modal / pinned messages screen.

Logic is pretty much identical to https://phab.comm.dev/D7505

Depends on D7735

Linear: https://linear.app/comm/issue/ENG-3455/show-a-pin-icon-next-to-pinned-messages

Test Plan

Test to confirm that the pin indicator appears in chat and not the other instances where Message is rendered (like the pin modal and screen).

For iOS, the confirmation is that there are no message height inconsistencies in the terminal. For Android, they appear on master, but the message height warnings don't show for all of the pinned messages so it makes me believe no changes made in this diff cause the warnings.

Diff Detail

Repository
rCOMM Comm
Branch
pinned_messages_native
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

rohan created this revision.May 8 2023, 6:52 AM
Herald added a subscriber: ashoat. · View Herald TranscriptMay 8 2023, 6:52 AM
rohan edited the test plan for this revision. (Show Details)May 8 2023, 6:54 AM
rohan attached a referenced file: F527585: Screen Recording 2023-05-08 at 9.42.20 AM.mov. (Show Details)
Harbormaster completed remote builds in B19137: Diff 26201.May 8 2023, 7:11 AM
rohan requested review of this revision.May 8 2023, 7:11 AM
atul requested changes to this revision.May 8 2023, 9:24 AM
atul added inline comments.
native/chat/composed-message.react.js
167

Can we memoize? Can probably put all of pinIcon in a useMemo?

This revision now requires changes to proceed.May 8 2023, 9:24 AM
atul accepted this revision.May 8 2023, 9:29 AM

Disregard... it's a class component. My bad.

This revision is now accepted and ready to land.May 8 2023, 9:29 AM
rohan updated this revision to Diff 26229.May 8 2023, 12:07 PM

Rebase

Harbormaster completed remote builds in B19158: Diff 26229.May 8 2023, 12:35 PM
Closed by commit rCOMM086aa7bc67da: [native] Show a pin icon next to pinned messages (authored by RohanK6 <rohan.khanderia@gmail.com>, committed by rohan). · Explain WhyMay 8 2023, 12:38 PM
This revision was automatically updated to reflect the committed changes.
rohan added a commit: rCOMM086aa7bc67da: [native] Show a pin icon next to pinned messages.

Revision Contents
Changeset List

Diff 26201

View Options

native/chat/composed-message.react.js

Loading...
View Options

native/chat/message-list.react.js

Loading...
View Options

native/chat/message-result.react.js

Loading...
View Options

native/chat/message.react.js

Loading...
View Options

native/chat/multimedia-message.react.js

Loading...
View Options

native/chat/text-message.react.js

Loading...