[native] Display the number of pinned messages in a banner across chat
ClosedPublic
Actions

Authored by rohan on Apr 25 2023, 10:28 AM.

Details

Reviewers

atul
ginsu
tomek

Commits

rCOMM4a39f676efa0: [native] Display the number of pinned messages in a banner across chat

Summary

We want to display the number of pinned messages in a banner on top of chat the displays the number of pinned messages (if > 0 and if threadInfo contains a pinnedCount field since we gate it by hasMinCodeVersion on native).

The code is largely similar to on web: https://phab.comm.dev/D7310

Depends on D7608

Test Plan

Check the banner to confirm the banner does not show if there are no pinned messages, and confirm that the banner appears for both 1 and more than 1 pinned messages.

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

rohan created this revision.Apr 25 2023, 10:28 AM

Herald added a subscriber: ashoat. · View Herald TranscriptApr 25 2023, 10:28 AM

rohan edited the summary of this revision. (Show Details)Apr 25 2023, 10:30 AM

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

rohan attached a referenced file: F507955: Simulator Screen Recording - iPhone 14 Pro - 2023-04-25 at 13.24.38.mp4. (Show Details)

Harbormaster completed remote builds in B18746: Diff 25684.Apr 25 2023, 10:48 AM

rohan requested review of this revision.Apr 25 2023, 10:48 AM

rohan added a child revision: D7672: [native] Create a new screen to display the thread's pinned messages.Apr 27 2023, 11:10 AM

Can we get rid of the React.useMemo(...) here?

native/chat/message-list-container.react.js
365–372 ↗	(On Diff #25684)	You don't need to use `React.useMemo(...)` here. The `useMemo()` hook is helpful for maintaining referential equality so that objects will be considered "shallowly equal" (`==` in JS) and we can avoid re-renders. This is helpful for objects (including `Map()`, `Set()`, etc), arrays (which are objects), and functions (which are objects). On the other hand, `string`s in JS are considered shallowly equal if they have the same contents, so we don't have to worry about re-renders if the "content" stays the same. See below: (In like C++, which you've been working w/ recently, `std::string`s can be allocated on the heap (unlike integers, booleans, etc) which may have been part of your reasoning that shallow equality would check reference instead of contents?)