Skip to content

Connect: Add useSearch hook#23035

Closed
ravicious wants to merge 6 commits intomasterfrom
ravicious/search-resources
Closed

Connect: Add useSearch hook#23035
ravicious wants to merge 6 commits intomasterfrom
ravicious/search-resources

Conversation

@ravicious
Copy link
Copy Markdown
Member

gravitational/teleport.e#991 needs to be merged first, otherwise merging this PR is going to break type check in e.

This PR adds a new hook which we can use for the search bar.

Before implementing the hook I had to make a couple of small changes related to how we provide the sort param to ResourcesService.

If you want to check out the hook in action, take a look at ravicious/spotlight-prototype.

@ravicious ravicious requested a review from gzdunek March 14, 2023 12:03
@ravicious ravicious force-pushed the ravicious/search-resources branch from 803064f to 6838a9e Compare March 14, 2023 12:14
The performance of the underlying ListResources RPC changes depending on
whether the sort field is provided or not. What follows from that is:

* A low-level layer such as tshd client should not implement business logic
  by providing a default sort.
* Since the sort field is so important, the callsite should be required
  to explicitly state it to demonstrate that it took into account
  the performance implications.
* It should be possible to provide no sort.
@ravicious ravicious force-pushed the ravicious/search-resources branch from 6838a9e to 953fef4 Compare March 14, 2023 12:27
Copy link
Copy Markdown
Contributor

@gzdunek gzdunek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

resourcesService.searchResources(cluster.uri, search)
);

return (await Promise.all(searchPromises)).flat();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice

}

/**
* searchResources searches for the given list of space-separated keywords across all resource
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* searchResources searches for the given list of space-separated keywords across all resource
* `searchResources` searches for the given list of space-separated keywords across all resource

@ravicious
Copy link
Copy Markdown
Member Author

Closing in favor of #23980.

@ravicious ravicious closed this Apr 3, 2023
@ravicious ravicious deleted the ravicious/search-resources branch April 6, 2023 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants