Page MenuHomePhabricator

[native] Barebones user selection in `CommunityCreationMembers`
ClosedPublic

Authored by atul on Jun 1 2023, 1:13 PM.
Tags
None
Referenced Files
F3512189: D8056.diff
Sat, Dec 21, 6:36 PM
Unknown Object (File)
Fri, Dec 20, 9:34 AM
Unknown Object (File)
Thu, Dec 19, 7:14 AM
Unknown Object (File)
Thu, Dec 19, 5:13 AM
Unknown Object (File)
Thu, Dec 19, 4:18 AM
Unknown Object (File)
Oct 28 2024, 1:56 PM
Unknown Object (File)
Oct 26 2024, 4:38 PM
Unknown Object (File)
Oct 15 2024, 12:26 PM
Subscribers

Details

Summary

Extremely barebones user selection screen in CommunityCreationMembers using TagInput and UserList.

There's a warning about nested scroll views, the styling isn't correct, and we aren't differentiating between COMMUNITY_ROOT and COMMUNITY_ANNOUNCEMENT_ROOT. Going to handle each of those in followup diffs to keep changes easier to review.

Test Plan

Works as expected:

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

atul requested review of this revision.Jun 1 2023, 1:33 PM
ashoat requested changes to this revision.Jun 1 2023, 1:35 PM

Back to you with questions:

  1. Do we really need another low-level component for this? Don't we have something we can reuse? If not, can't we refactor something / make it generic, rather than implementing a new one of these?
  2. Why do we need to differentiate between COMMUNITY_ROOT and COMMUNITY_ANNOUNCEMENT_ROOT?
This revision now requires changes to proceed.Jun 1 2023, 1:35 PM

Do we really need another low-level component for this? Don't we have something we can reuse? If not, can't we refactor something / make it generic, rather than implementing a new one of these?

We're reusing TagInput and UserList which "do a lot," but agree it makes sense to have some general-purpose component with TagInput and UserList "contained" within. At the moment there is no such higher level component we can just drop in here. I can try to pull something generic out of CommunityCreationMembers and see if it can be used in other components where we have TagInput and UserList (ComposeSubchannel, AddUsersModal, etc) later in this stack.

Why do we need to differentiate between COMMUNITY_ROOT and COMMUNITY_ANNOUNCEMENT_ROOT?

The last argument to getPotentialMemberItems(...) is threadType. My understanding is there's no "need" to differentiate at the moment, but might as well pass the correct threadType to the function since we're able to?

In D8056#239034, @atul wrote:

I can try to pull something generic out of CommunityCreationMembers and see if it can be used in other components where we have TagInput and UserList (ComposeSubchannel, AddUsersModal, etc) later in this stack.

That would be great

The last argument to getPotentialMemberItems(...) is threadType. My understanding is there's no "need" to differentiate at the moment, but might as well pass the correct threadType to the function since we're able to?

Thanks for explaining!

This revision is now accepted and ready to land.Jun 1 2023, 1:59 PM
In D8056#239034, @atul wrote:

I can try to pull something generic out of CommunityCreationMembers and see if it can be used in other components where we have TagInput and UserList (ComposeSubchannel, AddUsersModal, etc) later in this stack.

That would be great

https://linear.app/comm/issue/ENG-4042/higher-level-component-that-wraps-taginput-and-userlist

This revision was landed with ongoing or failed builds.Jun 2 2023, 9:03 AM
This revision was automatically updated to reflect the committed changes.