From 78ea85d978436caf893aa80ca0299e52263abdb2 Mon Sep 17 00:00:00 2001 From: Nick Peihl Date: Thu, 2 Mar 2023 11:54:17 -0500 Subject: [PATCH 01/18] Make changes to dashboard settings cancelable --- .../top_nav/use_dashboard_menu_items.tsx | 2 +- .../embeddable/api/index.ts | 2 +- .../embeddable/api/overlays/options.tsx | 54 +++--- .../api/overlays/settings_flyout.tsx | 183 ++++++++++++++++++ .../embeddable/api/show_options_popover.tsx | 61 ------ .../embeddable/api/show_settings.tsx | 39 ++++ .../embeddable/dashboard_container.tsx | 4 +- 7 files changed, 253 insertions(+), 92 deletions(-) create mode 100644 src/plugins/dashboard/public/dashboard_container/embeddable/api/overlays/settings_flyout.tsx delete mode 100644 src/plugins/dashboard/public/dashboard_container/embeddable/api/show_options_popover.tsx create mode 100644 src/plugins/dashboard/public/dashboard_container/embeddable/api/show_settings.tsx diff --git a/src/plugins/dashboard/public/dashboard_app/top_nav/use_dashboard_menu_items.tsx b/src/plugins/dashboard/public/dashboard_app/top_nav/use_dashboard_menu_items.tsx index 2c9d5db938962..07bbcb9de18f7 100644 --- a/src/plugins/dashboard/public/dashboard_app/top_nav/use_dashboard_menu_items.tsx +++ b/src/plugins/dashboard/public/dashboard_app/top_nav/use_dashboard_menu_items.tsx @@ -205,7 +205,7 @@ export const useDashboardMenuItems = ({ id: 'options', testId: 'dashboardOptionsButton', disableButton: isSaveInProgress, - run: (anchor) => dashboardContainer.showOptions(anchor), + run: () => dashboardContainer.showSettings(), } as TopNavMenuData, clone: { diff --git a/src/plugins/dashboard/public/dashboard_container/embeddable/api/index.ts b/src/plugins/dashboard/public/dashboard_container/embeddable/api/index.ts index fc58e3ad0aca5..2a2d20cc5da14 100644 --- a/src/plugins/dashboard/public/dashboard_container/embeddable/api/index.ts +++ b/src/plugins/dashboard/public/dashboard_container/embeddable/api/index.ts @@ -6,7 +6,7 @@ * Side Public License, v 1. */ -export { showOptions } from './show_options_popover'; +export { showSettings } from './show_settings'; export { addFromLibrary } from './add_panel_from_library'; export { runSaveAs, runQuickSave, runClone } from './run_save_functions'; export { addOrUpdateEmbeddable, replacePanel, showPlaceholderUntil } from './panel_management'; diff --git a/src/plugins/dashboard/public/dashboard_container/embeddable/api/overlays/options.tsx b/src/plugins/dashboard/public/dashboard_container/embeddable/api/overlays/options.tsx index b33e353881b0d..e206c17350049 100644 --- a/src/plugins/dashboard/public/dashboard_container/embeddable/api/overlays/options.tsx +++ b/src/plugins/dashboard/public/dashboard_container/embeddable/api/overlays/options.tsx @@ -9,32 +9,32 @@ import React from 'react'; import { i18n } from '@kbn/i18n'; -import { EuiForm, EuiFormRow, EuiSwitch } from '@elastic/eui'; -import { useDashboardContainerContext } from '../../../dashboard_container_renderer'; +import { EuiFormRow, EuiSwitch } from '@elastic/eui'; +import { DashboardContainerByValueInput } from '../../../../../common'; -export const DashboardOptions = () => { - const { - useEmbeddableDispatch, - useEmbeddableSelector: select, - actions: { setUseMargins, setSyncCursor, setSyncColors, setSyncTooltips, setHidePanelTitles }, - } = useDashboardContainerContext(); - const dispatch = useEmbeddableDispatch(); +interface DashboardOptionsProps { + initialInput: DashboardOptions; + updateDashboardSetting: (newSettings: Partial) => void; +} - const useMargins = select((state) => state.explicitInput.useMargins); - const syncColors = select((state) => state.explicitInput.syncColors); - const syncCursor = select((state) => state.explicitInput.syncCursor); - const syncTooltips = select((state) => state.explicitInput.syncTooltips); - const hidePanelTitles = select((state) => state.explicitInput.hidePanelTitles); +type DashboardOptions = Pick< + DashboardContainerByValueInput, + 'hidePanelTitles' | 'syncColors' | 'syncCursor' | 'syncTooltips' | 'useMargins' +>; +export const DashboardOptions = ({ + initialInput, + updateDashboardSetting, +}: DashboardOptionsProps) => { return ( - + <> dispatch(setUseMargins(event.target.checked))} + checked={initialInput.useMargins} + onChange={(event) => updateDashboardSetting({ useMargins: event.target.checked })} data-test-subj="dashboardMarginsCheckbox" /> @@ -44,8 +44,8 @@ export const DashboardOptions = () => { label={i18n.translate('dashboard.topNav.options.hideAllPanelTitlesSwitchLabel', { defaultMessage: 'Show panel titles', })} - checked={!hidePanelTitles} - onChange={(event) => dispatch(setHidePanelTitles(!event.target.checked))} + checked={!initialInput.hidePanelTitles} + onChange={(event) => updateDashboardSetting({ hidePanelTitles: !event.target.checked })} data-test-subj="dashboardPanelTitlesCheckbox" /> @@ -56,8 +56,8 @@ export const DashboardOptions = () => { label={i18n.translate('dashboard.topNav.options.syncColorsBetweenPanelsSwitchLabel', { defaultMessage: 'Sync color palettes across panels', })} - checked={syncColors} - onChange={(event) => dispatch(setSyncColors(event.target.checked))} + checked={initialInput.syncColors} + onChange={(event) => updateDashboardSetting({ syncColors: event.target.checked })} data-test-subj="dashboardSyncColorsCheckbox" /> @@ -66,8 +66,8 @@ export const DashboardOptions = () => { label={i18n.translate('dashboard.topNav.options.syncCursorBetweenPanelsSwitchLabel', { defaultMessage: 'Sync cursor across panels', })} - checked={syncCursor} - onChange={(event) => dispatch(setSyncCursor(event.target.checked))} + checked={initialInput.syncCursor} + onChange={(event) => updateDashboardSetting({ syncCursor: event.target.checked })} data-test-subj="dashboardSyncCursorCheckbox" /> @@ -79,14 +79,14 @@ export const DashboardOptions = () => { defaultMessage: 'Sync tooltips across panels', } )} - checked={syncTooltips} - disabled={!Boolean(syncCursor)} - onChange={(event) => dispatch(setSyncTooltips(event.target.checked))} + checked={initialInput.syncTooltips} + disabled={!Boolean(initialInput.syncCursor)} + onChange={(event) => updateDashboardSetting({ syncTooltips: event.target.checked })} data-test-subj="dashboardSyncTooltipsCheckbox" /> - + ); }; diff --git a/src/plugins/dashboard/public/dashboard_container/embeddable/api/overlays/settings_flyout.tsx b/src/plugins/dashboard/public/dashboard_container/embeddable/api/overlays/settings_flyout.tsx new file mode 100644 index 0000000000000..a52d564d6febd --- /dev/null +++ b/src/plugins/dashboard/public/dashboard_container/embeddable/api/overlays/settings_flyout.tsx @@ -0,0 +1,183 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import React, { useCallback, useState } from 'react'; +import { i18n } from '@kbn/i18n'; +import { + EuiFormRow, + EuiFieldText, + EuiTextArea, + EuiForm, + EuiButton, + EuiButtonEmpty, + EuiFlexGroup, + EuiFlexItem, + EuiFlyoutBody, + EuiFlyoutFooter, + EuiFlyoutHeader, + EuiTitle, +} from '@elastic/eui'; +import { FormattedMessage } from '@kbn/i18n-react'; +import { pluginServices } from '../../../../services/plugin_services'; +import { useDashboardContainerContext } from '../../../dashboard_container_renderer'; +import { DashboardOptions } from './options'; + +interface DashboardSettingsProps { + onClose: () => void; +} + +export const DashboardSettings = ({ onClose }: DashboardSettingsProps) => { + const { + savedObjectsTagging: { components }, + } = pluginServices.getServices(); + + const { + useEmbeddableDispatch, + useEmbeddableSelector: select, + actions: { setStateFromSaveModal }, + embeddableInstance: dashboardContainer, + } = useDashboardContainerContext(); + + const [dashboardSettingsState, setDashboardSettingsState] = useState({ + ...dashboardContainer.getInputAsValueType(), + }); + + const lastSavedId = select((state) => state.componentState.lastSavedId); + + const updateDashboardSetting = useCallback( + (newSettings) => { + setDashboardSettingsState({ + ...dashboardSettingsState, + ...newSettings, + }); + }, + [dashboardSettingsState] + ); + + const dispatch = useEmbeddableDispatch(); + + const renderTagSelector = () => { + if (!components) return; + return ( + + } + > + updateDashboardSetting({ tags: selectedTags })} + /> + + ); + }; + + return ( + <> + + +

+ +

+ + + + + + } + > + updateDashboardSetting({ title: event.target.value })} + aria-label={i18n.translate( + 'dashboardSettings.embeddableApi.flyout.optionsMenuForm.panelTitleInputAriaLabel', + { + defaultMessage: 'Change the dashboard title', + } + )} + /> + + + } + > + updateDashboardSetting({ description: event.target.value })} + aria-label={i18n.translate( + 'dashboardSettings.embeddableApi.flyout.optionsMenuForm.panelDescriptionAriaLabel', + { + defaultMessage: 'Change the dashboard description', + } + )} + /> + + {renderTagSelector()} + + + + + + + onClose()} + > + + + + + { + dispatch(setStateFromSaveModal({ lastSavedId, ...dashboardSettingsState })); + onClose(); + }} + fill + > + + + + + + + ); +}; diff --git a/src/plugins/dashboard/public/dashboard_container/embeddable/api/show_options_popover.tsx b/src/plugins/dashboard/public/dashboard_container/embeddable/api/show_options_popover.tsx deleted file mode 100644 index ebbd63bd6a93f..0000000000000 --- a/src/plugins/dashboard/public/dashboard_container/embeddable/api/show_options_popover.tsx +++ /dev/null @@ -1,61 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0 and the Server Side Public License, v 1; you may not use this file except - * in compliance with, at your election, the Elastic License 2.0 or the Server - * Side Public License, v 1. - */ - -import React from 'react'; -import ReactDOM from 'react-dom'; - -import { I18nProvider } from '@kbn/i18n-react'; -import { EuiWrappingPopover } from '@elastic/eui'; -import { KibanaThemeProvider } from '@kbn/kibana-react-plugin/public'; - -import { DashboardOptions } from './overlays/options'; -import { DashboardContainer } from '../dashboard_container'; -import { pluginServices } from '../../../services/plugin_services'; - -let isOpen = false; - -const container = document.createElement('div'); -const onClose = () => { - ReactDOM.unmountComponentAtNode(container); - isOpen = false; -}; - -export function showOptions(this: DashboardContainer, anchorElement: HTMLElement) { - const { - settings: { - theme: { theme$ }, - }, - } = pluginServices.getServices(); - - if (isOpen) { - onClose(); - return; - } - - isOpen = true; - const { Wrapper: DashboardReduxWrapper } = this.getReduxEmbeddableTools(); - - document.body.appendChild(container); - const element = ( - - - - - - - - - - ); - ReactDOM.render(element, container); -} diff --git a/src/plugins/dashboard/public/dashboard_container/embeddable/api/show_settings.tsx b/src/plugins/dashboard/public/dashboard_container/embeddable/api/show_settings.tsx new file mode 100644 index 0000000000000..218695d7d6729 --- /dev/null +++ b/src/plugins/dashboard/public/dashboard_container/embeddable/api/show_settings.tsx @@ -0,0 +1,39 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import React from 'react'; + +import { toMountPoint } from '@kbn/kibana-react-plugin/public'; + +import { DashboardSettings } from './overlays/settings_flyout'; +import { DashboardContainer } from '../dashboard_container'; +import { pluginServices } from '../../../services/plugin_services'; + +export function showSettings(this: DashboardContainer) { + const { + settings: { + theme: { theme$ }, + }, + overlays, + } = pluginServices.getServices(); + + const { Wrapper: DashboardReduxWrapper } = this.getReduxEmbeddableTools(); + + const handle = overlays.openFlyout( + toMountPoint( + + handle.close()} /> + , + { theme$ } + ), + { + size: 's', + 'data-test-subj': 'dashboardOptionsFlyout', + } + ); +} diff --git a/src/plugins/dashboard/public/dashboard_container/embeddable/dashboard_container.tsx b/src/plugins/dashboard/public/dashboard_container/embeddable/dashboard_container.tsx index c2205d68ba546..dcd88c0f103ff 100644 --- a/src/plugins/dashboard/public/dashboard_container/embeddable/dashboard_container.tsx +++ b/src/plugins/dashboard/public/dashboard_container/embeddable/dashboard_container.tsx @@ -37,7 +37,7 @@ import { ExitFullScreenButtonKibanaProvider } from '@kbn/shared-ux-button-exit-f import { runClone, runSaveAs, - showOptions, + showSettings, runQuickSave, replacePanel, addFromLibrary, @@ -515,7 +515,7 @@ export class DashboardContainer extends Container Date: Fri, 10 Mar 2023 16:18:39 -0500 Subject: [PATCH 02/18] Create dashboard settings flyout --- .../dashboard_app/_dashboard_app_strings.ts | 8 +- .../component/settings/index.ts | 9 + .../component/settings/settings_flyout.tsx | 328 ++++++++++++++++++ .../embeddable/api/overlays/options.tsx | 91 ----- .../api/overlays/settings_flyout.tsx | 183 ---------- .../embeddable/api/show_settings.tsx | 2 +- .../state/dashboard_container_reducers.ts | 48 +-- .../public/dashboard_container/types.ts | 4 +- 8 files changed, 369 insertions(+), 304 deletions(-) create mode 100644 src/plugins/dashboard/public/dashboard_container/component/settings/index.ts create mode 100644 src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx delete mode 100644 src/plugins/dashboard/public/dashboard_container/embeddable/api/overlays/options.tsx delete mode 100644 src/plugins/dashboard/public/dashboard_container/embeddable/api/overlays/settings_flyout.tsx diff --git a/src/plugins/dashboard/public/dashboard_app/_dashboard_app_strings.ts b/src/plugins/dashboard/public/dashboard_app/_dashboard_app_strings.ts index 4144d9cd5b3af..cc41c6c7208ce 100644 --- a/src/plugins/dashboard/public/dashboard_app/_dashboard_app_strings.ts +++ b/src/plugins/dashboard/public/dashboard_app/_dashboard_app_strings.ts @@ -324,11 +324,11 @@ export const topNavStrings = { }), }, options: { - label: i18n.translate('dashboard.topNave.optionsButtonAriaLabel', { - defaultMessage: 'options', + label: i18n.translate('dashboard.topNave.settingsButtonAriaLabel', { + defaultMessage: 'settings', }), - description: i18n.translate('dashboard.topNave.optionsConfigDescription', { - defaultMessage: 'Options', + description: i18n.translate('dashboard.topNave.settingsConfigDescription', { + defaultMessage: 'Settings', }), }, clone: { diff --git a/src/plugins/dashboard/public/dashboard_container/component/settings/index.ts b/src/plugins/dashboard/public/dashboard_container/component/settings/index.ts new file mode 100644 index 0000000000000..00d5be0ca3a0d --- /dev/null +++ b/src/plugins/dashboard/public/dashboard_container/component/settings/index.ts @@ -0,0 +1,9 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +export { DashboardSettings } from './settings_flyout'; diff --git a/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx b/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx new file mode 100644 index 0000000000000..2704c40cd1291 --- /dev/null +++ b/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx @@ -0,0 +1,328 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import React, { useCallback, useState } from 'react'; +import { i18n } from '@kbn/i18n'; +import { + EuiFormRow, + EuiFieldText, + EuiTextArea, + EuiForm, + EuiButton, + EuiButtonEmpty, + EuiFlexGroup, + EuiFlexItem, + EuiFlyoutBody, + EuiFlyoutFooter, + EuiFlyoutHeader, + EuiTitle, + EuiCallOut, + EuiSwitch, +} from '@elastic/eui'; +import { FormattedMessage } from '@kbn/i18n-react'; +import { DashboardContainerByValueInput } from '../../../../common'; +import { pluginServices } from '../../../services/plugin_services'; +import { useDashboardContainerContext } from '../../dashboard_container_context'; + +interface DashboardSettingsProps { + onClose: () => void; +} + +const DUPLICATE_TITLE_CALLOUT_ID = 'duplicateTitleCallout'; + +export const DashboardSettings = ({ onClose }: DashboardSettingsProps) => { + const { + savedObjectsTagging: { components }, + dashboardSavedObject: { checkForDuplicateDashboardTitle }, + } = pluginServices.getServices(); + + const { + useEmbeddableDispatch, + useEmbeddableSelector: select, + actions: { setStateFromSettingsFlyout }, + embeddableInstance: dashboardContainer, + } = useDashboardContainerContext(); + + const [dashboardSettingsState, setDashboardSettingsState] = useState({ + ...dashboardContainer.getInputAsValueType(), + }); + + const [isTitleDuplicate, setIsTitleDuplicate] = useState(false); + const [isTitleDuplicateConfirmed, setIsTitleDuplicateConfirmed] = useState(false); + + const lastSavedId = select((state) => state.componentState.lastSavedId); + const lastSavedTitle = select((state) => state.explicitInput.title); + + const onTitleDuplicate = () => { + setIsTitleDuplicate(true); + setIsTitleDuplicateConfirmed(true); + }; + + const updateDashboardSetting = useCallback( + (newSettings: Partial) => { + setDashboardSettingsState({ + ...dashboardSettingsState, + ...newSettings, + }); + }, + [dashboardSettingsState] + ); + + const dispatch = useEmbeddableDispatch(); + + const renderDuplicateTitleCallout = () => { + if (!isTitleDuplicate) { + return; + } + + return ( + + } + color="warning" + data-test-subj="duplicateTitleWarningMessage" + id={DUPLICATE_TITLE_CALLOUT_ID} + > +

+ +

+ + ); + }; + + const renderTagSelector = () => { + if (!components) return; + return ( + + } + > + updateDashboardSetting({ tags: selectedTags })} + /> + + ); + }; + + return ( + <> + + +

+ +

+ + + + {renderDuplicateTitleCallout()} + + + } + > + { + setIsTitleDuplicate(false); + setIsTitleDuplicateConfirmed(false); + updateDashboardSetting({ title: event.target.value }); + }} + aria-label={i18n.translate( + 'dashboardSettings.embeddableApi.flyout.optionsMenuForm.panelTitleInputAriaLabel', + { + defaultMessage: 'Change the dashboard title', + } + )} + aria-describedby={isTitleDuplicate ? DUPLICATE_TITLE_CALLOUT_ID : undefined} + /> + + + } + > + updateDashboardSetting({ description: event.target.value })} + aria-label={i18n.translate( + 'dashboardSettings.embeddableApi.flyout.optionsMenuForm.panelDescriptionAriaLabel', + { + defaultMessage: 'Change the dashboard description', + } + )} + /> + + {renderTagSelector()} + + } + > + updateDashboardSetting({ timeRestore: event.target.checked })} + label={ + + } + /> + + + updateDashboardSetting({ useMargins: event.target.checked })} + data-test-subj="dashboardMarginsCheckbox" + /> + + + + + updateDashboardSetting({ hidePanelTitles: !event.target.checked }) + } + data-test-subj="dashboardPanelTitlesCheckbox" + /> + + + <> + + updateDashboardSetting({ syncColors: event.target.checked })} + data-test-subj="dashboardSyncColorsCheckbox" + /> + + + updateDashboardSetting({ syncCursor: event.target.checked })} + data-test-subj="dashboardSyncCursorCheckbox" + /> + + + + updateDashboardSetting({ syncTooltips: event.target.checked }) + } + data-test-subj="dashboardSyncTooltipsCheckbox" + /> + + + + + + + + + onClose()} + > + + + + + { + if ( + await checkForDuplicateDashboardTitle({ + title: dashboardSettingsState.title, + copyOnSave: false, + lastSavedTitle, + onTitleDuplicate, + isTitleDuplicateConfirmed, + }) + ) { + dispatch(setStateFromSettingsFlyout({ lastSavedId, ...dashboardSettingsState })); + onClose(); + } + }} + fill + > + + + + + + + ); +}; diff --git a/src/plugins/dashboard/public/dashboard_container/embeddable/api/overlays/options.tsx b/src/plugins/dashboard/public/dashboard_container/embeddable/api/overlays/options.tsx deleted file mode 100644 index 8c41472331f91..0000000000000 --- a/src/plugins/dashboard/public/dashboard_container/embeddable/api/overlays/options.tsx +++ /dev/null @@ -1,91 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0 and the Server Side Public License, v 1; you may not use this file except - * in compliance with, at your election, the Elastic License 2.0 or the Server - * Side Public License, v 1. - */ - -import React from 'react'; -import { i18n } from '@kbn/i18n'; -import { EuiFormRow, EuiSwitch } from '@elastic/eui'; -import { DashboardContainerByValueInput } from '../../../../../common'; - -interface DashboardOptionsProps { - initialInput: DashboardOptions; - updateDashboardSetting: (newSettings: Partial) => void; -} - -type DashboardOptions = Pick< - DashboardContainerByValueInput, - 'hidePanelTitles' | 'syncColors' | 'syncCursor' | 'syncTooltips' | 'useMargins' ->; - -export const DashboardOptions = ({ - initialInput, - updateDashboardSetting, -}: DashboardOptionsProps) => { - return ( - <> - - updateDashboardSetting({ useMargins: event.target.checked })} - data-test-subj="dashboardMarginsCheckbox" - /> - - - - updateDashboardSetting({ hidePanelTitles: !event.target.checked })} - data-test-subj="dashboardPanelTitlesCheckbox" - /> - - - <> - - updateDashboardSetting({ syncColors: event.target.checked })} - data-test-subj="dashboardSyncColorsCheckbox" - /> - - - updateDashboardSetting({ syncCursor: event.target.checked })} - data-test-subj="dashboardSyncCursorCheckbox" - /> - - - updateDashboardSetting({ syncTooltips: event.target.checked })} - data-test-subj="dashboardSyncTooltipsCheckbox" - /> - - - - - ); -}; diff --git a/src/plugins/dashboard/public/dashboard_container/embeddable/api/overlays/settings_flyout.tsx b/src/plugins/dashboard/public/dashboard_container/embeddable/api/overlays/settings_flyout.tsx deleted file mode 100644 index a52d564d6febd..0000000000000 --- a/src/plugins/dashboard/public/dashboard_container/embeddable/api/overlays/settings_flyout.tsx +++ /dev/null @@ -1,183 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0 and the Server Side Public License, v 1; you may not use this file except - * in compliance with, at your election, the Elastic License 2.0 or the Server - * Side Public License, v 1. - */ - -import React, { useCallback, useState } from 'react'; -import { i18n } from '@kbn/i18n'; -import { - EuiFormRow, - EuiFieldText, - EuiTextArea, - EuiForm, - EuiButton, - EuiButtonEmpty, - EuiFlexGroup, - EuiFlexItem, - EuiFlyoutBody, - EuiFlyoutFooter, - EuiFlyoutHeader, - EuiTitle, -} from '@elastic/eui'; -import { FormattedMessage } from '@kbn/i18n-react'; -import { pluginServices } from '../../../../services/plugin_services'; -import { useDashboardContainerContext } from '../../../dashboard_container_renderer'; -import { DashboardOptions } from './options'; - -interface DashboardSettingsProps { - onClose: () => void; -} - -export const DashboardSettings = ({ onClose }: DashboardSettingsProps) => { - const { - savedObjectsTagging: { components }, - } = pluginServices.getServices(); - - const { - useEmbeddableDispatch, - useEmbeddableSelector: select, - actions: { setStateFromSaveModal }, - embeddableInstance: dashboardContainer, - } = useDashboardContainerContext(); - - const [dashboardSettingsState, setDashboardSettingsState] = useState({ - ...dashboardContainer.getInputAsValueType(), - }); - - const lastSavedId = select((state) => state.componentState.lastSavedId); - - const updateDashboardSetting = useCallback( - (newSettings) => { - setDashboardSettingsState({ - ...dashboardSettingsState, - ...newSettings, - }); - }, - [dashboardSettingsState] - ); - - const dispatch = useEmbeddableDispatch(); - - const renderTagSelector = () => { - if (!components) return; - return ( - - } - > - updateDashboardSetting({ tags: selectedTags })} - /> - - ); - }; - - return ( - <> - - -

- -

- - - - - - } - > - updateDashboardSetting({ title: event.target.value })} - aria-label={i18n.translate( - 'dashboardSettings.embeddableApi.flyout.optionsMenuForm.panelTitleInputAriaLabel', - { - defaultMessage: 'Change the dashboard title', - } - )} - /> - - - } - > - updateDashboardSetting({ description: event.target.value })} - aria-label={i18n.translate( - 'dashboardSettings.embeddableApi.flyout.optionsMenuForm.panelDescriptionAriaLabel', - { - defaultMessage: 'Change the dashboard description', - } - )} - /> - - {renderTagSelector()} - - - - - - - onClose()} - > - - - - - { - dispatch(setStateFromSaveModal({ lastSavedId, ...dashboardSettingsState })); - onClose(); - }} - fill - > - - - - - - - ); -}; diff --git a/src/plugins/dashboard/public/dashboard_container/embeddable/api/show_settings.tsx b/src/plugins/dashboard/public/dashboard_container/embeddable/api/show_settings.tsx index 218695d7d6729..68d2a8efadf1c 100644 --- a/src/plugins/dashboard/public/dashboard_container/embeddable/api/show_settings.tsx +++ b/src/plugins/dashboard/public/dashboard_container/embeddable/api/show_settings.tsx @@ -10,7 +10,7 @@ import React from 'react'; import { toMountPoint } from '@kbn/kibana-react-plugin/public'; -import { DashboardSettings } from './overlays/settings_flyout'; +import { DashboardSettings } from '../../component/settings/settings_flyout'; import { DashboardContainer } from '../dashboard_container'; import { pluginServices } from '../../../services/plugin_services'; diff --git a/src/plugins/dashboard/public/dashboard_container/state/dashboard_container_reducers.ts b/src/plugins/dashboard/public/dashboard_container/state/dashboard_container_reducers.ts index 4c47b41ad67d8..4525a4ead728c 100644 --- a/src/plugins/dashboard/public/dashboard_container/state/dashboard_container_reducers.ts +++ b/src/plugins/dashboard/public/dashboard_container/state/dashboard_container_reducers.ts @@ -7,7 +7,12 @@ */ import { PayloadAction } from '@reduxjs/toolkit'; -import { DashboardPublicState, DashboardReduxState, DashboardStateFromSaveModal } from '../types'; +import { + DashboardPublicState, + DashboardReduxState, + DashboardStateFromSaveModal, + DashboardStateFromSettingsFlyout, +} from '../types'; import { DashboardContainerByValueInput } from '../../../common'; export const dashboardContainerReducers = { @@ -50,6 +55,24 @@ export const dashboardContainerReducers = { } }, + setStateFromSettingsFlyout: ( + state: DashboardReduxState, + action: PayloadAction + ) => { + state.componentState.lastSavedId = action.payload.lastSavedId; + + state.explicitInput.tags = action.payload.tags; + state.explicitInput.title = action.payload.title; + state.explicitInput.description = action.payload.description; + state.explicitInput.timeRestore = action.payload.timeRestore; + + state.explicitInput.useMargins = action.payload.useMargins; + state.explicitInput.syncColors = action.payload.syncColors; + state.explicitInput.syncCursor = action.payload.syncCursor; + state.explicitInput.syncTooltips = action.payload.syncTooltips; + state.explicitInput.hidePanelTitles = action.payload.hidePanelTitles; + }, + setDescription: ( state: DashboardReduxState, action: PayloadAction @@ -106,29 +129,6 @@ export const dashboardContainerReducers = { state.explicitInput = state.componentState.lastSavedInput; }, - // ------------------------------------------------------------------------------ - // Options Reducers - // ------------------------------------------------------------------------------ - setUseMargins: (state: DashboardReduxState, action: PayloadAction) => { - state.explicitInput.useMargins = action.payload; - }, - - setSyncCursor: (state: DashboardReduxState, action: PayloadAction) => { - state.explicitInput.syncCursor = action.payload; - }, - - setSyncColors: (state: DashboardReduxState, action: PayloadAction) => { - state.explicitInput.syncColors = action.payload; - }, - - setSyncTooltips: (state: DashboardReduxState, action: PayloadAction) => { - state.explicitInput.syncTooltips = action.payload; - }, - - setHidePanelTitles: (state: DashboardReduxState, action: PayloadAction) => { - state.explicitInput.hidePanelTitles = action.payload; - }, - // ------------------------------------------------------------------------------ // Filtering Reducers // ------------------------------------------------------------------------------ diff --git a/src/plugins/dashboard/public/dashboard_container/types.ts b/src/plugins/dashboard/public/dashboard_container/types.ts index 6d09e75671db3..e0bcb230dea72 100644 --- a/src/plugins/dashboard/public/dashboard_container/types.ts +++ b/src/plugins/dashboard/public/dashboard_container/types.ts @@ -8,7 +8,7 @@ import type { ContainerOutput } from '@kbn/embeddable-plugin/public'; import type { ReduxEmbeddableState } from '@kbn/presentation-util-plugin/public'; -import type { DashboardContainerByValueInput } from '../../common/dashboard_container/types'; +import type { DashboardContainerByValueInput, DashboardOptions } from '../../common'; export type DashboardReduxState = ReduxEmbeddableState< DashboardContainerByValueInput, @@ -22,6 +22,8 @@ export type DashboardStateFromSaveModal = Pick< > & Pick; +export type DashboardStateFromSettingsFlyout = DashboardStateFromSaveModal & DashboardOptions; + export interface DashboardPublicState { lastSavedInput: DashboardContainerByValueInput; isEmbeddedExternally?: boolean; From 18ef0dfbb5b10ccd70d20c45b7502e282108c0a7 Mon Sep 17 00:00:00 2001 From: Nick Peihl Date: Fri, 10 Mar 2023 17:25:22 -0500 Subject: [PATCH 03/18] Disable dashboard quicksave button when flyout is open --- .../top_nav/use_dashboard_menu_items.tsx | 4 +++- .../component/settings/settings_flyout.tsx | 13 ++++++++++--- .../state/dashboard_container_reducers.ts | 8 ++++++++ .../dashboard/public/dashboard_container/types.ts | 1 + 4 files changed, 22 insertions(+), 4 deletions(-) diff --git a/src/plugins/dashboard/public/dashboard_app/top_nav/use_dashboard_menu_items.tsx b/src/plugins/dashboard/public/dashboard_app/top_nav/use_dashboard_menu_items.tsx index ba67283bef259..0701a5e9c4618 100644 --- a/src/plugins/dashboard/public/dashboard_app/top_nav/use_dashboard_menu_items.tsx +++ b/src/plugins/dashboard/public/dashboard_app/top_nav/use_dashboard_menu_items.tsx @@ -55,6 +55,7 @@ export const useDashboardMenuItems = ({ const dispatch = useEmbeddableDispatch(); const hasUnsavedChanges = select((state) => state.componentState.hasUnsavedChanges); + const hasOverlays = select((state) => state.componentState.hasOverlays); const lastSavedId = select((state) => state.componentState.lastSavedId); const dashboardTitle = select((state) => state.explicitInput.title); @@ -169,7 +170,7 @@ export const useDashboardMenuItems = ({ emphasize: true, isLoading: isSaveInProgress, testId: 'dashboardQuickSaveMenuItem', - disableButton: !hasUnsavedChanges || isSaveInProgress, + disableButton: !hasUnsavedChanges || isSaveInProgress || hasOverlays, run: () => quickSaveDashboard(), } as TopNavMenuData, @@ -220,6 +221,7 @@ export const useDashboardMenuItems = ({ quickSaveDashboard, dashboardContainer, hasUnsavedChanges, + hasOverlays, setFullScreenMode, isSaveInProgress, returnToViewMode, diff --git a/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx b/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx index 2704c40cd1291..de2745f46f582 100644 --- a/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx +++ b/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx @@ -44,7 +44,7 @@ export const DashboardSettings = ({ onClose }: DashboardSettingsProps) => { const { useEmbeddableDispatch, useEmbeddableSelector: select, - actions: { setStateFromSettingsFlyout }, + actions: { setStateFromSettingsFlyout, setHasOverlays }, embeddableInstance: dashboardContainer, } = useDashboardContainerContext(); @@ -75,6 +75,13 @@ export const DashboardSettings = ({ onClose }: DashboardSettingsProps) => { const dispatch = useEmbeddableDispatch(); + dispatch(setHasOverlays(true)); + + const closeFlyout = () => { + dispatch(setHasOverlays(false)); + onClose(); + }; + const renderDuplicateTitleCallout = () => { if (!isTitleDuplicate) { return; @@ -288,7 +295,7 @@ export const DashboardSettings = ({ onClose }: DashboardSettingsProps) => { onClose()} + onClick={() => closeFlyout()} > { }) ) { dispatch(setStateFromSettingsFlyout({ lastSavedId, ...dashboardSettingsState })); - onClose(); + closeFlyout(); } }} fill diff --git a/src/plugins/dashboard/public/dashboard_container/state/dashboard_container_reducers.ts b/src/plugins/dashboard/public/dashboard_container/state/dashboard_container_reducers.ts index 4525a4ead728c..9a5d135b0f61e 100644 --- a/src/plugins/dashboard/public/dashboard_container/state/dashboard_container_reducers.ts +++ b/src/plugins/dashboard/public/dashboard_container/state/dashboard_container_reducers.ts @@ -200,4 +200,12 @@ export const dashboardContainerReducers = { setFullScreenMode: (state: DashboardReduxState, action: PayloadAction) => { state.componentState.fullScreenMode = action.payload; }, + + // ------------------------------------------------------------------------------ + // Component state reducers + // ------------------------------------------------------------------------------ + + setHasOverlays: (state: DashboardReduxState, action: PayloadAction) => { + state.componentState.hasOverlays = action.payload; + }, }; diff --git a/src/plugins/dashboard/public/dashboard_container/types.ts b/src/plugins/dashboard/public/dashboard_container/types.ts index e0bcb230dea72..436a6ab0029d8 100644 --- a/src/plugins/dashboard/public/dashboard_container/types.ts +++ b/src/plugins/dashboard/public/dashboard_container/types.ts @@ -28,6 +28,7 @@ export interface DashboardPublicState { lastSavedInput: DashboardContainerByValueInput; isEmbeddedExternally?: boolean; hasUnsavedChanges?: boolean; + hasOverlays?: boolean; expandedPanelId?: string; fullScreenMode?: boolean; savedQueryId?: string; From fbab788e6184405d4a3e664780332b0dcd76612b Mon Sep 17 00:00:00 2001 From: Nick Peihl Date: Mon, 27 Mar 2023 08:49:51 -0400 Subject: [PATCH 04/18] Use updater function to avoid dependencies --- .../component/settings/settings_flyout.tsx | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx b/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx index de2745f46f582..e20854d5cb309 100644 --- a/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx +++ b/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx @@ -65,12 +65,14 @@ export const DashboardSettings = ({ onClose }: DashboardSettingsProps) => { const updateDashboardSetting = useCallback( (newSettings: Partial) => { - setDashboardSettingsState({ - ...dashboardSettingsState, - ...newSettings, + setDashboardSettingsState((prevDashboardSettingsState) => { + return { + ...prevDashboardSettingsState, + ...newSettings, + }; }); }, - [dashboardSettingsState] + [] ); const dispatch = useEmbeddableDispatch(); @@ -266,7 +268,14 @@ export const DashboardSettings = ({ onClose }: DashboardSettingsProps) => { } )} checked={dashboardSettingsState.syncCursor} - onChange={(event) => updateDashboardSetting({ syncCursor: event.target.checked })} + onChange={(event) => { + const syncCursor = event.target.checked; + if (!syncCursor && dashboardSettingsState.syncTooltips) { + updateDashboardSetting({ syncCursor, syncTooltips: false }); + } else { + updateDashboardSetting({ syncCursor }); + } + }} data-test-subj="dashboardSyncCursorCheckbox" /> From 620bfc53d34e278cc669c752ba999e0036aa1098 Mon Sep 17 00:00:00 2001 From: Nick Peihl Date: Tue, 28 Mar 2023 08:44:15 -0400 Subject: [PATCH 05/18] Add functional tests for dashboard settings --- .../dashboard_app/_dashboard_app_strings.ts | 2 +- .../top_nav/use_dashboard_menu_items.tsx | 8 +- .../component/settings/settings_flyout.tsx | 15 +- .../embeddable/api/show_settings.tsx | 43 ++++-- .../dashboard/group5/dashboard_options.ts | 56 -------- .../dashboard/group5/dashboard_settings.ts | 114 ++++++++++++++++ .../functional/page_objects/dashboard_page.ts | 57 +++----- .../services/dashboard/dashboard_settings.ts | 128 ++++++++++++++++++ test/functional/services/index.ts | 2 + 9 files changed, 300 insertions(+), 125 deletions(-) delete mode 100644 test/functional/apps/dashboard/group5/dashboard_options.ts create mode 100644 test/functional/apps/dashboard/group5/dashboard_settings.ts create mode 100644 test/functional/services/dashboard/dashboard_settings.ts diff --git a/src/plugins/dashboard/public/dashboard_app/_dashboard_app_strings.ts b/src/plugins/dashboard/public/dashboard_app/_dashboard_app_strings.ts index 76c77c6d0a098..c021cef1ef8f6 100644 --- a/src/plugins/dashboard/public/dashboard_app/_dashboard_app_strings.ts +++ b/src/plugins/dashboard/public/dashboard_app/_dashboard_app_strings.ts @@ -323,7 +323,7 @@ export const topNavStrings = { defaultMessage: 'Share Dashboard', }), }, - options: { + settings: { label: i18n.translate('dashboard.topNave.settingsButtonAriaLabel', { defaultMessage: 'settings', }), diff --git a/src/plugins/dashboard/public/dashboard_app/top_nav/use_dashboard_menu_items.tsx b/src/plugins/dashboard/public/dashboard_app/top_nav/use_dashboard_menu_items.tsx index 0701a5e9c4618..b18786392084a 100644 --- a/src/plugins/dashboard/public/dashboard_app/top_nav/use_dashboard_menu_items.tsx +++ b/src/plugins/dashboard/public/dashboard_app/top_nav/use_dashboard_menu_items.tsx @@ -201,10 +201,10 @@ export const useDashboardMenuItems = ({ run: showShare, } as TopNavMenuData, - options: { - ...topNavStrings.options, - id: 'options', - testId: 'dashboardOptionsButton', + settings: { + ...topNavStrings.settings, + id: 'settings', + testId: 'dashboardSettingsButton', disableButton: isSaveInProgress, run: () => dashboardContainer.showSettings(), } as TopNavMenuData, diff --git a/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx b/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx index e20854d5cb309..a7c8f8c39d37c 100644 --- a/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx +++ b/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx @@ -44,7 +44,7 @@ export const DashboardSettings = ({ onClose }: DashboardSettingsProps) => { const { useEmbeddableDispatch, useEmbeddableSelector: select, - actions: { setStateFromSettingsFlyout, setHasOverlays }, + actions: { setStateFromSettingsFlyout }, embeddableInstance: dashboardContainer, } = useDashboardContainerContext(); @@ -77,13 +77,6 @@ export const DashboardSettings = ({ onClose }: DashboardSettingsProps) => { const dispatch = useEmbeddableDispatch(); - dispatch(setHasOverlays(true)); - - const closeFlyout = () => { - dispatch(setHasOverlays(false)); - onClose(); - }; - const renderDuplicateTitleCallout = () => { if (!isTitleDuplicate) { return; @@ -304,7 +297,7 @@ export const DashboardSettings = ({ onClose }: DashboardSettingsProps) => { closeFlyout()} + onClick={() => onClose()} > { { if ( await checkForDuplicateDashboardTitle({ @@ -326,7 +319,7 @@ export const DashboardSettings = ({ onClose }: DashboardSettingsProps) => { }) ) { dispatch(setStateFromSettingsFlyout({ lastSavedId, ...dashboardSettingsState })); - closeFlyout(); + onClose(); } }} fill diff --git a/src/plugins/dashboard/public/dashboard_container/embeddable/api/show_settings.tsx b/src/plugins/dashboard/public/dashboard_container/embeddable/api/show_settings.tsx index 68d2a8efadf1c..266c0a414b5b9 100644 --- a/src/plugins/dashboard/public/dashboard_container/embeddable/api/show_settings.tsx +++ b/src/plugins/dashboard/public/dashboard_container/embeddable/api/show_settings.tsx @@ -22,18 +22,37 @@ export function showSettings(this: DashboardContainer) { overlays, } = pluginServices.getServices(); - const { Wrapper: DashboardReduxWrapper } = this.getReduxEmbeddableTools(); + const { + dispatch, + Wrapper: DashboardReduxWrapper, + actions: { setHasOverlays }, + } = this.getReduxEmbeddableTools(); + + // TODO Move this action into DashboardContainer.openOverlay + dispatch(setHasOverlays(true)); - const handle = overlays.openFlyout( - toMountPoint( - - handle.close()} /> - , - { theme$ } - ), - { - size: 's', - 'data-test-subj': 'dashboardOptionsFlyout', - } + this.openOverlay( + overlays.openFlyout( + toMountPoint( + + { + dispatch(setHasOverlays(false)); + this.clearOverlays(); + }} + /> + , + { theme$ } + ), + { + size: 's', + 'data-test-subj': 'dashboardSettingsFlyout', + onClose: (flyout) => { + this.clearOverlays(); + dispatch(setHasOverlays(false)); + flyout.close(); + }, + } + ) ); } diff --git a/test/functional/apps/dashboard/group5/dashboard_options.ts b/test/functional/apps/dashboard/group5/dashboard_options.ts deleted file mode 100644 index 096f8595072bf..0000000000000 --- a/test/functional/apps/dashboard/group5/dashboard_options.ts +++ /dev/null @@ -1,56 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0 and the Server Side Public License, v 1; you may not use this file except - * in compliance with, at your election, the Elastic License 2.0 or the Server - * Side Public License, v 1. - */ - -import expect from '@kbn/expect'; - -import { FtrProviderContext } from '../../../ftr_provider_context'; - -export default function ({ getService, getPageObjects }: FtrProviderContext) { - const retry = getService('retry'); - const kibanaServer = getService('kibanaServer'); - const PageObjects = getPageObjects(['common', 'dashboard']); - - describe('dashboard options', () => { - let originalTitles: string[] = []; - - before(async () => { - await kibanaServer.savedObjects.cleanStandardList(); - await kibanaServer.importExport.load( - 'test/functional/fixtures/kbn_archiver/dashboard/current/kibana' - ); - await kibanaServer.uiSettings.replace({ - defaultIndex: '0bf35f60-3dc9-11e8-8660-4d65aa086b3c', - }); - await PageObjects.common.navigateToApp('dashboard'); - await PageObjects.dashboard.preserveCrossAppState(); - await PageObjects.dashboard.loadSavedDashboard('few panels'); - await PageObjects.dashboard.switchToEditMode(); - originalTitles = await PageObjects.dashboard.getPanelTitles(); - }); - - after(async () => { - await kibanaServer.savedObjects.cleanStandardList(); - }); - - it('should be able to hide all panel titles', async () => { - await PageObjects.dashboard.checkHideTitle(); - await retry.try(async () => { - const titles = await PageObjects.dashboard.getPanelTitles(); - expect(titles[0]).to.eql(''); - }); - }); - - it('should be able to unhide all panel titles', async () => { - await PageObjects.dashboard.checkHideTitle(); - await retry.try(async () => { - const titles = await PageObjects.dashboard.getPanelTitles(); - expect(titles[0]).to.eql(originalTitles[0]); - }); - }); - }); -} diff --git a/test/functional/apps/dashboard/group5/dashboard_settings.ts b/test/functional/apps/dashboard/group5/dashboard_settings.ts new file mode 100644 index 0000000000000..bbfb867cdf176 --- /dev/null +++ b/test/functional/apps/dashboard/group5/dashboard_settings.ts @@ -0,0 +1,114 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import expect from '@kbn/expect'; + +import { FtrProviderContext } from '../../../ftr_provider_context'; + +export default function ({ getService, getPageObjects }: FtrProviderContext) { + const retry = getService('retry'); + const globalNav = getService('globalNav'); + const kibanaServer = getService('kibanaServer'); + const dashboardSettings = getService('dashboardSettings'); + const PageObjects = getPageObjects(['common', 'dashboard']); + + describe('dashboard settings', () => { + let originalTitles: string[] = []; + + before(async () => { + await kibanaServer.savedObjects.cleanStandardList(); + await kibanaServer.importExport.load( + 'test/functional/fixtures/kbn_archiver/dashboard/current/kibana' + ); + await kibanaServer.uiSettings.replace({ + defaultIndex: '0bf35f60-3dc9-11e8-8660-4d65aa086b3c', + }); + await PageObjects.common.navigateToApp('dashboard'); + await PageObjects.dashboard.preserveCrossAppState(); + await PageObjects.dashboard.loadSavedDashboard('few panels'); + await PageObjects.dashboard.switchToEditMode(); + originalTitles = await PageObjects.dashboard.getPanelTitles(); + }); + + after(async () => { + await kibanaServer.savedObjects.cleanStandardList(); + }); + + it('should be able to hide all panel titles', async () => { + await PageObjects.dashboard.openSettingsFlyout(); + await dashboardSettings.toggleShowPanelTitles(false); + await dashboardSettings.clickApplyButton(); + await retry.try(async () => { + const titles = await PageObjects.dashboard.getPanelTitles(); + expect(titles[0]).to.eql(''); + }); + }); + + it('should be able to unhide all panel titles', async () => { + await PageObjects.dashboard.openSettingsFlyout(); + await dashboardSettings.toggleShowPanelTitles(true); + await dashboardSettings.clickApplyButton(); + await retry.try(async () => { + const titles = await PageObjects.dashboard.getPanelTitles(); + expect(titles[0]).to.eql(originalTitles[0]); + }); + }); + + it('should update the title of the dashboard', async () => { + const newTitle = 'My awesome dashboard!!1'; + await PageObjects.dashboard.openSettingsFlyout(); + await dashboardSettings.setCustomPanelTitle(newTitle); + await dashboardSettings.clickApplyButton(); + await retry.try(async () => { + expect((await globalNav.getLastBreadcrumb()) === newTitle); + }); + }); + + it('should disable quick save when the settings are open', async () => { + await PageObjects.dashboard.expectQuickSaveButtonEnabled(); + await PageObjects.dashboard.openSettingsFlyout(); + await retry.try(async () => { + await PageObjects.dashboard.expectQuickSaveButtonDisabled(); + }); + await dashboardSettings.clickCancelButton(); + }); + + it('should enable quick save when the settings flyout is closed', async () => { + await PageObjects.dashboard.expectQuickSaveButtonEnabled(); + await PageObjects.dashboard.openSettingsFlyout(); + await dashboardSettings.clickCloseFlyoutButton(); + await retry.try(async () => { + await PageObjects.dashboard.expectQuickSaveButtonEnabled(); + }); + }); + + it('should warn when creating a duplicate title', async () => { + await PageObjects.dashboard.openSettingsFlyout(); + await dashboardSettings.setCustomPanelTitle('couple panels'); + await dashboardSettings.clickApplyButton(false); + await retry.try(async () => { + await dashboardSettings.expectDuplicateTitleWarningDisplayed(); + }); + await dashboardSettings.clickCancelButton(); + }); + + it('should allow duplicate title if warned once', async () => { + const newTitle = 'couple panels'; + await PageObjects.dashboard.openSettingsFlyout(); + await dashboardSettings.setCustomPanelTitle(newTitle); + await dashboardSettings.clickApplyButton(false); + await retry.try(async () => { + await dashboardSettings.expectDuplicateTitleWarningDisplayed(); + }); + await dashboardSettings.clickApplyButton(); + await retry.try(async () => { + expect((await globalNav.getLastBreadcrumb()) === newTitle); + }); + }); + }); +} diff --git a/test/functional/page_objects/dashboard_page.ts b/test/functional/page_objects/dashboard_page.ts index 41a99e046aa42..2fa2290a70a6c 100644 --- a/test/functional/page_objects/dashboard_page.ts +++ b/test/functional/page_objects/dashboard_page.ts @@ -393,16 +393,16 @@ export class DashboardPageObject extends FtrService { return this.testSubjects.exists('emptyListPrompt'); } - public async isOptionsOpen() { - this.log.debug('isOptionsOpen'); - return await this.testSubjects.exists('dashboardOptionsMenu'); + public async isSettingsOpen() { + this.log.debug('isSettingsOpen'); + return await this.testSubjects.exists('dashboardSettingsMenu'); } - public async openOptions() { - this.log.debug('openOptions'); - const isOpen = await this.isOptionsOpen(); + public async openSettingsFlyout() { + this.log.debug('openSettingsFlyout'); + const isOpen = await this.isSettingsOpen(); if (!isOpen) { - return await this.testSubjects.click('dashboardOptionsButton'); + return await this.testSubjects.click('dashboardSettingsButton'); } } @@ -414,34 +414,6 @@ export class DashboardPageObject extends FtrService { await this.gotoDashboardLandingPage(); } - public async isMarginsOn() { - this.log.debug('isMarginsOn'); - await this.openOptions(); - return await this.testSubjects.getAttribute('dashboardMarginsCheckbox', 'checked'); - } - - public async useMargins(on = true) { - await this.openOptions(); - const isMarginsOn = await this.isMarginsOn(); - if (isMarginsOn !== 'on') { - return await this.testSubjects.click('dashboardMarginsCheckbox'); - } - } - - public async isColorSyncOn() { - this.log.debug('isColorSyncOn'); - await this.openOptions(); - return await this.testSubjects.getAttribute('dashboardSyncColorsCheckbox', 'checked'); - } - - public async useColorSync(on = true) { - await this.openOptions(); - const isColorSyncOn = await this.isColorSyncOn(); - if (isColorSyncOn !== 'on') { - return await this.testSubjects.click('dashboardSyncColorsCheckbox'); - } - } - public async gotoDashboardEditMode(dashboardName: string) { await this.loadSavedDashboard(dashboardName); await this.switchToEditMode(); @@ -751,12 +723,6 @@ export class DashboardPageObject extends FtrService { }); } - public async checkHideTitle() { - this.log.debug('ensure that you can click on hide title checkbox'); - await this.openOptions(); - return await this.testSubjects.click('dashboardPanelTitlesCheckbox'); - } - public async expectMissingSaveOption() { await this.testSubjects.missingOrFail('dashboardSaveMenuItem'); } @@ -777,6 +743,15 @@ export class DashboardPageObject extends FtrService { } } + public async expectQuickSaveButtonDisabled() { + this.log.debug('expectQuickSaveButtonDisabled'); + const quickSaveButton = await this.testSubjects.find('dashboardQuickSaveMenuItem'); + const isDisabled = await quickSaveButton.getAttribute('disabled'); + if (!isDisabled) { + throw new Error('Quick save button not disabled'); + } + } + public async getNotLoadedVisualizations(vizList: string[]) { const checkList = []; for (const name of vizList) { diff --git a/test/functional/services/dashboard/dashboard_settings.ts b/test/functional/services/dashboard/dashboard_settings.ts new file mode 100644 index 0000000000000..8dab3e0f1dd2f --- /dev/null +++ b/test/functional/services/dashboard/dashboard_settings.ts @@ -0,0 +1,128 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { FtrProviderContext } from '../../ftr_provider_context'; + +export function DashboardSettingsProvider({ getService }: FtrProviderContext) { + const log = getService('log'); + const retry = getService('retry'); + const toasts = getService('toasts'); + const testSubjects = getService('testSubjects'); + + return new (class DashboardSettingsPanel { + public readonly FLYOUT_TEST_SUBJ = 'dashboardSettingsFlyout'; + public readonly SYNC_TOOLTIPS_DATA_SUBJ = 'dashboardSyncTooltipsCheckbox'; + + async expectDashboardSettingsFlyoutOpen() { + log.debug('expectDashboardSettingsFlyoutOpen'); + await testSubjects.existOrFail(this.FLYOUT_TEST_SUBJ); + } + + async expectDashboardSettingsFlyoutClosed() { + log.debug('expectDashboardSettingsFlyoutClosed'); + await testSubjects.missingOrFail(this.FLYOUT_TEST_SUBJ); + } + + async expectDuplicateTitleWarningDisplayed() { + log.debug('expectDuplicateTitleWarningDisplayed'); + await testSubjects.existOrFail('duplicateTitleWarningMessage'); + } + + async findFlyout() { + log.debug('findFlyout'); + return await testSubjects.find(this.FLYOUT_TEST_SUBJ); + } + + public async findFlyoutTestSubject(testSubject: string) { + log.debug(`findFlyoutTestSubject::${testSubject}`); + const flyout = await this.findFlyout(); + return await flyout.findByCssSelector(`[data-test-subj="${testSubject}"]`); + } + + public async setCustomPanelTitle(customTitle: string) { + log.debug(`setCustomPanelTitle::${customTitle}`); + await testSubjects.setValue('dashboardTitleInput', customTitle, { + clearWithKeyboard: customTitle === '', // if clearing the title using the empty string as the new value, 'clearWithKeyboard' must be true; otherwise, false + }); + } + + public async setCustomPanelDescription(customDescription: string) { + log.debug(`setCustomPanelDescription::${customDescription}`); + await testSubjects.setValue('dashboardDescriptionInput', customDescription, { + clearWithKeyboard: customDescription === '', // if clearing the description using the empty string as the new value, 'clearWithKeyboard' must be true; otherwise, false + }); + } + + public async toggleStoreTimeWithDashboard(value: boolean) { + const status = value ? 'check' : 'uncheck'; + log.debug(`toggleStoreTimeWithDashboard::${status}`); + await testSubjects.setEuiSwitch('storeTimeWithDashboard', status); + } + + public async toggleUseMarginsBetweenPanels(value: boolean) { + const status = value ? 'check' : 'uncheck'; + log.debug(`toggleUseMarginsBetweenPanels::${status}`); + await testSubjects.setEuiSwitch('dashboardMarginsCheckbox', status); + } + + public async toggleShowPanelTitles(value: boolean) { + const status = value ? 'check' : 'uncheck'; + log.debug(`toggleShowPanelTitles::${status}`); + await testSubjects.setEuiSwitch('dashboardPanelTitlesCheckbox', status); + } + + public async toggleSyncCursor(value: boolean) { + const status = value ? 'check' : 'uncheck'; + log.debug(`toggleSyncCursor::${status}`); + await testSubjects.setEuiSwitch('dashboardSyncCursorCheckbox', status); + } + + public async isSyncTooltipsEnabled() { + log.debug('isSyncTooltipsEnabled'); + return await testSubjects.isEuiSwitchChecked(this.SYNC_TOOLTIPS_DATA_SUBJ); + } + + public async toggleSyncTooltips(value: boolean) { + const status = value ? 'check' : 'uncheck'; + log.debug(`toggleSyncTooltips::${status}`); + if (await this.isSyncTooltipsEnabled) { + await testSubjects.setEuiSwitch(this.SYNC_TOOLTIPS_DATA_SUBJ, status); + } + } + + public async isShowingDuplicateTitleWarning() { + log.debug('isShowingDuplicateTitleWarning'); + await testSubjects.exists('duplicateTitleWarningMessage'); + } + + public async clickApplyButton(shouldClose: boolean = true) { + log.debug('clickApplyButton'); + await retry.try(async () => { + await toasts.dismissAllToasts(); + await testSubjects.click('applyCustomizeDashboardButton'); + if (shouldClose) await this.expectDashboardSettingsFlyoutClosed(); + }); + } + + public async clickCancelButton() { + log.debug('clickCancelButton'); + await retry.try(async () => { + await testSubjects.click('cancelCustomizeDashboardButton'); + await this.expectDashboardSettingsFlyoutClosed(); + }); + } + + public async clickCloseFlyoutButton() { + log.debug(); + await retry.try(async () => { + await (await this.findFlyoutTestSubject('euiFlyoutCloseButton')).click(); + await this.expectDashboardSettingsFlyoutClosed(); + }); + } + })(); +} diff --git a/test/functional/services/index.ts b/test/functional/services/index.ts index e13cac581ce52..11975f560e2d7 100644 --- a/test/functional/services/index.ts +++ b/test/functional/services/index.ts @@ -56,6 +56,7 @@ import { MenuToggleService } from './menu_toggle'; import { MonacoEditorService } from './monaco_editor'; import { UsageCollectionService } from './usage_collection'; import { SavedObjectsFinderService } from './saved_objects_finder'; +import { DashboardSettingsProvider } from './dashboard/dashboard_settings'; export const services = { ...commonServiceProviders, @@ -80,6 +81,7 @@ export const services = { dashboardBadgeActions: DashboardBadgeActionsProvider, dashboardDrilldownPanelActions: DashboardDrilldownPanelActionsProvider, dashboardDrilldownsManage: DashboardDrilldownsManageProvider, + dashboardSettings: DashboardSettingsProvider, flyout: FlyoutService, comboBox: ComboBoxService, dataGrid: DataGridService, From 56e8e6953d50b7ae477aca07c49c5ec106b85323 Mon Sep 17 00:00:00 2001 From: Nick Peihl Date: Tue, 28 Mar 2023 09:02:29 -0400 Subject: [PATCH 06/18] Ehh, forgot to save my changes --- .../public/dashboard_app/top_nav/use_dashboard_menu_items.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/dashboard/public/dashboard_app/top_nav/use_dashboard_menu_items.tsx b/src/plugins/dashboard/public/dashboard_app/top_nav/use_dashboard_menu_items.tsx index b18786392084a..ef080885e75c1 100644 --- a/src/plugins/dashboard/public/dashboard_app/top_nav/use_dashboard_menu_items.tsx +++ b/src/plugins/dashboard/public/dashboard_app/top_nav/use_dashboard_menu_items.tsx @@ -254,7 +254,7 @@ export const useDashboardMenuItems = ({ } else { editModeItems.push(menuItems.switchToViewMode, menuItems.saveAs); } - return [...labsMenuItem, menuItems.options, ...shareMenuItem, ...editModeItems]; + return [...labsMenuItem, menuItems.settings, ...shareMenuItem, ...editModeItems]; }, [lastSavedId, menuItems, share, isLabsEnabled]); return { viewModeTopNavConfig, editModeTopNavConfig }; From 7659a6877a33aa30b968a46b93dd95f0a60f81b7 Mon Sep 17 00:00:00 2001 From: Nick Peihl Date: Tue, 28 Mar 2023 09:37:42 -0400 Subject: [PATCH 07/18] Better descriptions for accessibility --- .../dashboard_app/_dashboard_app_strings.ts | 2 +- .../component/settings/settings_flyout.tsx | 16 ++++++++++++---- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/plugins/dashboard/public/dashboard_app/_dashboard_app_strings.ts b/src/plugins/dashboard/public/dashboard_app/_dashboard_app_strings.ts index c021cef1ef8f6..5208b277e0461 100644 --- a/src/plugins/dashboard/public/dashboard_app/_dashboard_app_strings.ts +++ b/src/plugins/dashboard/public/dashboard_app/_dashboard_app_strings.ts @@ -328,7 +328,7 @@ export const topNavStrings = { defaultMessage: 'settings', }), description: i18n.translate('dashboard.topNave.settingsConfigDescription', { - defaultMessage: 'Settings', + defaultMessage: 'Change settings for your dashboard', }), }, clone: { diff --git a/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx b/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx index a7c8f8c39d37c..996c85f648ba6 100644 --- a/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx +++ b/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx @@ -323,11 +323,19 @@ export const DashboardSettings = ({ onClose }: DashboardSettingsProps) => { } }} fill + aria-describedby={isTitleDuplicate ? DUPLICATE_TITLE_CALLOUT_ID : undefined} > - + {isTitleDuplicate ? ( + + ) : ( + + )} From 75ed8362bad4afb6124121e09d6af305bc6860b6 Mon Sep 17 00:00:00 2001 From: Nick Peihl Date: Tue, 28 Mar 2023 09:43:04 -0400 Subject: [PATCH 08/18] Fix i18n ids --- .../component/settings/settings_flyout.tsx | 48 +++++++++++-------- 1 file changed, 27 insertions(+), 21 deletions(-) diff --git a/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx b/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx index 996c85f648ba6..d8b8162e14ae1 100644 --- a/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx +++ b/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx @@ -86,7 +86,7 @@ export const DashboardSettings = ({ onClose }: DashboardSettingsProps) => { } @@ -96,7 +96,7 @@ export const DashboardSettings = ({ onClose }: DashboardSettingsProps) => { >

{ } @@ -144,7 +144,7 @@ export const DashboardSettings = ({ onClose }: DashboardSettingsProps) => { } @@ -162,7 +162,7 @@ export const DashboardSettings = ({ onClose }: DashboardSettingsProps) => { updateDashboardSetting({ title: event.target.value }); }} aria-label={i18n.translate( - 'dashboardSettings.embeddableApi.flyout.optionsMenuForm.panelTitleInputAriaLabel', + 'dashboardSettings.embeddableApi.showSettings.flyout.form.panelTitleInputAriaLabel', { defaultMessage: 'Change the dashboard title', } @@ -173,7 +173,7 @@ export const DashboardSettings = ({ onClose }: DashboardSettingsProps) => { } @@ -186,7 +186,7 @@ export const DashboardSettings = ({ onClose }: DashboardSettingsProps) => { value={dashboardSettingsState.description ?? ''} onChange={(event) => updateDashboardSetting({ description: event.target.value })} aria-label={i18n.translate( - 'dashboardSettings.embeddableApi.flyout.optionsMenuForm.panelDescriptionAriaLabel', + 'dashboardSettings.embeddableApi.showSettings.flyout.form.panelDescriptionAriaLabel', { defaultMessage: 'Change the dashboard description', } @@ -197,7 +197,7 @@ export const DashboardSettings = ({ onClose }: DashboardSettingsProps) => { } @@ -208,7 +208,7 @@ export const DashboardSettings = ({ onClose }: DashboardSettingsProps) => { onChange={(event) => updateDashboardSetting({ timeRestore: event.target.checked })} label={ } @@ -216,9 +216,12 @@ export const DashboardSettings = ({ onClose }: DashboardSettingsProps) => { updateDashboardSetting({ useMargins: event.target.checked })} data-test-subj="dashboardMarginsCheckbox" @@ -227,9 +230,12 @@ export const DashboardSettings = ({ onClose }: DashboardSettingsProps) => { updateDashboardSetting({ hidePanelTitles: !event.target.checked }) @@ -242,7 +248,7 @@ export const DashboardSettings = ({ onClose }: DashboardSettingsProps) => { { { { onClick={() => onClose()} > @@ -327,12 +333,12 @@ export const DashboardSettings = ({ onClose }: DashboardSettingsProps) => { > {isTitleDuplicate ? ( ) : ( )} From 61770df627519c5a016b8e6e14af36f6fc41d713 Mon Sep 17 00:00:00 2001 From: Nick Peihl Date: Tue, 28 Mar 2023 10:34:58 -0400 Subject: [PATCH 09/18] Fix functional tests --- test/accessibility/apps/dashboard.ts | 14 ++++++++++---- .../services/dashboard/dashboard_settings.ts | 6 ++++++ .../apps/dashboard/group2/sync_colors.ts | 9 +++++++-- .../functional/tests/dashboard_integration.ts | 5 ++++- 4 files changed, 27 insertions(+), 7 deletions(-) diff --git a/test/accessibility/apps/dashboard.ts b/test/accessibility/apps/dashboard.ts index 3db8b99c577a4..15a9b6d8982b1 100644 --- a/test/accessibility/apps/dashboard.ts +++ b/test/accessibility/apps/dashboard.ts @@ -12,6 +12,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { const PageObjects = getPageObjects(['common', 'dashboard', 'header', 'home', 'settings']); const a11y = getService('a11y'); const dashboardAddPanel = getService('dashboardAddPanel'); + const dashboardSettings = getService('dashboardSettings'); const testSubjects = getService('testSubjects'); const listingTable = getService('listingTable'); @@ -65,18 +66,23 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await a11y.testAppSnapshot(); }); - it('open options menu', async () => { - await PageObjects.dashboard.openOptions(); + it('open settings flyout', async () => { + await PageObjects.dashboard.openSettingsFlyout(); await a11y.testAppSnapshot(); }); it('Should be able to hide panel titles', async () => { - await testSubjects.click('dashboardPanelTitlesCheckbox'); + await dashboardSettings.toggleShowPanelTitles(false); await a11y.testAppSnapshot(); }); it('Should be able display panels without margins', async () => { - await testSubjects.click('dashboardMarginsCheckbox'); + await dashboardSettings.toggleUseMarginsBetweenPanels(true); + await a11y.testAppSnapshot(); + }); + + it('close settings flyout', async () => { + await dashboardSettings.clickCancelButton(); await a11y.testAppSnapshot(); }); diff --git a/test/functional/services/dashboard/dashboard_settings.ts b/test/functional/services/dashboard/dashboard_settings.ts index 8dab3e0f1dd2f..d849d609fbf42 100644 --- a/test/functional/services/dashboard/dashboard_settings.ts +++ b/test/functional/services/dashboard/dashboard_settings.ts @@ -76,6 +76,12 @@ export function DashboardSettingsProvider({ getService }: FtrProviderContext) { await testSubjects.setEuiSwitch('dashboardPanelTitlesCheckbox', status); } + public async toggleSyncColors(value: boolean) { + const status = value ? 'check' : 'uncheck'; + log.debug(`toggleSyncColors::${status}`); + await testSubjects.setEuiSwitch('dashboardSyncColorsCheckbox', status); + } + public async toggleSyncCursor(value: boolean) { const status = value ? 'check' : 'uncheck'; log.debug(`toggleSyncCursor::${status}`); diff --git a/x-pack/test/functional/apps/dashboard/group2/sync_colors.ts b/x-pack/test/functional/apps/dashboard/group2/sync_colors.ts index 612f6c44c2c27..fc68346ac5745 100644 --- a/x-pack/test/functional/apps/dashboard/group2/sync_colors.ts +++ b/x-pack/test/functional/apps/dashboard/group2/sync_colors.ts @@ -20,6 +20,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { 'timePicker', ]); const dashboardAddPanel = getService('dashboardAddPanel'); + const dashboardSettings = getService('dashboardSettings'); const filterBar = getService('filterBar'); const elasticChart = getService('elasticChart'); const kibanaServer = getService('kibanaServer'); @@ -87,7 +88,9 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await filterBar.addFilter({ field: 'geo.src', operation: 'is not', value: 'CN' }); await PageObjects.lens.save('vis2', false, true); - await PageObjects.dashboard.useColorSync(true); + await PageObjects.dashboard.openSettingsFlyout(); + await dashboardSettings.toggleSyncColors(true); + await dashboardSettings.clickApplyButton(); await PageObjects.header.waitUntilLoadingHasFinished(); await PageObjects.dashboard.waitForRenderComplete(); @@ -116,7 +119,9 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { }); it('should be possible to disable color sync', async () => { - await PageObjects.dashboard.useColorSync(false); + await PageObjects.dashboard.openSettingsFlyout(); + await dashboardSettings.toggleSyncColors(false); + await dashboardSettings.clickApplyButton(); await PageObjects.header.waitUntilLoadingHasFinished(); const colorMapping1 = getColorMapping(await PageObjects.dashboard.getPanelChartDebugState(0)); const colorMapping2 = getColorMapping(await PageObjects.dashboard.getPanelChartDebugState(1)); diff --git a/x-pack/test/saved_object_tagging/functional/tests/dashboard_integration.ts b/x-pack/test/saved_object_tagging/functional/tests/dashboard_integration.ts index 37da0d3205c9c..14d59767b35ee 100644 --- a/x-pack/test/saved_object_tagging/functional/tests/dashboard_integration.ts +++ b/x-pack/test/saved_object_tagging/functional/tests/dashboard_integration.ts @@ -14,6 +14,7 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) { const kibanaServer = getService('kibanaServer'); const listingTable = getService('listingTable'); const testSubjects = getService('testSubjects'); + const dashboardSettings = getService('dashboardSettings'); const PageObjects = getPageObjects(['dashboard', 'tagManagement', 'common']); describe('dashboard integration', () => { @@ -161,7 +162,9 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) { it('retains dashboard saved object tags after quicksave', async () => { // edit and save dashboard await PageObjects.dashboard.gotoDashboardEditMode('dashboard 4 with real data (tag-1)'); - await PageObjects.dashboard.useMargins(false); // turn margins off to cause quicksave to be enabled + await PageObjects.dashboard.openSettingsFlyout(); + await dashboardSettings.toggleUseMarginsBetweenPanels(false); // turn margins off to cause quicksave to be enabled + await dashboardSettings.clickApplyButton(); await PageObjects.dashboard.clickQuickSave(); // verify dashboard still has original tags From 78e3c33b7bfceffc1249b0fd8914ad57bcb5f9da Mon Sep 17 00:00:00 2001 From: Nick Peihl Date: Tue, 28 Mar 2023 10:53:22 -0400 Subject: [PATCH 10/18] Use correct i18n namespace --- .../component/settings/settings_flyout.tsx | 36 +++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx b/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx index d8b8162e14ae1..95c9eaa8d4334 100644 --- a/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx +++ b/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx @@ -86,7 +86,7 @@ export const DashboardSettings = ({ onClose }: DashboardSettingsProps) => { } @@ -96,7 +96,7 @@ export const DashboardSettings = ({ onClose }: DashboardSettingsProps) => { >

{ } @@ -132,7 +132,7 @@ export const DashboardSettings = ({ onClose }: DashboardSettingsProps) => {

@@ -144,7 +144,7 @@ export const DashboardSettings = ({ onClose }: DashboardSettingsProps) => { } @@ -162,7 +162,7 @@ export const DashboardSettings = ({ onClose }: DashboardSettingsProps) => { updateDashboardSetting({ title: event.target.value }); }} aria-label={i18n.translate( - 'dashboardSettings.embeddableApi.showSettings.flyout.form.panelTitleInputAriaLabel', + 'dashboard.embeddableApi.showSettings.flyout.form.panelTitleInputAriaLabel', { defaultMessage: 'Change the dashboard title', } @@ -173,7 +173,7 @@ export const DashboardSettings = ({ onClose }: DashboardSettingsProps) => { } @@ -186,7 +186,7 @@ export const DashboardSettings = ({ onClose }: DashboardSettingsProps) => { value={dashboardSettingsState.description ?? ''} onChange={(event) => updateDashboardSetting({ description: event.target.value })} aria-label={i18n.translate( - 'dashboardSettings.embeddableApi.showSettings.flyout.form.panelDescriptionAriaLabel', + 'dashboard.embeddableApi.showSettings.flyout.form.panelDescriptionAriaLabel', { defaultMessage: 'Change the dashboard description', } @@ -197,7 +197,7 @@ export const DashboardSettings = ({ onClose }: DashboardSettingsProps) => { } @@ -208,7 +208,7 @@ export const DashboardSettings = ({ onClose }: DashboardSettingsProps) => { onChange={(event) => updateDashboardSetting({ timeRestore: event.target.checked })} label={ } @@ -217,7 +217,7 @@ export const DashboardSettings = ({ onClose }: DashboardSettingsProps) => { { { { { { onClick={() => onClose()} > @@ -333,12 +333,12 @@ export const DashboardSettings = ({ onClose }: DashboardSettingsProps) => { > {isTitleDuplicate ? ( ) : ( )} From 936fd45bb48b54268a999d6b55688e2455b6f163 Mon Sep 17 00:00:00 2001 From: Nick Peihl Date: Tue, 28 Mar 2023 10:53:42 -0400 Subject: [PATCH 11/18] Update docs --- docs/user/dashboard/dashboard.asciidoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/user/dashboard/dashboard.asciidoc b/docs/user/dashboard/dashboard.asciidoc index cd6a024da8172..f4f3aa74a8c8f 100644 --- a/docs/user/dashboard/dashboard.asciidoc +++ b/docs/user/dashboard/dashboard.asciidoc @@ -363,7 +363,7 @@ Apply a set of design options to the entire dashboard. . If you're in view mode, click *Edit* in the toolbar. -. In the toolbar, *Options*, then use the following options: +. In the toolbar, click *Settings*, to open the *Dashboard settings* flyout, then use the following options: * *Use margins between panels* — Adds a margin of space between each panel. From cb91e7a82094dfe74765ee17f6932d40eba45610 Mon Sep 17 00:00:00 2001 From: Nick Peihl Date: Tue, 28 Mar 2023 14:42:39 -0400 Subject: [PATCH 12/18] Fix broken import --- test/functional/apps/dashboard/group5/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/functional/apps/dashboard/group5/index.ts b/test/functional/apps/dashboard/group5/index.ts index f78f7e2d549b8..f8f2f4c2d0df7 100644 --- a/test/functional/apps/dashboard/group5/index.ts +++ b/test/functional/apps/dashboard/group5/index.ts @@ -29,7 +29,7 @@ export default function ({ getService, loadTestFile }: FtrProviderContext) { // This has to be first since the other tests create some embeddables as side affects and our counting assumes // a fresh index. loadTestFile(require.resolve('./empty_dashboard')); - loadTestFile(require.resolve('./dashboard_options')); + loadTestFile(require.resolve('./dashboard_settings')); loadTestFile(require.resolve('./data_shared_attributes')); loadTestFile(require.resolve('./share')); loadTestFile(require.resolve('./embed_mode')); From 264cf31155b0f20ec7294100fdc8de1763680ae2 Mon Sep 17 00:00:00 2001 From: Nick Peihl Date: Tue, 28 Mar 2023 14:47:26 -0400 Subject: [PATCH 13/18] Update i18n --- x-pack/plugins/translations/translations/fr-FR.json | 12 +++++------- x-pack/plugins/translations/translations/ja-JP.json | 12 +++++------- x-pack/plugins/translations/translations/zh-CN.json | 12 +++++------- 3 files changed, 15 insertions(+), 21 deletions(-) diff --git a/x-pack/plugins/translations/translations/fr-FR.json b/x-pack/plugins/translations/translations/fr-FR.json index 7114cba4ea494..d70d4f209bab9 100644 --- a/x-pack/plugins/translations/translations/fr-FR.json +++ b/x-pack/plugins/translations/translations/fr-FR.json @@ -1205,11 +1205,11 @@ "dashboard.topNav.cloneModal.enterNewNameForDashboardDescription": "Veuillez saisir un autre nom pour votre tableau de bord.", "dashboard.topNav.labsButtonAriaLabel": "ateliers", "dashboard.topNav.labsConfigDescription": "Ateliers", - "dashboard.topNav.options.hideAllPanelTitlesSwitchLabel": "Afficher les titres de panneau", - "dashboard.topNav.options.syncColorsBetweenPanelsSwitchLabel": "Synchroniser les palettes de couleur de tous les panneaux", - "dashboard.topNav.options.syncCursorBetweenPanelsSwitchLabel": "Synchroniser le curseur de tous les panneaux", - "dashboard.topNav.options.syncTooltipsBetweenPanelsSwitchLabel": "Synchroniser les infobulles de tous les panneaux", - "dashboard.topNav.options.useMarginsBetweenPanelsSwitchLabel": "Utiliser des marges entre les panneaux", + "dashboard.embeddableApi.showSettings.flyout.form.hideAllPanelTitlesSwitchLabel": "Afficher les titres de panneau", + "dashboard.embeddableApi.showSettings.flyout.form.syncColorsBetweenPanelsSwitchLabel": "Synchroniser les palettes de couleur de tous les panneaux", + "dashboard.embeddableApi.showSettings.flyout.form.syncCursorBetweenPanelsSwitchLabel": "Synchroniser le curseur de tous les panneaux", + "dashboard.embeddableApi.showSettings.flyout.form.syncTooltipsBetweenPanelsSwitchLabel": "Synchroniser les infobulles de tous les panneaux", + "dashboard.embeddableApi.showSettings.flyout.form.useMarginsBetweenPanelsSwitchLabel": "Utiliser des marges entre les panneaux", "dashboard.topNav.saveModal.descriptionFormRowLabel": "Description", "dashboard.topNav.saveModal.objectType": "tableau de bord", "dashboard.topNav.saveModal.storeTimeWithDashboardFormRowHelpText": "Le filtre temporel est défini sur l’option sélectionnée chaque fois que ce tableau de bord est chargé.", @@ -1221,8 +1221,6 @@ "dashboard.topNave.editConfigDescription": "Basculer en mode Édition", "dashboard.topNave.fullScreenButtonAriaLabel": "plein écran", "dashboard.topNave.fullScreenConfigDescription": "Mode Plein écran", - "dashboard.topNave.optionsButtonAriaLabel": "options", - "dashboard.topNave.optionsConfigDescription": "Options", "dashboard.topNave.saveAsButtonAriaLabel": "enregistrer sous", "dashboard.topNave.saveAsConfigDescription": "Enregistrer en tant que nouveau tableau de bord", "dashboard.topNave.saveButtonAriaLabel": "enregistrer", diff --git a/x-pack/plugins/translations/translations/ja-JP.json b/x-pack/plugins/translations/translations/ja-JP.json index 11c83f035f1d0..88b35acefd403 100644 --- a/x-pack/plugins/translations/translations/ja-JP.json +++ b/x-pack/plugins/translations/translations/ja-JP.json @@ -1205,11 +1205,11 @@ "dashboard.topNav.cloneModal.enterNewNameForDashboardDescription": "ダッシュボードの新しい名前を入力してください。", "dashboard.topNav.labsButtonAriaLabel": "ラボ", "dashboard.topNav.labsConfigDescription": "ラボ", - "dashboard.topNav.options.hideAllPanelTitlesSwitchLabel": "パネルタイトルを表示", - "dashboard.topNav.options.syncColorsBetweenPanelsSwitchLabel": "パネル全体でカラーパレットを同期", - "dashboard.topNav.options.syncCursorBetweenPanelsSwitchLabel": "パネル全体でカーソルを同期", - "dashboard.topNav.options.syncTooltipsBetweenPanelsSwitchLabel": "パネル間でツールチップを同期", - "dashboard.topNav.options.useMarginsBetweenPanelsSwitchLabel": "パネルの間に余白を使用", + "dashboard.embeddableApi.showSettings.flyout.form.hideAllPanelTitlesSwitchLabel": "パネルタイトルを表示", + "dashboard.embeddableApi.showSettings.flyout.form.syncColorsBetweenPanelsSwitchLabel": "パネル全体でカラーパレットを同期", + "dashboard.embeddableApi.showSettings.flyout.form.syncCursorBetweenPanelsSwitchLabel": "パネル全体でカーソルを同期", + "dashboard.embeddableApi.showSettings.flyout.form.syncTooltipsBetweenPanelsSwitchLabel": "パネル間でツールチップを同期", + "dashboard.embeddableApi.showSettings.flyout.form.useMarginsBetweenPanelsSwitchLabel": "パネルの間に余白を使用", "dashboard.topNav.saveModal.descriptionFormRowLabel": "説明", "dashboard.topNav.saveModal.objectType": "ダッシュボード", "dashboard.topNav.saveModal.storeTimeWithDashboardFormRowHelpText": "有効化すると、ダッシュボードが読み込まれるごとに現在選択された時刻の時間フィルターが変更されます。", @@ -1221,8 +1221,6 @@ "dashboard.topNave.editConfigDescription": "編集モードに切り替えます", "dashboard.topNave.fullScreenButtonAriaLabel": "全画面", "dashboard.topNave.fullScreenConfigDescription": "全画面モード", - "dashboard.topNave.optionsButtonAriaLabel": "オプション", - "dashboard.topNave.optionsConfigDescription": "オプション", "dashboard.topNave.saveAsButtonAriaLabel": "名前を付けて保存", "dashboard.topNave.saveAsConfigDescription": "新しいダッシュボードとして保存", "dashboard.topNave.saveButtonAriaLabel": "保存", diff --git a/x-pack/plugins/translations/translations/zh-CN.json b/x-pack/plugins/translations/translations/zh-CN.json index aa75fdff53bd7..23c304f759ad7 100644 --- a/x-pack/plugins/translations/translations/zh-CN.json +++ b/x-pack/plugins/translations/translations/zh-CN.json @@ -1205,11 +1205,11 @@ "dashboard.topNav.cloneModal.enterNewNameForDashboardDescription": "请为您的仪表板输入新的名称。", "dashboard.topNav.labsButtonAriaLabel": "实验", "dashboard.topNav.labsConfigDescription": "实验", - "dashboard.topNav.options.hideAllPanelTitlesSwitchLabel": "显示面板标题", - "dashboard.topNav.options.syncColorsBetweenPanelsSwitchLabel": "在面板之间同步调色板", - "dashboard.topNav.options.syncCursorBetweenPanelsSwitchLabel": "在面板之间同步光标", - "dashboard.topNav.options.syncTooltipsBetweenPanelsSwitchLabel": "在面板之间同步工具提示", - "dashboard.topNav.options.useMarginsBetweenPanelsSwitchLabel": "在面板间使用边距", + "dashboard.embeddableApi.showSettings.flyout.form.hideAllPanelTitlesSwitchLabel": "显示面板标题", + "dashboard.embeddableApi.showSettings.flyout.form.syncColorsBetweenPanelsSwitchLabel": "在面板之间同步调色板", + "dashboard.embeddableApi.showSettings.flyout.form.syncCursorBetweenPanelsSwitchLabel": "在面板之间同步光标", + "dashboard.embeddableApi.showSettings.flyout.form.syncTooltipsBetweenPanelsSwitchLabel": "在面板之间同步工具提示", + "dashboard.embeddableApi.showSettings.flyout.form.useMarginsBetweenPanelsSwitchLabel": "在面板间使用边距", "dashboard.topNav.saveModal.descriptionFormRowLabel": "描述", "dashboard.topNav.saveModal.objectType": "仪表板", "dashboard.topNav.saveModal.storeTimeWithDashboardFormRowHelpText": "每次加载此仪表板时，都会将时间筛选更改为当前选定的时间。", @@ -1221,8 +1221,6 @@ "dashboard.topNave.editConfigDescription": "切换到编辑模式", "dashboard.topNave.fullScreenButtonAriaLabel": "全屏", "dashboard.topNave.fullScreenConfigDescription": "全屏模式", - "dashboard.topNave.optionsButtonAriaLabel": "选项", - "dashboard.topNave.optionsConfigDescription": "选项", "dashboard.topNave.saveAsButtonAriaLabel": "另存为", "dashboard.topNave.saveAsConfigDescription": "另存为新仪表板", "dashboard.topNave.saveButtonAriaLabel": "保存", From efd42f8d93fcab7449d707c8dee8352a3c8766f6 Mon Sep 17 00:00:00 2001 From: Nick Peihl Date: Wed, 29 Mar 2023 11:29:05 -0400 Subject: [PATCH 14/18] Review feedback --- .../dashboard_app/_dashboard_app_strings.ts | 2 +- .../top_nav/use_dashboard_menu_items.tsx | 8 ++--- .../component/settings/settings_flyout.tsx | 34 +++++++++++-------- 3 files changed, 25 insertions(+), 19 deletions(-) diff --git a/src/plugins/dashboard/public/dashboard_app/_dashboard_app_strings.ts b/src/plugins/dashboard/public/dashboard_app/_dashboard_app_strings.ts index 5208b277e0461..c10646430494d 100644 --- a/src/plugins/dashboard/public/dashboard_app/_dashboard_app_strings.ts +++ b/src/plugins/dashboard/public/dashboard_app/_dashboard_app_strings.ts @@ -328,7 +328,7 @@ export const topNavStrings = { defaultMessage: 'settings', }), description: i18n.translate('dashboard.topNave.settingsConfigDescription', { - defaultMessage: 'Change settings for your dashboard', + defaultMessage: 'Open dashboard settings', }), }, clone: { diff --git a/src/plugins/dashboard/public/dashboard_app/top_nav/use_dashboard_menu_items.tsx b/src/plugins/dashboard/public/dashboard_app/top_nav/use_dashboard_menu_items.tsx index ef080885e75c1..645aa42ec918d 100644 --- a/src/plugins/dashboard/public/dashboard_app/top_nav/use_dashboard_menu_items.tsx +++ b/src/plugins/dashboard/public/dashboard_app/top_nav/use_dashboard_menu_items.tsx @@ -176,7 +176,7 @@ export const useDashboardMenuItems = ({ saveAs: { description: topNavStrings.saveAs.description, - disableButton: isSaveInProgress, + disableButton: isSaveInProgress || hasOverlays, id: 'save', emphasize: !Boolean(lastSavedId), testId: 'dashboardSaveMenuItem', @@ -188,7 +188,7 @@ export const useDashboardMenuItems = ({ switchToViewMode: { ...topNavStrings.switchToViewMode, id: 'cancel', - disableButton: isSaveInProgress || !lastSavedId, + disableButton: isSaveInProgress || !lastSavedId || hasOverlays, testId: 'dashboardViewOnlyMode', run: () => returnToViewMode(), } as TopNavMenuData, @@ -197,7 +197,7 @@ export const useDashboardMenuItems = ({ ...topNavStrings.share, id: 'share', testId: 'shareTopNavButton', - disableButton: isSaveInProgress, + disableButton: isSaveInProgress || hasOverlays, run: showShare, } as TopNavMenuData, @@ -205,7 +205,7 @@ export const useDashboardMenuItems = ({ ...topNavStrings.settings, id: 'settings', testId: 'dashboardSettingsButton', - disableButton: isSaveInProgress, + disableButton: isSaveInProgress || hasOverlays, run: () => dashboardContainer.showSettings(), } as TopNavMenuData, diff --git a/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx b/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx index 95c9eaa8d4334..b428eea3556d9 100644 --- a/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx +++ b/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx @@ -54,6 +54,7 @@ export const DashboardSettings = ({ onClose }: DashboardSettingsProps) => { const [isTitleDuplicate, setIsTitleDuplicate] = useState(false); const [isTitleDuplicateConfirmed, setIsTitleDuplicateConfirmed] = useState(false); + const [isApplying, setIsApplying] = useState(false); const lastSavedId = select((state) => state.componentState.lastSavedId); const lastSavedTitle = select((state) => state.explicitInput.title); @@ -63,6 +64,23 @@ export const DashboardSettings = ({ onClose }: DashboardSettingsProps) => { setIsTitleDuplicateConfirmed(true); }; + const onApply = async () => { + setIsApplying(true); + if ( + await checkForDuplicateDashboardTitle({ + title: dashboardSettingsState.title, + copyOnSave: false, + lastSavedTitle, + onTitleDuplicate, + isTitleDuplicateConfirmed, + }) + ) { + dispatch(setStateFromSettingsFlyout({ lastSavedId, ...dashboardSettingsState })); + onClose(); + } + setIsApplying(false); + }; + const updateDashboardSetting = useCallback( (newSettings: Partial) => { setDashboardSettingsState((prevDashboardSettingsState) => { @@ -314,22 +332,10 @@ export const DashboardSettings = ({ onClose }: DashboardSettingsProps) => { { - if ( - await checkForDuplicateDashboardTitle({ - title: dashboardSettingsState.title, - copyOnSave: false, - lastSavedTitle, - onTitleDuplicate, - isTitleDuplicateConfirmed, - }) - ) { - dispatch(setStateFromSettingsFlyout({ lastSavedId, ...dashboardSettingsState })); - onClose(); - } - }} + onClick={onApply} fill aria-describedby={isTitleDuplicate ? DUPLICATE_TITLE_CALLOUT_ID : undefined} + isLoading={isApplying} > {isTitleDuplicate ? ( Date: Wed, 29 Mar 2023 15:10:37 -0400 Subject: [PATCH 15/18] Handle incomplete async functions when unmounted --- .../component/settings/settings_flyout.tsx | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx b/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx index b428eea3556d9..7e548054ce7a6 100644 --- a/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx +++ b/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx @@ -6,7 +6,7 @@ * Side Public License, v 1. */ -import React, { useCallback, useState } from 'react'; +import React, { useCallback, useEffect, useRef, useState } from 'react'; import { i18n } from '@kbn/i18n'; import { EuiFormRow, @@ -59,9 +59,19 @@ export const DashboardSettings = ({ onClose }: DashboardSettingsProps) => { const lastSavedId = select((state) => state.componentState.lastSavedId); const lastSavedTitle = select((state) => state.explicitInput.title); + const isMounted = useRef(false); + useEffect(() => { + isMounted.current = true; + return () => { + isMounted.current = false; + }; + }, []); + const onTitleDuplicate = () => { + if (!isMounted.current) return; setIsTitleDuplicate(true); setIsTitleDuplicateConfirmed(true); + setIsApplying(false); }; const onApply = async () => { @@ -78,7 +88,6 @@ export const DashboardSettings = ({ onClose }: DashboardSettingsProps) => { dispatch(setStateFromSettingsFlyout({ lastSavedId, ...dashboardSettingsState })); onClose(); } - setIsApplying(false); }; const updateDashboardSetting = useCallback( @@ -319,10 +328,7 @@ export const DashboardSettings = ({ onClose }: DashboardSettingsProps) => { - onClose()} - > + Date: Wed, 29 Mar 2023 16:18:25 -0400 Subject: [PATCH 16/18] Review feedback --- .../component/settings/settings_flyout.tsx | 35 +++++++++---------- 1 file changed, 16 insertions(+), 19 deletions(-) diff --git a/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx b/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx index 7e548054ce7a6..9d4f68f1c7944 100644 --- a/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx +++ b/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx @@ -6,7 +6,8 @@ * Side Public License, v 1. */ -import React, { useCallback, useEffect, useRef, useState } from 'react'; +import React, { useCallback, useState } from 'react'; +import useMountedState from 'react-use/lib/useMountedState'; import { i18n } from '@kbn/i18n'; import { EuiFormRow, @@ -59,32 +60,28 @@ export const DashboardSettings = ({ onClose }: DashboardSettingsProps) => { const lastSavedId = select((state) => state.componentState.lastSavedId); const lastSavedTitle = select((state) => state.explicitInput.title); - const isMounted = useRef(false); - useEffect(() => { - isMounted.current = true; - return () => { - isMounted.current = false; - }; - }, []); + const isMounted = useMountedState(); const onTitleDuplicate = () => { - if (!isMounted.current) return; + if (!isMounted()) return; setIsTitleDuplicate(true); setIsTitleDuplicateConfirmed(true); - setIsApplying(false); }; const onApply = async () => { setIsApplying(true); - if ( - await checkForDuplicateDashboardTitle({ - title: dashboardSettingsState.title, - copyOnSave: false, - lastSavedTitle, - onTitleDuplicate, - isTitleDuplicateConfirmed, - }) - ) { + const validTitle = await checkForDuplicateDashboardTitle({ + title: dashboardSettingsState.title, + copyOnSave: false, + lastSavedTitle, + onTitleDuplicate, + isTitleDuplicateConfirmed, + }); + setIsApplying(false); + + if (!isMounted()) return; + + if (validTitle) { dispatch(setStateFromSettingsFlyout({ lastSavedId, ...dashboardSettingsState })); onClose(); } From 26393ed01240788f6b48f19676d0b004c466256b Mon Sep 17 00:00:00 2001 From: Nick Peihl Date: Wed, 29 Mar 2023 16:33:51 -0400 Subject: [PATCH 17/18] Move setIsApplying function after isMounted check --- .../dashboard_container/component/settings/settings_flyout.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx b/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx index 9d4f68f1c7944..d7cd61cdd7f7c 100644 --- a/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx +++ b/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx @@ -77,10 +77,11 @@ export const DashboardSettings = ({ onClose }: DashboardSettingsProps) => { onTitleDuplicate, isTitleDuplicateConfirmed, }); - setIsApplying(false); if (!isMounted()) return; + setIsApplying(false); + if (validTitle) { dispatch(setStateFromSettingsFlyout({ lastSavedId, ...dashboardSettingsState })); onClose(); From e9529205a24c90d74318f0fa92da84944dd19d68 Mon Sep 17 00:00:00 2001 From: Nick Peihl Date: Thu, 30 Mar 2023 09:15:41 -0400 Subject: [PATCH 18/18] Fix flaky functional tests --- test/functional/services/dashboard/dashboard_settings.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/functional/services/dashboard/dashboard_settings.ts b/test/functional/services/dashboard/dashboard_settings.ts index d849d609fbf42..0796444ea7189 100644 --- a/test/functional/services/dashboard/dashboard_settings.ts +++ b/test/functional/services/dashboard/dashboard_settings.ts @@ -118,6 +118,7 @@ export function DashboardSettingsProvider({ getService }: FtrProviderContext) { public async clickCancelButton() { log.debug('clickCancelButton'); await retry.try(async () => { + await toasts.dismissAllToasts(); await testSubjects.click('cancelCustomizeDashboardButton'); await this.expectDashboardSettingsFlyoutClosed(); }); @@ -126,6 +127,7 @@ export function DashboardSettingsProvider({ getService }: FtrProviderContext) { public async clickCloseFlyoutButton() { log.debug(); await retry.try(async () => { + await toasts.dismissAllToasts(); await (await this.findFlyoutTestSubject('euiFlyoutCloseButton')).click(); await this.expectDashboardSettingsFlyoutClosed(); });