Page MenuHomePhabricator
Differential D10922

[web] introduce member list sidebar
ClosedPublic
Actions

Authored by ginsu on Feb 1 2024, 3:46 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Jun 30, 5:26 PM
Unknown Object (File)
Sun, Jun 30, 9:26 AM
Unknown Object (File)
Sat, Jun 29, 1:11 PM
Unknown Object (File)
Mon, Jun 24, 9:26 AM
Unknown Object (File)
Sun, Jun 23, 11:24 PM
Unknown Object (File)
Sun, Jun 23, 11:09 PM
Unknown Object (File)
Fri, Jun 21, 3:38 AM
Unknown Object (File)
Thu, Jun 20, 7:07 PM
View All 36 Files
Subscribers
ashoat
tomek

Details

Reviewers
atul
inka
Commits
rCOMM6931ec5b7d6e: [web] introduce member list sidebar
Summary

This diff introduces the member list sidebar component. Subsequent diffs will handle adding this component to the chat panel

Linear task: https://linear.app/comm/issue/ENG-5975/introduce-new-member-list-section

Depends on D10921

Test Plan

Please see the demo video below (the duplicate users here are just for testing purposes + not a part of this diff)

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

ginsu created this revision.Feb 1 2024, 3:46 PM
Herald added subscribers: tomek, ashoat. · View Herald TranscriptFeb 1 2024, 3:46 PM
ginsu edited the summary of this revision. (Show Details)Feb 1 2024, 3:52 PM
ginsu edited the test plan for this revision. (Show Details)
ginsu added reviewers: atul, inka.
ginsu added a parent revision: D10921: [web] introduce RoleGroupMemberList.
ginsu attached a referenced file: F1107150: Screen Recording 2024-02-01 at 6.49.51 PM.mov. (Show Details)
Harbormaster completed remote builds in B26442: Diff 36560.Feb 1 2024, 4:05 PM
ginsu requested review of this revision.Feb 1 2024, 4:05 PM
ginsu added a child revision: D10924: [web] update ThreadMember component to match new UI redesign.Feb 1 2024, 4:49 PM
ashoat added inline comments.Feb 2 2024, 7:26 AM
web/chat/member-list-sidebar/member-list-sidebar.react.js
26 ↗(On Diff #36560)

Should this be behind a memo like in D10921?

atul accepted this revision.Feb 5 2024, 11:07 AM
atul added inline comments.
web/chat/member-list-sidebar/member-list-sidebar.react.js
26 ↗(On Diff #36560)

+1, would be good to memoize groupedMemberList as well as the return value of MembersListSidebar

31 ↗(On Diff #36560)

IMO think it would make sense to push this "down" to the SortedMemberListGroup component as a groupLabel prop. Seems a bit weird that the group label is "floating" outside in the MembersListSidebar, feels like they should be coupled.

In the future would also make it easier to enable collapsing/expanding of groups, etc.

This revision is now accepted and ready to land.Feb 5 2024, 11:07 AM
ginsu updated this revision to Diff 36758.Feb 6 2024, 4:39 PM

address comments

Harbormaster completed remote builds in B26601: Diff 36758.Feb 6 2024, 4:54 PM
ginsu updated this revision to Diff 36821.Feb 7 2024, 4:41 PM

rebase before landing

This revision was landed with ongoing or failed builds.Feb 7 2024, 4:42 PM
Closed by commit rCOMM6931ec5b7d6e: [web] introduce member list sidebar (authored by ginsu). · Explain Why
This revision was automatically updated to reflect the committed changes.
ginsu added a commit: rCOMM6931ec5b7d6e: [web] introduce member list sidebar.
Harbormaster completed remote builds in B26643: Diff 36821.Feb 7 2024, 4:57 PM

Revision Contents
Changeset List

PathSize
web/
chat/
member-list-sidebar/
8 lines
52 lines

Diff 36824

View Options

web/chat/member-list-sidebar/member-list-sidebar.css

Loading...
View Options

web/chat/member-list-sidebar/member-list-sidebar.react.js

Loading...