Skip to content
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

[Lens] Add search provider for global search #77448

Merged
merged 5 commits into from
Sep 23, 2020

Conversation

flash1293
Copy link
Contributor

@flash1293 flash1293 commented Sep 15, 2020

Closes #77218

This PR adds a Lens entry to the new global search (ignore yellow background, that's a custom stylesheet):
Screenshot 2020-09-15 at 10 44 17

This is currently necessary because all apps not shown in the nav bar are not considered by the default application provider. This PR uses the same matching logic as the application provider to add a Lens entry (https://github.com/elastic/kibana/blob/c844187ee98c626c2cf1520d43ff889068d80ed9/x-pack/plugins/global_search_providers/public/providers/application.ts)

#72680 aims to add more entries to the search by considering "features" of individual apps - once that's implemented, the workaround introduced by this PR can be removed.

Considerations

We could use the Lens visualization icon as well, but I didn't do so yet because all other "Kibana" items are using the Kibana logo (ignore yellow background, that's a custom stylesheet):
Screenshot 2020-09-15 at 10 43 51

@flash1293 flash1293 added v8.0.0 Feature:Lens v7.10.0 release_note:skip Skip the PR/issue when compiling release notes labels Sep 15, 2020
@flash1293 flash1293 marked this pull request as ready for review September 15, 2020 11:39
@flash1293 flash1293 requested a review from a team September 15, 2020 11:39
@flash1293 flash1293 added REASSIGN from Team:Core UI Deprecated label for old Core UI team Team:Visualizations Visualization editors, elastic-charts and infrastructure labels Sep 15, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app (Team:KibanaApp)

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-core-ui (Team:Core UI)

@flash1293
Copy link
Contributor Author

cc @AlonaNadler

@ryankeairns
Copy link
Contributor

As Joe notes, all apps are currently using their parent solution icon. Further, saved objects are not using any icons. This was by design as we (organizationally) are moving away from the product-level logos/icons. That is not to say there aren't contrary positions, but that should likely be discussed at a broader level for the sake of consistency.

All that to say, I am a +1 for using the Kibana (solution) logo at this time.

Copy link
Contributor

@mbondyra mbondyra left a comment

Choose a reason for hiding this comment

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

Tested on FF, works good!

@myasonik
Copy link
Contributor

Congrats on being the first custom provider!!! 🎉

@pgayvallet could you review to make sure the provider stuff is as you expected it to be used?

@AlonaNadler
Copy link

This is awesome @flash1293 !
Few questions:

  • can we add a description next to Lens build charts and graphs using drag and drop?
  • can we show Lens in the suggestions when users are searching for visualize, chart?

@alexfrancoeur
Copy link

Well that was fast! Exciting to see a new provider in the mix already. As mentioned in the description, ideally we merge this into the future feature results provider (on deck).

"Lens Visualizations" sounds to me like it will show me a filtered list of visualizations created from Lens. I understand why we want visualizations in the title, but am wondering if this is the right approach. If I want to create a visualization, will the majority of users search for Lens? If I search for "visualization", would I choose Lens over Visualize? I've been meaning to open an issue to track "actions" from search. You can imagine "Create new case", "Create new alert", etc. I wonder if this is a good first candidate. "Create new visualization" or "Create new visualization from Lens" could be a good fit here if we want to start leading with Lens. I realize it's not the default experience today, but it is an opportunity to start driving more traffic to Lens with a specific task in mind.

We'll also need to make sure we have the appropriate metadata for Lens (#77114). Would the subtitle just be Kibana here? Feels like that's the right approach, but just making sure we start thinking this way with the introduction of a new provider.

@ryankeairns
Copy link
Contributor

...I've been meaning to open an issue to track "actions" from search. You can imagine "Create new case", "Create new alert", etc. I wonder if this is a good first candidate. "Create new visualization" or "Create new visualization from Lens" could be a good fit here if we want to start leading with Lens. I realize it's not the default experience today, but it is an opportunity to start driving more traffic to Lens with a specific task in mind.

I believe this issue covers the "actions" feature: #15019

We'll also need to make sure we have the appropriate metadata for Lens (#77114). Would the subtitle just be Kibana here? Feels like that's the right approach, but just making sure we start thinking this way with the introduction of a new provider.

For the metadata (i.e. subtitle text), we're heading toward using the category value which, for Lens, should probably be Kibana, as suggested.


can we add a description next to Lens build charts and graphs using drag and drop?

I'm hesitant to start putting promotional text in there and would like for us to have a separate discussion about what should/should not go into that metadata field.

can we show Lens in the suggestions when users are searching for visualize, chart?

I'll create a separate issue for this request as it's something the search feature should provide as we would want to manage keyword ownership in a centralized location.

@flash1293
Copy link
Contributor Author

can we add a description next to Lens build charts and graphs using drag and drop?

I'm not sure, can we @myasonik ?

can we show Lens in the suggestions when users are searching for visualize, chart

Yes, but I'm not sure whether it's confusing if the search term is not showing up in the result. @alexfrancoeur not sure how this was expected to work.

"Create new visualization" or "Create new visualization from Lens" could be a good fit here if we want to start leading with Lens

This makes sense to me, it's also how the Visual Studio Code command palette works:
Screenshot 2020-09-15 at 17 49 20

The action would also show up when typing "Create new"

@ryankeairns
Copy link
Contributor

@flash1293 see my reply that came in just seconds before yours :)

My personal preference would be to progress this PR as-is and take the other requests to a separate discussion as they have global implications.

@flash1293
Copy link
Contributor Author

@ryankeairns we had a little bit of a race condition here.

I'm hesitant to start putting promotional text in there and would like for us to have a separate discussion about what should/should not go into that metadata field.

OK, I will exclude that for now

can we show Lens in the suggestions when users are searching for visualize, chart?

I'll create a separate issue for this request as it's something the search feature should provide as we would want to manage keyword ownership in a centralized location.

It makes sense to create a separate issue to implement that for the general case, but for Lens specifically we could do it right now. The question is whether we should because it's not consistent with the other items

@ryankeairns
Copy link
Contributor

can we show Lens in the suggestions when users are searching for visualize, chart?

I'll create a separate issue for this request as it's something the search feature should provide as we would want to manage keyword ownership in a centralized location.

It makes sense to create a separate issue to implement that for the general case, but for Lens specifically we could do it right now. The question is whether we should because it's not consistent with the other items

I have serious reservations about doing this in this PR. Without it being centrally managed, it seems potential issues could develop if/when authors are potentially competing for the same or simliar keywords.

@joshdover
Copy link
Contributor

I have serious reservations about doing this in this PR. Without it being centrally managed, it seems potential issues could develop if/when authors are potentially competing for the same or simliar keywords.

I share similar concerns, most from a UX perpsective. I think we want to be careful to make the search results pretty consistent between versions since users are going to build muscle memory and usage habits based on these results. So if we're going to introduce new concepts like actions or reserve keywords for specific items, we should be fairly confident that these aren't going to change significantly with each release. Fine-tuning over time is to be expected, but I don't think we want to drastically change the interactions or ordering of results on a regular basis.

All that to say that if we're going to add additional keywords for this Lens result to match on, we should be confident that this fits into the longer-term vision of how we want Global Search to respond to user input over the next few releases and that anything we match on today isn't going to be removed in the near-term.

@flash1293
Copy link
Contributor Author

Got it - to summarize: We are moving out the "Actions" entries in the search, subtitles and magic matching words.

The only open question AFAICT is whether it should say "Lens Visualizations" or just "Lens". Any preference here @ryankeairns @alexfrancoeur @AlonaNadler ?

@AlonaNadler
Copy link

Lens Visualizations in this point since not everyone knows what Lens is

@AlonaNadler
Copy link

@ryankeairns regarding

I'm hesitant to start putting promotional text in there and would like for us to have a separate discussion about what should/should not go into that metadata field.

Its not promotional it's descriptive, many don't know what is Lens

@ryankeairns
Copy link
Contributor

ryankeairns commented Sep 17, 2020

@ryankeairns regarding

I'm hesitant to start putting promotional text in there and would like for us to have a separate discussion about what should/should not go into that metadata field.

Its not promotional it's descriptive, many don't know what is Lens

Understood, my main point is that we're currently pulling either the category (on apps) or type (on saved objects) from the existing plugin registry setup. Based upon my current understanding, there would be additional work to provide a custom description via the GS API and this also sets a new precedent for using longer descriptive text. Given those considerations, a separate issue to discuss further the approach seems an appropriate next step.

@alexfrancoeur
Copy link

At the end of the day, we want any search related to visualizing data to be routed to Lens (chart, graph, visualize, etc.). Given that some of the earlier proposed ways require a bit more planning (#77810, #77508, #15019) it feels like we should focus on how best to word this feature entry. This is a good exercise, because we may need to address at a broader scale in the next release with #72680.

Let's spend more time kicking the tires on text. I think we all agree that Lens is not descriptive enough. I'll throw some suggestions out there, but am hoping that @gchaps might be able to help as well.

It's worth noting that for 7.10, whatever result we end up choosing with the keyword visualize will also return this result. So we may not want to stray too far away.

image

  • Visualize: Lens - ugly, but straight to the point. This approach could eventually morph into however we end up presenting features in search
  • Create visualization with Lens - this was my original suggestion, but would be the only task oriented result. Pushed until we address Create a command palette with hotkeys for Kibana #15019.
  • Visualize with Lens - this also feels task oriented
  • Visualize your data in Lens - long, but similarly a task

Alternatively, we could wait until the next release and introduce the Lens results consistently with the other features and augment with searchable metadata (#77810). I only suggest this, because I'd like to avoid having too different of experiences between the next two minors. The benefits of introducing Lens in search probably outweigh the need for consistency here. If we can future proof as much as possible, it probably won't be a problem.

@flash1293
Copy link
Contributor Author

I think this is great input, thanks. “Visualize: Lens” is my preference at the moment.

Maybe another option: For 7.10 - do we even need to explain what Lens is in the global search? For apps it feels more of a command palette than something a user would actually “search” for, so it could be just a shortcut to get there quickly for those who know about it already. Given the high number of apps, it’s already pretty likely a user won’t be able to make sense out of all of the entries shown when opening the search. In that case we could go with just “Lens” until we have a unified “Actions” concept.

@flash1293
Copy link
Contributor Author

If you want to get a feel for it - I changed this PR to implement "Visualize: Lens" (also fixed a bug with missing permissions)
Screenshot 2020-09-21 at 13 48 47

@gchaps
Copy link
Contributor

gchaps commented Sep 21, 2020

@alexfrancoeur, @flash1293 Here are some additional suggestions:

  • Visualize--Lens - might look better with an em dash than a colon (:)

  • Lens: drag, drop, visualize - too promotional? Could also be Lens: create visualizations

  • Lens visualizations - puts Lens first

  • Visualizations with Lens - similar to Visualize with Lens, but not task-oriented

@AlonaNadler
Copy link

I think Lens: create visualizations is ideal
If possible lets put that first.
We are deemphasizing visualize. So any name that highlight visualize explicitly will be there only in the short term

Comment on lines 46 to 55
// maximum lev distance is length, we compute the match ratio (lower distance is better)
const ratio = Math.floor((1 - distance / length) * 100);
if (ratio >= 60) {
score = ratio;
}
}
if (score === 0) return EMPTY;
return from(
uiCapabilities.then(({ navLinks: { visualize: visualizeNavLink } }) =>
visualizeNavLink
Copy link
Contributor

Choose a reason for hiding this comment

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

NIT: the search logic is performed even when the navLinks.visualize capability is false. This could be avoided.

@flash1293
Copy link
Contributor Author

Fixed the nit from Pierre and switched to Lens: create visualizations

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

@kbn/optimizer bundle module count

id value diff baseline
lens 449 +2 447

page load bundle size

id value diff baseline
lens 1.1MB +2.8KB 1.1MB

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@alexfrancoeur
Copy link

I don't want to block additional discovery of lens if the team feels it's necessary to support in 7.10 vs 7.11 (both targeted). So I'm +1 on this text if the @gchaps @AlonaNadler and @ryankeairns agree.

However, if we go with Lens: create visualizations, we'll need to think through how this looks in the future. With a feature results provider up next (#72680), if we don't treat lens differently than other features, we'll end up with the following scenarios.

If lens is the input, we'd return the below results - both leading to the same view.

  • Lens
  • Lens: create visualizations

Very similar to how visualiz will work with this PR, but with different endpoints

  • Lens: create visualizations
  • Visualize

We'll likely have to live with these duplicate results for awhile unless we explicitly exclude lens, which isn't impossible, just an additional requirement. With that approach we likely cannot remove the lens specific registry until we have additional metadata (#77810) with the feature registry or commands (#15019). @ryankeairns and I can discuss priorities and scope.

One open question, how is the lens search registry prioritized over others? What's the hierarchy in the results?

@flash1293
Copy link
Contributor Author

One open question, how is the lens search registry prioritized over others? What's the hierarchy in the results?

The new search provider is using the same scoring algorithm than default application provider, not sure what the global search is doing on perfectly matching scores from different providers.

@ryankeairns
Copy link
Contributor

ryankeairns commented Sep 23, 2020

It seems we have a shared understanding that this particular change will very likely need to be adjusted/replaced in the near-ish future. Given that, I don't have any reservations about merging this as-is, especially due to the importance of promoting Lens.

@AlonaNadler
Copy link

We have a Lens first approach we need to have it in the global search, we can improve it in the future furthur but I think it is worth having it 7.10

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Lens REASSIGN from Team:Core UI Deprecated label for old Core UI team release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.10.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[GS] [Discuss] Add Lens to navigational search results