Page MenuHomePhabricator

[web] Search for the users on server
ClosedPublic

Authored by tomek on Jun 14 2022, 12:34 PM.
Tags
None
Referenced Files
F3488032: D4268.diff
Wed, Dec 18, 8:48 AM
Unknown Object (File)
Sun, Dec 1, 10:16 PM
Unknown Object (File)
Sun, Nov 24, 3:40 AM
Unknown Object (File)
Nov 14 2024, 1:50 PM
Unknown Object (File)
Nov 9 2024, 7:59 AM
Unknown Object (File)
Oct 14 2024, 12:46 AM
Unknown Object (File)
Oct 14 2024, 12:46 AM
Unknown Object (File)
Oct 14 2024, 12:46 AM

Details

Summary

Another source of the users is our server - there might be some users that aren't in the store, but it should be possible to friend / block them.

Depends on D4267

Test Plan

Render the component and check if all the users that match search text were returned from the server.

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

Considering this network request may take some time to complete, we should probably introduce a spinner or something here to indicate to the user that "something is happening."

Guessing that feedback will probably be more relevant later in the stack once you're working on the views though.

web/settings/relationship/add-users-list.react.js
22

If we move the const userStoreSearchIndex = ... line above this one, can we replace new Set() with:

new Set(userStoreSearchIndex.getSearchResults(searchText)

?

This revision is now accepted and ready to land.Jun 17 2022, 1:24 PM
In D4268#120826, @atul wrote:

Considering this network request may take some time to complete, we should probably introduce a spinner or something here to indicate to the user that "something is happening."

Guessing that feedback will probably be more relevant later in the stack once you're working on the views though.

We can do that but need to find a good place for it. We don't want to stop users from interacting with the component because we're fetching some data. Also, there will be some users displayed (from the store), so adding a spinner is not that beneficial... what do you think?

web/settings/relationship/add-users-list.react.js
22

Ok, that makes sense!

web/settings/relationship/add-users-list.react.js
22

But this code was introduced in D4267 so I will change it there.

This revision was automatically updated to reflect the committed changes.