-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
[Dashboard] Add visualization by value to Dashboard #68902
Changes from all commits
ab8d873
1922ee2
53719ab
6c6b1a1
eb4e150
0df45e2
857bd18
786431c
cb8f3cb
afee45a
cdfb577
d7bb297
74839ec
defaa39
0bb50b9
811cba1
dd1f287
ba3286f
7a6a891
2561ea1
aa11919
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
/* | ||
* Licensed to Elasticsearch B.V. under one or more contributor | ||
* license agreements. See the NOTICE file distributed with | ||
* this work for additional information regarding copyright | ||
* ownership. Elasticsearch B.V. licenses this file to you under | ||
* the Apache License, Version 2.0 (the "License"); you may | ||
* not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, | ||
* software distributed under the License is distributed on an | ||
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
* KIND, either express or implied. See the License for the | ||
* specific language governing permissions and limitations | ||
* under the License. | ||
*/ | ||
|
||
import { schema, TypeOf } from '@kbn/config-schema'; | ||
|
||
export const configSchema = schema.object({ | ||
showNewVisualizeFlow: schema.boolean({ defaultValue: false }), | ||
}); | ||
|
||
export type ConfigSchema = TypeOf<typeof configSchema>; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,8 +28,8 @@ import React from 'react'; | |
import { makeStateful, useVisualizeAppState } from './lib'; | ||
import { VisualizeConstants } from '../visualize_constants'; | ||
import { getEditBreadcrumbs } from '../breadcrumbs'; | ||
|
||
import { addHelpMenuToAppChrome } from '../help_menu/help_menu_util'; | ||
|
||
import { unhashUrl } from '../../../../kibana_utils/public'; | ||
import { MarkdownSimple, toMountPoint } from '../../../../kibana_react/public'; | ||
import { | ||
|
@@ -76,10 +76,11 @@ function VisualizeAppController($scope, $route, $injector, $timeout, kbnUrlState | |
I18nContext, | ||
setActiveUrl, | ||
visualizations, | ||
dashboard, | ||
embeddable, | ||
featureFlagConfig, | ||
scopedHistory, | ||
} = getServices(); | ||
|
||
const { | ||
filterManager, | ||
timefilter: { timefilter }, | ||
|
@@ -252,6 +253,24 @@ function VisualizeAppController($scope, $route, $injector, $timeout, kbnUrlState | |
}); | ||
}; | ||
|
||
const createVisReference = () => { | ||
const currentDashboardInput = dashboard.getLastLoadedDashboardAppDashboardInput(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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:
and then dashboard should check for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
embeddable | ||
.getEmbeddableFactory('dashboard') | ||
.create(currentDashboardInput) | ||
.then((currentDashboard) => { | ||
if (currentDashboard) { | ||
const input = { | ||
savedVis: { ...vis.serialize() }, | ||
}; | ||
currentDashboard.addNewEmbeddable('visualization', input).then(() => { | ||
const dashInput = currentDashboard.getInput(); | ||
dashboard.navigateToDashboard(dashInput); | ||
}); | ||
} | ||
}); | ||
}; | ||
|
||
const saveModal = ( | ||
<SavedObjectSaveModalOrigin | ||
documentInfo={savedVis} | ||
|
@@ -261,7 +280,12 @@ function VisualizeAppController($scope, $route, $injector, $timeout, kbnUrlState | |
originatingApp={$scope.getOriginatingApp()} | ||
/> | ||
); | ||
showSaveModal(saveModal, I18nContext); | ||
const lastAppType = $scope.getOriginatingApp(); | ||
if (lastAppType === 'dashboards' && featureFlagConfig.showNewVisualizeFlow) { | ||
createVisReference(); | ||
} else { | ||
showSaveModal(saveModal, I18nContext); | ||
} | ||
}, | ||
}, | ||
] | ||
|
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.
should we navigate to empty dashboard if one was not loaded ? should we allow for passing dashboardId to navigateToDashboard function ?