[web] house the thread list in a panel component
AcceptedPublic
Actions

Authored by ginsu on Jan 4 2024, 3:40 AM.

Details

Reviewers

atul
inka

Summary

PLEASE NOTE THAT THIS DIFF AND SUBSEQUENT DIFFS IN THIS STACK WILL NOT BE LANDED UNTIL MORE OF THE REDESIGN IS READY SINCE THIS WILL CAUSE REGRESSIONS IN PROD

This diff updates the chat component to use the panel component I introduced earlier. To make things easy to review I only included the changes for the thread list; however, in subsequent diffs I will bring back the message list and house it in this same panel component

Linear task: https://linear.app/comm/issue/ENG-5935/chat-thread-list-ui-redesign

Depends on D10527

Test Plan

Please see a the demo video below:

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

ginsu created this revision.Jan 4 2024, 3:40 AM

Herald added subscribers: tomek, ashoat. · View Herald TranscriptJan 4 2024, 3:40 AM

ginsu added inline comments.Jan 4 2024, 3:45 AM

web/chat/chat.css
1–6	Confirmed that this style was not being used anywhere
web/chat/chat.react.js
43	We move this here so both `ChatTabs` and `chatList` have context on the the thread list provider

ginsu edited the summary of this revision. (Show Details)Jan 4 2024, 3:46 AM

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

ginsu added reviewers: atul, inka.

ginsu added a parent revision: D10527: [web] update the chat tabs ui.

ginsu attached a referenced file: F1026654: Screen Recording 2024-01-04 at 6.42.18 AM.mov. (Show Details)

ginsu edited the summary of this revision. (Show Details)

Harbormaster completed remote builds in B25480: Diff 35194.Jan 4 2024, 3:58 AM

ginsu requested review of this revision.Jan 4 2024, 3:58 AM

ginsu added a child revision: D10529: [web] update thread list ui.Jan 4 2024, 4:11 AM

atul accepted this revision.Jan 4 2024, 1:55 PM

atul added inline comments.

web/chat/chat.css
1–6	FWIW most IDEs can automatically detect unused CSS selectors. Might be heavy weight to add a CSS stylecheck to CI, but flipping setting on in your IDE might help catch these early?
web/chat/chat.react.js
42–45	Would definitely be good to memoize this

This revision is now accepted and ready to land.Jan 4 2024, 1:55 PM

atul mentioned this in D10547: [web] Remove extraneous CSS selectors.Jan 4 2024, 1:58 PM

ginsu added inline comments.Jan 4 2024, 1:59 PM

web/chat/chat.react.js
42–45	It looks like on line 49 we are memoizing this component, is it still necessary to wrap this in a `useMemo`?

atul added inline comments.Jan 4 2024, 1:59 PM

web/chat/chat.css
1–6	Took 2 seconds to run, so put up the following with some obvious selectors that can be removed: https://phab.comm.dev/D10547 There are definitely more, but there were string matches in the codebase so I just unstaged those to keep this as quick as possible.