Details

Reviewers

tomek
• benschac
atul
ashoat

Commits

rCOMM3dc3f65cceb8: [web] Introduce `AddMembersList` component

Summary

Introduce component grouping users and rendering list based on search results in AddMembersModal.
I used common logic with native app, where only users from parent thread have "notice" set to "undefined". It's the reason, why after grouping the title: "Users in parent thread" is added manually.

Screenshot_Google Chrome_2022-04-15_134858.png (803×457 px, 37 KB)

Test Plan

The list can be tested after introducing AddMembersModal

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

• jacek created this revision.Apr 15 2022, 4:28 AM

Herald added subscribers: • abosh, • benschac, • karol and 4 others. · View Herald TranscriptApr 15 2022, 4:28 AM

• jacek edited the summary of this revision. (Show Details)Apr 15 2022, 4:50 AM

• jacek added reviewers: tomek, • benschac, atul.

• jacek added a parent revision: D3740: [web] Introduce `AddMemberGroup` component.

• jacek added a child revision: D3742: [web] Introduce `AddMembersModal` component.Apr 15 2022, 4:57 AM

Harbormaster completed remote builds in B8227: Diff 11511.Apr 15 2022, 8:23 AM

• jacek requested review of this revision.Apr 15 2022, 8:23 AM

tomek requested changes to this revision.Apr 15 2022, 9:22 AM

tomek added inline comments.

web/modals/threads/members/add-members-list.react.js
20–21 ↗	(On Diff #11511)	We don't memoize these so all the following memoizations will recompute after each render.
37–45 ↗	(On Diff #11511)	I think this code might become more readable when: We prepare all the data before this memo e.g. _toPairs(groupedAvailableUsersList) .filter(group => group[0] !== 'undefined') .sort((a, b) => a[0].localeCompare(b[0])) We store simple values instead of creating arrays just to destructure them (see suggested edit)

This revision now requires changes to proceed.Apr 15 2022, 9:22 AM

Add missing useMemo and make the component more readable

Harbormaster completed remote builds in B8439: Diff 11778.Apr 22 2022, 3:47 AM

Adding @ashoat as this diff affects the copy presented to users.

web/modals/threads/members/add-members-list.react.js
13 ↗	(On Diff #11778)
21 ↗	(On Diff #11778)	Shouldn't we use `alertText`?
32 ↗	(On Diff #11778)	Just wondering, maybe we can change the logic a bit and replace `undefined` by something defined, so that it's more maintainable?