Page MenuHomePhabricator

[web] Update MembersModal to call useUserSearchIndex
ClosedPublic

Authored by rohan on Dec 8 2023, 5:25 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mar 7 2024, 9:05 AM
Unknown Object (File)
Mar 7 2024, 7:48 AM
Unknown Object (File)
Mar 7 2024, 6:53 AM
Unknown Object (File)
Mar 7 2024, 5:56 AM
Unknown Object (File)
Mar 7 2024, 5:02 AM
Unknown Object (File)
Jan 17 2024, 4:14 AM
Unknown Object (File)
Jan 17 2024, 1:05 AM
Unknown Object (File)
Jan 16 2024, 12:23 AM
Subscribers

Details

Summary

There are several search experiences in the app that do not use get/usePotentialMemberItems. This means we need to update them individually, so I'm separating out diffs to do exactly this to make it easy for reviewers. [[ https://linear.app/comm/issue/ENG-6019/[after-landing]-try-to-update-remaining-search-experiences-to-use | ENG-6019 ]] tracks attempting to update these search experiences to use usePotentialMemberItems. Each diff will affect one file ideally.

This diff affects MembersModal.

This component is used when viewing channel members on web.

Addresses ENG-5434

Depends on D10231

Test Plan

Please see the video below where I view the channel members on web

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

rohan edited the test plan for this revision. (Show Details)
Harbormaster returned this revision to the author for changes because remote builds failed.Dec 8 2023, 6:10 AM
Harbormaster failed remote builds in B24886: Diff 34416!
rohan requested review of this revision.Dec 8 2023, 6:15 AM
web/modals/threads/members/members-modal.react.js
37–49 ↗(On Diff #34416)

There is some logic in this file that made sure that even though we were using userStoreSearchIndex, we weren't displaying users from outside of this thread. It can probably be simplified now.

web/modals/threads/members/members-modal.react.js
37–49 ↗(On Diff #34416)

Would you mind clarifying your original comment?

My understanding with the original logic is:

  1. If there is no search term (like just opening the modal), we display threadMembersNotFiltered
  2. If there is a search term, we only display the threadMembersNotFiltereds that are a match in the search index

We naturally prevented external users from being shown like you said by using threadMembersNotFiltered

This is the same now, except now our search index just doesn't include external users. Still though, we'd need to either (1) show all members if no search term is provided or (2) only show threadMembersNotFiltereds that are a match in the search index

Let me know if I'm misunderstanding though

inka added inline comments.
web/modals/threads/members/members-modal.react.js
37–49 ↗(On Diff #34416)

You are right, because userIDs are ids, not the whole UserInfos, we still need to do this, sorry

This revision is now accepted and ready to land.Dec 13 2023, 4:10 AM