From b4972daf7624debc2a202c72f63f4ad89e42ba18 Mon Sep 17 00:00:00 2001 From: Maja Grubic Date: Wed, 15 Jul 2020 13:17:21 +0100 Subject: [PATCH 01/13] First version of new by-value editor Fixing broken behavior and applying relevant changes Adding changes to dashboard Removing unnecessary empty line Removing unnecessary deepClone Fixing some stuff in dashboard container Extracting logic into common components Fixing eslint Fix breadcrumbs Fixing error in search interceptor Reintroduce mistakenly removed empty lines Renaming function --- .../application/dashboard_app_controller.tsx | 4 +- .../embeddable/dashboard_container.tsx | 8 +- .../lib/actions/edit_panel_action.test.tsx | 10 +- .../public/lib/actions/edit_panel_action.ts | 20 +++- .../public/lib/state_transfer/types.ts | 1 + .../public/default_editor_controller.tsx | 1 - .../create_vis_embeddable_from_object.ts | 3 +- src/plugins/visualize/config.ts | 2 +- .../visualize/public/application/app.tsx | 10 +- .../public/application/components/index.ts | 1 + .../components/visualize_byvalue_editor.tsx | 102 ++++++++++++++++ .../components/visualize_editor.tsx | 55 +++------ .../components/visualize_editor_common.tsx | 102 ++++++++++++++++ .../components/visualize_top_nav.tsx | 1 + .../public/application/utils/breadcrumbs.ts | 5 +- .../utils/create_visualize_app_state.ts | 100 +++++++++------- .../application/utils/get_top_nav_config.tsx | 49 ++++---- .../utils/get_visualization_instance.ts | 111 +++++++++++++----- .../public/application/utils/use/index.ts | 1 + .../utils/use/use_editor_updates.ts | 13 +- .../utils/use/use_saved_vis_instance.ts | 1 - .../application/utils/use/use_vis_byvalue.ts | 94 +++++++++++++++ .../utils/use/use_visualize_app_state.tsx | 4 +- .../public/application/utils/utils.ts | 2 +- .../public/application/visualize_constants.ts | 1 + 25 files changed, 550 insertions(+), 151 deletions(-) create mode 100644 src/plugins/visualize/public/application/components/visualize_byvalue_editor.tsx create mode 100644 src/plugins/visualize/public/application/components/visualize_editor_common.tsx create mode 100644 src/plugins/visualize/public/application/utils/use/use_vis_byvalue.ts diff --git a/src/plugins/dashboard/public/application/dashboard_app_controller.tsx b/src/plugins/dashboard/public/application/dashboard_app_controller.tsx index 8138e1c7f4dfd..a8dc285a29e3c 100644 --- a/src/plugins/dashboard/public/application/dashboard_app_controller.tsx +++ b/src/plugins/dashboard/public/application/dashboard_app_controller.tsx @@ -436,12 +436,12 @@ export class DashboardAppController { savedObjectId: incomingState.id, }); } else if ('input' in incomingState) { - const input = incomingState.input; + const { input, type, embeddableId } = incomingState; delete input.id; const explicitInput = { savedVis: input, }; - container.addOrUpdateEmbeddable(incomingState.type, explicitInput); + container.addOrUpdateEmbeddable(type, explicitInput, embeddableId); } } } diff --git a/src/plugins/dashboard/public/application/embeddable/dashboard_container.tsx b/src/plugins/dashboard/public/application/embeddable/dashboard_container.tsx index ff74580ba256b..036880a1d088b 100644 --- a/src/plugins/dashboard/public/application/embeddable/dashboard_container.tsx +++ b/src/plugins/dashboard/public/application/embeddable/dashboard_container.tsx @@ -171,6 +171,7 @@ export class DashboardContainer extends Container = IEmbeddable - >(type: string, explicitInput: Partial) { - if (explicitInput.id && this.input.panels[explicitInput.id]) { - this.replacePanel(this.input.panels[explicitInput.id], { + >(type: string, explicitInput: Partial, embeddableId?: string) { + const idToReplace = embeddableId || explicitInput.id; + if (idToReplace && this.input.panels[idToReplace]) { + this.replacePanel(this.input.panels[idToReplace], { type, explicitInput: { ...explicitInput, diff --git a/src/plugins/embeddable/public/lib/actions/edit_panel_action.test.tsx b/src/plugins/embeddable/public/lib/actions/edit_panel_action.test.tsx index 594a7ad73c396..8c3d7ab9c30d0 100644 --- a/src/plugins/embeddable/public/lib/actions/edit_panel_action.test.tsx +++ b/src/plugins/embeddable/public/lib/actions/edit_panel_action.test.tsx @@ -56,12 +56,18 @@ test('is compatible when edit url is available, in edit mode and editable', asyn test('redirects to app using state transfer', async () => { applicationMock.currentAppId$ = of('superCoolCurrentApp'); const action = new EditPanelAction(getFactory, applicationMock, stateTransferMock); - const embeddable = new EditableEmbeddable({ id: '123', viewMode: ViewMode.EDIT }, true); + const input = { id: '123', viewMode: ViewMode.EDIT }; + const embeddable = new EditableEmbeddable(input, true); embeddable.getOutput = jest.fn(() => ({ editApp: 'ultraVisualize', editPath: '/123' })); await action.execute({ embeddable }); expect(stateTransferMock.navigateToEditor).toHaveBeenCalledWith('ultraVisualize', { path: '/123', - state: { originatingApp: 'superCoolCurrentApp' }, + state: { + originatingApp: 'superCoolCurrentApp', + byValueMode: true, + embeddableId: '123', + valueInput: input, + }, }); }); diff --git a/src/plugins/embeddable/public/lib/actions/edit_panel_action.ts b/src/plugins/embeddable/public/lib/actions/edit_panel_action.ts index 9177a77d547b0..8d12ddd0299e7 100644 --- a/src/plugins/embeddable/public/lib/actions/edit_panel_action.ts +++ b/src/plugins/embeddable/public/lib/actions/edit_panel_action.ts @@ -24,7 +24,12 @@ import { take } from 'rxjs/operators'; import { ViewMode } from '../types'; import { EmbeddableFactoryNotFoundError } from '../errors'; import { EmbeddableStart } from '../../plugin'; -import { IEmbeddable, EmbeddableEditorState, EmbeddableStateTransfer } from '../..'; +import { + IEmbeddable, + EmbeddableEditorState, + EmbeddableStateTransfer, + SavedObjectEmbeddableInput, +} from '../..'; export const ACTION_EDIT_PANEL = 'editPanel'; @@ -109,8 +114,17 @@ export class EditPanelAction implements Action { const app = embeddable ? embeddable.getOutput().editApp : undefined; const path = embeddable ? embeddable.getOutput().editPath : undefined; if (app && path) { - const state = this.currentAppId ? { originatingApp: this.currentAppId } : undefined; - return { app, path, state }; + if (this.currentAppId) { + const byValueMode = !(embeddable.getInput() as SavedObjectEmbeddableInput).savedObjectId; + const state: EmbeddableEditorState = { + originatingApp: this.currentAppId, + byValueMode, + valueInput: byValueMode ? embeddable.getInput() : undefined, + embeddableId: embeddable.id, + }; + return { app, path, state }; + } + return { app, path }; } } diff --git a/src/plugins/embeddable/public/lib/state_transfer/types.ts b/src/plugins/embeddable/public/lib/state_transfer/types.ts index a6721784302ac..4d487a022be4e 100644 --- a/src/plugins/embeddable/public/lib/state_transfer/types.ts +++ b/src/plugins/embeddable/public/lib/state_transfer/types.ts @@ -27,6 +27,7 @@ export interface EmbeddableEditorState { originatingApp: string; byValueMode?: boolean; valueInput?: EmbeddableInput; + embeddableId?: string; } export function isEmbeddableEditorState(state: unknown): state is EmbeddableEditorState { diff --git a/src/plugins/vis_default_editor/public/default_editor_controller.tsx b/src/plugins/vis_default_editor/public/default_editor_controller.tsx index 3158582438558..56fb15ea8354a 100644 --- a/src/plugins/vis_default_editor/public/default_editor_controller.tsx +++ b/src/plugins/vis_default_editor/public/default_editor_controller.tsx @@ -69,7 +69,6 @@ class DefaultEditorController { ] : visType.editorConfig.optionTabs), ]; - this.state = { vis, optionTabs, diff --git a/src/plugins/visualizations/public/embeddable/create_vis_embeddable_from_object.ts b/src/plugins/visualizations/public/embeddable/create_vis_embeddable_from_object.ts index 45c750de05ae1..194deef82a5f0 100644 --- a/src/plugins/visualizations/public/embeddable/create_vis_embeddable_from_object.ts +++ b/src/plugins/visualizations/public/embeddable/create_vis_embeddable_from_object.ts @@ -41,7 +41,8 @@ export const createVisEmbeddableFromObject = (deps: VisualizeEmbeddableFactoryDe try { const visId = vis.id as string; - const editPath = visId ? savedVisualizations.urlFor(visId) : ''; + const editPath = visId ? savedVisualizations.urlFor(visId) : '#/edit_by_value'; + const editUrl = visId ? getHttp().basePath.prepend(`/app/visualize${savedVisualizations.urlFor(visId)}`) : ''; diff --git a/src/plugins/visualize/config.ts b/src/plugins/visualize/config.ts index ee79a37717f26..6f01c8d1d5e8b 100644 --- a/src/plugins/visualize/config.ts +++ b/src/plugins/visualize/config.ts @@ -20,7 +20,7 @@ import { schema, TypeOf } from '@kbn/config-schema'; export const configSchema = schema.object({ - showNewVisualizeFlow: schema.boolean({ defaultValue: false }), + showNewVisualizeFlow: schema.boolean({ defaultValue: true }), }); export type ConfigSchema = TypeOf; diff --git a/src/plugins/visualize/public/application/app.tsx b/src/plugins/visualize/public/application/app.tsx index 0e71d72a3d4c7..8dd6b2ace8413 100644 --- a/src/plugins/visualize/public/application/app.tsx +++ b/src/plugins/visualize/public/application/app.tsx @@ -24,7 +24,12 @@ import { Route, Switch, useLocation } from 'react-router-dom'; import { syncQueryStateWithUrl } from '../../../data/public'; import { useKibana } from '../../../kibana_react/public'; import { VisualizeServices } from './types'; -import { VisualizeEditor, VisualizeListing, VisualizeNoMatch } from './components'; +import { + VisualizeEditor, + VisualizeListing, + VisualizeNoMatch, + VisualizeByValueEditor, +} from './components'; import { VisualizeConstants } from './visualize_constants'; export const VisualizeApp = () => { @@ -48,6 +53,9 @@ export const VisualizeApp = () => { return ( + + + diff --git a/src/plugins/visualize/public/application/components/index.ts b/src/plugins/visualize/public/application/components/index.ts index a3a7fde1d6569..1666bae9b72e0 100644 --- a/src/plugins/visualize/public/application/components/index.ts +++ b/src/plugins/visualize/public/application/components/index.ts @@ -20,3 +20,4 @@ export { VisualizeListing } from './visualize_listing'; export { VisualizeEditor } from './visualize_editor'; export { VisualizeNoMatch } from './visualize_no_match'; +export { VisualizeByValueEditor } from './visualize_byvalue_editor'; diff --git a/src/plugins/visualize/public/application/components/visualize_byvalue_editor.tsx b/src/plugins/visualize/public/application/components/visualize_byvalue_editor.tsx new file mode 100644 index 0000000000000..da0b35c6c04de --- /dev/null +++ b/src/plugins/visualize/public/application/components/visualize_byvalue_editor.tsx @@ -0,0 +1,102 @@ +/* + * 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 './visualize_editor.scss'; +import React, { useEffect, useState } from 'react'; +import { EventEmitter } from 'events'; + +import { VisualizeInput } from 'src/plugins/visualizations/public'; +import { useKibana } from '../../../../kibana_react/public'; +import { + useChromeVisibility, + useVisByValue, + useVisualizeAppState, + useEditorUpdates, + useLinkedSearchUpdates, +} from '../utils'; +import { VisualizeServices } from '../types'; +import { VisualizeEditorCommon } from './visualize_editor_common'; + +export const VisualizeByValueEditor = () => { + const [originatingApp, setOriginatingApp] = useState(); + const { services } = useKibana(); + const [eventEmitter] = useState(new EventEmitter()); + const [hasUnsavedChanges, setHasUnsavedChanges] = useState(true); + const [embeddableId, setEmbeddableId] = useState(); + const [valueInput, setValueInput] = useState(); + + useEffect(() => { + const { originatingApp: value, embeddableId: embeddableIdValue, valueInput: valueInputValue } = + services.embeddable + .getStateTransfer(services.scopedHistory) + .getIncomingEditorState({ keysToRemoveAfterFetch: ['id', 'embeddableId', 'valueInput'] }) || + {}; + setOriginatingApp(value); + setValueInput(valueInputValue); + setEmbeddableId(embeddableIdValue); + }, [services]); + + const isChromeVisible = useChromeVisibility(services.chrome); + + const { savedVisInstance, visEditorRef, visEditorController } = useVisByValue( + services, + eventEmitter, + isChromeVisible, + valueInput + ); + const { appState, hasUnappliedChanges } = useVisualizeAppState( + services, + eventEmitter, + true, + savedVisInstance + ); + const { isEmbeddableRendered, currentAppState } = useEditorUpdates( + services, + eventEmitter, + setHasUnsavedChanges, + appState, + savedVisInstance, + visEditorController, + true + ); + useLinkedSearchUpdates(services, eventEmitter, appState, savedVisInstance); + + useEffect(() => { + // clean up all registered listeners if any is left + return () => { + eventEmitter.removeAllListeners(); + }; + }, [eventEmitter]); + + return ( + + ); +}; diff --git a/src/plugins/visualize/public/application/components/visualize_editor.tsx b/src/plugins/visualize/public/application/components/visualize_editor.tsx index c571a5fb078bc..91626b60d1672 100644 --- a/src/plugins/visualize/public/application/components/visualize_editor.tsx +++ b/src/plugins/visualize/public/application/components/visualize_editor.tsx @@ -21,8 +21,6 @@ import './visualize_editor.scss'; import React, { useEffect, useState } from 'react'; import { useParams } from 'react-router-dom'; import { EventEmitter } from 'events'; -import { FormattedMessage } from '@kbn/i18n/react'; -import { EuiScreenReaderOnly } from '@elastic/eui'; import { useKibana } from '../../../../kibana_react/public'; import { @@ -33,8 +31,7 @@ import { useLinkedSearchUpdates, } from '../utils'; import { VisualizeServices } from '../types'; -import { ExperimentalVisInfo } from './experimental_vis_info'; -import { VisualizeTopNav } from './visualize_top_nav'; +import { VisualizeEditorCommon } from './visualize_editor_common'; export const VisualizeEditor = () => { const { id: visualizationIdFromUrl } = useParams(); @@ -53,6 +50,7 @@ export const VisualizeEditor = () => { const { appState, hasUnappliedChanges } = useVisualizeAppState( services, eventEmitter, + false, savedVisInstance ); const { isEmbeddableRendered, currentAppState } = useEditorUpdates( @@ -67,7 +65,9 @@ export const VisualizeEditor = () => { useEffect(() => { const { originatingApp: value } = - services.embeddable.getStateTransfer(services.scopedHistory).getIncomingEditorState() || {}; + services.embeddable + .getStateTransfer(services.scopedHistory) + .getIncomingEditorState({ keysToRemoveAfterFetch: ['id', 'input'] }) || {}; setOriginatingApp(value); }, [services]); @@ -79,37 +79,18 @@ export const VisualizeEditor = () => { }, [eventEmitter]); return ( -
- {savedVisInstance && appState && currentAppState && ( - - )} - {savedVisInstance?.vis?.type?.isExperimental && } - {savedVisInstance && ( - -

- -

- - )} -
-
+ ); }; diff --git a/src/plugins/visualize/public/application/components/visualize_editor_common.tsx b/src/plugins/visualize/public/application/components/visualize_editor_common.tsx new file mode 100644 index 0000000000000..82aea09755f2e --- /dev/null +++ b/src/plugins/visualize/public/application/components/visualize_editor_common.tsx @@ -0,0 +1,102 @@ +/* + * 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 './visualize_editor.scss'; +import React, { RefObject } from 'react'; +import { FormattedMessage } from '@kbn/i18n/react'; +import { EuiScreenReaderOnly } from '@elastic/eui'; +import { VisualizeTopNav } from './visualize_top_nav'; +import { ExperimentalVisInfo } from './experimental_vis_info'; +import { SavedVisInstance, VisualizeAppState, VisualizeAppStateContainer } from '../types'; + +interface VisualizeEditorCommonProps { + savedVisInstance?: SavedVisInstance; + appState: VisualizeAppStateContainer | null; + currentAppState?: VisualizeAppState; + isChromeVisible?: boolean; + hasUnsavedChanges: boolean; + setHasUnsavedChanges: (value: boolean) => void; + hasUnappliedChanges: boolean; + isEmbeddableRendered: boolean; + visEditorRef: RefObject; + originatingApp?: string; + visualizationIdFromUrl?: string; + embeddableId?: string; +} + +export const VisualizeEditorCommon = ({ + savedVisInstance, + appState, + currentAppState, + isChromeVisible, + hasUnsavedChanges, + setHasUnsavedChanges, + hasUnappliedChanges, + isEmbeddableRendered, + originatingApp, + visualizationIdFromUrl, + embeddableId, + visEditorRef, +}: VisualizeEditorCommonProps) => { + return ( +
+ {savedVisInstance && appState && currentAppState && ( + + )} + {savedVisInstance?.vis?.type?.isExperimental && } + {savedVisInstance && ( + +

+ {savedVisInstance.savedVis ? ( + + ) : ( + + )} +

+ + )} +
+
+ ); +}; diff --git a/src/plugins/visualize/public/application/components/visualize_top_nav.tsx b/src/plugins/visualize/public/application/components/visualize_top_nav.tsx index 2e7dba46487ad..b3950fabac7d6 100644 --- a/src/plugins/visualize/public/application/components/visualize_top_nav.tsx +++ b/src/plugins/visualize/public/application/components/visualize_top_nav.tsx @@ -104,6 +104,7 @@ const TopNav = ({ stateContainer, visualizationIdFromUrl, services, + embeddableId, ]); const [indexPattern, setIndexPattern] = useState(vis.data.indexPattern); const showDatePicker = () => { diff --git a/src/plugins/visualize/public/application/utils/breadcrumbs.ts b/src/plugins/visualize/public/application/utils/breadcrumbs.ts index a1e5a9e8912e1..7197b3dc35c1d 100644 --- a/src/plugins/visualize/public/application/utils/breadcrumbs.ts +++ b/src/plugins/visualize/public/application/utils/breadcrumbs.ts @@ -43,7 +43,10 @@ export function getCreateBreadcrumbs() { ]; } -export function getEditBreadcrumbs(text: string) { +export function getEditBreadcrumbs(text?: string) { + if (!text) { + return [...getLandingBreadcrumbs(), { text: 'Edit' }]; + } return [ ...getLandingBreadcrumbs(), { diff --git a/src/plugins/visualize/public/application/utils/create_visualize_app_state.ts b/src/plugins/visualize/public/application/utils/create_visualize_app_state.ts index 52b7e3ede298b..7a7e04d78354b 100644 --- a/src/plugins/visualize/public/application/utils/create_visualize_app_state.ts +++ b/src/plugins/visualize/public/application/utils/create_visualize_app_state.ts @@ -32,6 +32,7 @@ const STATE_STORAGE_KEY = '_a'; interface Arguments { kbnUrlStateStorage: IKbnUrlStateStorage; stateDefaults: VisualizeAppState; + byValue?: boolean; } function toObject(state: PureVisState): PureVisState { @@ -40,55 +41,67 @@ function toObject(state: PureVisState): PureVisState { }) as PureVisState; } -export function createVisualizeAppState({ stateDefaults, kbnUrlStateStorage }: Arguments) { +const pureTransitions = { + set: (state) => (prop, value) => ({ ...state, [prop]: value }), + setVis: (state) => (vis) => ({ + ...state, + vis: { + ...state.vis, + ...vis, + }, + }), + unlinkSavedSearch: (state) => ({ query, parentFilters = [] }) => ({ + ...state, + query: query || state.query, + filters: union(state.filters, parentFilters), + linked: false, + }), + updateVisState: (state) => (newVisState) => ({ ...state, vis: toObject(newVisState) }), + updateSavedQuery: (state) => (savedQueryId) => { + const updatedState = { + ...state, + savedQuery: savedQueryId, + }; + + if (!savedQueryId) { + delete updatedState.savedQuery; + } + + return updatedState; + }, +} as VisualizeAppStateTransitions; + +function createVisualizeByValueAppState(stateDefaults: VisualizeAppState) { + const initialState = migrateAppState({ + ...stateDefaults, + ...stateDefaults, + }); + const stateContainer = createStateContainer( + initialState, + pureTransitions + ); + const stopStateSync = () => {}; + return { stateContainer, stopStateSync }; +} + +function createDefaultVisualizeAppState({ stateDefaults, kbnUrlStateStorage }: Arguments) { const urlState = kbnUrlStateStorage.get(STATE_STORAGE_KEY); const initialState = migrateAppState({ ...stateDefaults, ...urlState, }); - /* - make sure url ('_a') matches initial state - Initializing appState does two things - first it translates the defaults into AppState, - second it updates appState based on the url (the url trumps the defaults). This means if - we update the state format at all and want to handle BWC, we must not only migrate the - data stored with saved vis, but also any old state in the url. - */ + make sure url ('_a') matches initial state + Initializing appState does two things - first it translates the defaults into AppState, + second it updates appState based on the url (the url trumps the defaults). This means if + we update the state format at all and want to handle BWC, we must not only migrate the + data stored with saved vis, but also any old state in the url. + */ kbnUrlStateStorage.set(STATE_STORAGE_KEY, initialState, { replace: true }); - const stateContainer = createStateContainer( initialState, - { - set: (state) => (prop, value) => ({ ...state, [prop]: value }), - setVis: (state) => (vis) => ({ - ...state, - vis: { - ...state.vis, - ...vis, - }, - }), - unlinkSavedSearch: (state) => ({ query, parentFilters = [] }) => ({ - ...state, - query: query || state.query, - filters: union(state.filters, parentFilters), - linked: false, - }), - updateVisState: (state) => (newVisState) => ({ ...state, vis: toObject(newVisState) }), - updateSavedQuery: (state) => (savedQueryId) => { - const updatedState = { - ...state, - savedQuery: savedQueryId, - }; - - if (!savedQueryId) { - delete updatedState.savedQuery; - } - - return updatedState; - }, - } + pureTransitions ); - const { start: startStateSync, stop: stopStateSync } = syncState({ storageKey: STATE_STORAGE_KEY, stateContainer: { @@ -102,9 +115,14 @@ export function createVisualizeAppState({ stateDefaults, kbnUrlStateStorage }: A }, stateStorage: kbnUrlStateStorage, }); - // start syncing the appState with the ('_a') url startStateSync(); - return { stateContainer, stopStateSync }; } + +export function createVisualizeAppState({ stateDefaults, kbnUrlStateStorage, byValue }: Arguments) { + if (byValue) { + return createVisualizeByValueAppState(stateDefaults); + } + return createDefaultVisualizeAppState({ stateDefaults, kbnUrlStateStorage }); +} diff --git a/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx b/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx index 96f64c6478fa9..ba7b2dc6b0678 100644 --- a/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx +++ b/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx @@ -43,6 +43,7 @@ interface TopNavConfigParams { savedVisInstance: SavedVisInstance; stateContainer: VisualizeAppStateContainer; visualizationIdFromUrl?: string; + embeddableId?: string; } export const getTopNavConfig = ( @@ -55,6 +56,7 @@ export const getTopNavConfig = ( savedVisInstance: { embeddableHandler, savedVis, vis }, stateContainer, visualizationIdFromUrl, + embeddableId, }: TopNavConfigParams, { application, @@ -142,8 +144,26 @@ export const getTopNavConfig = ( } } + const createVisReference = () => { + if (!originatingApp) { + return; + } + const state = { + input: { + ...vis.serialize(), + id: embeddableId ? embeddableId : uuid.v4(), + }, + type: VISUALIZE_EMBEDDABLE_TYPE, + embeddableId: '', + }; + if (embeddableId) { + state.embeddableId = embeddableId; + } + embeddable.getStateTransfer().navigateToWithEmbeddablePackage(originatingApp, { state }); + }; + const topNavMenu: TopNavMenuData[] = [ - ...(originatingApp && savedVis.id + ...(originatingApp && ((savedVis && savedVis.id) || embeddableId) ? [ { id: 'saveAndReturn', @@ -175,24 +195,27 @@ export const getTopNavConfig = ( confirmOverwrite: false, returnToOrigin: true, }; + if (originatingApp === 'dashboards' && featureFlagConfig.showNewVisualizeFlow) { + return createVisReference(); + } return doSave(saveOptions); }, }, ] : []), - ...(visualizeCapabilities.save + ...(visualizeCapabilities.save && !embeddableId ? [ { id: 'save', label: - savedVis.id && originatingApp + savedVis && savedVis.id && originatingApp ? i18n.translate('visualize.topNavMenu.saveVisualizationAsButtonLabel', { defaultMessage: 'save as', }) : i18n.translate('visualize.topNavMenu.saveVisualizationButtonLabel', { defaultMessage: 'save', }), - emphasize: !savedVis.id || !originatingApp, + emphasize: !savedVis || !savedVis.id || !originatingApp, description: i18n.translate('visualize.topNavMenu.saveVisualizationButtonAriaLabel', { defaultMessage: 'Save Visualization', }), @@ -234,20 +257,6 @@ export const getTopNavConfig = ( } return response; }; - - const createVisReference = () => { - if (!originatingApp) { - return; - } - const input = { - ...vis.serialize(), - id: uuid.v4(), - }; - embeddable.getStateTransfer().navigateToWithEmbeddablePackage(originatingApp, { - state: { input, type: VISUALIZE_EMBEDDABLE_TYPE }, - }); - }; - const saveModal = ( { - if (share) { + if (share && savedVis) { share.toggleShareContextMenu({ anchorElement, allowEmbed: true, @@ -292,7 +301,7 @@ export const getTopNavConfig = ( } }, // disable the Share button if no action specified - disableButton: !share, + disableButton: !share || embeddableId, }, { id: 'inspector', diff --git a/src/plugins/visualize/public/application/utils/get_visualization_instance.ts b/src/plugins/visualize/public/application/utils/get_visualization_instance.ts index a75c84cf0b71c..e145da9e47f44 100644 --- a/src/plugins/visualize/public/application/utils/get_visualization_instance.ts +++ b/src/plugins/visualize/public/application/utils/get_visualization_instance.ts @@ -18,46 +18,31 @@ */ import { i18n } from '@kbn/i18n'; -import { VisSavedObject, VisualizeEmbeddableContract } from 'src/plugins/visualizations/public'; +import { + SerializedVis, + Vis, + VisSavedObject, + VisualizeEmbeddableContract, + VisualizeInput, +} from 'src/plugins/visualizations/public'; import { SearchSourceFields } from 'src/plugins/data/public'; import { SavedObject } from 'src/plugins/saved_objects/public'; +import { cloneDeep } from 'lodash'; import { createSavedSearchesLoader } from '../../../../discover/public'; import { VisualizeServices } from '../types'; -export const getVisualizationInstance = async ( - { +const createVisualizeEmbeddableAndLinkSavedSearch = async ( + vis: Vis, + visualizeServices: VisualizeServices +) => { + const { chrome, data, overlays, - visualizations, createVisEmbeddableFromObject, savedObjects, - savedVisualizations, toastNotifications, - }: VisualizeServices, - /** - * opts can be either a saved visualization id passed as string, - * or an object of new visualization params. - * Both come from url search query - */ - opts?: Record | string -) => { - const savedVis: VisSavedObject = await savedVisualizations.get(opts); - - if (typeof opts !== 'string') { - savedVis.searchSourceFields = { index: opts?.indexPattern } as SearchSourceFields; - } - const serializedVis = visualizations.convertToSerializedVis(savedVis); - let vis = await visualizations.createVis(serializedVis.type, serializedVis); - - if (vis.type.setup) { - try { - vis = await vis.type.setup(vis); - } catch { - // skip this catch block - } - } - + } = visualizeServices; const embeddableHandler = (await createVisEmbeddableFromObject(vis, { timeRange: data.query.timefilter.timefilter.getTime(), filters: data.query.filterManager.getFilters(), @@ -86,5 +71,71 @@ export const getVisualizationInstance = async ( }).get(vis.data.savedSearchId); } - return { vis, savedVis, savedSearch, embeddableHandler }; + return { savedSearch, embeddableHandler }; +}; + +export const getVisualizationInstanceFromInput = async ( + visualizeServices: VisualizeServices, + input?: VisualizeInput +) => { + if (!input) { + return; + } + const { visualizations } = visualizeServices; + const visState = input.savedVis as SerializedVis; + + let vis = await visualizations.createVis(visState.type, cloneDeep(visState)); + if (vis.type.setup) { + try { + vis = await vis.type.setup(vis); + } catch { + // skip this catch block + } + } + const { embeddableHandler, savedSearch } = await createVisualizeEmbeddableAndLinkSavedSearch( + vis, + visualizeServices + ); + return { + vis, + embeddableHandler, + savedSearch, + }; +}; + +export const getVisualizationInstance = async ( + visualizeServices: VisualizeServices, + /** + * opts can be either a saved visualization id passed as string, + * or an object of new visualization params. + * Both come from url search query + */ + opts?: Record | string +) => { + const { visualizations, savedVisualizations } = visualizeServices; + const savedVis: VisSavedObject = await savedVisualizations.get(opts); + + if (typeof opts !== 'string') { + savedVis.searchSourceFields = { index: opts?.indexPattern } as SearchSourceFields; + } + const serializedVis = visualizations.convertToSerializedVis(savedVis); + let vis = await visualizations.createVis(serializedVis.type, serializedVis); + if (vis.type.setup) { + try { + vis = await vis.type.setup(vis); + } catch { + // skip this catch block + } + } + + const { embeddableHandler, savedSearch } = await createVisualizeEmbeddableAndLinkSavedSearch( + vis, + visualizeServices + ); + return { + vis, + embeddableHandler, + savedSearch, + savedVis, + }; }; diff --git a/src/plugins/visualize/public/application/utils/use/index.ts b/src/plugins/visualize/public/application/utils/use/index.ts index 8bd9456b10572..98d1f11d81a8e 100644 --- a/src/plugins/visualize/public/application/utils/use/index.ts +++ b/src/plugins/visualize/public/application/utils/use/index.ts @@ -22,3 +22,4 @@ export { useEditorUpdates } from './use_editor_updates'; export { useSavedVisInstance } from './use_saved_vis_instance'; export { useVisualizeAppState } from './use_visualize_app_state'; export { useLinkedSearchUpdates } from './use_linked_search_updates'; +export { useVisByValue } from './use_vis_byvalue'; diff --git a/src/plugins/visualize/public/application/utils/use/use_editor_updates.ts b/src/plugins/visualize/public/application/utils/use/use_editor_updates.ts index 360e7560b1932..10a9b777cfbce 100644 --- a/src/plugins/visualize/public/application/utils/use/use_editor_updates.ts +++ b/src/plugins/visualize/public/application/utils/use/use_editor_updates.ts @@ -37,7 +37,8 @@ export const useEditorUpdates = ( setHasUnsavedChanges: (value: boolean) => void, appState: VisualizeAppStateContainer | null, savedVisInstance: SavedVisInstance | undefined, - visEditorController: IEditorController | undefined + visEditorController: IEditorController | undefined, + byValue?: boolean ) => { const [isEmbeddableRendered, setIsEmbeddableRendered] = useState(false); const [currentAppState, setCurrentAppState] = useState(); @@ -84,15 +85,17 @@ export const useEditorUpdates = ( }); const handleLinkedSearch = (linked: boolean) => { - if (linked && !savedVis.savedSearchId && savedSearch) { + if (linked && savedVis && !savedVis.savedSearchId && savedSearch) { savedVis.savedSearchId = savedSearch.id; vis.data.savedSearchId = savedSearch.id; if (vis.data.searchSource) { vis.data.searchSource.setParent(savedSearch.searchSource); } - } else if (!linked && savedVis.savedSearchId) { + } else if (!linked && savedVis && savedVis.savedSearchId) { delete savedVis.savedSearchId; delete vis.data.savedSearchId; + } else { + // TODO: something to do for when it's not a saved vis? } }; @@ -110,8 +113,7 @@ export const useEditorUpdates = ( const unsubscribeStateUpdates = appState.subscribe((state) => { setCurrentAppState(state); - - if (savedVis.id && !services.history.location.pathname.includes(savedVis.id)) { + if (savedVis && savedVis.id && !services.history.location.pathname.includes(savedVis.id)) { // this filters out the case when manipulating the browser history back/forward // and initializing different visualizations return; @@ -127,6 +129,7 @@ export const useEditorUpdates = ( // if the browser history was changed manually we need to reflect changes in the editor if ( + savedVis && !isEqual( { ...services.visualizations.convertFromSerializedVis(vis.serialize()).visState, diff --git a/src/plugins/visualize/public/application/utils/use/use_saved_vis_instance.ts b/src/plugins/visualize/public/application/utils/use/use_saved_vis_instance.ts index 764bcb4a327c0..ec815b8cfcbee 100644 --- a/src/plugins/visualize/public/application/utils/use/use_saved_vis_instance.ts +++ b/src/plugins/visualize/public/application/utils/use/use_saved_vis_instance.ts @@ -59,7 +59,6 @@ export const useSavedVisInstance = ( const getSavedVisInstance = async () => { try { let savedVisInstance: SavedVisInstance; - if (history.location.pathname === '/create') { const searchParams = parse(history.location.search); const visTypes = services.visualizations.all(); diff --git a/src/plugins/visualize/public/application/utils/use/use_vis_byvalue.ts b/src/plugins/visualize/public/application/utils/use/use_vis_byvalue.ts new file mode 100644 index 0000000000000..9e119a575825a --- /dev/null +++ b/src/plugins/visualize/public/application/utils/use/use_vis_byvalue.ts @@ -0,0 +1,94 @@ +/* + * 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 { EventEmitter } from 'events'; +import { useEffect, useRef, useState } from 'react'; +import { VisualizeInput } from 'src/plugins/visualizations/public'; +import { IEditorController, SavedVisInstance, VisualizeServices } from '../../types'; +import { getVisualizationInstanceFromInput } from '../get_visualization_instance'; +import { getEditBreadcrumbs } from '../breadcrumbs'; +import { DefaultEditorController } from '../../../../../vis_default_editor/public'; + +export const useVisByValue = ( + services: VisualizeServices, + eventEmitter: EventEmitter, + isChromeVisible: boolean | undefined, + valueInput?: VisualizeInput +) => { + const [state, setState] = useState<{ + savedVisInstance?: SavedVisInstance; + visEditorController?: IEditorController; + }>({}); + const visEditorRef = useRef(null); + const loaded = useRef(false); + useEffect(() => { + const { chrome } = services; + const getVisInstance = async () => { + if (!valueInput || loaded.current) { + return; + } + const savedVisInstance = await getVisualizationInstanceFromInput(services, valueInput); + const { embeddableHandler, vis } = savedVisInstance; + const Editor = vis.type.editor || DefaultEditorController; + const visEditorController = new Editor( + visEditorRef.current, + vis, + eventEmitter, + embeddableHandler + ); + + if (chrome) { + chrome.setBreadcrumbs(getEditBreadcrumbs()); + } + + loaded.current = true; + setState({ + savedVisInstance, + visEditorController, + }); + }; + + getVisInstance(); + }, [ + eventEmitter, + isChromeVisible, + services, + state.savedVisInstance, + state.visEditorController, + valueInput, + ]); + + useEffect(() => { + return () => { + if (state.visEditorController) { + state.visEditorController.destroy(); + } else if (state.savedVisInstance?.embeddableHandler) { + state.savedVisInstance.embeddableHandler.destroy(); + } + if (state.savedVisInstance?.savedVis) { + state.savedVisInstance.savedVis.destroy(); + } + }; + }, [state]); + + return { + ...state, + visEditorRef, + }; +}; diff --git a/src/plugins/visualize/public/application/utils/use/use_visualize_app_state.tsx b/src/plugins/visualize/public/application/utils/use/use_visualize_app_state.tsx index e4d891472fbfd..caee64dc99153 100644 --- a/src/plugins/visualize/public/application/utils/use/use_visualize_app_state.tsx +++ b/src/plugins/visualize/public/application/utils/use/use_visualize_app_state.tsx @@ -37,6 +37,7 @@ import { VisualizeConstants } from '../../visualize_constants'; export const useVisualizeAppState = ( services: VisualizeServices, eventEmitter: EventEmitter, + byValue: boolean, instance?: SavedVisInstance ) => { const [hasUnappliedChanges, setHasUnappliedChanges] = useState(false); @@ -49,6 +50,7 @@ export const useVisualizeAppState = ( const { stateContainer, stopStateSync } = createVisualizeAppState({ stateDefaults, kbnUrlStateStorage: services.kbnUrlStateStorage, + byValue, }); const onDirtyStateChange = ({ isDirty }: { isDirty: boolean }) => { @@ -114,7 +116,7 @@ export const useVisualizeAppState = ( stopSyncingAppFilters(); }; } - }, [eventEmitter, instance, services]); + }, [eventEmitter, instance, services, byValue]); return { appState, hasUnappliedChanges }; }; diff --git a/src/plugins/visualize/public/application/utils/utils.ts b/src/plugins/visualize/public/application/utils/utils.ts index 9f32da3f785b5..c470509827037 100644 --- a/src/plugins/visualize/public/application/utils/utils.ts +++ b/src/plugins/visualize/public/application/utils/utils.ts @@ -66,6 +66,6 @@ export const visStateToEditorState = ( query: vis.data.searchSource?.getOwnField('query') || getDefaultQuery(services), filters: (vis.data.searchSource?.getOwnField('filter') as Filter[]) || [], vis: { ...savedVisState.visState, title: vis.title }, - linked: !!savedVis.savedSearchId, + linked: savedVis ? !!savedVis.savedSearchId : !!savedVisState.savedSearchId, }; }; diff --git a/src/plugins/visualize/public/application/visualize_constants.ts b/src/plugins/visualize/public/application/visualize_constants.ts index adcf27f17dc25..1950fff2733d4 100644 --- a/src/plugins/visualize/public/application/visualize_constants.ts +++ b/src/plugins/visualize/public/application/visualize_constants.ts @@ -25,4 +25,5 @@ export const VisualizeConstants = { WIZARD_STEP_2_PAGE_PATH: '/new/configure', CREATE_PATH: '/create', EDIT_PATH: '/edit', + EDIT_BY_VALUE_PATH: '/edit_by_value', }; From f5061c7c5139da7de62e8ede2e12fe38b45cab2f Mon Sep 17 00:00:00 2001 From: Maja Grubic Date: Fri, 17 Jul 2020 13:10:46 +0100 Subject: [PATCH 02/13] Adding missing null check --- src/plugins/visualize/config.ts | 2 +- .../public/application/components/visualize_top_nav.tsx | 3 +++ src/plugins/visualize/public/application/utils/utils.ts | 3 ++- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/plugins/visualize/config.ts b/src/plugins/visualize/config.ts index 6f01c8d1d5e8b..ee79a37717f26 100644 --- a/src/plugins/visualize/config.ts +++ b/src/plugins/visualize/config.ts @@ -20,7 +20,7 @@ import { schema, TypeOf } from '@kbn/config-schema'; export const configSchema = schema.object({ - showNewVisualizeFlow: schema.boolean({ defaultValue: true }), + showNewVisualizeFlow: schema.boolean({ defaultValue: false }), }); export type ConfigSchema = TypeOf; diff --git a/src/plugins/visualize/public/application/components/visualize_top_nav.tsx b/src/plugins/visualize/public/application/components/visualize_top_nav.tsx index b3950fabac7d6..5a8d7ff7a8e81 100644 --- a/src/plugins/visualize/public/application/components/visualize_top_nav.tsx +++ b/src/plugins/visualize/public/application/components/visualize_top_nav.tsx @@ -43,6 +43,7 @@ interface VisualizeTopNavProps { savedVisInstance: SavedVisInstance; stateContainer: VisualizeAppStateContainer; visualizationIdFromUrl?: string; + embeddableId?: string; } const TopNav = ({ @@ -56,6 +57,7 @@ const TopNav = ({ savedVisInstance, stateContainer, visualizationIdFromUrl, + embeddableId, }: VisualizeTopNavProps) => { const { services } = useKibana(); const { TopNavMenu } = services.navigation.ui; @@ -89,6 +91,7 @@ const TopNav = ({ savedVisInstance, stateContainer, visualizationIdFromUrl, + embeddableId, }, services ); diff --git a/src/plugins/visualize/public/application/utils/utils.ts b/src/plugins/visualize/public/application/utils/utils.ts index c470509827037..f8a724503d409 100644 --- a/src/plugins/visualize/public/application/utils/utils.ts +++ b/src/plugins/visualize/public/application/utils/utils.ts @@ -62,7 +62,8 @@ export const visStateToEditorState = ( ) => { const savedVisState = services.visualizations.convertFromSerializedVis(vis.serialize()); return { - uiState: savedVis.uiStateJSON ? JSON.parse(savedVis.uiStateJSON) : vis.uiState.toJSON(), + uiState: + savedVis && savedVis.uiStateJSON ? JSON.parse(savedVis.uiStateJSON) : vis.uiState.toJSON(), query: vis.data.searchSource?.getOwnField('query') || getDefaultQuery(services), filters: (vis.data.searchSource?.getOwnField('filter') as Filter[]) || [], vis: { ...savedVisState.visState, title: vis.title }, From 8d334c04a568daa3cd4e88b832d6e7fc76118018 Mon Sep 17 00:00:00 2001 From: Maja Grubic Date: Mon, 20 Jul 2020 14:12:44 +0100 Subject: [PATCH 03/13] Making typescript play nicely --- .../application/dashboard_app_controller.tsx | 7 +++-- .../public/lib/state_transfer/types.ts | 1 + .../components/visualize_byvalue_editor.tsx | 14 ++++----- .../components/visualize_editor.tsx | 3 +- .../components/visualize_editor_common.tsx | 29 ++++++++++------- .../components/visualize_top_nav.tsx | 16 +++++----- .../visualize/public/application/types.ts | 8 +++++ .../application/utils/get_top_nav_config.tsx | 31 +++++++++++++------ .../utils/get_visualization_instance.ts | 6 +--- .../utils/use/use_editor_updates.ts | 21 +++++-------- .../utils/use/use_linked_search_updates.ts | 21 ++++++------- .../application/utils/use/use_vis_byvalue.ts | 19 +++++------- .../utils/use/use_visualize_app_state.tsx | 14 +++++---- .../public/application/utils/utils.ts | 8 +++-- 14 files changed, 107 insertions(+), 91 deletions(-) diff --git a/src/plugins/dashboard/public/application/dashboard_app_controller.tsx b/src/plugins/dashboard/public/application/dashboard_app_controller.tsx index a8dc285a29e3c..3b81dff7681df 100644 --- a/src/plugins/dashboard/public/application/dashboard_app_controller.tsx +++ b/src/plugins/dashboard/public/application/dashboard_app_controller.tsx @@ -61,6 +61,7 @@ import { ContainerOutput, EmbeddableInput, SavedObjectEmbeddableInput, + EmbeddablePackageState, } from '../../../embeddable/public'; import { NavAction, SavedDashboardPanel } from '../types'; @@ -429,18 +430,20 @@ export class DashboardAppController { const incomingState = embeddable .getStateTransfer(scopedHistory()) - .getIncomingEmbeddablePackage(); + .getIncomingEmbeddablePackage() as EmbeddablePackageState; if (incomingState) { if ('id' in incomingState) { container.addOrUpdateEmbeddable(incomingState.type, { savedObjectId: incomingState.id, }); } else if ('input' in incomingState) { - const { input, type, embeddableId } = incomingState; + const { input, type } = incomingState; delete input.id; const explicitInput = { savedVis: input, }; + const embeddableId = + 'embeddableId' in incomingState ? incomingState.embeddableId : undefined; container.addOrUpdateEmbeddable(type, explicitInput, embeddableId); } } diff --git a/src/plugins/embeddable/public/lib/state_transfer/types.ts b/src/plugins/embeddable/public/lib/state_transfer/types.ts index 4d487a022be4e..3f3456d914728 100644 --- a/src/plugins/embeddable/public/lib/state_transfer/types.ts +++ b/src/plugins/embeddable/public/lib/state_transfer/types.ts @@ -50,6 +50,7 @@ export interface EmbeddablePackageByReferenceState { export interface EmbeddablePackageByValueState { type: string; input: EmbeddableInput; + embeddableId?: string; } export type EmbeddablePackageState = diff --git a/src/plugins/visualize/public/application/components/visualize_byvalue_editor.tsx b/src/plugins/visualize/public/application/components/visualize_byvalue_editor.tsx index da0b35c6c04de..01ac403cea653 100644 --- a/src/plugins/visualize/public/application/components/visualize_byvalue_editor.tsx +++ b/src/plugins/visualize/public/application/components/visualize_byvalue_editor.tsx @@ -54,7 +54,7 @@ export const VisualizeByValueEditor = () => { const isChromeVisible = useChromeVisibility(services.chrome); - const { savedVisInstance, visEditorRef, visEditorController } = useVisByValue( + const { byValueVisInstance, visEditorRef, visEditorController } = useVisByValue( services, eventEmitter, isChromeVisible, @@ -63,19 +63,17 @@ export const VisualizeByValueEditor = () => { const { appState, hasUnappliedChanges } = useVisualizeAppState( services, eventEmitter, - true, - savedVisInstance + byValueVisInstance ); const { isEmbeddableRendered, currentAppState } = useEditorUpdates( services, eventEmitter, setHasUnsavedChanges, appState, - savedVisInstance, - visEditorController, - true + byValueVisInstance, + visEditorController ); - useLinkedSearchUpdates(services, eventEmitter, appState, savedVisInstance); + useLinkedSearchUpdates(services, eventEmitter, appState, byValueVisInstance); useEffect(() => { // clean up all registered listeners if any is left @@ -86,7 +84,7 @@ export const VisualizeByValueEditor = () => { return ( { const { appState, hasUnappliedChanges } = useVisualizeAppState( services, eventEmitter, - false, savedVisInstance ); const { isEmbeddableRendered, currentAppState } = useEditorUpdates( @@ -80,7 +79,7 @@ export const VisualizeEditor = () => { return ( { return ( -
- {savedVisInstance && appState && currentAppState && ( +
+ {visInstance && appState && currentAppState && ( )} - {savedVisInstance?.vis?.type?.isExperimental && } - {savedVisInstance && ( + {visInstance?.vis?.type?.isExperimental && } + {visInstance && (

- {savedVisInstance.savedVis ? ( + {'savedVis' in visInstance && visInstance.savedVis.id ? ( ) : ( @@ -88,7 +93,7 @@ export const VisualizeEditorCommon = ({ id="visualize.byValue_pageHeading" defaultMessage="Visualization of type {chartType} embedded into {originatingApp} app" values={{ - chartType: savedVisInstance.vis.type.title, + chartType: visInstance.vis.type.title, originatingApp: originatingApp || 'dashboards', }} /> diff --git a/src/plugins/visualize/public/application/components/visualize_top_nav.tsx b/src/plugins/visualize/public/application/components/visualize_top_nav.tsx index 5a8d7ff7a8e81..6ceb047335d2f 100644 --- a/src/plugins/visualize/public/application/components/visualize_top_nav.tsx +++ b/src/plugins/visualize/public/application/components/visualize_top_nav.tsx @@ -27,7 +27,7 @@ import { VisualizeServices, VisualizeAppState, VisualizeAppStateContainer, - SavedVisInstance, + VisualizeEditorVisInstance, } from '../types'; import { APP_NAME } from '../visualize_constants'; import { getTopNavConfig } from '../utils'; @@ -40,7 +40,7 @@ interface VisualizeTopNavProps { setHasUnsavedChanges: (value: boolean) => void; hasUnappliedChanges: boolean; originatingApp?: string; - savedVisInstance: SavedVisInstance; + visInstance: VisualizeEditorVisInstance; stateContainer: VisualizeAppStateContainer; visualizationIdFromUrl?: string; embeddableId?: string; @@ -54,14 +54,14 @@ const TopNav = ({ setHasUnsavedChanges, hasUnappliedChanges, originatingApp, - savedVisInstance, + visInstance, stateContainer, visualizationIdFromUrl, embeddableId, }: VisualizeTopNavProps) => { const { services } = useKibana(); const { TopNavMenu } = services.navigation.ui; - const { embeddableHandler, vis } = savedVisInstance; + const { embeddableHandler, vis } = visInstance; const [inspectorSession, setInspectorSession] = useState(); const openInspector = useCallback(() => { const session = embeddableHandler.openInspector(); @@ -73,10 +73,10 @@ const TopNav = ({ if (!isEqual(currentAppState.query, query)) { stateContainer.transitions.set('query', query || currentAppState.query); } else { - savedVisInstance.embeddableHandler.reload(); + visInstance.embeddableHandler.reload(); } }, - [currentAppState.query, savedVisInstance.embeddableHandler, stateContainer.transitions] + [currentAppState.query, visInstance.embeddableHandler, stateContainer.transitions] ); const config = useMemo(() => { @@ -88,7 +88,7 @@ const TopNav = ({ hasUnappliedChanges, openInspector, originatingApp, - savedVisInstance, + visInstance, stateContainer, visualizationIdFromUrl, embeddableId, @@ -103,7 +103,7 @@ const TopNav = ({ hasUnappliedChanges, openInspector, originatingApp, - savedVisInstance, + visInstance, stateContainer, visualizationIdFromUrl, services, diff --git a/src/plugins/visualize/public/application/types.ts b/src/plugins/visualize/public/application/types.ts index 02ae1cc155dd2..65b88485b2f06 100644 --- a/src/plugins/visualize/public/application/types.ts +++ b/src/plugins/visualize/public/application/types.ts @@ -121,6 +121,14 @@ export interface SavedVisInstance { embeddableHandler: VisualizeEmbeddableContract; } +export interface ByValueVisInstance { + vis: Vis; + savedSearch?: SavedObject; + embeddableHandler: VisualizeEmbeddableContract; +} + +export type VisualizeEditorVisInstance = SavedVisInstance | ByValueVisInstance; + export interface IEditorController { render(props: EditorRenderProps): void; destroy(): void; diff --git a/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx b/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx index ba7b2dc6b0678..f6a319e0993b7 100644 --- a/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx +++ b/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx @@ -31,16 +31,21 @@ import { } from '../../../../saved_objects/public'; import { unhashUrl } from '../../../../kibana_utils/public'; -import { SavedVisInstance, VisualizeServices, VisualizeAppStateContainer } from '../types'; +import { + VisualizeServices, + VisualizeAppStateContainer, + VisualizeEditorVisInstance, +} from '../types'; import { VisualizeConstants } from '../visualize_constants'; import { getEditBreadcrumbs } from './breadcrumbs'; + interface TopNavConfigParams { hasUnsavedChanges: boolean; setHasUnsavedChanges: (value: boolean) => void; openInspector: () => void; originatingApp?: string; hasUnappliedChanges: boolean; - savedVisInstance: SavedVisInstance; + visInstance: VisualizeEditorVisInstance; stateContainer: VisualizeAppStateContainer; visualizationIdFromUrl?: string; embeddableId?: string; @@ -53,7 +58,7 @@ export const getTopNavConfig = ( openInspector, originatingApp, hasUnappliedChanges, - savedVisInstance: { embeddableHandler, savedVis, vis }, + visInstance, stateContainer, visualizationIdFromUrl, embeddableId, @@ -71,10 +76,15 @@ export const getTopNavConfig = ( featureFlagConfig, }: VisualizeServices ) => { + const { vis, embeddableHandler } = visInstance; + const savedVis = 'savedVis' in visInstance ? visInstance.savedVis : undefined; /** * Called when the user clicks "Save" button. */ async function doSave(saveOptions: SavedObjectSaveOpts) { + if (!savedVis) { + return {}; + } const newlyCreated = !Boolean(savedVis.id) || savedVis.copyOnSave; // vis.title was not bound and it's needed to reflect title into visState stateContainer.transitions.setVis({ @@ -240,6 +250,9 @@ export const getTopNavConfig = ( newDescription, returnToOrigin, }: OnSaveProps & { returnToOrigin: boolean }) => { + if (!savedVis) { + return; + } const currentTitle = savedVis.title; savedVis.title = newTitle; savedVis.copyOnSave = newCopyOnSave; @@ -259,7 +272,7 @@ export const getTopNavConfig = ( }; const saveModal = ( {}} @@ -268,7 +281,7 @@ export const getTopNavConfig = ( ); if (originatingApp === 'dashboards' && featureFlagConfig.showNewVisualizeFlow) { createVisReference(); - } else { + } else if (savedVis) { showSaveModal(saveModal, I18nContext); } }, @@ -285,23 +298,23 @@ export const getTopNavConfig = ( }), testId: 'shareTopNavButton', run: (anchorElement) => { - if (share && savedVis) { + if (share && savedVis && savedVis.id) { share.toggleShareContextMenu({ anchorElement, allowEmbed: true, allowShortUrl: visualizeCapabilities.createShortUrl, shareableUrl: unhashUrl(window.location.href), - objectId: savedVis.id, + objectId: savedVis?.id, objectType: 'visualization', sharingData: { - title: savedVis.title, + title: savedVis?.title, }, isDirty: hasUnappliedChanges || hasUnsavedChanges, }); } }, // disable the Share button if no action specified - disableButton: !share || embeddableId, + disableButton: (!share || embeddableId) as boolean, }, { id: 'inspector', diff --git a/src/plugins/visualize/public/application/utils/get_visualization_instance.ts b/src/plugins/visualize/public/application/utils/get_visualization_instance.ts index e145da9e47f44..3ffca578f8052 100644 --- a/src/plugins/visualize/public/application/utils/get_visualization_instance.ts +++ b/src/plugins/visualize/public/application/utils/get_visualization_instance.ts @@ -76,14 +76,10 @@ const createVisualizeEmbeddableAndLinkSavedSearch = async ( export const getVisualizationInstanceFromInput = async ( visualizeServices: VisualizeServices, - input?: VisualizeInput + input: VisualizeInput ) => { - if (!input) { - return; - } const { visualizations } = visualizeServices; const visState = input.savedVis as SerializedVis; - let vis = await visualizations.createVis(visState.type, cloneDeep(visState)); if (vis.type.setup) { try { diff --git a/src/plugins/visualize/public/application/utils/use/use_editor_updates.ts b/src/plugins/visualize/public/application/utils/use/use_editor_updates.ts index 10a9b777cfbce..d3e93cb82bd7f 100644 --- a/src/plugins/visualize/public/application/utils/use/use_editor_updates.ts +++ b/src/plugins/visualize/public/application/utils/use/use_editor_updates.ts @@ -27,8 +27,8 @@ import { VisualizeServices, VisualizeAppState, VisualizeAppStateContainer, - SavedVisInstance, IEditorController, + VisualizeEditorVisInstance, } from '../../types'; export const useEditorUpdates = ( @@ -36,20 +36,20 @@ export const useEditorUpdates = ( eventEmitter: EventEmitter, setHasUnsavedChanges: (value: boolean) => void, appState: VisualizeAppStateContainer | null, - savedVisInstance: SavedVisInstance | undefined, - visEditorController: IEditorController | undefined, - byValue?: boolean + visInstance: VisualizeEditorVisInstance | undefined, + visEditorController: IEditorController | undefined ) => { const [isEmbeddableRendered, setIsEmbeddableRendered] = useState(false); const [currentAppState, setCurrentAppState] = useState(); useEffect(() => { - if (appState && savedVisInstance) { + if (appState && visInstance) { const { timefilter: { timefilter }, filterManager, } = services.data.query; - const { embeddableHandler, savedVis, savedSearch, vis } = savedVisInstance; + const { embeddableHandler, savedSearch, vis } = visInstance; + const savedVis = 'savedVis' in visInstance ? visInstance.savedVis : undefined; const initialState = appState.getState(); setCurrentAppState(initialState); @@ -172,14 +172,7 @@ export const useEditorUpdates = ( unsubscribeStateUpdates(); }; } - }, [ - appState, - eventEmitter, - savedVisInstance, - services, - setHasUnsavedChanges, - visEditorController, - ]); + }, [appState, eventEmitter, visInstance, services, setHasUnsavedChanges, visEditorController]); return { isEmbeddableRendered, currentAppState }; }; diff --git a/src/plugins/visualize/public/application/utils/use/use_linked_search_updates.ts b/src/plugins/visualize/public/application/utils/use/use_linked_search_updates.ts index e257b72ee751b..7bc38ba6e2842 100644 --- a/src/plugins/visualize/public/application/utils/use/use_linked_search_updates.ts +++ b/src/plugins/visualize/public/application/utils/use/use_linked_search_updates.ts @@ -22,24 +22,23 @@ import { i18n } from '@kbn/i18n'; import { EventEmitter } from 'events'; import { Filter } from 'src/plugins/data/public'; -import { VisualizeServices, VisualizeAppStateContainer, SavedVisInstance } from '../../types'; +import { + VisualizeServices, + VisualizeAppStateContainer, + VisualizeEditorVisInstance, +} from '../../types'; export const useLinkedSearchUpdates = ( services: VisualizeServices, eventEmitter: EventEmitter, appState: VisualizeAppStateContainer | null, - savedVisInstance: SavedVisInstance | undefined + visInstance: VisualizeEditorVisInstance | undefined ) => { useEffect(() => { - if ( - appState && - savedVisInstance && - savedVisInstance.savedSearch && - savedVisInstance.vis.data.searchSource - ) { - const { savedSearch } = savedVisInstance; + if (appState && visInstance && visInstance.savedSearch && visInstance.vis.data.searchSource) { + const { savedSearch } = visInstance; // SearchSource is a promise-based stream of search results that can inherit from other search sources. - const { searchSource } = savedVisInstance.vis.data; + const { searchSource } = visInstance.vis.data; const unlinkFromSavedSearch = () => { const searchSourceParent = savedSearch.searchSource; @@ -70,5 +69,5 @@ export const useLinkedSearchUpdates = ( eventEmitter.off('unlinkFromSavedSearch', unlinkFromSavedSearch); }; } - }, [appState, eventEmitter, savedVisInstance, services.toastNotifications]); + }, [appState, eventEmitter, visInstance, services.toastNotifications]); }; diff --git a/src/plugins/visualize/public/application/utils/use/use_vis_byvalue.ts b/src/plugins/visualize/public/application/utils/use/use_vis_byvalue.ts index 9e119a575825a..d743d9540265a 100644 --- a/src/plugins/visualize/public/application/utils/use/use_vis_byvalue.ts +++ b/src/plugins/visualize/public/application/utils/use/use_vis_byvalue.ts @@ -20,7 +20,7 @@ import { EventEmitter } from 'events'; import { useEffect, useRef, useState } from 'react'; import { VisualizeInput } from 'src/plugins/visualizations/public'; -import { IEditorController, SavedVisInstance, VisualizeServices } from '../../types'; +import { ByValueVisInstance, IEditorController, VisualizeServices } from '../../types'; import { getVisualizationInstanceFromInput } from '../get_visualization_instance'; import { getEditBreadcrumbs } from '../breadcrumbs'; import { DefaultEditorController } from '../../../../../vis_default_editor/public'; @@ -32,7 +32,7 @@ export const useVisByValue = ( valueInput?: VisualizeInput ) => { const [state, setState] = useState<{ - savedVisInstance?: SavedVisInstance; + byValueVisInstance?: ByValueVisInstance; visEditorController?: IEditorController; }>({}); const visEditorRef = useRef(null); @@ -43,8 +43,8 @@ export const useVisByValue = ( if (!valueInput || loaded.current) { return; } - const savedVisInstance = await getVisualizationInstanceFromInput(services, valueInput); - const { embeddableHandler, vis } = savedVisInstance; + const byValueVisInstance = await getVisualizationInstanceFromInput(services, valueInput); + const { embeddableHandler, vis } = byValueVisInstance; const Editor = vis.type.editor || DefaultEditorController; const visEditorController = new Editor( visEditorRef.current, @@ -59,7 +59,7 @@ export const useVisByValue = ( loaded.current = true; setState({ - savedVisInstance, + byValueVisInstance, visEditorController, }); }; @@ -69,7 +69,7 @@ export const useVisByValue = ( eventEmitter, isChromeVisible, services, - state.savedVisInstance, + state.byValueVisInstance, state.visEditorController, valueInput, ]); @@ -78,11 +78,8 @@ export const useVisByValue = ( return () => { if (state.visEditorController) { state.visEditorController.destroy(); - } else if (state.savedVisInstance?.embeddableHandler) { - state.savedVisInstance.embeddableHandler.destroy(); - } - if (state.savedVisInstance?.savedVis) { - state.savedVisInstance.savedVis.destroy(); + } else if (state.byValueVisInstance?.embeddableHandler) { + state.byValueVisInstance.embeddableHandler.destroy(); } }; }, [state]); diff --git a/src/plugins/visualize/public/application/utils/use/use_visualize_app_state.tsx b/src/plugins/visualize/public/application/utils/use/use_visualize_app_state.tsx index caee64dc99153..ff073d45430d4 100644 --- a/src/plugins/visualize/public/application/utils/use/use_visualize_app_state.tsx +++ b/src/plugins/visualize/public/application/utils/use/use_visualize_app_state.tsx @@ -25,11 +25,14 @@ import { i18n } from '@kbn/i18n'; import { MarkdownSimple, toMountPoint } from '../../../../../kibana_react/public'; import { esFilters, connectToQueryState } from '../../../../../data/public'; -import { VisualizeServices, VisualizeAppStateContainer, SavedVisInstance } from '../../types'; +import { + VisualizeServices, + VisualizeAppStateContainer, + VisualizeEditorVisInstance, +} from '../../types'; import { visStateToEditorState } from '../utils'; import { createVisualizeAppState } from '../create_visualize_app_state'; import { VisualizeConstants } from '../../visualize_constants'; - /** * This effect is responsible for instantiating the visualize app state container, * which is in sync with "_a" url param @@ -37,8 +40,7 @@ import { VisualizeConstants } from '../../visualize_constants'; export const useVisualizeAppState = ( services: VisualizeServices, eventEmitter: EventEmitter, - byValue: boolean, - instance?: SavedVisInstance + instance?: VisualizeEditorVisInstance ) => { const [hasUnappliedChanges, setHasUnappliedChanges] = useState(false); const [appState, setAppState] = useState(null); @@ -46,7 +48,7 @@ export const useVisualizeAppState = ( useEffect(() => { if (instance) { const stateDefaults = visStateToEditorState(instance, services); - + const byValue = !('savedVis' in instance); const { stateContainer, stopStateSync } = createVisualizeAppState({ stateDefaults, kbnUrlStateStorage: services.kbnUrlStateStorage, @@ -116,7 +118,7 @@ export const useVisualizeAppState = ( stopSyncingAppFilters(); }; } - }, [eventEmitter, instance, services, byValue]); + }, [eventEmitter, instance, services]); return { appState, hasUnappliedChanges }; }; diff --git a/src/plugins/visualize/public/application/utils/utils.ts b/src/plugins/visualize/public/application/utils/utils.ts index f8a724503d409..c31aa4ecda771 100644 --- a/src/plugins/visualize/public/application/utils/utils.ts +++ b/src/plugins/visualize/public/application/utils/utils.ts @@ -21,7 +21,7 @@ import { i18n } from '@kbn/i18n'; import { ChromeStart, DocLinksStart } from 'kibana/public'; import { Filter, UI_SETTINGS } from '../../../../data/public'; -import { VisualizeServices, SavedVisInstance } from '../types'; +import { VisualizeServices, VisualizeEditorVisInstance } from '../types'; export const addHelpMenuToAppChrome = (chrome: ChromeStart, docLinks: DocLinksStart) => { chrome.setHelpExtension({ @@ -57,16 +57,18 @@ export const getDefaultQuery = ({ localStorage, uiSettings }: VisualizeServices) }); export const visStateToEditorState = ( - { vis, savedVis }: SavedVisInstance, + visInstance: VisualizeEditorVisInstance, services: VisualizeServices ) => { + const vis = visInstance.vis; const savedVisState = services.visualizations.convertFromSerializedVis(vis.serialize()); + const savedVis = 'savedVis' in visInstance ? visInstance.savedVis : undefined; return { uiState: savedVis && savedVis.uiStateJSON ? JSON.parse(savedVis.uiStateJSON) : vis.uiState.toJSON(), query: vis.data.searchSource?.getOwnField('query') || getDefaultQuery(services), filters: (vis.data.searchSource?.getOwnField('filter') as Filter[]) || [], vis: { ...savedVisState.visState, title: vis.title }, - linked: savedVis ? !!savedVis.savedSearchId : !!savedVisState.savedSearchId, + linked: savedVis && savedVis.id ? !!savedVis.savedSearchId : !!savedVisState.savedSearchId, }; }; From 6295a084cf456273734341aa1bcefd0f175a5bb9 Mon Sep 17 00:00:00 2001 From: Maja Grubic Date: Mon, 20 Jul 2020 18:48:18 +0100 Subject: [PATCH 04/13] Fixing failing tests --- .../public/application/utils/get_top_nav_config.tsx | 5 +++-- .../application/utils/use/use_visualize_app_state.test.ts | 1 + 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx b/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx index f6a319e0993b7..42204a740069d 100644 --- a/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx +++ b/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx @@ -225,7 +225,7 @@ export const getTopNavConfig = ( : i18n.translate('visualize.topNavMenu.saveVisualizationButtonLabel', { defaultMessage: 'save', }), - emphasize: !savedVis || !savedVis.id || !originatingApp, + emphasize: (savedVis && !savedVis.id) || !originatingApp, description: i18n.translate('visualize.topNavMenu.saveVisualizationButtonAriaLabel', { defaultMessage: 'Save Visualization', }), @@ -298,7 +298,8 @@ export const getTopNavConfig = ( }), testId: 'shareTopNavButton', run: (anchorElement) => { - if (share && savedVis && savedVis.id) { + if (share && !embeddableId) { + // TODO: support sharing in by-value mode share.toggleShareContextMenu({ anchorElement, allowEmbed: true, diff --git a/src/plugins/visualize/public/application/utils/use/use_visualize_app_state.test.ts b/src/plugins/visualize/public/application/utils/use/use_visualize_app_state.test.ts index e885067c58184..79e3aca64e8ae 100644 --- a/src/plugins/visualize/public/application/utils/use/use_visualize_app_state.test.ts +++ b/src/plugins/visualize/public/application/utils/use/use_visualize_app_state.test.ts @@ -90,6 +90,7 @@ describe('useVisualizeAppState', () => { expect(createVisualizeAppState).toHaveBeenCalledWith({ stateDefaults: visualizeAppStateStub, kbnUrlStateStorage: undefined, + byValue: false, }); expect(mockServices.data.query.filterManager.setAppFilters).toHaveBeenCalledWith( visualizeAppStateStub.filters From 8e871ce11a6314d8075774a754e3de96ddd02c3c Mon Sep 17 00:00:00 2001 From: Maja Grubic Date: Mon, 10 Aug 2020 13:51:00 +0100 Subject: [PATCH 05/13] Applying PR comments --- .../visualize/public/application/utils/breadcrumbs.ts | 9 +++++---- .../public/application/utils/get_top_nav_config.tsx | 6 +++--- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/plugins/visualize/public/application/utils/breadcrumbs.ts b/src/plugins/visualize/public/application/utils/breadcrumbs.ts index 7197b3dc35c1d..fa6b74e25694f 100644 --- a/src/plugins/visualize/public/application/utils/breadcrumbs.ts +++ b/src/plugins/visualize/public/application/utils/breadcrumbs.ts @@ -21,6 +21,10 @@ import { i18n } from '@kbn/i18n'; import { VisualizeConstants } from '../visualize_constants'; +const defaultEditText = i18n.translate('visualize.editor.defaultEditBreadcrumbText', { + defaultMessage: 'Edit', +}); + export function getLandingBreadcrumbs() { return [ { @@ -43,10 +47,7 @@ export function getCreateBreadcrumbs() { ]; } -export function getEditBreadcrumbs(text?: string) { - if (!text) { - return [...getLandingBreadcrumbs(), { text: 'Edit' }]; - } +export function getEditBreadcrumbs(text: string = defaultEditText) { return [ ...getLandingBreadcrumbs(), { diff --git a/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx b/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx index aff93d2321529..bef4e78cd5e9d 100644 --- a/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx +++ b/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx @@ -178,7 +178,7 @@ export const getTopNavConfig = ( }; const topNavMenu: TopNavMenuData[] = [ - ...(originatingApp && ((savedVis && savedVis.id) || embeddableId) + ...(originatingApp && (savedVis?.id || embeddableId) ? [ { id: 'saveAndReturn', @@ -223,7 +223,7 @@ export const getTopNavConfig = ( { id: 'save', label: - savedVis && savedVis.id && originatingApp + savedVis?.id && originatingApp ? i18n.translate('visualize.topNavMenu.saveVisualizationAsButtonLabel', { defaultMessage: 'save as', }) @@ -320,7 +320,7 @@ export const getTopNavConfig = ( } }, // disable the Share button if no action specified - disableButton: (!share || embeddableId) as boolean, + disableButton: !share || !!embeddableId }, { id: 'inspector', From 09a8f9b4269dee572397a4920dccd880d669644c Mon Sep 17 00:00:00 2001 From: Maja Grubic Date: Mon, 10 Aug 2020 14:59:18 +0100 Subject: [PATCH 06/13] Fixing eslint errors --- .../dashboard/public/application/dashboard_app_controller.tsx | 4 +++- .../visualize/public/application/utils/get_top_nav_config.tsx | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/plugins/dashboard/public/application/dashboard_app_controller.tsx b/src/plugins/dashboard/public/application/dashboard_app_controller.tsx index 6ad3e49928300..7a19514eebe17 100644 --- a/src/plugins/dashboard/public/application/dashboard_app_controller.tsx +++ b/src/plugins/dashboard/public/application/dashboard_app_controller.tsx @@ -469,7 +469,9 @@ export class DashboardAppController { savedVis: input, }; const embeddableId = - 'embeddableId' in incomingEmbeddable ? incomingEmbeddable.embeddableId : undefined; + 'embeddableId' in incomingEmbeddable + ? incomingEmbeddable.embeddableId + : undefined; container.addOrUpdateEmbeddable( incomingEmbeddable.type, explicitInput, diff --git a/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx b/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx index bef4e78cd5e9d..c99128889f024 100644 --- a/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx +++ b/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx @@ -320,7 +320,7 @@ export const getTopNavConfig = ( } }, // disable the Share button if no action specified - disableButton: !share || !!embeddableId + disableButton: !share || !!embeddableId, }, { id: 'inspector', From a201c2a6c3e974292aa28faabc260616e12bcf3d Mon Sep 17 00:00:00 2001 From: Maja Grubic Date: Tue, 11 Aug 2020 14:11:36 +0100 Subject: [PATCH 07/13] Fix save as behavior --- .../public/top_nav_menu/top_nav_menu_item.tsx | 1 + .../public/application/utils/get_top_nav_config.tsx | 10 ++++++++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/plugins/navigation/public/top_nav_menu/top_nav_menu_item.tsx b/src/plugins/navigation/public/top_nav_menu/top_nav_menu_item.tsx index 1c5642f9b75b7..b058ef0de448b 100644 --- a/src/plugins/navigation/public/top_nav_menu/top_nav_menu_item.tsx +++ b/src/plugins/navigation/public/top_nav_menu/top_nav_menu_item.tsx @@ -46,6 +46,7 @@ export function TopNavMenuItem(props: TopNavMenuData) { iconType: props.iconType, iconSide: props.iconSide, 'data-test-subj': props.testId, + className: props.className, }; const btn = props.emphasize ? ( diff --git a/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx b/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx index c99128889f024..0310d3d061b12 100644 --- a/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx +++ b/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx @@ -234,6 +234,7 @@ export const getTopNavConfig = ( description: i18n.translate('visualize.topNavMenu.saveVisualizationButtonAriaLabel', { defaultMessage: 'Save Visualization', }), + className: savedVis?.id && originatingApp ? 'saveAsButton' : '', testId: 'visualizeSaveButton', disableButton: hasUnappliedChanges, tooltip() { @@ -246,7 +247,7 @@ export const getTopNavConfig = ( ); } }, - run: () => { + run: (anchorElement: HTMLAnchorElement) => { const onSave = async ({ newTitle, newCopyOnSave, @@ -284,7 +285,12 @@ export const getTopNavConfig = ( originatingApp={originatingApp} /> ); - if (originatingApp === 'dashboards' && featureFlagConfig.showNewVisualizeFlow) { + const isSaveAsButton = anchorElement.classList.contains('saveAsButton'); + if ( + originatingApp === 'dashboards' && + featureFlagConfig.showNewVisualizeFlow && + !isSaveAsButton + ) { createVisReference(); } else if (savedVis) { showSaveModal(saveModal, I18nContext); From 24c194873ecff2e758e9d5e17ee9c00b919af0e4 Mon Sep 17 00:00:00 2001 From: Maja Grubic Date: Tue, 11 Aug 2020 14:26:29 +0100 Subject: [PATCH 08/13] Fixing HTMLElement type --- .../visualize/public/application/utils/get_top_nav_config.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx b/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx index 0310d3d061b12..3a59af626fc50 100644 --- a/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx +++ b/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx @@ -247,7 +247,7 @@ export const getTopNavConfig = ( ); } }, - run: (anchorElement: HTMLAnchorElement) => { + run: (anchorElement: HTMLElement) => { const onSave = async ({ newTitle, newCopyOnSave, From d4b69dcb0a899d9996422f50d57040f21239d7b8 Mon Sep 17 00:00:00 2001 From: Maja Grubic Date: Tue, 11 Aug 2020 16:30:45 +0100 Subject: [PATCH 09/13] Passing in setOriginatingApp parameter --- .../public/application/components/visualize_byvalue_editor.tsx | 1 + .../public/application/components/visualize_editor.tsx | 1 + .../public/application/components/visualize_editor_common.tsx | 3 +++ 3 files changed, 5 insertions(+) diff --git a/src/plugins/visualize/public/application/components/visualize_byvalue_editor.tsx b/src/plugins/visualize/public/application/components/visualize_byvalue_editor.tsx index 01ac403cea653..1b7e7bb098009 100644 --- a/src/plugins/visualize/public/application/components/visualize_byvalue_editor.tsx +++ b/src/plugins/visualize/public/application/components/visualize_byvalue_editor.tsx @@ -92,6 +92,7 @@ export const VisualizeByValueEditor = () => { hasUnappliedChanges={hasUnappliedChanges} isEmbeddableRendered={isEmbeddableRendered} originatingApp={originatingApp} + setOriginatingApp={setOriginatingApp} setHasUnsavedChanges={setHasUnsavedChanges} visEditorRef={visEditorRef} embeddableId={embeddableId} diff --git a/src/plugins/visualize/public/application/components/visualize_editor.tsx b/src/plugins/visualize/public/application/components/visualize_editor.tsx index 2d4e29d752c98..0bf5b26e1339f 100644 --- a/src/plugins/visualize/public/application/components/visualize_editor.tsx +++ b/src/plugins/visualize/public/application/components/visualize_editor.tsx @@ -87,6 +87,7 @@ export const VisualizeEditor = () => { hasUnappliedChanges={hasUnappliedChanges} isEmbeddableRendered={isEmbeddableRendered} originatingApp={originatingApp} + setOriginatingApp={setOriginatingApp} visualizationIdFromUrl={visualizationIdFromUrl} setHasUnsavedChanges={setHasUnsavedChanges} visEditorRef={visEditorRef} diff --git a/src/plugins/visualize/public/application/components/visualize_editor_common.tsx b/src/plugins/visualize/public/application/components/visualize_editor_common.tsx index abe711e01fd91..b811936c63b14 100644 --- a/src/plugins/visualize/public/application/components/visualize_editor_common.tsx +++ b/src/plugins/visualize/public/application/components/visualize_editor_common.tsx @@ -40,6 +40,7 @@ interface VisualizeEditorCommonProps { isEmbeddableRendered: boolean; visEditorRef: RefObject; originatingApp?: string; + setOriginatingApp?: (originatingApp: string | undefined) => void; visualizationIdFromUrl?: string; embeddableId?: string; } @@ -54,6 +55,7 @@ export const VisualizeEditorCommon = ({ hasUnappliedChanges, isEmbeddableRendered, originatingApp, + setOriginatingApp, visualizationIdFromUrl, embeddableId, visEditorRef, @@ -69,6 +71,7 @@ export const VisualizeEditorCommon = ({ isEmbeddableRendered={isEmbeddableRendered} hasUnappliedChanges={hasUnappliedChanges} originatingApp={originatingApp} + setOriginatingApp={setOriginatingApp} visInstance={visInstance} stateContainer={appState} visualizationIdFromUrl={visualizationIdFromUrl} From 875be622697b35a5e51a16cddf02f765d0e82823 Mon Sep 17 00:00:00 2001 From: Maja Grubic Date: Wed, 12 Aug 2020 17:14:20 +0100 Subject: [PATCH 10/13] Redirect back to dashboard if input is missing --- .../components/visualize_byvalue_editor.tsx | 6 +++++- .../public/application/utils/breadcrumbs.ts | 13 +++++++++++++ .../public/application/utils/get_top_nav_config.tsx | 2 +- .../public/application/utils/use/use_vis_byvalue.ts | 10 +++++++--- 4 files changed, 26 insertions(+), 5 deletions(-) diff --git a/src/plugins/visualize/public/application/components/visualize_byvalue_editor.tsx b/src/plugins/visualize/public/application/components/visualize_byvalue_editor.tsx index 1b7e7bb098009..a78633d6841e5 100644 --- a/src/plugins/visualize/public/application/components/visualize_byvalue_editor.tsx +++ b/src/plugins/visualize/public/application/components/visualize_byvalue_editor.tsx @@ -50,6 +50,9 @@ export const VisualizeByValueEditor = () => { setOriginatingApp(value); setValueInput(valueInputValue); setEmbeddableId(embeddableIdValue); + if (!valueInputValue) { + history.back(); + } }, [services]); const isChromeVisible = useChromeVisibility(services.chrome); @@ -58,7 +61,8 @@ export const VisualizeByValueEditor = () => { services, eventEmitter, isChromeVisible, - valueInput + valueInput, + originatingApp ); const { appState, hasUnappliedChanges } = useVisualizeAppState( services, diff --git a/src/plugins/visualize/public/application/utils/breadcrumbs.ts b/src/plugins/visualize/public/application/utils/breadcrumbs.ts index fa6b74e25694f..f2dfcbf8bb7dc 100644 --- a/src/plugins/visualize/public/application/utils/breadcrumbs.ts +++ b/src/plugins/visualize/public/application/utils/breadcrumbs.ts @@ -21,6 +21,14 @@ import { i18n } from '@kbn/i18n'; import { VisualizeConstants } from '../visualize_constants'; +const appPrefixes: Record = { + dashboards: { + text: i18n.translate('dashboard.listing.breadcrumb', { + defaultMessage: 'Dashboard', + }), + }, +}; + const defaultEditText = i18n.translate('visualize.editor.defaultEditBreadcrumbText', { defaultMessage: 'Edit', }); @@ -47,6 +55,11 @@ export function getCreateBreadcrumbs() { ]; } +export function getBreadcrumbsPrefixedWithApp(originatingApp: string) { + const originatingAppBreadcrumb = appPrefixes[originatingApp]; + return [originatingAppBreadcrumb, ...getLandingBreadcrumbs(), { text: defaultEditText }]; +} + export function getEditBreadcrumbs(text: string = defaultEditText) { return [ ...getLandingBreadcrumbs(), diff --git a/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx b/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx index 3a59af626fc50..b7c0fe21d31a2 100644 --- a/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx +++ b/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx @@ -178,7 +178,7 @@ export const getTopNavConfig = ( }; const topNavMenu: TopNavMenuData[] = [ - ...(originatingApp && (savedVis?.id || embeddableId) + ...(originatingApp && ((savedVis && savedVis.id) || embeddableId) ? [ { id: 'saveAndReturn', diff --git a/src/plugins/visualize/public/application/utils/use/use_vis_byvalue.ts b/src/plugins/visualize/public/application/utils/use/use_vis_byvalue.ts index d743d9540265a..f2758d0cc01a4 100644 --- a/src/plugins/visualize/public/application/utils/use/use_vis_byvalue.ts +++ b/src/plugins/visualize/public/application/utils/use/use_vis_byvalue.ts @@ -22,14 +22,15 @@ import { useEffect, useRef, useState } from 'react'; import { VisualizeInput } from 'src/plugins/visualizations/public'; import { ByValueVisInstance, IEditorController, VisualizeServices } from '../../types'; import { getVisualizationInstanceFromInput } from '../get_visualization_instance'; -import { getEditBreadcrumbs } from '../breadcrumbs'; +import { getBreadcrumbsPrefixedWithApp, getEditBreadcrumbs } from '../breadcrumbs'; import { DefaultEditorController } from '../../../../../vis_default_editor/public'; export const useVisByValue = ( services: VisualizeServices, eventEmitter: EventEmitter, isChromeVisible: boolean | undefined, - valueInput?: VisualizeInput + valueInput?: VisualizeInput, + originatingApp?: string ) => { const [state, setState] = useState<{ byValueVisInstance?: ByValueVisInstance; @@ -53,7 +54,9 @@ export const useVisByValue = ( embeddableHandler ); - if (chrome) { + if (chrome && originatingApp) { + chrome.setBreadcrumbs(getBreadcrumbsPrefixedWithApp(originatingApp)); + } else if (chrome) { chrome.setBreadcrumbs(getEditBreadcrumbs()); } @@ -72,6 +75,7 @@ export const useVisByValue = ( state.byValueVisInstance, state.visEditorController, valueInput, + originatingApp, ]); useEffect(() => { From cc59f71ed3d8da2741e08c04417d7b5d39bdf3b0 Mon Sep 17 00:00:00 2001 From: Maja Grubic Date: Thu, 13 Aug 2020 09:48:26 +0100 Subject: [PATCH 11/13] Fixing i18n error --- src/plugins/visualize/public/application/utils/breadcrumbs.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/visualize/public/application/utils/breadcrumbs.ts b/src/plugins/visualize/public/application/utils/breadcrumbs.ts index f2dfcbf8bb7dc..a5c246c539c54 100644 --- a/src/plugins/visualize/public/application/utils/breadcrumbs.ts +++ b/src/plugins/visualize/public/application/utils/breadcrumbs.ts @@ -23,7 +23,7 @@ import { VisualizeConstants } from '../visualize_constants'; const appPrefixes: Record = { dashboards: { - text: i18n.translate('dashboard.listing.breadcrumb', { + text: i18n.translate('visualize.dashboard.prefix.breadcrumb', { defaultMessage: 'Dashboard', }), }, From 1741aaeae85cb9c19cf3cc283695e459e784aa6f Mon Sep 17 00:00:00 2001 From: Maja Grubic Date: Thu, 13 Aug 2020 14:39:33 +0100 Subject: [PATCH 12/13] Unlink saved search --- .../public/application/utils/use/use_editor_updates.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/plugins/visualize/public/application/utils/use/use_editor_updates.ts b/src/plugins/visualize/public/application/utils/use/use_editor_updates.ts index 626b4a8e70d51..c29f6337a6246 100644 --- a/src/plugins/visualize/public/application/utils/use/use_editor_updates.ts +++ b/src/plugins/visualize/public/application/utils/use/use_editor_updates.ts @@ -89,8 +89,9 @@ export const useEditorUpdates = ( } else if (!linked && savedVis && savedVis.savedSearchId) { delete savedVis.savedSearchId; delete vis.data.savedSearchId; - } else { - // TODO: something to do for when it's not a saved vis? + } else if (!linked && !savedVis) { + // delete link when it's not a saved vis + delete vis.data.savedSearchId; } }; From 228d007393cad3ef856504ef81030d5bfe5dad2d Mon Sep 17 00:00:00 2001 From: Maja Grubic Date: Mon, 17 Aug 2020 11:57:35 +0100 Subject: [PATCH 13/13] Fix duplicating embeddable by reference --- .../public/application/utils/get_top_nav_config.tsx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx b/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx index b7c0fe21d31a2..87a6437192aa9 100644 --- a/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx +++ b/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx @@ -210,7 +210,11 @@ export const getTopNavConfig = ( confirmOverwrite: false, returnToOrigin: true, }; - if (originatingApp === 'dashboards' && featureFlagConfig.showNewVisualizeFlow) { + if ( + originatingApp === 'dashboards' && + featureFlagConfig.showNewVisualizeFlow && + !savedVis + ) { return createVisReference(); } return doSave(saveOptions);