Page MenuHomePhabricator

[web] Display list of pending users to add in `AddMembersModal`
ClosedPublic

Authored by jacek on Apr 29 2022, 5:01 AM.
Tags
None
Referenced Files
F3357538: D3876.diff
Sun, Nov 24, 12:09 AM
Unknown Object (File)
Mon, Nov 18, 3:08 PM
Unknown Object (File)
Mon, Nov 18, 10:15 AM
Unknown Object (File)
Wed, Nov 13, 10:47 AM
Unknown Object (File)
Sun, Nov 10, 12:39 AM
Unknown Object (File)
Fri, Nov 8, 8:21 PM
Unknown Object (File)
Fri, Nov 8, 8:21 PM
Unknown Object (File)
Fri, Nov 8, 8:21 PM

Details

Summary

Introduce list of pending users to add below search bar in AddMembersModal.
The diff also excludes pending users from main list in Modal, so they don't appear twice.

Test Plan

Open "Members Modal", try to add users from list, pending users appears below search bar.

Diff Detail

Repository
rCOMM Comm
Branch
jacek/pending-users-to-add
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

tomek added a reviewer: ashoat.

This looks amazing! Adding @ashoat so he can take a look.

web/modals/threads/members/add-members-modal.react.js
117

In some places we use Array.from(pendingUsersToAdd) and here we use spread operator. It doesn't matter that much, but it might be a nice idea to be consistent

130

Have you considered using a callback here?

web/modals/threads/members/add-members-modal.react.js
117

You're right. In many other places, I used Array.from when converting set to an array. I'll replace it to be consistent

130

Yes, I was considering it, but I concluded it's not necessary here, as it's all inside the useMemo.

Looks great!!

web/modals/threads/members/add-members-modal.react.js
117

I personally like the spread operator but it's all okay

130

@def-au1t's reasoning makes sense to me

This revision is now accepted and ready to land.Apr 30 2022, 4:18 PM

replace spread with Array.from & rebase before landing