[Dashboard] Add visualization by value to Dashboard #68902

majagrubic · 2020-06-11T14:35:43Z

Summary

This is the first pass in adding Visualization to Dashboard by value. It should be fully functional. However, this solution is based on passing dashboard input to visualize editor, and then using this input to pass visualization data back to dashboard, which is a bit hacky (and won't work in case of a page reload). I will change this logic after #67064 is merged.

I'm hiding the new flow behind a "feature flag" by utilizing the server-side config. The current flow should remain unaffected. In order to test this locally, the value of the showNewVisualizeFlow flag needs to be set to "true" in src/plugins/visualize/config.ts and src/plugins/visualize/server/index.ts.

Checklist

Delete any items that are not applicable to this PR.

~~- [] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support~~
~~- [ ] Documentation was added for features that require explanation or tutorials~~

Unit or functional tests were updated or added to match the most common scenarios
This was checked for keyboard-only and screenreader accessibility
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser
This was checked for cross-browser compatibility, including a check against IE11

For maintainers

This was checked for breaking API changes and was labeled appropriately

majagrubic · 2020-06-11T18:05:58Z

@elasticmachine merge upstream

elasticmachine · 2020-06-17T09:26:24Z

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

ppisljar

cool! i think we should throw some things around but its very close.

ppisljar · 2020-06-17T11:16:49Z

src/plugins/dashboard/public/plugin.tsx

@@ -342,6 +359,32 @@ export class DashboardPlugin
    }
  }

+  private navigateToDashboard(core: CoreStart, dashInput: EmbeddableInput) {
+    if (!this.getActiveUrl) {


should we navigate to empty dashboard if one was not loaded ? should we allow for passing dashboardId to navigateToDashboard function ?

ppisljar · 2020-06-17T11:18:15Z

src/plugins/visualizations/public/embeddable/visualize_embeddable_factory.tsx

@@ -133,14 +134,21 @@ export class VisualizeEmbeddableFactory
    }
  }

-  public async create() {
+  public async create(input: VisualizeInput & { hideVisModal: boolean }, parent?: IContainer) {


could we rather say that input can also be undefined, and if its undefined we show the modal else we create from the input

input won't be undefined in case of a "regular save", it contains timeRange and query and a few other parameters:

kibana/src/plugins/visualizations/public/embeddable/visualize_embeddable.ts

Line 61 in 5348110

export interface VisualizeInput extends EmbeddableInput {

src/plugins/visualizations/public/embeddable/visualize_embeddable_factory.tsx

src/plugins/visualize/public/application/editor/editor.js

ppisljar · 2020-06-17T11:25:21Z

src/plugins/visualize/public/application/editor/editor.js

@@ -253,6 +253,35 @@ function VisualizeAppController($scope, $route, $injector, $timeout, kbnUrlState
                });
              };

+              const createVisReference = () => {
+                const currentDashboardInput = dashboard.getLastLoadedDashboardAppDashboardInput();


i think we should not be loading the dashboard and manipulating it in visualize (this requires internal knowledge about dashboard structure)

rather we should navigate to dashboard and let dashboard take care of that:

navigateToApp('dashboard', { newVis: vis.serialize(); })

and then dashboard should check for newVis on its state, if its there it can append it to its panels

I agree. The problem here is that we started working on a better inter-app communication logic and the PR hasn't been merged yet. As I mentioned in the description, this code is definitely going to change after #67064 is merged, and what you're describing is pretty similar to how it is going to work. We'll navigate to dashboard and pass the serialized vis as input, and the dashboard is going to render.

I did not want to base this PR off an unmerged PR, but I also didn't want to be blocked by that change.

I think this way of communicating between dashboard and visualization is good enough for now.

ppisljar

code LGTM, looking forward to improved communication between visualize and dashboard as well.

stratoula

Code LGTM. I was more thorough on the vis editor code. Tested it on Chrome and it works as expected.

kibanamachine · 2020-06-24T17:40:13Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: aa11919
Pipeline Steps (look for red circles / failed steps)
Interpreting CI Failures

Failed CI Steps

Build metrics

@kbn/optimizer bundle module count

id	value	diff	baseline
dashboard	111	+1	110

History

💚 Build #55872 succeeded 2561ea1
💔 Build #55843 failed 7a6a891
💚 Build #55474 succeeded dd1f287
💚 Build #54701 succeeded 811cba1
💔 Build #54647 failed 0bb50b9

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

majagrubic · 2020-06-25T10:11:01Z

Thanks @ppisljar , @stratoula and @ThomThomson for your reviews. I'm closing this PR in favor of: #69898

and will ping you for another review there 😃

majagrubic force-pushed the add-vis-to-dashboard branch from bece04b to cd6735d Compare June 12, 2020 10:19

majagrubic changed the title ~~[Dashboard] Add visualization as a reference to Dashboard~~ [Dashboard] Add visualization as value to Dashboard Jun 12, 2020

majagrubic changed the title ~~[Dashboard] Add visualization as value to Dashboard~~ [Dashboard] Add visualization by value to Dashboard Jun 12, 2020

majagrubic marked this pull request as ready for review June 17, 2020 09:25

majagrubic requested a review from a team June 17, 2020 09:25

majagrubic requested a review from a team as a code owner June 17, 2020 09:25

majagrubic requested review from ThomThomson, flash1293 and ppisljar June 17, 2020 09:25

majagrubic added v7.1.0 v8.0.0 release_note:roadmap Feature:Dashboard Dashboard related features Team:Visualizations Visualization editors, elastic-charts and infrastructure labels Jun 17, 2020

Plugging in DashboardStart dependency

ab8d873

ppisljar requested changes Jun 17, 2020

View reviewed changes

Maja Grubic added 12 commits June 17, 2020 12:30

Create embeddable by reference and navigate back to dashboard

1922ee2

Trying to feature flag the new flow

53719ab

Feature flagging new visualize flow

6c6b1a1

Removing unnecessary console statement

eb4e150

Fixing typescript errors

0df45e2

Adding a functional test for new functionality

857bd18

Adding a functional test for new functionality

786431c

Fixing test name

cb8f3cb

Changing test name

afee45a

Moving functional test to a separate folder

cdfb577

Trying to fix the config file

d7bb297

Adding an index file

74839ec

Maja Grubic added 2 commits June 17, 2020 12:30

Remove falsly included file

defaa39

Adding aggs and params to vis input

0bb50b9

majagrubic force-pushed the add-vis-to-dashboard branch from 12addcd to 0bb50b9 Compare June 17, 2020 11:39

Serializing vis before passing it as an input

811cba1

majagrubic mentioned this pull request Jun 18, 2020

Feature flag mechanism for hiding the new feature #69255

Closed

majagrubic requested a review from ppisljar June 18, 2020 08:28

ppisljar approved these changes Jun 18, 2020

View reviewed changes

Merge branch 'master' into add-vis-to-dashboard

dd1f287

majagrubic requested review from stratoula and removed request for flash1293 June 23, 2020 13:11

stratoula approved these changes Jun 24, 2020

View reviewed changes

Maja Grubic added 4 commits June 24, 2020 10:08

Merge master

ba3286f

Merge master

7a6a891

Remove duplicate indentifiers

2561ea1

Readding dashboard as a dependency

aa11919

majagrubic closed this Jun 25, 2020

majagrubic mentioned this pull request Jun 25, 2020

[Dashboard] Add visualization by value to dashboard #69898

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Dashboard] Add visualization by value to Dashboard #68902

[Dashboard] Add visualization by value to Dashboard #68902

majagrubic commented Jun 11, 2020 •

edited

Loading

majagrubic commented Jun 11, 2020

elasticmachine commented Jun 17, 2020

ppisljar left a comment

ppisljar Jun 17, 2020

ppisljar Jun 17, 2020

majagrubic Jun 17, 2020

ppisljar Jun 17, 2020

majagrubic Jun 17, 2020 •

edited

Loading

ppisljar left a comment

stratoula left a comment

kibanamachine commented Jun 24, 2020

majagrubic commented Jun 25, 2020

[Dashboard] Add visualization by value to Dashboard #68902

[Dashboard] Add visualization by value to Dashboard #68902

Conversation

majagrubic commented Jun 11, 2020 • edited Loading

Summary

Checklist

For maintainers

majagrubic commented Jun 11, 2020

elasticmachine commented Jun 17, 2020

ppisljar left a comment

Choose a reason for hiding this comment

ppisljar Jun 17, 2020

Choose a reason for hiding this comment

ppisljar Jun 17, 2020

Choose a reason for hiding this comment

majagrubic Jun 17, 2020

Choose a reason for hiding this comment

ppisljar Jun 17, 2020

Choose a reason for hiding this comment

majagrubic Jun 17, 2020 • edited Loading

Choose a reason for hiding this comment

ppisljar left a comment

Choose a reason for hiding this comment

stratoula left a comment

Choose a reason for hiding this comment

kibanamachine commented Jun 24, 2020

💔 Build Failed

Failed CI Steps

Build metrics

@kbn/optimizer bundle module count

History

majagrubic commented Jun 25, 2020

majagrubic commented Jun 11, 2020 •

edited

Loading

majagrubic Jun 17, 2020 •

edited

Loading