-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[GS] Allows apps to register searchable keywords for Global Search #85686
Conversation
@joshdover @pgayvallet This is still very much work in progress. I wanted to open a draft early so that we can discuss what we still need for this feature and what we decide to strip out (e.g. removing keywords from application deep links) |
x-pack/plugins/global_search_providers/public/providers/get_app_results.ts
Show resolved
Hide resolved
x-pack/plugins/global_search_providers/public/providers/get_app_results.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/global_search_providers/public/providers/get_app_results.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/global_search_providers/public/providers/get_app_results.ts
Outdated
Show resolved
Hide resolved
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 looks fantastic. I do think we should move the searchDeepLinks
property into the meta
object so that these two concepts are closer to one another in the API. We don't have to do that in this PR though if you'd prefer a follow up for that.
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.
Looking great for now!
+1 for josh's proposal to move searchDeepLinks
into the new meta
property
x-pack/plugins/global_search_providers/public/providers/get_app_results.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/global_search_providers/public/providers/get_app_results.ts
Show resolved
Hide resolved
x-pack/plugins/global_search_providers/public/providers/get_app_results.ts
Outdated
Show resolved
Hide resolved
Pinging @elastic/kibana-core (Team:Core) |
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.
A few nits, but overall the implementation is great.
LGTM
src/core/public/application/types.ts
Outdated
* | ||
* @remarks | ||
* Used to populate navigational search results (where available). | ||
* Can be updated using the {@link App.updater$} observable. See {@link AppSubLink} for more details. | ||
* | ||
* @example |
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: was already present before this PR, but the link to AppSubLink
is broken. I guess it should point to PublicAppSearchDeepLinkInfo
?
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'll fix it while waiting for the @elastic/kibana-app team's review.
x-pack/plugins/global_search_providers/public/providers/get_app_results.test.ts
Outdated
Show resolved
Hide resolved
const scores = keywords.map((keyword) => { | ||
return scoreAppByTerms(term, keyword); |
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.
optional: we could break/return earlier if a keyword's score is 100.
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.
We can add that as an optional optimization strategy if/when plugin owners start adding their keywords. Right now, I don't think it's needed.
x-pack/plugins/global_search_providers/public/providers/get_app_results.ts
Outdated
Show resolved
Hide resolved
💚 Build SucceededMetrics [docs]Distributable file count
Page load bundle
History
To update your PR or re-run it, just comment with: |
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.
KibanaApp code changes LGTM, didn't test locally
Resolves #76778
Summary
Allows applications to register keywords as search terms for Global Search.
Since keywords are not data, they are have been added to the
App
type as ameta
property. This PR movessearchDeepLinks
into the newmeta
property for the same reason (they are not data).Scoring by keywords implements the same algorithm as searching for applications by title and weights the scores in favor of matching by title (keywords score * 0.8). The weighting factor is an arbitrary constant.
This PR makes no assumptions on what keywords to assign to apps, rather leaving it up to application developers and solutions to choose their own.
How to test this
The unit tests for
get_app_info
, e.g. https://github.com/elastic/kibana/pull/85686/files#diff-5d4a779847b1ac5095589e30e25778e35a3350c09b4c6d9bdfcca0b22bc820fbR122 show how the keywords are added to an app and the unit tests forget_app_results
, e.g. https://github.com/elastic/kibana/pull/85686/files#diff-3656691fb1cb333c33264ea83656f59878c49b13d7c3cdda8399ddbcc4e7d03eR113 show how scoring by keywords works.To test this 'live', you'll need to add keywords or add a new plugin that has keywords and, optionally, deep links (see https://github.com/elastic/kibana/pull/85686/files#diff-efac386db0738f497e14a9772c5da15dde935e474ee2dc3b62fe5fca2e0e92d8R247 as an example), after pulling the code.
Example
Plugin: Uptime
Modified registration:
Run the code and search for 'awake'. This should show a direct link to 'Uptime'
Plugin: APM
Modified registration:
Searching for 'performance' or 'application monitoring' or any shorter version of these keywords will show a direct link to the 'APM' app:
Keywords in deep links work similarly.
Release Notes
Kibana's navigation search now implements keyword registration for applications, allowing you to search for applications by keywords as well as by title.
Checklist
Delete any items that are not applicable to this PR.
For maintainers