diff --git a/src/plugins/dashboard/public/application/listing/confirm_overlays.tsx b/src/plugins/dashboard/public/application/listing/confirm_overlays.tsx index 41b27b4fd69260..d302bb4216bc49 100644 --- a/src/plugins/dashboard/public/application/listing/confirm_overlays.tsx +++ b/src/plugins/dashboard/public/application/listing/confirm_overlays.tsx @@ -40,6 +40,60 @@ export const confirmDiscardUnsavedChanges = ( } }); +export type DiscardOrKeepSelection = 'cancel' | 'discard' | 'keep'; + +export const confirmDiscardOrKeepUnsavedChanges = ( + overlays: OverlayStart +): Promise => { + return new Promise((resolve) => { + const session = overlays.openModal( + toMountPoint( + <> + + {leaveConfirmStrings.getLeaveEditModeTitle()} + + + + {leaveConfirmStrings.getLeaveEditModeSubtitle()} + + + + session.close()} + > + {leaveConfirmStrings.getCancelButtonText()} + + { + session.close(); + resolve('keep'); + }} + > + {leaveConfirmStrings.getKeepChangesText()} + + { + session.close(); + resolve('discard'); + }} + > + {leaveConfirmStrings.getConfirmButtonText()} + + + + ), + { + 'data-test-subj': 'dashboardDiscardConfirmModal', + } + ); + }); +}; + export const confirmCreateWithUnsaved = ( overlays: OverlayStart, startBlankCallback: () => void, diff --git a/src/plugins/dashboard/public/application/top_nav/dashboard_top_nav.tsx b/src/plugins/dashboard/public/application/top_nav/dashboard_top_nav.tsx index 786afc81c400cd..e68d371ebb2703 100644 --- a/src/plugins/dashboard/public/application/top_nav/dashboard_top_nav.tsx +++ b/src/plugins/dashboard/public/application/top_nav/dashboard_top_nav.tsx @@ -43,7 +43,7 @@ import { showOptionsPopover } from './show_options_popover'; import { TopNavIds } from './top_nav_ids'; import { ShowShareModal } from './show_share_modal'; import { PanelToolbar } from './panel_toolbar'; -import { confirmDiscardUnsavedChanges } from '../listing/confirm_overlays'; +import { confirmDiscardOrKeepUnsavedChanges } from '../listing/confirm_overlays'; import { OverlayRef } from '../../../../../core/public'; import { getNewDashboardTitle } from '../../dashboard_strings'; import { DASHBOARD_PANELS_UNSAVED_ID } from '../lib/dashboard_panel_storage'; @@ -152,34 +152,53 @@ export function DashboardTopNav({ } }, [state.addPanelOverlay]); - const onDiscardChanges = useCallback(() => { - function revertChangesAndExitEditMode() { - dashboardStateManager.resetState(); - dashboardStateManager.clearUnsavedPanels(); - - // We need to do a hard reset of the timepicker. appState will not reload like - // it does on 'open' because it's been saved to the url and the getAppState.previouslyStored() check on - // reload will cause it not to sync. - if (dashboardStateManager.getIsTimeSavedWithDashboard()) { - dashboardStateManager.syncTimefilterWithDashboardTime(timefilter); - dashboardStateManager.syncTimefilterWithDashboardRefreshInterval(timefilter); - } - dashboardStateManager.switchViewMode(ViewMode.VIEW); - } - confirmDiscardUnsavedChanges(core.overlays, revertChangesAndExitEditMode); - }, [core.overlays, dashboardStateManager, timefilter]); - const onChangeViewMode = useCallback( (newMode: ViewMode) => { clearAddPanel(); - if (savedDashboard?.id && allowByValueEmbeddables) { - const { getFullEditPath, title, id } = savedDashboard; - chrome.recentlyAccessed.add(getFullEditPath(newMode === ViewMode.EDIT), title, id); + const isPageRefresh = newMode === dashboardStateManager.getViewMode(); + const isLeavingEditMode = !isPageRefresh && newMode === ViewMode.VIEW; + const willLoseChanges = isLeavingEditMode && dashboardStateManager.getIsDirty(timefilter); + + function switchViewMode() { + dashboardStateManager.switchViewMode(newMode); + dashboardStateManager.restorePanels(); + + if (savedDashboard?.id && allowByValueEmbeddables) { + const { getFullEditPath, title, id } = savedDashboard; + chrome.recentlyAccessed.add(getFullEditPath(newMode === ViewMode.EDIT), title, id); + } + } + + if (!willLoseChanges) { + switchViewMode(); + return; } - dashboardStateManager.switchViewMode(newMode); - dashboardStateManager.restorePanels(); + + function discardChanges() { + dashboardStateManager.resetState(); + dashboardStateManager.clearUnsavedPanels(); + + // We need to do a hard reset of the timepicker. appState will not reload like + // it does on 'open' because it's been saved to the url and the getAppState.previouslyStored() check on + // reload will cause it not to sync. + if (dashboardStateManager.getIsTimeSavedWithDashboard()) { + dashboardStateManager.syncTimefilterWithDashboardTime(timefilter); + dashboardStateManager.syncTimefilterWithDashboardRefreshInterval(timefilter); + } + dashboardStateManager.switchViewMode(ViewMode.VIEW); + } + confirmDiscardOrKeepUnsavedChanges(core.overlays).then((selection) => { + if (selection === 'discard') { + discardChanges(); + } + if (selection !== 'cancel') { + switchViewMode(); + } + }); }, [ + timefilter, + core.overlays, clearAddPanel, savedDashboard, dashboardStateManager, @@ -381,7 +400,6 @@ export function DashboardTopNav({ }, [TopNavIds.EXIT_EDIT_MODE]: () => onChangeViewMode(ViewMode.VIEW), [TopNavIds.ENTER_EDIT_MODE]: () => onChangeViewMode(ViewMode.EDIT), - [TopNavIds.DISCARD_CHANGES]: onDiscardChanges, [TopNavIds.SAVE]: runSave, [TopNavIds.QUICK_SAVE]: runQuickSave, [TopNavIds.CLONE]: runClone, @@ -417,7 +435,6 @@ export function DashboardTopNav({ }, [ dashboardCapabilities, dashboardStateManager, - onDiscardChanges, onChangeViewMode, savedDashboard, runClone, diff --git a/src/plugins/dashboard/public/application/top_nav/get_top_nav_config.ts b/src/plugins/dashboard/public/application/top_nav/get_top_nav_config.ts index abc128369017c5..26eea1b5f718de 100644 --- a/src/plugins/dashboard/public/application/top_nav/get_top_nav_config.ts +++ b/src/plugins/dashboard/public/application/top_nav/get_top_nav_config.ts @@ -41,14 +41,12 @@ export function getTopNavConfig( getOptionsConfig(actions[TopNavIds.OPTIONS]), getShareConfig(actions[TopNavIds.SHARE]), getViewConfig(actions[TopNavIds.EXIT_EDIT_MODE]), - getDiscardConfig(actions[TopNavIds.DISCARD_CHANGES]), getSaveConfig(actions[TopNavIds.SAVE], options.isNewDashboard), ] : [ getOptionsConfig(actions[TopNavIds.OPTIONS]), getShareConfig(actions[TopNavIds.SHARE]), getViewConfig(actions[TopNavIds.EXIT_EDIT_MODE]), - getDiscardConfig(actions[TopNavIds.DISCARD_CHANGES]), getSaveConfig(actions[TopNavIds.SAVE]), getQuickSave(actions[TopNavIds.QUICK_SAVE]), ]; @@ -154,23 +152,6 @@ function getViewConfig(action: NavAction) { }; } -/** - * @returns {kbnTopNavConfig} - */ -function getDiscardConfig(action: NavAction) { - return { - id: 'discard', - label: i18n.translate('dashboard.topNave.discardlButtonAriaLabel', { - defaultMessage: 'discard', - }), - description: i18n.translate('dashboard.topNave.discardConfigDescription', { - defaultMessage: 'Discard unsaved changes', - }), - testId: 'dashboardDiscardChanges', - run: action, - }; -} - /** * @returns {kbnTopNavConfig} */ diff --git a/src/plugins/dashboard/public/application/top_nav/top_nav_ids.ts b/src/plugins/dashboard/public/application/top_nav/top_nav_ids.ts index 92a0db6bd0ba2e..ee3d08e2330ae9 100644 --- a/src/plugins/dashboard/public/application/top_nav/top_nav_ids.ts +++ b/src/plugins/dashboard/public/application/top_nav/top_nav_ids.ts @@ -13,7 +13,6 @@ export const TopNavIds = { SAVE: 'save', EXIT_EDIT_MODE: 'exitEditMode', ENTER_EDIT_MODE: 'enterEditMode', - DISCARD_CHANGES: 'discard', CLONE: 'clone', FULL_SCREEN: 'fullScreenMode', }; diff --git a/src/plugins/dashboard/public/dashboard_strings.ts b/src/plugins/dashboard/public/dashboard_strings.ts index 96bd32088ec38b..8588fbc1bbdc7f 100644 --- a/src/plugins/dashboard/public/dashboard_strings.ts +++ b/src/plugins/dashboard/public/dashboard_strings.ts @@ -253,6 +253,18 @@ export const leaveConfirmStrings = { i18n.translate('dashboard.appLeaveConfirmModal.unsavedChangesSubtitle', { defaultMessage: 'Leave Dashboard with unsaved work?', }), + getKeepChangesText: () => + i18n.translate('dashboard.appLeaveConfirmModal.keepUnsavedChangesButtonLabel', { + defaultMessage: 'Keep unsaved changes', + }), + getLeaveEditModeTitle: () => + i18n.translate('dashboard.changeViewModeConfirmModal.leaveEditMode', { + defaultMessage: 'Leave edit mode with unsaved work?', + }), + getLeaveEditModeSubtitle: () => + i18n.translate('dashboard.changeViewModeConfirmModal.discardChangesOptionalDescription', { + defaultMessage: `If you discard your changes, there's no getting them back.`, + }), getDiscardTitle: () => i18n.translate('dashboard.changeViewModeConfirmModal.discardChangesTitle', { defaultMessage: 'Discard changes to dashboard?', diff --git a/test/accessibility/apps/dashboard.ts b/test/accessibility/apps/dashboard.ts index 0171a462b13680..08d577b3df08cd 100644 --- a/test/accessibility/apps/dashboard.ts +++ b/test/accessibility/apps/dashboard.ts @@ -110,12 +110,13 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { }); it('Exit out of edit mode', async () => { - await PageObjects.dashboard.clickDiscardChanges(); + await PageObjects.dashboard.clickDiscardChanges(false); await a11y.testAppSnapshot(); }); it('Discard changes', async () => { - await PageObjects.common.clickConfirmOnModal(); + await testSubjects.exists('dashboardDiscardConfirmDiscard'); + await testSubjects.click('dashboardDiscardConfirmDiscard'); await PageObjects.dashboard.getIsInViewMode(); await a11y.testAppSnapshot(); }); diff --git a/test/functional/apps/dashboard/dashboard_unsaved_state.ts b/test/functional/apps/dashboard/dashboard_unsaved_state.ts index 851d7ab7461ed0..eaf0d2f2a97df2 100644 --- a/test/functional/apps/dashboard/dashboard_unsaved_state.ts +++ b/test/functional/apps/dashboard/dashboard_unsaved_state.ts @@ -80,6 +80,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { it('retains unsaved panel count after returning to edit mode', async () => { await PageObjects.header.waitUntilLoadingHasFinished(); await PageObjects.dashboard.switchToEditMode(); + await PageObjects.header.waitUntilLoadingHasFinished(); const currentPanelCount = await PageObjects.dashboard.getPanelCount(); expect(currentPanelCount).to.eql(unsavedPanelCount); }); diff --git a/test/functional/apps/dashboard/view_edit.ts b/test/functional/apps/dashboard/view_edit.ts index 5242e59efa0e99..6c7d60c9a15aa1 100644 --- a/test/functional/apps/dashboard/view_edit.ts +++ b/test/functional/apps/dashboard/view_edit.ts @@ -15,6 +15,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { const esArchiver = getService('esArchiver'); const kibanaServer = getService('kibanaServer'); const dashboardAddPanel = getService('dashboardAddPanel'); + const testSubjects = getService('testSubjects'); const PageObjects = getPageObjects(['dashboard', 'header', 'common', 'visualize', 'timePicker']); const dashboardName = 'dashboard with filter'; const filterBar = getService('filterBar'); @@ -74,9 +75,6 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { ); await PageObjects.dashboard.clickDiscardChanges(); - // confirm lose changes - await PageObjects.common.clickConfirmOnModal(); - const newTime = await PageObjects.timePicker.getTimeConfig(); expect(newTime.start).to.equal(originalTime.start); @@ -90,9 +88,6 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await PageObjects.dashboard.clickDiscardChanges(); - // confirm lose changes - await PageObjects.common.clickConfirmOnModal(); - const query = await queryBar.getQueryString(); expect(query).to.equal(originalQuery); }); @@ -113,9 +108,6 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await PageObjects.dashboard.clickDiscardChanges(); - // confirm lose changes - await PageObjects.common.clickConfirmOnModal(); - hasFilter = await filterBar.hasFilter('animal', 'dog'); expect(hasFilter).to.be(true); }); @@ -133,12 +125,13 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { redirectToOrigin: true, }); - await PageObjects.dashboard.clickDiscardChanges(); + await PageObjects.dashboard.clickDiscardChanges(false); // for this sleep see https://github.com/elastic/kibana/issues/22299 await PageObjects.common.sleep(500); // confirm lose changes - await PageObjects.common.clickConfirmOnModal(); + await testSubjects.exists('dashboardDiscardConfirmDiscard'); + await testSubjects.click('dashboardDiscardConfirmDiscard'); const panelCount = await PageObjects.dashboard.getPanelCount(); expect(panelCount).to.eql(originalPanelCount); @@ -150,9 +143,6 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await dashboardAddPanel.addVisualization('new viz panel'); await PageObjects.dashboard.clickDiscardChanges(); - // confirm lose changes - await PageObjects.common.clickConfirmOnModal(); - const panelCount = await PageObjects.dashboard.getPanelCount(); expect(panelCount).to.eql(originalPanelCount); }); @@ -171,9 +161,10 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { 'Sep 19, 2015 @ 06:31:44.000', 'Sep 19, 2015 @ 06:31:44.000' ); - await PageObjects.dashboard.clickDiscardChanges(); + await PageObjects.dashboard.clickDiscardChanges(false); - await PageObjects.common.clickCancelOnModal(); + await testSubjects.exists('dashboardDiscardConfirmCancel'); + await testSubjects.click('dashboardDiscardConfirmCancel'); await PageObjects.dashboard.saveDashboard(dashboardName, { storeTimeWithDashboard: true, }); @@ -200,9 +191,10 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { ); const newTime = await PageObjects.timePicker.getTimeConfig(); - await PageObjects.dashboard.clickDiscardChanges(); + await PageObjects.dashboard.clickDiscardChanges(false); - await PageObjects.common.clickCancelOnModal(); + await testSubjects.exists('dashboardDiscardConfirmCancel'); + await testSubjects.click('dashboardDiscardConfirmCancel'); await PageObjects.dashboard.saveDashboard(dashboardName, { storeTimeWithDashboard: true }); await PageObjects.dashboard.loadSavedDashboard(dashboardName); @@ -223,7 +215,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { 'Oct 19, 2014 @ 06:31:44.000', 'Dec 19, 2014 @ 06:31:44.000' ); - await PageObjects.dashboard.clickCancelOutOfEditMode(); + await PageObjects.dashboard.clickCancelOutOfEditMode(false); await PageObjects.common.expectConfirmModalOpenState(false); }); @@ -235,7 +227,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { const originalQuery = await queryBar.getQueryString(); await queryBar.setQuery(`${originalQuery}extra stuff`); - await PageObjects.dashboard.clickCancelOutOfEditMode(); + await PageObjects.dashboard.clickCancelOutOfEditMode(false); await PageObjects.common.expectConfirmModalOpenState(false); diff --git a/test/functional/page_objects/dashboard_page.ts b/test/functional/page_objects/dashboard_page.ts index 4291d67a6bc082..9c571f0f0ef86b 100644 --- a/test/functional/page_objects/dashboard_page.ts +++ b/test/functional/page_objects/dashboard_page.ts @@ -246,14 +246,26 @@ export function DashboardPageProvider({ getService, getPageObjects }: FtrProvide return await testSubjects.exists('dashboardEditMode'); } - public async clickCancelOutOfEditMode() { + public async clickCancelOutOfEditMode(accept = true) { log.debug('clickCancelOutOfEditMode'); await testSubjects.click('dashboardViewOnlyMode'); + if (accept) { + const confirmation = await testSubjects.exists('dashboardDiscardConfirmKeep'); + if (confirmation) { + await testSubjects.click('dashboardDiscardConfirmKeep'); + } + } } - public async clickDiscardChanges() { + public async clickDiscardChanges(accept = true) { log.debug('clickDiscardChanges'); - await testSubjects.click('dashboardDiscardChanges'); + await testSubjects.click('dashboardViewOnlyMode'); + if (accept) { + const confirmation = await testSubjects.exists('dashboardDiscardConfirmDiscard'); + if (confirmation) { + await testSubjects.click('dashboardDiscardConfirmDiscard'); + } + } } public async clickQuickSave() {