-
Couldn't load subscription status.
- Fork 1.1k
Add assets search and enhance search commands #10789
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add assets search and enhance search commands #10789
Conversation
Signed-off-by: Lin Wang <[email protected]>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #10789 +/- ##
==========================================
+ Coverage 60.54% 60.55% +0.01%
==========================================
Files 4511 4513 +2
Lines 121359 121430 +71
Branches 20266 20279 +13
==========================================
+ Hits 73478 73536 +58
- Misses 42735 42745 +10
- Partials 5146 5149 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Lin Wang <[email protected]>
Signed-off-by: Lin Wang <[email protected]>
Signed-off-by: Lin Wang <[email protected]>
Signed-off-by: Lin Wang <[email protected]>
|
|
||
| export interface GlobalSearchServiceSetupContract { | ||
| registerSearchCommand(searchCommand: GlobalSearchCommand): void; | ||
| registerSearchSubmitCommand(searchResultCommand: GlobalSearchSubmitCommand): void; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's the difference between registerSearchCommand and registerSearchSubmitCommand? would be nice to add comment to each of them
| public setup(): GlobalSearchServiceSetupContract { | ||
| return { | ||
| registerSearchCommand: this.registerSearchCommand.bind(this), | ||
| registerSearchSubmitCommand: this.registerSearchSubmitCommand, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't seem to see where is this function called registerSearchSubmitCommand, did I miss anything?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For now the only search submit command is in the dashboards-assistant repo. Will raise a new PR for adding an example for this new methods.
|
Can we specify the type of assets when searching? Right now it seems to search all types of saved object, can we specify type in the query? for example: |
Signed-off-by: Lin Wang <[email protected]>
| /** | ||
| * @experimental | ||
| */ | ||
| export interface GlobalSearchSubmitCommand { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed to use an unified Command interface, instead of introducing a new GlobalSearchSubmitCommand, it should extend the existing interface GlobalSearchCommand
| * @param callback callback function when search is done | ||
| */ | ||
| run(value: string, callback?: () => void): Promise<ReactNode[]>; | ||
| run(value: string, callback?: () => void, abortSignal?: AbortSignal): Promise<ReactNode[]>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we update the function interface for better extensibility? in case there will be more context we will need to pass in the future
| run(value: string, callback?: () => void, abortSignal?: AbortSignal): Promise<ReactNode[]>; | |
| run(value: string, options: {callback?: () => void, abortSignal?: AbortSignal}): Promise<ReactNode[]>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good suggestion. I'm prefer to keep value: string and callback?: () => void as the old implementation. Since I don't want to change the navigation search implementation. The new abortSignal can be wrapped with options. What do you think about it?
Signed-off-by: Lin Wang <[email protected]>
Signed-off-by: Lin Wang <[email protected]>
I think it can be a feature support in the future. Store a mapping for assets types and register more alias. We can reuse the assets search logic. We need to figure out the alias conflicts due to |
Signed-off-by: Lin Wang <[email protected]>
Signed-off-by: Lin Wang <[email protected]>
Signed-off-by: Lin Wang <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved with non-blocking comment
| ongoingAbortControllersRef.current.push({ controller: abortController, query: value }); | ||
| if (enterKeyDownRef.current) { | ||
| globalSearchCommands | ||
| .filter((item) => !!item.action) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: this filter seem unnecessary, because it checks command.action before calling it in forEach
| const commandsWithoutActions = globalSearchCommands.filter( | ||
| (command) => command.type !== 'ACTIONS' | ||
| ); | ||
| const filteredCommands = commandsWithoutActions.filter((command) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you leave a comment here to explain why having filteredCommands? It seem it is sorted as [alias/search command, action commands]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. In the current implementation, only alias command will be execute if user start with alias. Add filter here to make sure all action commands can be executed and rendered in the end. Let me added in the next PR.
| const sections = Object.entries(searchResults).map(([key, items]) => { | ||
| const sectionHeader = SearchCommandTypes[key as SearchCommandKeyTypes].description; | ||
| return resultSection(items, sectionHeader); | ||
| return resultSection(items, key !== 'ACTIONS' ? sectionHeader : undefined); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would suggest not have this special logic key !== 'ACTIONS', instead we could have a extra displayName property to SearchCommandTypes which can be set to empty
| defaultMessage: 'Search the menu', | ||
| })} | ||
| placeholder={ | ||
| globalSearchCommands.find((item) => item.inputPlaceholder)?.inputPlaceholder ?? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes it to only display the first command that has inputPlaceholder, I would suggest to make the placeholder to be computed from all different commands.
| */ | ||
| export class GlobalSearchService { | ||
| private searchCommands = [] as GlobalSearchCommand[]; | ||
| private searchCommands$ = new BehaviorSubject<GlobalSearchCommand[]>([]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: why did we change it observable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to register some search commands in the start phase. I've expose a registerSearchCommands method in the start contract.
|
Checked failed CI not related to this PR. Should be safe to merge. |
Description
This PR enhances the global search functionality by adding two new capabilities:
Assets Search Command: Enables searching for dashboards and visualizations directly from the global search bar. Users can now quickly find and navigate to saved objects without leaving their current context.
Enhanced Command Actions: Extended the
GlobalSearchCommandinterface to support action-based commands that execute when users press Enter in the search bar, providing a more intuitive way to perform search-related actions.Key changes include:
searchAssetsfunction to search for dashboards and visualizations via the saved objects APIGlobalSearchCommandinterface withactionproperty to support Enter-key triggered actionsIssues
#10741
Screenshot
Changelog
Check List
yarn test:jestyarn test:jest_integration