Page MenuHomePhabricator
Differential D4269

[web] Merge store and server search results
ClosedPublic
Actions

Authored by tomek on Jun 14 2022, 12:37 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Jun 28, 10:59 AM
Unknown Object (File)
Fri, Jun 28, 10:55 AM
Unknown Object (File)
Thu, Jun 27, 11:51 PM
Unknown Object (File)
Wed, Jun 26, 9:58 PM
Unknown Object (File)
Wed, Jun 26, 9:33 AM
Unknown Object (File)
Wed, Jun 26, 7:05 AM
Unknown Object (File)
Sat, Jun 22, 4:04 AM
Unknown Object (File)
Mon, Jun 17, 11:40 PM
View All 98 Files
Subscribers
• abosh
• adrian
ashoat

Details

Reviewers
• jacek
atul
• benschac
Commits
rCOMM87b2cca30c62: [web] Merge store and server search results
Summary

Merge the results from both sources and store them in a memo.

Depends on D4268

Test Plan

Render the component and check if merged result contains only the users matched by the search text.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

tomek created this revision.Jun 14 2022, 12:37 PM
Herald added subscribers: • abosh, • adrian, ashoat. · View Herald TranscriptJun 14 2022, 12:37 PM
Harbormaster completed remote builds in B9758: Diff 13508.Jun 14 2022, 12:44 PM
tomek requested review of this revision.Jun 14 2022, 12:44 PM
atul accepted this revision.Jun 17 2022, 1:30 PM
atul added inline comments.
web/settings/relationship/add-users-list.react.js
44 ↗(On Diff #13508)

Could maybe use isSet defined in lib/shared/core.js? It's essentially an alias for blah !== null && blah !=undefined

54 ↗(On Diff #13508)

And could then change this to isSet(searchTextPresent)

This revision is now accepted and ready to land.Jun 17 2022, 1:30 PM
tomek added inline comments.Jun 21 2022, 5:00 AM
web/settings/relationship/add-users-list.react.js
44 ↗(On Diff #13508)

This won't work because searchText is always non-null. What we should do instead is to check if it's empty.

54 ↗(On Diff #13508)

The intention was for this memo to depend on a boolean flag instead of the text, so that changing the text wouldn't cause recomputation.

tomek updated this revision to Diff 13618.Jun 21 2022, 5:02 AM

Check text length

Harbormaster completed remote builds in B9843: Diff 13618.Jun 21 2022, 5:09 AM
tomek added a child revision: D4305: [web] Prepare a list of users that should be displayed.Jun 21 2022, 5:14 AM
Closed by commit rCOMM87b2cca30c62: [web] Merge store and server search results (authored by tomek). · Explain WhyJun 22 2022, 9:16 AM
This revision was automatically updated to reflect the committed changes.
tomek added a commit: rCOMM87b2cca30c62: [web] Merge store and server search results.

Revision Contents
Changeset List

PathSize
web/
settings/
relationship/
30 lines

Diff 13669

View Options

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

Loading...