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] Visualizations triggered by Discover should be handled by Lens #67415

Closed
AlonaNadler opened this issue May 26, 2020 · 8 comments · Fixed by #77873
Closed

[Lens] Visualizations triggered by Discover should be handled by Lens #67415

AlonaNadler opened this issue May 26, 2020 · 8 comments · Fixed by #77873
Assignees
Labels
enhancement New value added to drive a business result Feature:Lens Team:Visualizations Visualization editors, elastic-charts and infrastructure

Comments

@AlonaNadler
Copy link

In Discover top values there is a way to jump into a visualization that lands the users in Visuzlie editor.

image

For Lens GA I would like to suggest to change it to land in Lens instead of Visualize

@wylieconlon
Copy link
Contributor

Depends on #59845

@wylieconlon wylieconlon added Feature:Discover Discover Application Team:Visualizations Visualization editors, elastic-charts and infrastructure labels May 26, 2020
@elasticmachine
Copy link
Contributor

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

@wylieconlon
Copy link
Contributor

wylieconlon commented Aug 12, 2020

Going to update this issue because we have a simpler option, which is no longer blocked by the larger-scope concept of "opening a pre-filled Lens editor". The simpler option is to implement a handler for the VISUALIZE_FIELD action, based on @stratoula's work in #74121

In Lens terms, we already have a function called getDatasourceSuggestionsForField which should be called when the VISUALIZE_FIELD action is received. This is the same function that is used when users drag + drop in Lens, so the results will be predictable and easy to understand.

@wylieconlon wylieconlon added Feature:Lens and removed blocked Feature:Discover Discover Application labels Aug 12, 2020
@wylieconlon wylieconlon changed the title Discover top values visualize to open in Lens instead of Visualize [Lens] Visualizations triggered by Discover should be handled by Lens Aug 12, 2020
@flash1293 flash1293 mentioned this issue Aug 17, 2020
5 tasks
@flash1293
Copy link
Contributor

flash1293 commented Sep 9, 2020

Proposal:

  • Implement "Go to Lens" action for VISUALIZE_FIELD_TRIGGER
  • Lens deregisters the OSS Visualize action handler in start phase (UIActions.unregisterAction(id))
  • When executed this action will call navigateToApp to navigate to Lens using a special URL /by_field?fieldName=<fieldname>&indexPatternId=<indexPatternId> (should use state option of navigateToApp, see open questions)
  • field name and index pattern id params are parsed and passed through mounter and app to the running editor frame instance
  • The current getDatasourceSuggestionsForField API is almost what we need, but not exactly - it acts on an unknown field type controlled by the data sources, e.g. SQL would have something completely different in there. I propose to add an API getDatasourceSuggestionsForVisualizeFieldTrigger (need to work on the naming) which explicitly takes an index pattern id and a field name. For the index pattern datasource it would translate these two things into the local field type and fetch the suggestions.
  • Editor frame instance initializes as usually, if all data sources are ready it calls getDatasourceSuggestionsForVisualizeFieldTrigger on existing datasources and follows almost the same flow as if a field has been dropped (pass all suggested tables to the visualizations, order them, then take the first one and make it the actual state.

Untitled drawing

The downside of this is that it's building on top of the already messy initalization logic within the editor frame effect hooks. We have plans to refactor this and move the state management out of react, but it's not done yet. We could put this on hold until the state management is done, but I'm would definitely be fine with implementing it in the current code base.

Open questions:

  • Can we use the statetransfer service to pass the context into the Lens app? I didn't mention in the propsal because it seems to be coupled to embeddables
    • Even better - we can use the state option of navigateToApp to pass the context in a private way
  • Is Lens ready in 7.10/7.11 to do this switch?
    • yes

@stratoula
Copy link
Contributor

stratoula commented Sep 14, 2020

I have tested the unregisterAction of UIActions and it works! So when the Lens plugin starts it should call the UIActions.unregisterAction(id) where the id should be added on visualizeFieldAction. After syncing with Anton he suggests to add it on Lens start because in setup it probably could be a race condition, that lens first unregisters, but then visualize registers it.
cc @flash1293 cc @Dosant

@flash1293
Copy link
Contributor

Sounds great, thanks @stratoula - I think we have all of the building blocks in place and can start implementing this as soon as it fits in.

@stratoula
Copy link
Contributor

stratoula commented Sep 18, 2020

@flash1293 what we will lose with not having a special link is the getHref of the uiActions meaning the right click and open in new Tab from Discover. But maybe we don't want this for Lens (?)

@flash1293
Copy link
Contributor

@stratoula Let's implement it without an href for now (just a regular button). We can always extend later, but a URL scheme might be hard to take back

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New value added to drive a business result Feature:Lens Team:Visualizations Visualization editors, elastic-charts and infrastructure
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants