From 1e2bb42f2ea659c32ec36b458115eaf55aa3ba7d Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Thu, 24 Dec 2020 10:57:09 -0500 Subject: [PATCH 01/30] First pass at removing panels from URL. Everything should work --- .../application/dashboard_state_manager.ts | 112 +++++++++++++----- .../hooks/use_dashboard_breadcrumbs.ts | 22 +--- .../hooks/use_dashboard_state_manager.ts | 7 +- .../application/top_nav/dashboard_top_nav.tsx | 5 + src/plugins/dashboard/public/plugin.tsx | 2 +- src/plugins/dashboard/public/types.ts | 3 +- 6 files changed, 94 insertions(+), 57 deletions(-) diff --git a/src/plugins/dashboard/public/application/dashboard_state_manager.ts b/src/plugins/dashboard/public/application/dashboard_state_manager.ts index daa0bbdfc9f8a..9dbebdd4d87f2 100644 --- a/src/plugins/dashboard/public/application/dashboard_state_manager.ts +++ b/src/plugins/dashboard/public/application/dashboard_state_manager.ts @@ -38,6 +38,7 @@ import { } from '../types'; import { ViewMode } from '../services/embeddable'; +import { Storage } from '../../../kibana_utils/public'; import { UsageCollectionSetup } from '../services/usage_collection'; import { Filter, Query, TimefilterContract as Timefilter } from '../services/data'; import type { SavedObjectTagDecoratorTypeGuard } from '../services/saved_objects_tagging_oss'; @@ -49,6 +50,9 @@ import { syncState, } from '../services/kibana_utils'; +const DASHBOARD_PANELS_SESSION_KEY = 'dashboardStateManagerPanels'; +const DASHBOARD_PANELS_UNSAVED_ID = 'unsavedDashboard'; + /** * Dashboard state manager handles connecting angular and redux state between the angular and react portions of the * app. There are two "sources of truth" that need to stay in sync - AppState (aka the `_a` portion of the url) and @@ -84,8 +88,10 @@ export class DashboardStateManager { private readonly stateContainerChangeSub: Subscription; private readonly STATE_STORAGE_KEY = '_a'; private readonly kbnUrlStateStorage: IKbnUrlStateStorage; + private readonly sessionStorage: Storage; private readonly stateSyncRef: ISyncStateRef; - private readonly history: History; + private readonly allowByValueEmbeddables: boolean; + private readonly usageCollection: UsageCollectionSetup | undefined; public readonly hasTaggingCapabilities: SavedObjectTagDecoratorTypeGuard; @@ -103,6 +109,7 @@ export class DashboardStateManager { kbnUrlStateStorage, history, usageCollection, + allowByValueEmbeddables, hasTaggingCapabilities, }: { savedDashboard: DashboardSavedObject; @@ -111,14 +118,15 @@ export class DashboardStateManager { kbnUrlStateStorage: IKbnUrlStateStorage; history: History; usageCollection?: UsageCollectionSetup; + allowByValueEmbeddables: boolean; hasTaggingCapabilities: SavedObjectTagDecoratorTypeGuard; }) { - this.history = history; this.kibanaVersion = kibanaVersion; this.savedDashboard = savedDashboard; this.hideWriteControls = hideWriteControls; this.usageCollection = usageCollection; this.hasTaggingCapabilities = hasTaggingCapabilities; + this.allowByValueEmbeddables = allowByValueEmbeddables; // get state defaults from saved dashboard, make sure it is migrated this.stateDefaults = migrateAppState( @@ -126,7 +134,7 @@ export class DashboardStateManager { kibanaVersion, usageCollection ); - + this.sessionStorage = new Storage(sessionStorage); this.kbnUrlStateStorage = kbnUrlStateStorage; // setup initial state by merging defaults with state from url @@ -134,6 +142,7 @@ export class DashboardStateManager { const initialState = migrateAppState( { ...this.stateDefaults, + ...this.getUnsavedPanelState(), ...this.kbnUrlStateStorage.get(this.STATE_STORAGE_KEY), }, kibanaVersion, @@ -176,10 +185,10 @@ export class DashboardStateManager { stateContainer: { ...this.stateContainer, get: () => this.toUrlState(this.stateContainer.get()), - set: (state: DashboardAppStateInUrl | null) => { + set: (stateFromUrl: DashboardAppStateInUrl | null) => { // sync state required state container to be able to handle null // overriding set() so it could handle null coming from url - if (state) { + if (stateFromUrl) { // Skip this update if current dashboardId in the url is different from what we have in the current instance of state manager // As dashboard is driven by angular at the moment, the destroy cycle happens async, // If the dashboardId has changed it means this instance @@ -190,7 +199,8 @@ export class DashboardStateManager { this.stateContainer.set({ ...this.stateDefaults, - ...state, + ...(this.getUnsavedPanelState() || {}), + ...stateFromUrl, }); } else { // Do nothing in case when state from url is empty, @@ -263,6 +273,8 @@ export class DashboardStateManager { if (dirty) { this.stateContainer.transitions.set('panels', Object.values(convertedPanelStateMap)); + this.setUnsavedPanels(this.getPanels()); + if (dirtyBecauseOfInitialStateMigration) { this.saveState({ replace: true }); } @@ -477,7 +489,14 @@ export class DashboardStateManager { } public getViewMode() { - return this.hideWriteControls ? ViewMode.VIEW : this.appState.viewMode; + if (this.hideWriteControls) { + return ViewMode.VIEW; + } + // get viewMode should work properly even before the state container is created + return this.stateContainer + ? this.appState.viewMode + : this.kbnUrlStateStorage.get(this.STATE_STORAGE_KEY)?.viewMode ?? + ViewMode.VIEW; } public getIsViewMode() { @@ -593,22 +612,6 @@ export class DashboardStateManager { return this.kbnUrlStateStorage.flush({ replace }); } - // TODO: find nicer solution for this - // this function helps to make just 1 browser history update, when we imperatively changing the dashboard url - // It could be that there is pending *dashboardStateManager* updates, which aren't flushed yet to the url. - // So to prevent 2 browser updates: - // 1. Force flush any pending state updates (syncing state to query) - // 2. If url was updated, then apply path change with replace - public changeDashboardUrl(pathname: string) { - // synchronously persist current state to url with push() - const updated = this.saveState({ replace: false }); - // change pathname - this.history[updated ? 'replace' : 'push']({ - ...this.history.location, - pathname, - }); - } - public setQuery(query: Query) { this.stateContainer.transitions.set('query', query); } @@ -638,6 +641,60 @@ export class DashboardStateManager { } } + public hydrateUnsavedPanels() { + const unsavedState = this.getUnsavedPanelState(); + if (!unsavedState || unsavedState.panels?.length === 0) { + return; + } + this.stateContainer.set( + migrateAppState( + { + ...this.stateDefaults, + ...unsavedState, + ...this.kbnUrlStateStorage.get(this.STATE_STORAGE_KEY), + }, + this.kibanaVersion, + this.usageCollection + ) + ); + } + + public clearUnsavedPanels() { + if (!this.allowByValueEmbeddables) { + return; + } + const sessionStoragePanels = this.sessionStorage.get(DASHBOARD_PANELS_SESSION_KEY) || {}; + sessionStoragePanels[this.savedDashboard.id || DASHBOARD_PANELS_UNSAVED_ID] = undefined; + this.sessionStorage.set(DASHBOARD_PANELS_SESSION_KEY, sessionStoragePanels); + } + + private getUnsavedPanelState(): { panels?: SavedDashboardPanel[] } { + if (!this.allowByValueEmbeddables || this.getIsViewMode()) { + return {}; + } + const panels = this.sessionStorage.get(DASHBOARD_PANELS_SESSION_KEY)?.[ + this.savedDashboard.id || DASHBOARD_PANELS_UNSAVED_ID + ]; + return panels ? { panels } : {}; + } + + private setUnsavedPanels(newPanels: SavedDashboardPanel[]) { + if (!this.allowByValueEmbeddables || this.getIsViewMode() || !this.getIsDirty()) { + return; + } + const sessionStoragePanels = this.sessionStorage.get(DASHBOARD_PANELS_SESSION_KEY) || {}; + sessionStoragePanels[this.savedDashboard.id || DASHBOARD_PANELS_UNSAVED_ID] = newPanels; + this.sessionStorage.set(DASHBOARD_PANELS_SESSION_KEY, sessionStoragePanels); + } + + private toUrlState(state: DashboardAppState): DashboardAppStateInUrl { + if (this.getIsEditMode() && !this.allowByValueEmbeddables) { + return state; + } + const { panels, ...stateWithoutPanels } = state; + return stateWithoutPanels; + } + private checkIsDirty() { // Filters need to be compared manually because they sometimes have a $$hashkey stored on the object. // Query needs to be compared manually because saved legacy queries get migrated in app state automatically @@ -647,13 +704,4 @@ export class DashboardStateManager { const current = _.omit(this.stateContainer.get(), propsToIgnore); return !_.isEqual(initial, current); } - - private toUrlState(state: DashboardAppState): DashboardAppStateInUrl { - if (state.viewMode === ViewMode.VIEW) { - const { panels, ...stateWithoutPanels } = state; - return stateWithoutPanels; - } - - return state; - } } diff --git a/src/plugins/dashboard/public/application/hooks/use_dashboard_breadcrumbs.ts b/src/plugins/dashboard/public/application/hooks/use_dashboard_breadcrumbs.ts index 2a9e3e0a5a9b2..ce4a6c4713aaf 100644 --- a/src/plugins/dashboard/public/application/hooks/use_dashboard_breadcrumbs.ts +++ b/src/plugins/dashboard/public/application/hooks/use_dashboard_breadcrumbs.ts @@ -49,32 +49,12 @@ export const useDashboardBreadcrumbs = ( return; } - const { - getConfirmButtonText, - getCancelButtonText, - getLeaveTitle, - getLeaveSubtitle, - } = leaveConfirmStrings; - setBreadcrumbs([ { text: getDashboardBreadcrumb(), 'data-test-subj': 'dashboardListingBreadcrumb', onClick: () => { - if (dashboardStateManager.getIsDirty()) { - openConfirm(getLeaveSubtitle(), { - confirmButtonText: getConfirmButtonText(), - cancelButtonText: getCancelButtonText(), - defaultFocusedButton: EUI_MODAL_CANCEL_BUTTON, - title: getLeaveTitle(), - }).then((isConfirmed) => { - if (isConfirmed) { - redirectTo({ destination: 'listing' }); - } - }); - } else { - redirectTo({ destination: 'listing' }); - } + redirectTo({ destination: 'listing' }); }, }, { diff --git a/src/plugins/dashboard/public/application/hooks/use_dashboard_state_manager.ts b/src/plugins/dashboard/public/application/hooks/use_dashboard_state_manager.ts index 7aadfe40ebf08..ca61368ae0539 100644 --- a/src/plugins/dashboard/public/application/hooks/use_dashboard_state_manager.ts +++ b/src/plugins/dashboard/public/application/hooks/use_dashboard_state_manager.ts @@ -39,6 +39,7 @@ import { createSessionRestorationDataProvider } from '../lib/session_restoration import { DashboardStateManager } from '../dashboard_state_manager'; import { getDashboardTitle } from '../../dashboard_strings'; import { DashboardAppServices } from '../types'; +import { DashboardFeatureFlagConfig } from '../..'; // TS is picky with type guards, we can't just inline `() => false` function defaultTaggingGuard(_obj: SavedObject): _obj is TagDecoratedSavedObject { @@ -55,8 +56,8 @@ export const useDashboardStateManager = ( uiSettings, usageCollection, initializerContext, - dashboardCapabilities, savedObjectsTagging, + dashboardCapabilities, } = useKibana().services; // Destructure and rename services; makes the Effect hook more specific, makes later @@ -85,6 +86,8 @@ export const useDashboardStateManager = ( useHash: uiSettings.get('state:storeInSessionStorage'), ...withNotifyOnErrors(toasts), }); + const allowByValueEmbeddables = initializerContext.config.get() + .allowByValueEmbeddables; const stateManager = new DashboardStateManager({ hasTaggingCapabilities, @@ -94,6 +97,7 @@ export const useDashboardStateManager = ( kibanaVersion, savedDashboard, usageCollection, + allowByValueEmbeddables, }); // sync initial app filters from state to filterManager @@ -183,6 +187,7 @@ export const useDashboardStateManager = ( dataPlugin, filterManager, hasTaggingCapabilities, + initializerContext.config, hideWriteControls, history, kibanaVersion, 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 915f245fbcd19..d875c0a74c74d 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 @@ -147,6 +147,9 @@ export function DashboardTopNav({ if (!willLoseChanges) { dashboardStateManager.switchViewMode(newMode); + if (newMode === ViewMode.EDIT) { + dashboardStateManager.hydrateUnsavedPanels(); + } return; } @@ -154,6 +157,7 @@ export function DashboardTopNav({ dashboardStateManager.resetState(); // This is only necessary for new dashboards, which will default to Edit mode. dashboardStateManager.switchViewMode(ViewMode.VIEW); + 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 @@ -206,6 +210,7 @@ export function DashboardTopNav({ 'data-test-subj': 'saveDashboardSuccess', }); + dashboardStateManager.clearUnsavedPanels(); if (id !== lastDashboardId) { redirectTo({ destination: 'dashboard', id }); } else { diff --git a/src/plugins/dashboard/public/plugin.tsx b/src/plugins/dashboard/public/plugin.tsx index 4dff423098c5a..12e64669cdb9b 100644 --- a/src/plugins/dashboard/public/plugin.tsx +++ b/src/plugins/dashboard/public/plugin.tsx @@ -308,10 +308,10 @@ export class DashboardPlugin appUnMounted, usageCollection, onAppLeave: params.onAppLeave, - initializerContext: this.initializerContext, restorePreviousUrl, element: params.element, scopedHistory: this.currentHistory!, + initializerContext: this.initializerContext, setHeaderActionMenu: params.setHeaderActionMenu, }); }, diff --git a/src/plugins/dashboard/public/types.ts b/src/plugins/dashboard/public/types.ts index 7e859a81d9d4d..af0aed9203581 100644 --- a/src/plugins/dashboard/public/types.ts +++ b/src/plugins/dashboard/public/types.ts @@ -91,8 +91,7 @@ export type DashboardAppStateDefaults = DashboardAppState & { }; /** - * In URL panels are optional, - * Panels are not added to the URL when in "view" mode + * Panels are not added to the URL */ export type DashboardAppStateInUrl = Omit & { panels?: SavedDashboardPanel[]; From 8b2ac8b3bcd29fa35cee9f268cc429ec2dd29de3 Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Thu, 24 Dec 2020 14:38:50 -0500 Subject: [PATCH 02/30] Fix types --- .../dashboard/public/application/dashboard_state.test.ts | 1 + .../public/application/hooks/use_dashboard_breadcrumbs.ts | 7 +------ 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/plugins/dashboard/public/application/dashboard_state.test.ts b/src/plugins/dashboard/public/application/dashboard_state.test.ts index b07ea762f35e0..3cff8ff03be9a 100644 --- a/src/plugins/dashboard/public/application/dashboard_state.test.ts +++ b/src/plugins/dashboard/public/application/dashboard_state.test.ts @@ -52,6 +52,7 @@ describe('DashboardState', function () { dashboardState = new DashboardStateManager({ savedDashboard, hideWriteControls: false, + allowByValueEmbeddables: false, kibanaVersion: '7.0.0', kbnUrlStateStorage: createKbnUrlStateStorage(), history: createBrowserHistory(), diff --git a/src/plugins/dashboard/public/application/hooks/use_dashboard_breadcrumbs.ts b/src/plugins/dashboard/public/application/hooks/use_dashboard_breadcrumbs.ts index ce4a6c4713aaf..d16f46a1ba29e 100644 --- a/src/plugins/dashboard/public/application/hooks/use_dashboard_breadcrumbs.ts +++ b/src/plugins/dashboard/public/application/hooks/use_dashboard_breadcrumbs.ts @@ -19,16 +19,11 @@ import { useEffect } from 'react'; import _ from 'lodash'; -import { EUI_MODAL_CANCEL_BUTTON } from '@elastic/eui'; import { useKibana } from '../../services/kibana_react'; import { DashboardStateManager } from '../dashboard_state_manager'; -import { - getDashboardBreadcrumb, - getDashboardTitle, - leaveConfirmStrings, -} from '../../dashboard_strings'; +import { getDashboardBreadcrumb, getDashboardTitle } from '../../dashboard_strings'; import { DashboardAppServices, DashboardRedirect } from '../types'; export const useDashboardBreadcrumbs = ( From aa1140e13bebde3825bffc781a5d4a7104e3cad3 Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Thu, 7 Jan 2021 19:01:10 -0500 Subject: [PATCH 03/30] First version of unsaved panels listing --- .../public/application/_dashboard_app.scss | 10 ++ .../public/application/dashboard_router.tsx | 5 +- .../application/dashboard_state_manager.ts | 80 +++++---- .../hooks/use_dashboard_state_manager.ts | 8 + .../application/hooks/use_saved_dashboard.ts | 9 +- .../lib/dashboard_panel_storage.ts | 52 ++++++ .../dashboard/public/application/lib/index.ts | 1 + .../application/listing/dashboard_listing.tsx | 5 +- .../listing/dashboard_unsaved_listing.tsx | 170 ++++++++++++++++++ .../listing/discard_changes_confirm.tsx | 37 ++++ .../application/top_nav/dashboard_top_nav.tsx | 19 +- .../dashboard/public/application/types.ts | 4 +- .../dashboard/public/dashboard_constants.ts | 7 +- .../dashboard/public/dashboard_strings.ts | 30 +++- src/plugins/dashboard/public/plugin.tsx | 2 +- .../dashboard/public/services/kibana_utils.ts | 1 + .../table_list_view/table_list_view.tsx | 1 + 17 files changed, 370 insertions(+), 71 deletions(-) create mode 100644 src/plugins/dashboard/public/application/lib/dashboard_panel_storage.ts create mode 100644 src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.tsx create mode 100644 src/plugins/dashboard/public/application/listing/discard_changes_confirm.tsx diff --git a/src/plugins/dashboard/public/application/_dashboard_app.scss b/src/plugins/dashboard/public/application/_dashboard_app.scss index f969f936ddebc..7c29f47e8da63 100644 --- a/src/plugins/dashboard/public/application/_dashboard_app.scss +++ b/src/plugins/dashboard/public/application/_dashboard_app.scss @@ -33,3 +33,13 @@ margin-left: $euiSizeS; text-align: center; } + +.dshUnsavedListingItemTitle { + margin-bottom: 0 !important; +} + +.dshUnsavedListingButtons { + margin-left: 15px; +} + + diff --git a/src/plugins/dashboard/public/application/dashboard_router.tsx b/src/plugins/dashboard/public/application/dashboard_router.tsx index 9673737372478..8ae606854374a 100644 --- a/src/plugins/dashboard/public/application/dashboard_router.tsx +++ b/src/plugins/dashboard/public/application/dashboard_router.tsx @@ -26,7 +26,7 @@ import { Switch, Route, RouteComponentProps, HashRouter } from 'react-router-dom import { DashboardListing } from './listing'; import { DashboardApp } from './dashboard_app'; -import { addHelpMenuToAppChrome } from './lib'; +import { addHelpMenuToAppChrome, DashboardPanelStorage } from './lib'; import { createDashboardListingFilterUrl } from '../dashboard_constants'; import { getDashboardPageTitle, dashboardReadonlyBadge } from '../dashboard_strings'; import { createDashboardEditUrl, DashboardConstants } from '../dashboard_constants'; @@ -102,6 +102,7 @@ export async function mountApp({ indexPatterns: dataStart.indexPatterns, savedQueryService: dataStart.query.savedQueries, savedObjectsClient: coreStart.savedObjects.client, + dashboardPanelStorage: new DashboardPanelStorage(), savedDashboards: dashboardStart.getSavedDashboardLoader(), savedObjectsTagging: savedObjectsTaggingOss?.getTaggingApi(), dashboardCapabilities: { @@ -129,7 +130,7 @@ export async function mountApp({ let destination; if (redirectTo.destination === 'dashboard') { destination = redirectTo.id - ? createDashboardEditUrl(redirectTo.id) + ? createDashboardEditUrl(redirectTo.id, redirectTo.editMode) : DashboardConstants.CREATE_NEW_DASHBOARD_URL; } else { destination = createDashboardListingFilterUrl(redirectTo.filter); diff --git a/src/plugins/dashboard/public/application/dashboard_state_manager.ts b/src/plugins/dashboard/public/application/dashboard_state_manager.ts index 9dbebdd4d87f2..9e3f23f7dedf9 100644 --- a/src/plugins/dashboard/public/application/dashboard_state_manager.ts +++ b/src/plugins/dashboard/public/application/dashboard_state_manager.ts @@ -27,7 +27,12 @@ import { FilterUtils } from './lib/filter_utils'; import { DashboardContainer } from './embeddable'; import { DashboardSavedObject } from '../saved_dashboards'; import { migrateLegacyQuery } from './lib/migrate_legacy_query'; -import { getAppStateDefaults, migrateAppState, getDashboardIdFromUrl } from './lib'; +import { + getAppStateDefaults, + migrateAppState, + getDashboardIdFromUrl, + DashboardPanelStorage, +} from './lib'; import { convertPanelStateToSavedDashboardPanel } from '../../common/embeddable/embeddable_saved_object_converters'; import { DashboardAppState, @@ -38,7 +43,6 @@ import { } from '../types'; import { ViewMode } from '../services/embeddable'; -import { Storage } from '../../../kibana_utils/public'; import { UsageCollectionSetup } from '../services/usage_collection'; import { Filter, Query, TimefilterContract as Timefilter } from '../services/data'; import type { SavedObjectTagDecoratorTypeGuard } from '../services/saved_objects_tagging_oss'; @@ -49,9 +53,7 @@ import { ReduxLikeStateContainer, syncState, } from '../services/kibana_utils'; - -const DASHBOARD_PANELS_SESSION_KEY = 'dashboardStateManagerPanels'; -const DASHBOARD_PANELS_UNSAVED_ID = 'unsavedDashboard'; +import { STATE_STORAGE_KEY } from '../url_generator'; /** * Dashboard state manager handles connecting angular and redux state between the angular and react portions of the @@ -86,9 +88,8 @@ export class DashboardStateManager { DashboardAppStateTransitions >; private readonly stateContainerChangeSub: Subscription; - private readonly STATE_STORAGE_KEY = '_a'; private readonly kbnUrlStateStorage: IKbnUrlStateStorage; - private readonly sessionStorage: Storage; + private readonly dashboardPanelStorage?: DashboardPanelStorage; private readonly stateSyncRef: ISyncStateRef; private readonly allowByValueEmbeddables: boolean; @@ -103,22 +104,24 @@ export class DashboardStateManager { * @param */ constructor({ + history, + kibanaVersion, savedDashboard, + usageCollection, hideWriteControls, - kibanaVersion, kbnUrlStateStorage, - history, - usageCollection, - allowByValueEmbeddables, + dashboardPanelStorage, hasTaggingCapabilities, + allowByValueEmbeddables, }: { - savedDashboard: DashboardSavedObject; - hideWriteControls: boolean; - kibanaVersion: string; - kbnUrlStateStorage: IKbnUrlStateStorage; history: History; - usageCollection?: UsageCollectionSetup; + kibanaVersion: string; + hideWriteControls: boolean; allowByValueEmbeddables: boolean; + savedDashboard: DashboardSavedObject; + usageCollection?: UsageCollectionSetup; + kbnUrlStateStorage: IKbnUrlStateStorage; + dashboardPanelStorage?: DashboardPanelStorage; hasTaggingCapabilities: SavedObjectTagDecoratorTypeGuard; }) { this.kibanaVersion = kibanaVersion; @@ -134,16 +137,16 @@ export class DashboardStateManager { kibanaVersion, usageCollection ); - this.sessionStorage = new Storage(sessionStorage); + this.dashboardPanelStorage = dashboardPanelStorage; this.kbnUrlStateStorage = kbnUrlStateStorage; - // setup initial state by merging defaults with state from url + // setup initial state by merging defaults with state from url & panels storage // also run migration, as state in url could be of older version const initialState = migrateAppState( { ...this.stateDefaults, ...this.getUnsavedPanelState(), - ...this.kbnUrlStateStorage.get(this.STATE_STORAGE_KEY), + ...this.kbnUrlStateStorage.get(STATE_STORAGE_KEY), }, kibanaVersion, usageCollection @@ -179,9 +182,9 @@ export class DashboardStateManager { this.changeListeners.forEach((listener) => listener({ dirty: this.isDirty })); }); - // setup state syncing utils. state container will be synced with url into `this.STATE_STORAGE_KEY` query param + // setup state syncing utils. state container will be synced with url into `STATE_STORAGE_KEY` query param this.stateSyncRef = syncState({ - storageKey: this.STATE_STORAGE_KEY, + storageKey: STATE_STORAGE_KEY, stateContainer: { ...this.stateContainer, get: () => this.toUrlState(this.stateContainer.get()), @@ -273,10 +276,10 @@ export class DashboardStateManager { if (dirty) { this.stateContainer.transitions.set('panels', Object.values(convertedPanelStateMap)); - this.setUnsavedPanels(this.getPanels()); - if (dirtyBecauseOfInitialStateMigration) { this.saveState({ replace: true }); + } else { + this.setUnsavedPanels(this.getPanels()); } } @@ -495,7 +498,7 @@ export class DashboardStateManager { // get viewMode should work properly even before the state container is created return this.stateContainer ? this.appState.viewMode - : this.kbnUrlStateStorage.get(this.STATE_STORAGE_KEY)?.viewMode ?? + : this.kbnUrlStateStorage.get(STATE_STORAGE_KEY)?.viewMode ?? ViewMode.VIEW; } @@ -605,7 +608,7 @@ export class DashboardStateManager { private saveState({ replace }: { replace: boolean }): boolean { // schedules setting current state to url this.kbnUrlStateStorage.set( - this.STATE_STORAGE_KEY, + STATE_STORAGE_KEY, this.toUrlState(this.stateContainer.get()) ); // immediately forces scheduled updates and changes location @@ -641,7 +644,7 @@ export class DashboardStateManager { } } - public hydrateUnsavedPanels() { + public restoreUnsavedPanels() { const unsavedState = this.getUnsavedPanelState(); if (!unsavedState || unsavedState.panels?.length === 0) { return; @@ -651,7 +654,7 @@ export class DashboardStateManager { { ...this.stateDefaults, ...unsavedState, - ...this.kbnUrlStateStorage.get(this.STATE_STORAGE_KEY), + ...this.kbnUrlStateStorage.get(STATE_STORAGE_KEY), }, this.kibanaVersion, this.usageCollection @@ -660,31 +663,30 @@ export class DashboardStateManager { } public clearUnsavedPanels() { - if (!this.allowByValueEmbeddables) { + if (!this.allowByValueEmbeddables || !this.dashboardPanelStorage) { return; } - const sessionStoragePanels = this.sessionStorage.get(DASHBOARD_PANELS_SESSION_KEY) || {}; - sessionStoragePanels[this.savedDashboard.id || DASHBOARD_PANELS_UNSAVED_ID] = undefined; - this.sessionStorage.set(DASHBOARD_PANELS_SESSION_KEY, sessionStoragePanels); + this.dashboardPanelStorage.clearPanels(this.savedDashboard?.id); } private getUnsavedPanelState(): { panels?: SavedDashboardPanel[] } { - if (!this.allowByValueEmbeddables || this.getIsViewMode()) { + if (!this.allowByValueEmbeddables || this.getIsViewMode() || !this.dashboardPanelStorage) { return {}; } - const panels = this.sessionStorage.get(DASHBOARD_PANELS_SESSION_KEY)?.[ - this.savedDashboard.id || DASHBOARD_PANELS_UNSAVED_ID - ]; + const panels = this.dashboardPanelStorage.getPanels(this.savedDashboard?.id); return panels ? { panels } : {}; } private setUnsavedPanels(newPanels: SavedDashboardPanel[]) { - if (!this.allowByValueEmbeddables || this.getIsViewMode() || !this.getIsDirty()) { + if ( + !this.allowByValueEmbeddables || + this.getIsViewMode() || + !this.getIsDirty() || + !this.dashboardPanelStorage + ) { return; } - const sessionStoragePanels = this.sessionStorage.get(DASHBOARD_PANELS_SESSION_KEY) || {}; - sessionStoragePanels[this.savedDashboard.id || DASHBOARD_PANELS_UNSAVED_ID] = newPanels; - this.sessionStorage.set(DASHBOARD_PANELS_SESSION_KEY, sessionStoragePanels); + this.dashboardPanelStorage.setPanels(this.savedDashboard?.id, newPanels); } private toUrlState(state: DashboardAppState): DashboardAppStateInUrl { diff --git a/src/plugins/dashboard/public/application/hooks/use_dashboard_state_manager.ts b/src/plugins/dashboard/public/application/hooks/use_dashboard_state_manager.ts index ca61368ae0539..a41642c4f0dd6 100644 --- a/src/plugins/dashboard/public/application/hooks/use_dashboard_state_manager.ts +++ b/src/plugins/dashboard/public/application/hooks/use_dashboard_state_manager.ts @@ -58,6 +58,7 @@ export const useDashboardStateManager = ( initializerContext, savedObjectsTagging, dashboardCapabilities, + dashboardPanelStorage, } = useKibana().services; // Destructure and rename services; makes the Effect hook more specific, makes later @@ -86,11 +87,13 @@ export const useDashboardStateManager = ( useHash: uiSettings.get('state:storeInSessionStorage'), ...withNotifyOnErrors(toasts), }); + const allowByValueEmbeddables = initializerContext.config.get() .allowByValueEmbeddables; const stateManager = new DashboardStateManager({ hasTaggingCapabilities, + dashboardPanelStorage, hideWriteControls, history, kbnUrlStateStorage, @@ -175,6 +178,10 @@ export const useDashboardStateManager = ( }) ); + if (stateManager.getIsEditMode()) { + stateManager.restoreUnsavedPanels(); + } + setDashboardStateManager(stateManager); return () => { @@ -188,6 +195,7 @@ export const useDashboardStateManager = ( filterManager, hasTaggingCapabilities, initializerContext.config, + dashboardPanelStorage, hideWriteControls, history, kibanaVersion, diff --git a/src/plugins/dashboard/public/application/hooks/use_saved_dashboard.ts b/src/plugins/dashboard/public/application/hooks/use_saved_dashboard.ts index f0d8b5f5e000d..f08db33d7e941 100644 --- a/src/plugins/dashboard/public/application/hooks/use_saved_dashboard.ts +++ b/src/plugins/dashboard/public/application/hooks/use_saved_dashboard.ts @@ -36,7 +36,7 @@ export const useSavedDashboard = (savedDashboardId: string | undefined, history: // abstraction of service dependencies easier. const { indexPatterns } = data; const { recentlyAccessed: recentlyAccessedPaths, docTitle } = chrome; - const { addDanger: showDangerToast, addWarning: showWarningToast } = core.notifications.toasts; + const { toasts } = core.notifications; useEffect(() => { (async function loadSavedDashboard() { @@ -46,7 +46,7 @@ export const useSavedDashboard = (savedDashboardId: string | undefined, history: pathname: DashboardConstants.CREATE_NEW_DASHBOARD_URL, }); - showWarningToast(getDashboard60Warning()); + toasts.addWarning(getDashboard60Warning()); return; } @@ -63,20 +63,19 @@ export const useSavedDashboard = (savedDashboardId: string | undefined, history: setSavedDashboard(dashboard); } catch (error) { // E.g. a corrupt or deleted dashboard - showDangerToast(error.message); + toasts.addDanger(error.message); history.push(DashboardConstants.LANDING_PAGE_PATH); } })(); return () => setSavedDashboard(null); }, [ + toasts, docTitle, history, indexPatterns, recentlyAccessedPaths, savedDashboardId, savedDashboards, - showDangerToast, - showWarningToast, ]); return savedDashboard; diff --git a/src/plugins/dashboard/public/application/lib/dashboard_panel_storage.ts b/src/plugins/dashboard/public/application/lib/dashboard_panel_storage.ts new file mode 100644 index 0000000000000..237b7e26800f1 --- /dev/null +++ b/src/plugins/dashboard/public/application/lib/dashboard_panel_storage.ts @@ -0,0 +1,52 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { Storage } from '../../services/kibana_utils'; +import { SavedDashboardPanel } from '..'; + +export const DASHBOARD_PANELS_UNSAVED_ID = 'unsavedDashboard'; +const DASHBOARD_PANELS_SESSION_KEY = 'dashboardStateManagerPanels'; + +export class DashboardPanelStorage { + private sessionStorage: Storage; + + constructor() { + this.sessionStorage = new Storage(sessionStorage); + } + + public clearPanels(id = DASHBOARD_PANELS_UNSAVED_ID) { + const sessionStoragePanels = this.sessionStorage.get(DASHBOARD_PANELS_SESSION_KEY) || {}; + delete sessionStoragePanels[id]; + this.sessionStorage.set(DASHBOARD_PANELS_SESSION_KEY, sessionStoragePanels); + } + + public getPanels(id = DASHBOARD_PANELS_UNSAVED_ID): SavedDashboardPanel[] | undefined { + return this.sessionStorage.get(DASHBOARD_PANELS_SESSION_KEY)?.[id]; + } + + public setPanels(id = DASHBOARD_PANELS_UNSAVED_ID, newPanels: SavedDashboardPanel[]) { + const sessionStoragePanels = this.sessionStorage.get(DASHBOARD_PANELS_SESSION_KEY) || {}; + sessionStoragePanels[id] = newPanels; + this.sessionStorage.set(DASHBOARD_PANELS_SESSION_KEY, sessionStoragePanels); + } + + public getDashboardIdsWithUnsavedChanges() { + return Object.keys(this.sessionStorage.get(DASHBOARD_PANELS_SESSION_KEY) || {}); + } +} diff --git a/src/plugins/dashboard/public/application/lib/index.ts b/src/plugins/dashboard/public/application/lib/index.ts index 03825ad7765f8..f593b4dbaebf8 100644 --- a/src/plugins/dashboard/public/application/lib/index.ts +++ b/src/plugins/dashboard/public/application/lib/index.ts @@ -24,3 +24,4 @@ export { getDashboardIdFromUrl } from './url'; export { createSessionRestorationDataProvider } from './session_restoration'; export { addHelpMenuToAppChrome } from './help_menu_util'; export { attemptLoadDashboardByTitle } from './load_dashboard_by_title'; +export { DashboardPanelStorage } from './dashboard_panel_storage'; diff --git a/src/plugins/dashboard/public/application/listing/dashboard_listing.tsx b/src/plugins/dashboard/public/application/listing/dashboard_listing.tsx index 40c033322799f..9887760e2a67b 100644 --- a/src/plugins/dashboard/public/application/listing/dashboard_listing.tsx +++ b/src/plugins/dashboard/public/application/listing/dashboard_listing.tsx @@ -30,6 +30,7 @@ import { syncQueryStateWithUrl } from '../../services/data'; import { IKbnUrlStateStorage } from '../../services/kibana_utils'; import { TableListView, useKibana } from '../../services/kibana_react'; import { SavedObjectsTaggingApi } from '../../services/saved_objects_tagging_oss'; +import { DashboardUnsavedListing } from './dashboard_unsaved_listing'; export interface DashboardListingProps { kbnUrlStateStorage: IKbnUrlStateStorage; @@ -170,7 +171,9 @@ export const DashboardListing = ({ listingLimit, tableColumns, }} - /> + > + + ); }; diff --git a/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.tsx b/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.tsx new file mode 100644 index 0000000000000..7931bc2c3de18 --- /dev/null +++ b/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.tsx @@ -0,0 +1,170 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { + EuiButtonEmpty, + EuiButtonIconPropsForButton, + EuiCallOut, + EuiFlexGroup, + EuiFlexItem, + EuiIcon, + EuiListGroup, + EuiListGroupItemProps, + EuiSpacer, + EuiTitle, +} from '@elastic/eui'; +import React, { useCallback, useEffect, useState } from 'react'; +import useMount from 'react-use/lib/useMount'; +import { createDashboardEditUrl, DashboardConstants, DashboardSavedObject } from '../..'; +import { dashboardUnsavedListingStrings, getNewDashboardTitle } from '../../dashboard_strings'; +import { useKibana } from '../../services/kibana_react'; +import { DASHBOARD_PANELS_UNSAVED_ID } from '../lib/dashboard_panel_storage'; +import { DashboardAppServices, DashboardRedirect } from '../types'; +import { confirmDiscardUnsavedChanges } from './discard_changes_confirm'; + +const DashboardUnsavedItem = ({ + dashboard, + onOpenClick, + onDiscardClick, +}: { + dashboard?: DashboardSavedObject; + onOpenClick: () => void; + onDiscardClick: () => void; +}) => { + return ( + <> + {/* + + + + + +

{dashboard?.title ?? getNewDashboardTitle()}

+
+ + + + {dashboardUnsavedListingStrings.getEditTitle()} + + + + + {dashboardUnsavedListingStrings.getDiscardTitle()} + + + +
+
*/} + + +

+ {dashboard?.title ?? getNewDashboardTitle()} +

+
+ + + + {dashboardUnsavedListingStrings.getEditTitle()} + + + + + {dashboardUnsavedListingStrings.getDiscardTitle()} + + + + + ); +}; + +export const DashboardUnsavedListing = ({ redirectTo }: { redirectTo: DashboardRedirect }) => { + const { + services: { + dashboardPanelStorage, + savedDashboards, + core: { overlays }, + }, + } = useKibana(); + + const [items, setItems] = useState([]); + const [dashboardIds, setDashboardIds] = useState([]); + + const onOpen = useCallback( + (id?: string) => { + redirectTo({ destination: 'dashboard', id, editMode: true }); + }, + [redirectTo] + ); + + const onDiscard = useCallback( + (id?: string) => { + confirmDiscardUnsavedChanges(overlays, () => { + dashboardPanelStorage.clearPanels(id); + setDashboardIds(dashboardPanelStorage.getDashboardIdsWithUnsavedChanges()); + }); + }, + [overlays, dashboardPanelStorage] + ); + + useMount(() => { + setDashboardIds(dashboardPanelStorage.getDashboardIdsWithUnsavedChanges()); + }); + + useEffect(() => { + let hasNewDashboard = false; + const dashPromises = dashboardIds + .filter((id) => { + if (id !== DASHBOARD_PANELS_UNSAVED_ID) { + return true; + } + hasNewDashboard = true; + return false; + }) + .map((dashboardId) => savedDashboards.get(dashboardId)); + Promise.all(dashPromises).then((dashboards: DashboardSavedObject[]) => { + const newItems = dashboards.map((dashboard) => ( + onOpen(dashboard.id)} + onDiscardClick={() => onDiscard(dashboard.id)} + /> + )); + if (hasNewDashboard) { + newItems.unshift( + onOpen()} + onDiscardClick={() => onDiscard()} + /> + ); + } + setItems(newItems); + }); + }, [dashboardIds, onOpen, onDiscard, savedDashboards]); + + return items.length === 0 ? null : ( + <> + 1)}> + {items} + + + + ); +}; diff --git a/src/plugins/dashboard/public/application/listing/discard_changes_confirm.tsx b/src/plugins/dashboard/public/application/listing/discard_changes_confirm.tsx new file mode 100644 index 0000000000000..5e2f7d172a46a --- /dev/null +++ b/src/plugins/dashboard/public/application/listing/discard_changes_confirm.tsx @@ -0,0 +1,37 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { EUI_MODAL_CANCEL_BUTTON } from '@elastic/eui'; +import { OverlayStart } from '../../../../../core/public'; +import { leaveConfirmStrings } from '../../dashboard_strings'; + +export const confirmDiscardUnsavedChanges = (overlays: OverlayStart, discardCallback: () => void) => + overlays + .openConfirm(leaveConfirmStrings.getDiscardSubtitle(), { + confirmButtonText: leaveConfirmStrings.getConfirmButtonText(), + cancelButtonText: leaveConfirmStrings.getCancelButtonText(), + buttonColor: 'danger', + defaultFocusedButton: EUI_MODAL_CANCEL_BUTTON, + title: leaveConfirmStrings.getDiscardTitle(), + }) + .then((isConfirmed) => { + if (isConfirmed) { + discardCallback(); + } + }); 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 d875c0a74c74d..6043dc1874687 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 @@ -17,8 +17,6 @@ * under the License. */ -import { EUI_MODAL_CANCEL_BUTTON } from '@elastic/eui'; - import { i18n } from '@kbn/i18n'; import angular from 'angular'; import React, { useCallback, useEffect, useMemo, useState } from 'react'; @@ -42,7 +40,6 @@ import { import { NavAction } from '../../types'; import { DashboardSavedObject } from '../..'; import { DashboardStateManager } from '../dashboard_state_manager'; -import { leaveConfirmStrings } from '../../dashboard_strings'; import { saveDashboard } from '../lib'; import { DashboardAppServices, @@ -57,6 +54,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/discard_changes_confirm'; import { DashboardContainer } from '..'; export interface DashboardTopNavState { @@ -148,7 +146,7 @@ export function DashboardTopNav({ if (!willLoseChanges) { dashboardStateManager.switchViewMode(newMode); if (newMode === ViewMode.EDIT) { - dashboardStateManager.hydrateUnsavedPanels(); + dashboardStateManager.restoreUnsavedPanels(); } return; } @@ -169,18 +167,7 @@ export function DashboardTopNav({ redirectTo({ destination: 'dashboard', id: savedDashboard.id }); } - core.overlays - .openConfirm(leaveConfirmStrings.getDiscardSubtitle(), { - confirmButtonText: leaveConfirmStrings.getConfirmButtonText(), - cancelButtonText: leaveConfirmStrings.getCancelButtonText(), - defaultFocusedButton: EUI_MODAL_CANCEL_BUTTON, - title: leaveConfirmStrings.getDiscardTitle(), - }) - .then((isConfirmed) => { - if (isConfirmed) { - revertChangesAndExitEditMode(); - } - }); + confirmDiscardUnsavedChanges(core.overlays, revertChangesAndExitEditMode); }, [redirectTo, timefilter, core.overlays, savedDashboard.id, dashboardStateManager] ); diff --git a/src/plugins/dashboard/public/application/types.ts b/src/plugins/dashboard/public/application/types.ts index d1caaa349d80b..2db20315e7d73 100644 --- a/src/plugins/dashboard/public/application/types.ts +++ b/src/plugins/dashboard/public/application/types.ts @@ -33,10 +33,11 @@ import { NavigationPublicPluginStart } from '../services/navigation'; import { SavedObjectsTaggingApi } from '../services/saved_objects_tagging_oss'; import { DataPublicPluginStart, IndexPatternsContract } from '../services/data'; import { SavedObjectLoader, SavedObjectsStart } from '../services/saved_objects'; +import { DashboardPanelStorage } from './lib'; export type DashboardRedirect = (props: RedirectToProps) => void; export type RedirectToProps = - | { destination: 'dashboard'; id?: string; useReplace?: boolean } + | { destination: 'dashboard'; id?: string; useReplace?: boolean; editMode?: boolean } | { destination: 'listing'; filter?: string; useReplace?: boolean }; export interface DashboardEmbedSettings { @@ -80,6 +81,7 @@ export interface DashboardAppServices { indexPatterns: IndexPatternsContract; usageCollection?: UsageCollectionSetup; navigation: NavigationPublicPluginStart; + dashboardPanelStorage: DashboardPanelStorage; dashboardCapabilities: DashboardCapabilities; initializerContext: PluginInitializerContext; onAppLeave: AppMountParameters['onAppLeave']; diff --git a/src/plugins/dashboard/public/dashboard_constants.ts b/src/plugins/dashboard/public/dashboard_constants.ts index 1498485816adc..a24bba1d1ef0e 100644 --- a/src/plugins/dashboard/public/dashboard_constants.ts +++ b/src/plugins/dashboard/public/dashboard_constants.ts @@ -17,6 +17,8 @@ * under the License. */ +import { STATE_STORAGE_KEY } from './url_generator'; + export const DashboardConstants = { LANDING_PAGE_PATH: '/list', CREATE_NEW_DASHBOARD_URL: '/create', @@ -28,8 +30,9 @@ export const DashboardConstants = { SEARCH_SESSION_ID: 'searchSessionId', }; -export function createDashboardEditUrl(id: string) { - return `${DashboardConstants.VIEW_DASHBOARD_URL}/${id}`; +export function createDashboardEditUrl(id: string, editMode?: boolean) { + const edit = editMode ? `?${STATE_STORAGE_KEY}=(viewMode:edit)` : ''; + return `${DashboardConstants.VIEW_DASHBOARD_URL}/${id}${edit}`; } export function createDashboardListingFilterUrl(filter: string | undefined) { diff --git a/src/plugins/dashboard/public/dashboard_strings.ts b/src/plugins/dashboard/public/dashboard_strings.ts index 9bede02c75b94..405ad68d91b23 100644 --- a/src/plugins/dashboard/public/dashboard_strings.ts +++ b/src/plugins/dashboard/public/dashboard_strings.ts @@ -35,10 +35,7 @@ export function getDashboardTitle( ): string { const isEditMode = viewMode === ViewMode.EDIT; let displayTitle: string; - const newDashboardTitle = i18n.translate('dashboard.savedDashboard.newDashboardTitle', { - defaultMessage: 'New Dashboard', - }); - const dashboardTitle = isNew ? newDashboardTitle : title; + const dashboardTitle = isNew ? getNewDashboardTitle() : title; if (isEditMode && isDirty) { displayTitle = i18n.translate('dashboard.strings.dashboardUnsavedEditTitle', { @@ -187,6 +184,11 @@ export const dashboardReplacePanelAction = { /* Dashboard Editor */ +export const getNewDashboardTitle = () => + i18n.translate('dashboard.savedDashboard.newDashboardTitle', { + defaultMessage: 'New Dashboard', + }); + export const shareModalStrings = { getTopMenuCheckbox: () => i18n.translate('dashboard.embedUrlParamExtension.topMenu', { @@ -318,3 +320,23 @@ export const dashboardListingTable = { defaultMessage: 'Description', }), }; + +export const dashboardUnsavedListingStrings = { + getUnsavedChangesTitle: (plural = false) => + i18n.translate('dashboard.listing.unsaved.unsavedChangesTitle', { + defaultMessage: 'You have unsaved changes in the following {dash}', + values: { + dash: plural + ? dashboardListingTable.getEntityNamePlural() + : dashboardListingTable.getEntityName(), + }, + }), + getEditTitle: () => + i18n.translate('dashboard.listing.unsaved.editTitle', { + defaultMessage: 'Continue Editing', + }), + getDiscardTitle: () => + i18n.translate('dashboard.listing.unsaved.discardTitle', { + defaultMessage: 'Discard changes', + }), +}; diff --git a/src/plugins/dashboard/public/plugin.tsx b/src/plugins/dashboard/public/plugin.tsx index 12e64669cdb9b..54c522d97a13e 100644 --- a/src/plugins/dashboard/public/plugin.tsx +++ b/src/plugins/dashboard/public/plugin.tsx @@ -307,9 +307,9 @@ export class DashboardPlugin core, appUnMounted, usageCollection, - onAppLeave: params.onAppLeave, restorePreviousUrl, element: params.element, + onAppLeave: params.onAppLeave, scopedHistory: this.currentHistory!, initializerContext: this.initializerContext, setHeaderActionMenu: params.setHeaderActionMenu, diff --git a/src/plugins/dashboard/public/services/kibana_utils.ts b/src/plugins/dashboard/public/services/kibana_utils.ts index 876e9c0773542..46827964a6b98 100644 --- a/src/plugins/dashboard/public/services/kibana_utils.ts +++ b/src/plugins/dashboard/public/services/kibana_utils.ts @@ -18,6 +18,7 @@ */ export { + Storage, unhashUrl, syncState, ISyncStateRef, diff --git a/src/plugins/kibana_react/public/table_list_view/table_list_view.tsx b/src/plugins/kibana_react/public/table_list_view/table_list_view.tsx index 75fd2c30a995a..91c0c097db232 100644 --- a/src/plugins/kibana_react/public/table_list_view/table_list_view.tsx +++ b/src/plugins/kibana_react/public/table_list_view/table_list_view.tsx @@ -529,6 +529,7 @@ class TableListView extends React.Component + {this.props.children} {this.renderListingLimitWarning()} {this.renderFetchError()} From 946c7c4d7f31221fc4c1fd65ece2dda86df35021 Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Tue, 12 Jan 2021 10:38:51 -0500 Subject: [PATCH 04/30] type fix --- .../public/application/listing/dashboard_listing.test.tsx | 2 ++ .../public/application/listing/dashboard_unsaved_listing.tsx | 5 +---- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/plugins/dashboard/public/application/listing/dashboard_listing.test.tsx b/src/plugins/dashboard/public/application/listing/dashboard_listing.test.tsx index 8172be46e9f3a..7f3b3c42b195c 100644 --- a/src/plugins/dashboard/public/application/listing/dashboard_listing.test.tsx +++ b/src/plugins/dashboard/public/application/listing/dashboard_listing.test.tsx @@ -40,6 +40,7 @@ import { chromeServiceMock, coreMock } from '../../../../../core/public/mocks'; import { I18nProvider } from '@kbn/i18n/react'; import React from 'react'; import { UrlForwardingStart } from '../../../../url_forwarding/public'; +import { DashboardPanelStorage } from '../lib'; function makeDefaultServices(): DashboardAppServices { const core = coreMock.createStart(); @@ -63,6 +64,7 @@ function makeDefaultServices(): DashboardAppServices { savedObjects: savedObjectsPluginMock.createStartContract(), embeddable: embeddablePluginMock.createInstance().doStart(), dashboardCapabilities: {} as DashboardCapabilities, + dashboardPanelStorage: {} as DashboardPanelStorage, initializerContext: {} as PluginInitializerContext, chrome: chromeServiceMock.createStartContract(), navigation: {} as NavigationPublicPluginStart, diff --git a/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.tsx b/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.tsx index 7931bc2c3de18..7278d0c8594c2 100644 --- a/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.tsx +++ b/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.tsx @@ -19,19 +19,16 @@ import { EuiButtonEmpty, - EuiButtonIconPropsForButton, EuiCallOut, EuiFlexGroup, EuiFlexItem, EuiIcon, - EuiListGroup, - EuiListGroupItemProps, EuiSpacer, EuiTitle, } from '@elastic/eui'; import React, { useCallback, useEffect, useState } from 'react'; import useMount from 'react-use/lib/useMount'; -import { createDashboardEditUrl, DashboardConstants, DashboardSavedObject } from '../..'; +import { DashboardSavedObject } from '../..'; import { dashboardUnsavedListingStrings, getNewDashboardTitle } from '../../dashboard_strings'; import { useKibana } from '../../services/kibana_react'; import { DASHBOARD_PANELS_UNSAVED_ID } from '../lib/dashboard_panel_storage'; From d4b0c11ca9de3177a66584b01ea7f4cf05cc5b5c Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Tue, 12 Jan 2021 18:03:48 -0500 Subject: [PATCH 05/30] Added confirm to create new when new dashboard already has unsaved edits. Changed edit link in listing page to actually edit. Removed unsaved edits when dashboard is not dirty --- .../application/dashboard_state_manager.ts | 13 ++++--- .../lib/dashboard_panel_storage.ts | 10 ++++-- ...anges_confirm.tsx => confirm_overlays.tsx} | 21 +++++++++++- .../application/listing/dashboard_listing.tsx | 34 ++++++++++++++++--- .../listing/dashboard_unsaved_listing.tsx | 25 +------------- .../application/top_nav/dashboard_top_nav.tsx | 2 +- .../dashboard/public/dashboard_strings.ts | 19 +++++++++++ 7 files changed, 88 insertions(+), 36 deletions(-) rename src/plugins/dashboard/public/application/listing/{discard_changes_confirm.tsx => confirm_overlays.tsx} (68%) diff --git a/src/plugins/dashboard/public/application/dashboard_state_manager.ts b/src/plugins/dashboard/public/application/dashboard_state_manager.ts index a0bdef68a9f8f..f9a10b954cb14 100644 --- a/src/plugins/dashboard/public/application/dashboard_state_manager.ts +++ b/src/plugins/dashboard/public/application/dashboard_state_manager.ts @@ -179,6 +179,9 @@ export class DashboardStateManager { this.stateContainerChangeSub = this.stateContainer.state$.subscribe(() => { this.isDirty = this.checkIsDirty(); + if (!this.isDirty && this.getIsViewMode()) { + this.clearUnsavedPanels(); + } this.changeListeners.forEach((listener) => listener({ dirty: this.isDirty })); }); @@ -504,11 +507,13 @@ export class DashboardStateManager { if (this.hideWriteControls) { return ViewMode.VIEW; } + if (this.stateContainer) { + return this.appState.viewMode; + } // get viewMode should work properly even before the state container is created - return this.stateContainer - ? this.appState.viewMode - : this.kbnUrlStateStorage.get(STATE_STORAGE_KEY)?.viewMode ?? - ViewMode.VIEW; + return this.savedDashboard.id + ? this.kbnUrlStateStorage.get(STATE_STORAGE_KEY)?.viewMode ?? ViewMode.VIEW + : ViewMode.EDIT; } public getIsViewMode() { diff --git a/src/plugins/dashboard/public/application/lib/dashboard_panel_storage.ts b/src/plugins/dashboard/public/application/lib/dashboard_panel_storage.ts index 237b7e26800f1..1cd66c0efb143 100644 --- a/src/plugins/dashboard/public/application/lib/dashboard_panel_storage.ts +++ b/src/plugins/dashboard/public/application/lib/dashboard_panel_storage.ts @@ -32,8 +32,10 @@ export class DashboardPanelStorage { public clearPanels(id = DASHBOARD_PANELS_UNSAVED_ID) { const sessionStoragePanels = this.sessionStorage.get(DASHBOARD_PANELS_SESSION_KEY) || {}; - delete sessionStoragePanels[id]; - this.sessionStorage.set(DASHBOARD_PANELS_SESSION_KEY, sessionStoragePanels); + if (sessionStoragePanels[id]) { + delete sessionStoragePanels[id]; + this.sessionStorage.set(DASHBOARD_PANELS_SESSION_KEY, sessionStoragePanels); + } } public getPanels(id = DASHBOARD_PANELS_UNSAVED_ID): SavedDashboardPanel[] | undefined { @@ -49,4 +51,8 @@ export class DashboardPanelStorage { public getDashboardIdsWithUnsavedChanges() { return Object.keys(this.sessionStorage.get(DASHBOARD_PANELS_SESSION_KEY) || {}); } + + public dashboardHasUnsavedEdits(id = DASHBOARD_PANELS_UNSAVED_ID) { + return this.getDashboardIdsWithUnsavedChanges().indexOf(id) !== -1; + } } diff --git a/src/plugins/dashboard/public/application/listing/discard_changes_confirm.tsx b/src/plugins/dashboard/public/application/listing/confirm_overlays.tsx similarity index 68% rename from src/plugins/dashboard/public/application/listing/discard_changes_confirm.tsx rename to src/plugins/dashboard/public/application/listing/confirm_overlays.tsx index 5e2f7d172a46a..cb3226963efb8 100644 --- a/src/plugins/dashboard/public/application/listing/discard_changes_confirm.tsx +++ b/src/plugins/dashboard/public/application/listing/confirm_overlays.tsx @@ -19,7 +19,7 @@ import { EUI_MODAL_CANCEL_BUTTON } from '@elastic/eui'; import { OverlayStart } from '../../../../../core/public'; -import { leaveConfirmStrings } from '../../dashboard_strings'; +import { createConfirmStrings, leaveConfirmStrings } from '../../dashboard_strings'; export const confirmDiscardUnsavedChanges = (overlays: OverlayStart, discardCallback: () => void) => overlays @@ -35,3 +35,22 @@ export const confirmDiscardUnsavedChanges = (overlays: OverlayStart, discardCall discardCallback(); } }); + +export const confirmCreateWithUnsaved = ( + overlays: OverlayStart, + startBlankCallback: () => void, + contineCallback: () => void +) => + overlays + .openConfirm(leaveConfirmStrings.getDiscardSubtitle(), { + confirmButtonText: createConfirmStrings.getConfirmButtonText(), + cancelButtonText: createConfirmStrings.getCancelButtonText(), + defaultFocusedButton: EUI_MODAL_CANCEL_BUTTON, + title: createConfirmStrings.getCreateTitle(), + }) + .then((isConfirmed) => { + if (isConfirmed) { + startBlankCallback(); + } + contineCallback(); + }); diff --git a/src/plugins/dashboard/public/application/listing/dashboard_listing.tsx b/src/plugins/dashboard/public/application/listing/dashboard_listing.tsx index 9887760e2a67b..4723ddbe6f5c1 100644 --- a/src/plugins/dashboard/public/application/listing/dashboard_listing.tsx +++ b/src/plugins/dashboard/public/application/listing/dashboard_listing.tsx @@ -31,6 +31,7 @@ import { IKbnUrlStateStorage } from '../../services/kibana_utils'; import { TableListView, useKibana } from '../../services/kibana_react'; import { SavedObjectsTaggingApi } from '../../services/saved_objects_tagging_oss'; import { DashboardUnsavedListing } from './dashboard_unsaved_listing'; +import { confirmCreateWithUnsaved } from './confirm_overlays'; export interface DashboardListingProps { kbnUrlStateStorage: IKbnUrlStateStorage; @@ -54,6 +55,7 @@ export const DashboardListing = ({ savedObjectsClient, savedObjectsTagging, dashboardCapabilities, + dashboardPanelStorage, chrome: { setBreadcrumbs }, }, } = useKibana(); @@ -95,8 +97,16 @@ export const DashboardListing = ({ const tableColumns = useMemo( () => - getTableColumns((id) => redirectTo({ destination: 'dashboard', id }), savedObjectsTagging), - [savedObjectsTagging, redirectTo] + getTableColumns( + (id) => + redirectTo({ + destination: 'dashboard', + id, + editMode: dashboardPanelStorage.dashboardHasUnsavedEdits(id), + }), + savedObjectsTagging + ), + [savedObjectsTagging, redirectTo, dashboardPanelStorage] ); const noItemsFragment = useMemo( @@ -134,10 +144,26 @@ export const DashboardListing = ({ ); const editItem = useCallback( - ({ id }: { id: string | undefined }) => redirectTo({ destination: 'dashboard', id }), + ({ id }: { id: string | undefined }) => + redirectTo({ destination: 'dashboard', id, editMode: true }), [redirectTo] ); + const createItem = useCallback(() => { + if (dashboardPanelStorage.dashboardHasUnsavedEdits()) { + redirectTo({ destination: 'dashboard' }); + } else { + confirmCreateWithUnsaved( + core.overlays, + () => { + dashboardPanelStorage.clearPanels(); + redirectTo({ destination: 'dashboard' }); + }, + () => redirectTo({ destination: 'dashboard' }) + ); + } + }, [dashboardPanelStorage, redirectTo, core.overlays]); + const searchFilters = useMemo(() => { return savedObjectsTagging ? [savedObjectsTagging.ui.getSearchBarFilter({ useName: true })] @@ -152,7 +178,7 @@ export const DashboardListing = ({ } = dashboardListingTable; return ( redirectTo({ destination: 'dashboard' })} + createItem={hideWriteControls ? undefined : createItem} deleteItems={hideWriteControls ? undefined : deleteItems} initialPageSize={savedObjects.settings.getPerPage()} editItem={hideWriteControls ? undefined : editItem} diff --git a/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.tsx b/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.tsx index 7278d0c8594c2..051022419a014 100644 --- a/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.tsx +++ b/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.tsx @@ -33,7 +33,7 @@ import { dashboardUnsavedListingStrings, getNewDashboardTitle } from '../../dash import { useKibana } from '../../services/kibana_react'; import { DASHBOARD_PANELS_UNSAVED_ID } from '../lib/dashboard_panel_storage'; import { DashboardAppServices, DashboardRedirect } from '../types'; -import { confirmDiscardUnsavedChanges } from './discard_changes_confirm'; +import { confirmDiscardUnsavedChanges } from './confirm_overlays'; const DashboardUnsavedItem = ({ dashboard, @@ -46,29 +46,6 @@ const DashboardUnsavedItem = ({ }) => { return ( <> - {/* - - - - - -

{dashboard?.title ?? getNewDashboardTitle()}

-
- - - - {dashboardUnsavedListingStrings.getEditTitle()} - - - - - {dashboardUnsavedListingStrings.getDiscardTitle()} - - - -
-
*/} -

{dashboard?.title ?? getNewDashboardTitle()} 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 e0f516224c230..97c69a3925a32 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 @@ -54,7 +54,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/discard_changes_confirm'; +import { confirmDiscardUnsavedChanges } from '../listing/confirm_overlays'; import { OverlayRef } from '../../../../../core/public'; import { DashboardContainer } from '..'; diff --git a/src/plugins/dashboard/public/dashboard_strings.ts b/src/plugins/dashboard/public/dashboard_strings.ts index 405ad68d91b23..6028006cea644 100644 --- a/src/plugins/dashboard/public/dashboard_strings.ts +++ b/src/plugins/dashboard/public/dashboard_strings.ts @@ -255,6 +255,25 @@ export const leaveConfirmStrings = { }), }; +export const createConfirmStrings = { + getCreateTitle: () => + i18n.translate('dashboard.createConfirmModal.unsavedChangesTitle', { + defaultMessage: 'New dashboard already in progress', + }), + getCreateSubtitle: () => + i18n.translate('dashboard.createConfirmModal.unsavedChangesSubtitle', { + defaultMessage: 'You can continue editing or start with a blank dashboard.', + }), + getConfirmButtonText: () => + i18n.translate('dashboard.createConfirmModal.confirmButtonLabel', { + defaultMessage: 'Start over', + }), + getCancelButtonText: () => + i18n.translate('dashboard.createConfirmModal.cancelButtonLabel', { + defaultMessage: 'Continue editing', + }), +}; + /* Empty Screen */ From 25da78c8fafeae733f454013252ff370d10fe002 Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Wed, 13 Jan 2021 13:38:07 -0500 Subject: [PATCH 06/30] Fixed typo, ensured that snapshot share links work properly --- .../public/application/dashboard_app_functions.ts | 1 - .../public/application/dashboard_state_manager.ts | 12 +++++++++--- .../public/application/listing/confirm_overlays.tsx | 2 +- .../public/application/listing/dashboard_listing.tsx | 2 +- 4 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/plugins/dashboard/public/application/dashboard_app_functions.ts b/src/plugins/dashboard/public/application/dashboard_app_functions.ts index af7a485296ea0..0e6f0f460859d 100644 --- a/src/plugins/dashboard/public/application/dashboard_app_functions.ts +++ b/src/plugins/dashboard/public/application/dashboard_app_functions.ts @@ -195,7 +195,6 @@ export const getInputSubscription = ({ dashboardContainer.getInput().filters ); } - dashboardStateManager.handleDashboardContainerChanges(dashboardContainer); }); diff --git a/src/plugins/dashboard/public/application/dashboard_state_manager.ts b/src/plugins/dashboard/public/application/dashboard_state_manager.ts index f9a10b954cb14..bc127e761b2ef 100644 --- a/src/plugins/dashboard/public/application/dashboard_state_manager.ts +++ b/src/plugins/dashboard/public/application/dashboard_state_manager.ts @@ -142,16 +142,24 @@ export class DashboardStateManager { // setup initial state by merging defaults with state from url & panels storage // also run migration, as state in url could be of older version + const initialUrlState = this.kbnUrlStateStorage.get(STATE_STORAGE_KEY); const initialState = migrateAppState( { ...this.stateDefaults, ...this.getUnsavedPanelState(), - ...this.kbnUrlStateStorage.get(STATE_STORAGE_KEY), + ...initialUrlState, }, kibanaVersion, usageCollection ); + this.isDirty = false; + + if (initialUrlState?.panels && !_.isEqual(initialUrlState.panels, this.stateDefaults.panels)) { + this.isDirty = true; + this.setUnsavedPanels(initialState.panels); + } + // setup state container using initial state both from defaults and from url this.stateContainer = createStateContainer( initialState, @@ -167,8 +175,6 @@ export class DashboardStateManager { } ); - this.isDirty = false; - // We can't compare the filters stored on this.appState to this.savedDashboard because in order to apply // the filters to the visualizations, we need to save it on the dashboard. We keep track of the original // filter state in order to let the user know if their filters changed and provide this specific information diff --git a/src/plugins/dashboard/public/application/listing/confirm_overlays.tsx b/src/plugins/dashboard/public/application/listing/confirm_overlays.tsx index cb3226963efb8..5fd9a2c0fdf75 100644 --- a/src/plugins/dashboard/public/application/listing/confirm_overlays.tsx +++ b/src/plugins/dashboard/public/application/listing/confirm_overlays.tsx @@ -42,7 +42,7 @@ export const confirmCreateWithUnsaved = ( contineCallback: () => void ) => overlays - .openConfirm(leaveConfirmStrings.getDiscardSubtitle(), { + .openConfirm(createConfirmStrings.getCreateSubtitle(), { confirmButtonText: createConfirmStrings.getConfirmButtonText(), cancelButtonText: createConfirmStrings.getCancelButtonText(), defaultFocusedButton: EUI_MODAL_CANCEL_BUTTON, diff --git a/src/plugins/dashboard/public/application/listing/dashboard_listing.tsx b/src/plugins/dashboard/public/application/listing/dashboard_listing.tsx index 4723ddbe6f5c1..d2a8b7d788170 100644 --- a/src/plugins/dashboard/public/application/listing/dashboard_listing.tsx +++ b/src/plugins/dashboard/public/application/listing/dashboard_listing.tsx @@ -150,7 +150,7 @@ export const DashboardListing = ({ ); const createItem = useCallback(() => { - if (dashboardPanelStorage.dashboardHasUnsavedEdits()) { + if (!dashboardPanelStorage.dashboardHasUnsavedEdits()) { redirectTo({ destination: 'dashboard' }); } else { confirmCreateWithUnsaved( From ea9162d917400bf769c801524b53ffe2ba7498f9 Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Thu, 14 Jan 2021 12:25:02 -0500 Subject: [PATCH 07/30] Added unit testing for unsaved listing --- .../application/listing/confirm_overlays.tsx | 70 ++++++-- .../dashboard_unsaved_listing.test.tsx | 160 ++++++++++++++++++ .../listing/dashboard_unsaved_listing.tsx | 15 +- .../dashboard/public/dashboard_strings.ts | 5 +- 4 files changed, 231 insertions(+), 19 deletions(-) create mode 100644 src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.test.tsx diff --git a/src/plugins/dashboard/public/application/listing/confirm_overlays.tsx b/src/plugins/dashboard/public/application/listing/confirm_overlays.tsx index 5fd9a2c0fdf75..a11e909559570 100644 --- a/src/plugins/dashboard/public/application/listing/confirm_overlays.tsx +++ b/src/plugins/dashboard/public/application/listing/confirm_overlays.tsx @@ -17,9 +17,21 @@ * under the License. */ -import { EUI_MODAL_CANCEL_BUTTON } from '@elastic/eui'; +import { + EuiButton, + EuiButtonEmpty, + EuiModal, + EuiModalBody, + EuiModalFooter, + EuiModalHeader, + EuiModalHeaderTitle, + EuiText, + EUI_MODAL_CANCEL_BUTTON, +} from '@elastic/eui'; +import React from 'react'; import { OverlayStart } from '../../../../../core/public'; import { createConfirmStrings, leaveConfirmStrings } from '../../dashboard_strings'; +import { toMountPoint } from '../../services/kibana_react'; export const confirmDiscardUnsavedChanges = (overlays: OverlayStart, discardCallback: () => void) => overlays @@ -40,17 +52,45 @@ export const confirmCreateWithUnsaved = ( overlays: OverlayStart, startBlankCallback: () => void, contineCallback: () => void -) => - overlays - .openConfirm(createConfirmStrings.getCreateSubtitle(), { - confirmButtonText: createConfirmStrings.getConfirmButtonText(), - cancelButtonText: createConfirmStrings.getCancelButtonText(), - defaultFocusedButton: EUI_MODAL_CANCEL_BUTTON, - title: createConfirmStrings.getCreateTitle(), - }) - .then((isConfirmed) => { - if (isConfirmed) { - startBlankCallback(); - } - contineCallback(); - }); +) => { + const session = overlays.openModal( + toMountPoint( + session.close()}> + + {createConfirmStrings.getCreateTitle()} + + + + {createConfirmStrings.getCreateSubtitle()} + + + + session.close()}> + {createConfirmStrings.getCancelButtonText()} + + { + startBlankCallback(); + session.close(); + }} + > + {createConfirmStrings.getStartOverButtonText()} + + { + contineCallback(); + session.close(); + }} + > + {createConfirmStrings.getContinueButtonText()} + + + + ), + { + 'data-test-subj': 'dashboardCreateConfirmModal', + } + ); +}; diff --git a/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.test.tsx b/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.test.tsx new file mode 100644 index 0000000000000..4c25edb3618ba --- /dev/null +++ b/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.test.tsx @@ -0,0 +1,160 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { I18nProvider } from '@kbn/i18n/react'; +import { findTestSubject } from '@elastic/eui/lib/test'; +import { waitFor } from '@testing-library/react'; +import { mount } from 'enzyme'; +import React from 'react'; +import { DashboardSavedObject } from '../..'; +import { coreMock } from '../../../../../core/public/mocks'; +import { KibanaContextProvider } from '../../services/kibana_react'; +import { SavedObjectLoader } from '../../services/saved_objects'; +import { DashboardPanelStorage } from '../lib'; +import { DASHBOARD_PANELS_UNSAVED_ID } from '../lib/dashboard_panel_storage'; +import { DashboardAppServices, DashboardRedirect } from '../types'; +import { DashboardUnsavedListing } from './dashboard_unsaved_listing'; + +const mockedDashboards: { [key: string]: DashboardSavedObject } = { + dashboardUnsavedOne: { + id: `dashboardUnsavedOne`, + title: `Dashboard Unsaved One`, + } as DashboardSavedObject, + dashboardUnsavedTwo: { + id: `dashboardUnsavedTwo`, + title: `Dashboard Unsaved Two`, + } as DashboardSavedObject, + dashboardUnsavedThree: { + id: `dashboardUnsavedThree`, + title: `Dashboard Unsaved Three`, + } as DashboardSavedObject, +}; + +function makeDefaultServices(): DashboardAppServices { + const core = coreMock.createStart(); + core.overlays.openConfirm = jest.fn().mockResolvedValue(true); + const savedDashboards = {} as SavedObjectLoader; + savedDashboards.get = jest.fn().mockImplementation((id: string) => mockedDashboards[id]); + const dashboardPanelStorage = {} as DashboardPanelStorage; + dashboardPanelStorage.clearPanels = jest.fn(); + dashboardPanelStorage.getDashboardIdsWithUnsavedChanges = jest + .fn() + .mockImplementation(() => [ + 'dashboardUnsavedOne', + 'dashboardUnsavedTwo', + 'dashboardUnsavedThree', + ]); + return ({ + dashboardPanelStorage, + savedDashboards, + core, + } as unknown) as DashboardAppServices; +} + +const makeDefaultProps = () => ({ redirectTo: jest.fn() }); + +function mountWith({ + services: incomingServices, + props: incomingProps, +}: { + services?: DashboardAppServices; + props?: { redirectTo: DashboardRedirect }; +}) { + const services = incomingServices ?? makeDefaultServices(); + const props = incomingProps ?? makeDefaultProps(); + const wrappingComponent: React.FC<{ + children: React.ReactNode; + }> = ({ children }) => { + return ( + + {children} + + ); + }; + const component = mount(, { wrappingComponent }); + return { component, props, services }; +} + +describe('Unsaved listing', () => { + it('Gets information for each unsaved dashboard', async () => { + const { services } = mountWith({}); + await waitFor(() => { + expect(services.savedDashboards.get).toHaveBeenCalledTimes(3); + }); + }); + + it('Does not attempt to get unsaved dashboard id', async () => { + const services = makeDefaultServices(); + services.dashboardPanelStorage.getDashboardIdsWithUnsavedChanges = jest + .fn() + .mockImplementation(() => ['dashboardUnsavedOne', DASHBOARD_PANELS_UNSAVED_ID]); + mountWith({ services }); + await waitFor(() => { + expect(services.savedDashboards.get).toHaveBeenCalledTimes(1); + }); + }); + + it('Redirects to the requested dashboard in edit mode when continue editing clicked', async () => { + const { props, component } = mountWith({}); + await waitFor(() => { + component.update(); + const editButton = findTestSubject(component, 'edit-unsaved-dashboardUnsavedOne'); + editButton.simulate('click'); + component.update(); + expect(props.redirectTo).toHaveBeenCalledWith({ + destination: 'dashboard', + id: 'dashboardUnsavedOne', + editMode: true, + }); + }); + }); + + it('Redirects to new dashboard when continue editing clicked', async () => { + const services = makeDefaultServices(); + services.dashboardPanelStorage.getDashboardIdsWithUnsavedChanges = jest + .fn() + .mockImplementation(() => [DASHBOARD_PANELS_UNSAVED_ID]); + const { props, component } = mountWith({ services }); + await waitFor(() => { + component.update(); + const editButton = findTestSubject(component, `edit-unsaved-${DASHBOARD_PANELS_UNSAVED_ID}`); + editButton.simulate('click'); + component.update(); + expect(props.redirectTo).toHaveBeenCalledWith({ + destination: 'dashboard', + id: undefined, + editMode: true, + }); + }); + }); + + it('Shows a warning then clears changes when delete unsaved changes is pressed', async () => { + const { services, component } = mountWith({}); + await waitFor(() => { + component.update(); + const discardButton = findTestSubject(component, 'discard-unsaved-dashboardUnsavedOne'); + discardButton.simulate('click'); + component.update(); + expect(services.core.overlays.openConfirm).toHaveBeenCalled(); + expect(services.dashboardPanelStorage.clearPanels).toHaveBeenCalledWith( + 'dashboardUnsavedOne' + ); + }); + }); +}); diff --git a/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.tsx b/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.tsx index 051022419a014..1764894ddde92 100644 --- a/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.tsx +++ b/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.tsx @@ -53,12 +53,23 @@ const DashboardUnsavedItem = ({ - + {dashboardUnsavedListingStrings.getEditTitle()} - + {dashboardUnsavedListingStrings.getDiscardTitle()} diff --git a/src/plugins/dashboard/public/dashboard_strings.ts b/src/plugins/dashboard/public/dashboard_strings.ts index 6028006cea644..6b57389b01d0d 100644 --- a/src/plugins/dashboard/public/dashboard_strings.ts +++ b/src/plugins/dashboard/public/dashboard_strings.ts @@ -264,13 +264,14 @@ export const createConfirmStrings = { i18n.translate('dashboard.createConfirmModal.unsavedChangesSubtitle', { defaultMessage: 'You can continue editing or start with a blank dashboard.', }), - getConfirmButtonText: () => + getStartOverButtonText: () => i18n.translate('dashboard.createConfirmModal.confirmButtonLabel', { defaultMessage: 'Start over', }), + getContinueButtonText: () => leaveConfirmStrings.getCancelButtonText(), getCancelButtonText: () => i18n.translate('dashboard.createConfirmModal.cancelButtonLabel', { - defaultMessage: 'Continue editing', + defaultMessage: 'Cancel', }), }; From de19cb72c88c0a80db13180eed8eaef9502424bd Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Thu, 14 Jan 2021 16:26:50 -0500 Subject: [PATCH 08/30] Changed recently accessed functionality. Ensured proper unsaved state is removed on save. --- .../public/application/dashboard_router.tsx | 9 ++++- .../hooks/use_dashboard_state_manager.ts | 6 ++-- .../application/hooks/use_saved_dashboard.ts | 7 +--- .../listing/dashboard_listing.test.tsx | 1 + .../application/top_nav/dashboard_top_nav.tsx | 33 +++++++++++++++++-- .../dashboard/public/application/types.ts | 1 + .../dashboard/public/dashboard_constants.ts | 7 ++-- .../saved_dashboards/saved_dashboard.ts | 7 +++- 8 files changed, 54 insertions(+), 17 deletions(-) diff --git a/src/plugins/dashboard/public/application/dashboard_router.tsx b/src/plugins/dashboard/public/application/dashboard_router.tsx index 71fe948485893..358f0b8a834d1 100644 --- a/src/plugins/dashboard/public/application/dashboard_router.tsx +++ b/src/plugins/dashboard/public/application/dashboard_router.tsx @@ -31,7 +31,12 @@ import { createDashboardListingFilterUrl } from '../dashboard_constants'; import { getDashboardPageTitle, dashboardReadonlyBadge } from '../dashboard_strings'; import { createDashboardEditUrl, DashboardConstants } from '../dashboard_constants'; import { DashboardAppServices, DashboardEmbedSettings, RedirectToProps } from './types'; -import { DashboardSetupDependencies, DashboardStart, DashboardStartDependencies } from '../plugin'; +import { + DashboardFeatureFlagConfig, + DashboardSetupDependencies, + DashboardStart, + DashboardStartDependencies, +} from '../plugin'; import { createKbnUrlStateStorage, withNotifyOnErrors } from '../services/kibana_utils'; import { KibanaContextProvider } from '../services/kibana_react'; @@ -108,6 +113,8 @@ export async function mountApp({ dashboardPanelStorage: new DashboardPanelStorage(), savedDashboards: dashboardStart.getSavedDashboardLoader(), savedObjectsTagging: savedObjectsTaggingOss?.getTaggingApi(), + allowByValueEmbeddables: initializerContext.config.get() + .allowByValueEmbeddables, dashboardCapabilities: { hideWriteControls: dashboardConfig.getHideWriteControls(), show: Boolean(coreStart.application.capabilities.dashboard.show), diff --git a/src/plugins/dashboard/public/application/hooks/use_dashboard_state_manager.ts b/src/plugins/dashboard/public/application/hooks/use_dashboard_state_manager.ts index 041f4456027d0..c36dbf502a15c 100644 --- a/src/plugins/dashboard/public/application/hooks/use_dashboard_state_manager.ts +++ b/src/plugins/dashboard/public/application/hooks/use_dashboard_state_manager.ts @@ -39,7 +39,6 @@ import { createSessionRestorationDataProvider } from '../lib/session_restoration import { DashboardStateManager } from '../dashboard_state_manager'; import { getDashboardTitle } from '../../dashboard_strings'; import { DashboardAppServices } from '../types'; -import { DashboardFeatureFlagConfig } from '../..'; import { ViewMode } from '../../services/embeddable'; // TS is picky with type guards, we can't just inline `() => false` @@ -66,6 +65,7 @@ export const useDashboardStateManager = ( savedObjectsTagging, dashboardCapabilities, dashboardPanelStorage, + allowByValueEmbeddables, } = useKibana().services; // Destructure and rename services; makes the Effect hook more specific, makes later @@ -96,9 +96,6 @@ export const useDashboardStateManager = ( ...withNotifyOnErrors(toasts), }); - const allowByValueEmbeddables = initializerContext.config.get() - .allowByValueEmbeddables; - const stateManager = new DashboardStateManager({ hasTaggingCapabilities, dashboardPanelStorage, @@ -216,6 +213,7 @@ export const useDashboardStateManager = ( toasts, uiSettings, usageCollection, + allowByValueEmbeddables, ]); return { dashboardStateManager, viewMode, setViewMode }; diff --git a/src/plugins/dashboard/public/application/hooks/use_saved_dashboard.ts b/src/plugins/dashboard/public/application/hooks/use_saved_dashboard.ts index f08db33d7e941..5afcfbab4e59a 100644 --- a/src/plugins/dashboard/public/application/hooks/use_saved_dashboard.ts +++ b/src/plugins/dashboard/public/application/hooks/use_saved_dashboard.ts @@ -54,12 +54,7 @@ export const useSavedDashboard = (savedDashboardId: string | undefined, history: try { const dashboard = (await savedDashboards.get(savedDashboardId)) as DashboardSavedObject; - const { title, getFullPath } = dashboard; - if (savedDashboardId) { - recentlyAccessedPaths.add(getFullPath(), title, savedDashboardId); - } - - docTitle.change(title); + docTitle.change(dashboard.title); setSavedDashboard(dashboard); } catch (error) { // E.g. a corrupt or deleted dashboard diff --git a/src/plugins/dashboard/public/application/listing/dashboard_listing.test.tsx b/src/plugins/dashboard/public/application/listing/dashboard_listing.test.tsx index 7f3b3c42b195c..7f778363a3a1d 100644 --- a/src/plugins/dashboard/public/application/listing/dashboard_listing.test.tsx +++ b/src/plugins/dashboard/public/application/listing/dashboard_listing.test.tsx @@ -78,6 +78,7 @@ function makeDefaultServices(): DashboardAppServices { uiSettings: {} as IUiSettingsClient, restorePreviousUrl: () => {}, onAppLeave: (handler) => {}, + allowByValueEmbeddables: true, savedDashboards, core, }; 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 97c69a3925a32..a294976893c95 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 @@ -56,6 +56,8 @@ import { ShowShareModal } from './show_share_modal'; import { PanelToolbar } from './panel_toolbar'; import { confirmDiscardUnsavedChanges } from '../listing/confirm_overlays'; import { OverlayRef } from '../../../../../core/public'; +import { getNewDashboardTitle } from '../../dashboard_strings'; +import { DASHBOARD_PANELS_UNSAVED_ID } from '../lib/dashboard_panel_storage'; import { DashboardContainer } from '..'; export interface DashboardTopNavState { @@ -100,6 +102,8 @@ export function DashboardTopNav({ setHeaderActionMenu, savedObjectsTagging, dashboardCapabilities, + dashboardPanelStorage, + allowByValueEmbeddables, } = useKibana().services; const [state, setState] = useState({ chromeIsVisible: false }); @@ -108,8 +112,16 @@ export function DashboardTopNav({ const visibleSubscription = chrome.getIsVisible$().subscribe((chromeIsVisible) => { setState((s) => ({ ...s, chromeIsVisible })); }); + const { id, title, getFullEditPath } = savedDashboard; + if (id || allowByValueEmbeddables) { + chrome.recentlyAccessed.add( + getFullEditPath(dashboardStateManager.getIsEditMode()), + title || getNewDashboardTitle(), + id || DASHBOARD_PANELS_UNSAVED_ID + ); + } return () => visibleSubscription.unsubscribe(); - }, [chrome]); + }, [chrome, allowByValueEmbeddables, dashboardStateManager, savedDashboard]); const addFromLibrary = useCallback(() => { if (!isErrorEmbeddable(dashboardContainer)) { @@ -158,6 +170,11 @@ export function DashboardTopNav({ const isLeavingEditMode = !isPageRefresh && newMode === ViewMode.VIEW; const willLoseChanges = isLeavingEditMode && dashboardStateManager.getIsDirty(timefilter); + if (savedDashboard?.id && allowByValueEmbeddables) { + const { getFullEditPath, title, id } = savedDashboard; + chrome.recentlyAccessed.add(getFullEditPath(newMode === ViewMode.EDIT), title, id); + } + if (!willLoseChanges) { dashboardStateManager.switchViewMode(newMode); if (newMode === ViewMode.EDIT) { @@ -184,7 +201,16 @@ export function DashboardTopNav({ confirmDiscardUnsavedChanges(core.overlays, revertChangesAndExitEditMode); }, - [redirectTo, timefilter, core.overlays, savedDashboard.id, dashboardStateManager, clearAddPanel] + [ + redirectTo, + timefilter, + clearAddPanel, + core.overlays, + savedDashboard, + dashboardStateManager, + allowByValueEmbeddables, + chrome.recentlyAccessed, + ] ); /** @@ -212,7 +238,7 @@ export function DashboardTopNav({ 'data-test-subj': 'saveDashboardSuccess', }); - dashboardStateManager.clearUnsavedPanels(); + dashboardPanelStorage.clearPanels(lastDashboardId); if (id !== lastDashboardId) { redirectTo({ destination: 'dashboard', id }); } else { @@ -239,6 +265,7 @@ export function DashboardTopNav({ [ core.notifications.toasts, dashboardStateManager, + dashboardPanelStorage, lastDashboardId, chrome.docTitle, redirectTo, diff --git a/src/plugins/dashboard/public/application/types.ts b/src/plugins/dashboard/public/application/types.ts index 0f652ddb0aa4a..4d5cdf12917a3 100644 --- a/src/plugins/dashboard/public/application/types.ts +++ b/src/plugins/dashboard/public/application/types.ts @@ -77,6 +77,7 @@ export interface DashboardAppServices { uiSettings: IUiSettingsClient; restorePreviousUrl: () => void; savedObjects: SavedObjectsStart; + allowByValueEmbeddables: boolean; urlForwarding: UrlForwardingStart; savedDashboards: SavedObjectLoader; scopedHistory: () => ScopedHistory; diff --git a/src/plugins/dashboard/public/dashboard_constants.ts b/src/plugins/dashboard/public/dashboard_constants.ts index a24bba1d1ef0e..f07f55d6b7a30 100644 --- a/src/plugins/dashboard/public/dashboard_constants.ts +++ b/src/plugins/dashboard/public/dashboard_constants.ts @@ -30,9 +30,12 @@ export const DashboardConstants = { SEARCH_SESSION_ID: 'searchSessionId', }; -export function createDashboardEditUrl(id: string, editMode?: boolean) { +export function createDashboardEditUrl(id?: string, editMode?: boolean) { const edit = editMode ? `?${STATE_STORAGE_KEY}=(viewMode:edit)` : ''; - return `${DashboardConstants.VIEW_DASHBOARD_URL}/${id}${edit}`; + if (id) { + return `${DashboardConstants.VIEW_DASHBOARD_URL}/${id}${edit}`; + } + return `${DashboardConstants.CREATE_NEW_DASHBOARD_URL}${edit}`; } export function createDashboardListingFilterUrl(filter: string | undefined) { diff --git a/src/plugins/dashboard/public/saved_dashboards/saved_dashboard.ts b/src/plugins/dashboard/public/saved_dashboards/saved_dashboard.ts index e9645b36af660..ebb20fc6e25c1 100644 --- a/src/plugins/dashboard/public/saved_dashboards/saved_dashboard.ts +++ b/src/plugins/dashboard/public/saved_dashboards/saved_dashboard.ts @@ -40,6 +40,7 @@ export interface DashboardSavedObject extends SavedObject { searchSource: ISearchSource; getQuery(): Query; getFilters(): Filter[]; + getFullEditPath: (editMode?: boolean) => string; } // Used only by the savedDashboards service, usually no reason to change this @@ -116,7 +117,7 @@ export function createSavedDashboardClass( refreshInterval: undefined, }, }); - this.getFullPath = () => `/app/dashboards#${createDashboardEditUrl(String(this.id))}`; + this.getFullPath = () => `/app/dashboards#${createDashboardEditUrl(this.id)}`; } getQuery() { @@ -126,6 +127,10 @@ export function createSavedDashboardClass( getFilters() { return this.searchSource!.getOwnField('filter') || []; } + + getFullEditPath = (editMode?: boolean) => { + return `/app/dashboards#${createDashboardEditUrl(this.id, editMode)}`; + }; } // Unfortunately this throws a typescript error without the casting. I think it's due to the From e5f68d92f677fb957ba12fffb75a30f05471717f Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Thu, 14 Jan 2021 18:52:51 -0500 Subject: [PATCH 09/30] Removed issue caused by dashboard_security importing from dashboard_constants --- .../dashboard/public/application/dashboard_app_functions.ts | 1 + src/plugins/dashboard/public/dashboard_constants.ts | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/plugins/dashboard/public/application/dashboard_app_functions.ts b/src/plugins/dashboard/public/application/dashboard_app_functions.ts index 0e6f0f460859d..af7a485296ea0 100644 --- a/src/plugins/dashboard/public/application/dashboard_app_functions.ts +++ b/src/plugins/dashboard/public/application/dashboard_app_functions.ts @@ -195,6 +195,7 @@ export const getInputSubscription = ({ dashboardContainer.getInput().filters ); } + dashboardStateManager.handleDashboardContainerChanges(dashboardContainer); }); diff --git a/src/plugins/dashboard/public/dashboard_constants.ts b/src/plugins/dashboard/public/dashboard_constants.ts index f07f55d6b7a30..6e279a9215721 100644 --- a/src/plugins/dashboard/public/dashboard_constants.ts +++ b/src/plugins/dashboard/public/dashboard_constants.ts @@ -17,7 +17,7 @@ * under the License. */ -import { STATE_STORAGE_KEY } from './url_generator'; +const DASHBOARD_STATE_STORAGE_KEY = '_a'; export const DashboardConstants = { LANDING_PAGE_PATH: '/list', @@ -31,7 +31,7 @@ export const DashboardConstants = { }; export function createDashboardEditUrl(id?: string, editMode?: boolean) { - const edit = editMode ? `?${STATE_STORAGE_KEY}=(viewMode:edit)` : ''; + const edit = editMode ? `?${DASHBOARD_STATE_STORAGE_KEY}=(viewMode:edit)` : ''; if (id) { return `${DashboardConstants.VIEW_DASHBOARD_URL}/${id}${edit}`; } From b9e1c9a2ddc1857361bba0c846c25f32614d1957 Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Mon, 18 Jan 2021 16:58:38 -0500 Subject: [PATCH 10/30] split discarding changes from view / edit mode. --- .../application/dashboard_state_manager.ts | 17 +++-- .../hooks/use_dashboard_state_manager.ts | 2 +- .../application/top_nav/dashboard_top_nav.tsx | 69 ++++++++++--------- .../application/top_nav/get_top_nav_config.ts | 26 +++++-- .../public/application/top_nav/top_nav_ids.ts | 1 + .../dashboard/public/dashboard_strings.ts | 16 ++++- 6 files changed, 87 insertions(+), 44 deletions(-) diff --git a/src/plugins/dashboard/public/application/dashboard_state_manager.ts b/src/plugins/dashboard/public/application/dashboard_state_manager.ts index 7438acf3eba18..87d4282da4a4c 100644 --- a/src/plugins/dashboard/public/application/dashboard_state_manager.ts +++ b/src/plugins/dashboard/public/application/dashboard_state_manager.ts @@ -185,9 +185,6 @@ export class DashboardStateManager { this.stateContainerChangeSub = this.stateContainer.state$.subscribe(() => { this.isDirty = this.checkIsDirty(); - if (!this.isDirty && this.getIsViewMode()) { - this.clearUnsavedPanels(); - } this.changeListeners.forEach((listener) => listener({ dirty: this.isDirty })); }); @@ -209,9 +206,14 @@ export class DashboardStateManager { const currentDashboardIdInUrl = getDashboardIdFromUrl(history.location.pathname); if (currentDashboardIdInUrl !== this.savedDashboard.id) return; + // set View mode before the rest of the state so unsaved panels can be added correctly. + if (this.appState.viewMode !== stateFromUrl.viewMode) { + this.switchViewMode(stateFromUrl.viewMode); + } + this.stateContainer.set({ ...this.stateDefaults, - ...(this.getUnsavedPanelState() || {}), + ...this.getUnsavedPanelState(), ...stateFromUrl, }); } else { @@ -296,6 +298,11 @@ export class DashboardStateManager { } else { this.setUnsavedPanels(this.getPanels()); } + + // If a panel has been changed, and the state is now equal to the state in the saved object, remove the unsaved panels + if (!this.isDirty && this.getIsEditMode()) { + this.clearUnsavedPanels(); + } } if (input.isFullScreenMode !== this.getFullScreenMode()) { @@ -672,7 +679,7 @@ export class DashboardStateManager { } } - public restoreUnsavedPanels() { + public restorePanels() { const unsavedState = this.getUnsavedPanelState(); if (!unsavedState || unsavedState.panels?.length === 0) { return; diff --git a/src/plugins/dashboard/public/application/hooks/use_dashboard_state_manager.ts b/src/plugins/dashboard/public/application/hooks/use_dashboard_state_manager.ts index c36dbf502a15c..a2b5dcf595134 100644 --- a/src/plugins/dashboard/public/application/hooks/use_dashboard_state_manager.ts +++ b/src/plugins/dashboard/public/application/hooks/use_dashboard_state_manager.ts @@ -184,7 +184,7 @@ export const useDashboardStateManager = ( ); if (stateManager.getIsEditMode()) { - stateManager.restoreUnsavedPanels(); + stateManager.restorePanels(); } setDashboardStateManager(stateManager); 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 a294976893c95..d85b597c4601f 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 @@ -56,7 +56,7 @@ import { ShowShareModal } from './show_share_modal'; import { PanelToolbar } from './panel_toolbar'; import { confirmDiscardUnsavedChanges } from '../listing/confirm_overlays'; import { OverlayRef } from '../../../../../core/public'; -import { getNewDashboardTitle } from '../../dashboard_strings'; +import { getNewDashboardTitle, unsavedChangesRetainedStrings } from '../../dashboard_strings'; import { DASHBOARD_PANELS_UNSAVED_ID } from '../lib/dashboard_panel_storage'; import { DashboardContainer } from '..'; @@ -163,53 +163,54 @@ export function DashboardTopNav({ } }, [state.addPanelOverlay]); + const onDiscardChanges = useCallback(() => { + function revertChangesAndExitEditMode() { + dashboardStateManager.resetState(); + // This is only necessary for new dashboards, which will default to Edit mode. + dashboardStateManager.switchViewMode(ViewMode.VIEW); + 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); + } + redirectTo({ destination: 'dashboard', id: savedDashboard.id }); + } + + confirmDiscardUnsavedChanges(core.overlays, revertChangesAndExitEditMode); + }, [core.overlays, dashboardStateManager, redirectTo, timefilter, savedDashboard.id]); + const onChangeViewMode = useCallback( (newMode: ViewMode) => { clearAddPanel(); - const isPageRefresh = newMode === dashboardStateManager.getViewMode(); - const isLeavingEditMode = !isPageRefresh && newMode === ViewMode.VIEW; - const willLoseChanges = isLeavingEditMode && dashboardStateManager.getIsDirty(timefilter); - if (savedDashboard?.id && allowByValueEmbeddables) { const { getFullEditPath, title, id } = savedDashboard; chrome.recentlyAccessed.add(getFullEditPath(newMode === ViewMode.EDIT), title, id); } - - if (!willLoseChanges) { - dashboardStateManager.switchViewMode(newMode); - if (newMode === ViewMode.EDIT) { - dashboardStateManager.restoreUnsavedPanels(); - } - return; - } - - function revertChangesAndExitEditMode() { - dashboardStateManager.resetState(); - // This is only necessary for new dashboards, which will default to Edit mode. - dashboardStateManager.switchViewMode(ViewMode.VIEW); - 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); - } - redirectTo({ destination: 'dashboard', id: savedDashboard.id }); + if ( + newMode === ViewMode.VIEW && + dashboardStateManager.getIsEditMode() && + dashboardStateManager.isDirty + ) { + core.notifications.toasts.addSuccess({ + title: unsavedChangesRetainedStrings.getTitle(), + text: unsavedChangesRetainedStrings.getText(), + }); } - confirmDiscardUnsavedChanges(core.overlays, revertChangesAndExitEditMode); + dashboardStateManager.switchViewMode(newMode); + dashboardStateManager.restorePanels(); }, [ - redirectTo, - timefilter, clearAddPanel, - core.overlays, savedDashboard, dashboardStateManager, allowByValueEmbeddables, chrome.recentlyAccessed, + core.notifications.toasts, ] ); @@ -240,7 +241,7 @@ export function DashboardTopNav({ dashboardPanelStorage.clearPanels(lastDashboardId); if (id !== lastDashboardId) { - redirectTo({ destination: 'dashboard', id }); + redirectTo({ destination: 'dashboard', id, useReplace: !lastDashboardId }); } else { chrome.docTitle.change(dashboardStateManager.savedDashboard.lastSavedTitle); dashboardStateManager.switchViewMode(ViewMode.VIEW); @@ -379,6 +380,7 @@ 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.CLONE]: runClone, [TopNavIds.ADD_EXISTING]: addFromLibrary, @@ -415,6 +417,7 @@ export function DashboardTopNav({ }, [ dashboardCapabilities, dashboardStateManager, + onDiscardChanges, onChangeViewMode, savedDashboard, addFromLibrary, 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 3db8e3fc792c0..fc1c93f400b44 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 @@ -52,6 +52,7 @@ export function getTopNavConfig( getShareConfig(actions[TopNavIds.SHARE]), getAddConfig(actions[TopNavIds.ADD_EXISTING]), getViewConfig(actions[TopNavIds.EXIT_EDIT_MODE]), + getDiscardConfig(actions[TopNavIds.DISCARD_CHANGES]), getSaveConfig(actions[TopNavIds.SAVE]), getCreateNewConfig(actions[TopNavIds.VISUALIZE]), ]; @@ -118,18 +119,35 @@ function getSaveConfig(action: NavAction) { */ function getViewConfig(action: NavAction) { return { - id: 'cancel', - label: i18n.translate('dashboard.topNave.cancelButtonAriaLabel', { - defaultMessage: 'cancel', + id: 'view', + label: i18n.translate('dashboard.topNave.viewButtonAriaLabel', { + defaultMessage: 'view', }), description: i18n.translate('dashboard.topNave.viewConfigDescription', { - defaultMessage: 'Cancel editing and switch to view-only mode', + defaultMessage: 'Return to view mode', }), testId: 'dashboardViewOnlyMode', run: action, }; } +/** + * @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 748bfaaab6141..309391046ee11 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 @@ -23,6 +23,7 @@ export const TopNavIds = { SAVE: 'save', EXIT_EDIT_MODE: 'exitEditMode', ENTER_EDIT_MODE: 'enterEditMode', + DISCARD_CHANGES: 'discard', CLONE: 'clone', FULL_SCREEN: 'fullScreenMode', VISUALIZE: 'visualize', diff --git a/src/plugins/dashboard/public/dashboard_strings.ts b/src/plugins/dashboard/public/dashboard_strings.ts index 87c14eb43ff1f..ba409d064dd82 100644 --- a/src/plugins/dashboard/public/dashboard_strings.ts +++ b/src/plugins/dashboard/public/dashboard_strings.ts @@ -317,6 +317,20 @@ export const emptyScreenStrings = { }), }; +/* + Toasts +*/ +export const unsavedChangesRetainedStrings = { + getTitle: () => + i18n.translate('dashboard.changesRetainedToast.title', { + defaultMessage: 'Your changes are still here!', + }), + getText: () => + i18n.translate('dashboard.changesRetainedToast.text', { + defaultMessage: 'Return to edit mode to continue from where you left off.', + }), +}; + /* Dashboard Listing Page */ @@ -353,7 +367,7 @@ export const dashboardUnsavedListingStrings = { }), getEditTitle: () => i18n.translate('dashboard.listing.unsaved.editTitle', { - defaultMessage: 'Continue Editing', + defaultMessage: 'Continue editing', }), getDiscardTitle: () => i18n.translate('dashboard.listing.unsaved.discardTitle', { From 03214ef952f2f1c435eb601668bbaf108964f3c4 Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Mon, 18 Jan 2021 17:56:24 -0500 Subject: [PATCH 11/30] Listing entries for Dashboards with unsaved edits now open in view mode. Fixed tests --- .../public/application/_dashboard_app.scss | 2 - .../application/listing/dashboard_listing.tsx | 3 +- .../dashboard_unsaved_listing.test.tsx | 41 +++++++++++-------- .../translations/translations/ja-JP.json | 1 - .../translations/translations/zh-CN.json | 1 - 5 files changed, 24 insertions(+), 24 deletions(-) diff --git a/src/plugins/dashboard/public/application/_dashboard_app.scss b/src/plugins/dashboard/public/application/_dashboard_app.scss index 7c29f47e8da63..56dfc1ad6146e 100644 --- a/src/plugins/dashboard/public/application/_dashboard_app.scss +++ b/src/plugins/dashboard/public/application/_dashboard_app.scss @@ -41,5 +41,3 @@ .dshUnsavedListingButtons { margin-left: 15px; } - - diff --git a/src/plugins/dashboard/public/application/listing/dashboard_listing.tsx b/src/plugins/dashboard/public/application/listing/dashboard_listing.tsx index d2a8b7d788170..dbe7fb3441aaf 100644 --- a/src/plugins/dashboard/public/application/listing/dashboard_listing.tsx +++ b/src/plugins/dashboard/public/application/listing/dashboard_listing.tsx @@ -102,11 +102,10 @@ export const DashboardListing = ({ redirectTo({ destination: 'dashboard', id, - editMode: dashboardPanelStorage.dashboardHasUnsavedEdits(id), }), savedObjectsTagging ), - [savedObjectsTagging, redirectTo, dashboardPanelStorage] + [savedObjectsTagging, redirectTo] ); const noItemsFragment = useMemo( diff --git a/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.test.tsx b/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.test.tsx index 4c25edb3618ba..6c9201a6fa25c 100644 --- a/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.test.tsx +++ b/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.test.tsx @@ -112,16 +112,16 @@ describe('Unsaved listing', () => { it('Redirects to the requested dashboard in edit mode when continue editing clicked', async () => { const { props, component } = mountWith({}); + const getEditButton = () => findTestSubject(component, 'edit-unsaved-dashboardUnsavedOne'); await waitFor(() => { component.update(); - const editButton = findTestSubject(component, 'edit-unsaved-dashboardUnsavedOne'); - editButton.simulate('click'); - component.update(); - expect(props.redirectTo).toHaveBeenCalledWith({ - destination: 'dashboard', - id: 'dashboardUnsavedOne', - editMode: true, - }); + expect(getEditButton().length).toEqual(1); + }); + getEditButton().simulate('click'); + expect(props.redirectTo).toHaveBeenCalledWith({ + destination: 'dashboard', + id: 'dashboardUnsavedOne', + editMode: true, }); }); @@ -131,25 +131,30 @@ describe('Unsaved listing', () => { .fn() .mockImplementation(() => [DASHBOARD_PANELS_UNSAVED_ID]); const { props, component } = mountWith({ services }); + const getEditButton = () => + findTestSubject(component, `edit-unsaved-${DASHBOARD_PANELS_UNSAVED_ID}`); await waitFor(() => { component.update(); - const editButton = findTestSubject(component, `edit-unsaved-${DASHBOARD_PANELS_UNSAVED_ID}`); - editButton.simulate('click'); - component.update(); - expect(props.redirectTo).toHaveBeenCalledWith({ - destination: 'dashboard', - id: undefined, - editMode: true, - }); + expect(getEditButton().length).toBe(1); + }); + getEditButton().simulate('click'); + expect(props.redirectTo).toHaveBeenCalledWith({ + destination: 'dashboard', + id: undefined, + editMode: true, }); }); it('Shows a warning then clears changes when delete unsaved changes is pressed', async () => { const { services, component } = mountWith({}); + const getDiscardButton = () => + findTestSubject(component, 'discard-unsaved-dashboardUnsavedOne'); await waitFor(() => { component.update(); - const discardButton = findTestSubject(component, 'discard-unsaved-dashboardUnsavedOne'); - discardButton.simulate('click'); + expect(getDiscardButton().length).toBe(1); + }); + getDiscardButton().simulate('click'); + waitFor(() => { component.update(); expect(services.core.overlays.openConfirm).toHaveBeenCalled(); expect(services.dashboardPanelStorage.clearPanels).toHaveBeenCalledWith( diff --git a/x-pack/plugins/translations/translations/ja-JP.json b/x-pack/plugins/translations/translations/ja-JP.json index 6dd72d179b210..eda6a2afaa825 100644 --- a/x-pack/plugins/translations/translations/ja-JP.json +++ b/x-pack/plugins/translations/translations/ja-JP.json @@ -668,7 +668,6 @@ "dashboard.topNave.addConfigDescription": "既存のビジュアライゼーションをダッシュボードに追加", "dashboard.topNave.addNewButtonAriaLabel": "パネルの作成", "dashboard.topNave.addNewConfigDescription": "このダッシュボードに新規パネルを作成", - "dashboard.topNave.cancelButtonAriaLabel": "キャンセル", "dashboard.topNave.cloneButtonAriaLabel": "クローンを作成", "dashboard.topNave.cloneConfigDescription": "ダッシュボードのコピーを作成します", "dashboard.topNave.editButtonAriaLabel": "編集", diff --git a/x-pack/plugins/translations/translations/zh-CN.json b/x-pack/plugins/translations/translations/zh-CN.json index 3ab1c7ee56a32..02901686c0fb3 100644 --- a/x-pack/plugins/translations/translations/zh-CN.json +++ b/x-pack/plugins/translations/translations/zh-CN.json @@ -668,7 +668,6 @@ "dashboard.topNave.addConfigDescription": "将现有可视化添加到仪表板", "dashboard.topNave.addNewButtonAriaLabel": "创建面板", "dashboard.topNave.addNewConfigDescription": "在此仪表板上创建新的面板", - "dashboard.topNave.cancelButtonAriaLabel": "取消", "dashboard.topNave.cloneButtonAriaLabel": "克隆", "dashboard.topNave.cloneConfigDescription": "创建仪表板的副本", "dashboard.topNave.editButtonAriaLabel": "编辑", From c19d0735d9a83e929ea974867b9229db0dcf2143 Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Tue, 19 Jan 2021 16:02:19 -0500 Subject: [PATCH 12/30] doc title change for New Dashboards, removed redirectTo from onDiscardChanges --- .../public/application/hooks/use_saved_dashboard.ts | 4 ++-- .../public/application/top_nav/dashboard_top_nav.tsx | 7 ++----- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/plugins/dashboard/public/application/hooks/use_saved_dashboard.ts b/src/plugins/dashboard/public/application/hooks/use_saved_dashboard.ts index 5afcfbab4e59a..e417056183c5c 100644 --- a/src/plugins/dashboard/public/application/hooks/use_saved_dashboard.ts +++ b/src/plugins/dashboard/public/application/hooks/use_saved_dashboard.ts @@ -25,7 +25,7 @@ import { useKibana } from '../../services/kibana_react'; import { DashboardConstants } from '../..'; import { DashboardSavedObject } from '../../saved_dashboards'; -import { getDashboard60Warning } from '../../dashboard_strings'; +import { getDashboard60Warning, getNewDashboardTitle } from '../../dashboard_strings'; import { DashboardAppServices } from '../types'; export const useSavedDashboard = (savedDashboardId: string | undefined, history: History) => { @@ -54,7 +54,7 @@ export const useSavedDashboard = (savedDashboardId: string | undefined, history: try { const dashboard = (await savedDashboards.get(savedDashboardId)) as DashboardSavedObject; - docTitle.change(dashboard.title); + docTitle.change(dashboard.title || getNewDashboardTitle()); setSavedDashboard(dashboard); } catch (error) { // E.g. a corrupt or deleted dashboard 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 d85b597c4601f..a393ffcfad587 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 @@ -166,8 +166,6 @@ export function DashboardTopNav({ const onDiscardChanges = useCallback(() => { function revertChangesAndExitEditMode() { dashboardStateManager.resetState(); - // This is only necessary for new dashboards, which will default to Edit mode. - dashboardStateManager.switchViewMode(ViewMode.VIEW); dashboardStateManager.clearUnsavedPanels(); // We need to do a hard reset of the timepicker. appState will not reload like @@ -177,11 +175,10 @@ export function DashboardTopNav({ dashboardStateManager.syncTimefilterWithDashboardTime(timefilter); dashboardStateManager.syncTimefilterWithDashboardRefreshInterval(timefilter); } - redirectTo({ destination: 'dashboard', id: savedDashboard.id }); + dashboardStateManager.switchViewMode(ViewMode.VIEW); } - confirmDiscardUnsavedChanges(core.overlays, revertChangesAndExitEditMode); - }, [core.overlays, dashboardStateManager, redirectTo, timefilter, savedDashboard.id]); + }, [core.overlays, dashboardStateManager, timefilter]); const onChangeViewMode = useCallback( (newMode: ViewMode) => { From 0e6f6b29daff315dc7986b20e0241cc8fde31d14 Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Tue, 19 Jan 2021 16:33:08 -0500 Subject: [PATCH 13/30] fixing functional tests --- test/accessibility/apps/dashboard.ts | 2 +- test/functional/apps/dashboard/view_edit.ts | 14 +++++++------- test/functional/page_objects/dashboard_page.ts | 5 +++++ 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/test/accessibility/apps/dashboard.ts b/test/accessibility/apps/dashboard.ts index c0d73b92d5e5a..f3638c63c2419 100644 --- a/test/accessibility/apps/dashboard.ts +++ b/test/accessibility/apps/dashboard.ts @@ -121,7 +121,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { }); it('Exit out of edit mode', async () => { - await PageObjects.dashboard.clickCancelOutOfEditMode(); + await PageObjects.dashboard.clickDiscardChanges(); await a11y.testAppSnapshot(); }); diff --git a/test/functional/apps/dashboard/view_edit.ts b/test/functional/apps/dashboard/view_edit.ts index b0e5d0b08dba2..be5c081b573c4 100644 --- a/test/functional/apps/dashboard/view_edit.ts +++ b/test/functional/apps/dashboard/view_edit.ts @@ -83,7 +83,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { 'Sep 19, 2013 @ 06:31:44.000', 'Sep 19, 2013 @ 06:31:44.000' ); - await PageObjects.dashboard.clickCancelOutOfEditMode(); + await PageObjects.dashboard.clickDiscardChanges(); // confirm lose changes await PageObjects.common.clickConfirmOnModal(); @@ -99,7 +99,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await queryBar.setQuery(`${originalQuery}and extra stuff`); await queryBar.submitQuery(); - await PageObjects.dashboard.clickCancelOutOfEditMode(); + await PageObjects.dashboard.clickDiscardChanges(); // confirm lose changes await PageObjects.common.clickConfirmOnModal(); @@ -122,7 +122,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { hasFilter = await filterBar.hasFilter('animal', 'dog'); expect(hasFilter).to.be(false); - await PageObjects.dashboard.clickCancelOutOfEditMode(); + await PageObjects.dashboard.clickDiscardChanges(); // confirm lose changes await PageObjects.common.clickConfirmOnModal(); @@ -144,7 +144,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { redirectToOrigin: true, }); - await PageObjects.dashboard.clickCancelOutOfEditMode(); + await PageObjects.dashboard.clickDiscardChanges(); // for this sleep see https://github.com/elastic/kibana/issues/22299 await PageObjects.common.sleep(500); @@ -159,7 +159,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { const originalPanelCount = await PageObjects.dashboard.getPanelCount(); await dashboardAddPanel.addVisualization('new viz panel'); - await PageObjects.dashboard.clickCancelOutOfEditMode(); + await PageObjects.dashboard.clickDiscardChanges(); // confirm lose changes await PageObjects.common.clickConfirmOnModal(); @@ -182,7 +182,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { 'Sep 19, 2015 @ 06:31:44.000', 'Sep 19, 2015 @ 06:31:44.000' ); - await PageObjects.dashboard.clickCancelOutOfEditMode(); + await PageObjects.dashboard.clickDiscardChanges(); await PageObjects.common.clickCancelOnModal(); await PageObjects.dashboard.saveDashboard(dashboardName, { @@ -211,7 +211,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { ); const newTime = await PageObjects.timePicker.getTimeConfig(); - await PageObjects.dashboard.clickCancelOutOfEditMode(); + await PageObjects.dashboard.clickDiscardChanges(); await PageObjects.common.clickCancelOnModal(); await PageObjects.dashboard.saveDashboard(dashboardName, { storeTimeWithDashboard: true }); diff --git a/test/functional/page_objects/dashboard_page.ts b/test/functional/page_objects/dashboard_page.ts index cc1420e4825c2..375d77da01e9a 100644 --- a/test/functional/page_objects/dashboard_page.ts +++ b/test/functional/page_objects/dashboard_page.ts @@ -227,6 +227,11 @@ export function DashboardPageProvider({ getService, getPageObjects }: FtrProvide await testSubjects.click('dashboardViewOnlyMode'); } + public async clickDiscardChanges() { + log.debug('clickDiscardChanges'); + await testSubjects.click('dashboardDiscardChanges'); + } + public async clickNewDashboard() { await listingTable.clickNewButton('createDashboardPromptButton'); // make sure the dashboard page is shown From f2cc0368311d5b6fd4f63ef1f1a70158430af7f8 Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Tue, 19 Jan 2021 16:46:59 -0500 Subject: [PATCH 14/30] added aria-labels to edit and discard buttons in unsaved listing --- .../public/application/listing/dashboard_unsaved_listing.tsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.tsx b/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.tsx index 1764894ddde92..e7a4de3d3b0aa 100644 --- a/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.tsx +++ b/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.tsx @@ -44,6 +44,7 @@ const DashboardUnsavedItem = ({ onOpenClick: () => void; onDiscardClick: () => void; }) => { + const title = dashboard?.title ?? getNewDashboardTitle(); return ( <> @@ -59,6 +60,7 @@ const DashboardUnsavedItem = ({ color="primary" onClick={onOpenClick} data-test-subj={`edit-unsaved-${dashboard?.id ?? DASHBOARD_PANELS_UNSAVED_ID}`} + aria-label={`${dashboardUnsavedListingStrings.getEditTitle()} ${title}`} > {dashboardUnsavedListingStrings.getEditTitle()} @@ -69,6 +71,7 @@ const DashboardUnsavedItem = ({ color="danger" onClick={onDiscardClick} data-test-subj={`discard-unsaved-${dashboard?.id ?? DASHBOARD_PANELS_UNSAVED_ID}`} + aria-label={`${dashboardUnsavedListingStrings.getDiscardTitle()} ${title}`} > {dashboardUnsavedListingStrings.getDiscardTitle()} From 9503272eb5d36fda24ea927d73aec7a62fee0cdc Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Tue, 19 Jan 2021 17:24:38 -0500 Subject: [PATCH 15/30] i18n fix --- .../application/listing/dashboard_unsaved_listing.tsx | 4 ++-- src/plugins/dashboard/public/dashboard_strings.ts | 10 ++++++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.tsx b/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.tsx index e7a4de3d3b0aa..69b013942458c 100644 --- a/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.tsx +++ b/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.tsx @@ -60,7 +60,7 @@ const DashboardUnsavedItem = ({ color="primary" onClick={onOpenClick} data-test-subj={`edit-unsaved-${dashboard?.id ?? DASHBOARD_PANELS_UNSAVED_ID}`} - aria-label={`${dashboardUnsavedListingStrings.getEditTitle()} ${title}`} + aria-label={dashboardUnsavedListingStrings.getEditAriaLabel(title)} > {dashboardUnsavedListingStrings.getEditTitle()} @@ -71,7 +71,7 @@ const DashboardUnsavedItem = ({ color="danger" onClick={onDiscardClick} data-test-subj={`discard-unsaved-${dashboard?.id ?? DASHBOARD_PANELS_UNSAVED_ID}`} - aria-label={`${dashboardUnsavedListingStrings.getDiscardTitle()} ${title}`} + aria-label={dashboardUnsavedListingStrings.getDiscardAriaLabel(title)} > {dashboardUnsavedListingStrings.getDiscardTitle()} diff --git a/src/plugins/dashboard/public/dashboard_strings.ts b/src/plugins/dashboard/public/dashboard_strings.ts index ba409d064dd82..b6b3fa42b62fb 100644 --- a/src/plugins/dashboard/public/dashboard_strings.ts +++ b/src/plugins/dashboard/public/dashboard_strings.ts @@ -365,10 +365,20 @@ export const dashboardUnsavedListingStrings = { : dashboardListingTable.getEntityName(), }, }), + getEditAriaLabel: (title: string) => + i18n.translate('dashboard.listing.unsaved.editAria', { + defaultMessage: 'Continue editing {title}', + values: { title }, + }), getEditTitle: () => i18n.translate('dashboard.listing.unsaved.editTitle', { defaultMessage: 'Continue editing', }), + getDiscardAriaLabel: (title: string) => + i18n.translate('dashboard.listing.unsaved.discardAria', { + defaultMessage: 'Discard changes to {title}', + values: { title }, + }), getDiscardTitle: () => i18n.translate('dashboard.listing.unsaved.discardTitle', { defaultMessage: 'Discard changes', From 809199a50c4276a3205fb9f89c61e4563909c571 Mon Sep 17 00:00:00 2001 From: Ryan Keairns Date: Tue, 19 Jan 2021 16:53:16 -0600 Subject: [PATCH 16/30] design tweaks to align with notifications UI --- .../public/application/_dashboard_app.scss | 14 ++++++-- .../listing/dashboard_unsaved_listing.tsx | 36 +++++++++++++------ .../dashboard/public/dashboard_strings.ts | 2 +- 3 files changed, 37 insertions(+), 15 deletions(-) diff --git a/src/plugins/dashboard/public/application/_dashboard_app.scss b/src/plugins/dashboard/public/application/_dashboard_app.scss index 56dfc1ad6146e..63a73d6f80c8f 100644 --- a/src/plugins/dashboard/public/application/_dashboard_app.scss +++ b/src/plugins/dashboard/public/application/_dashboard_app.scss @@ -34,10 +34,18 @@ text-align: center; } -.dshUnsavedListingItemTitle { +.dshUnsavedListingItem { + margin-top: $euiSizeM; +} + +.dshUnsavedListingItem__icon { + margin-right: $euiSizeM; +} + +.dshUnsavedListingItem__title { margin-bottom: 0 !important; } -.dshUnsavedListingButtons { - margin-left: 15px; +.dshUnsavedListingItem__actions { + margin-left: $euiSizeL + $euiSizeXS; } diff --git a/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.tsx b/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.tsx index 1764894ddde92..a52af34b48133 100644 --- a/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.tsx +++ b/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.tsx @@ -45,17 +45,28 @@ const DashboardUnsavedItem = ({ onDiscardClick: () => void; }) => { return ( - <> - -

- {dashboard?.title ?? getNewDashboardTitle()} -

-
- +
+ + + + + + +

+ {dashboard?.title ?? getNewDashboardTitle()} +

+
+
+
+ - +
); }; @@ -146,7 +157,10 @@ export const DashboardUnsavedListing = ({ redirectTo }: { redirectTo: DashboardR return items.length === 0 ? null : ( <> - 1)}> + 1)} + > {items} diff --git a/src/plugins/dashboard/public/dashboard_strings.ts b/src/plugins/dashboard/public/dashboard_strings.ts index ba409d064dd82..b80d8df03743f 100644 --- a/src/plugins/dashboard/public/dashboard_strings.ts +++ b/src/plugins/dashboard/public/dashboard_strings.ts @@ -358,7 +358,7 @@ export const dashboardListingTable = { export const dashboardUnsavedListingStrings = { getUnsavedChangesTitle: (plural = false) => i18n.translate('dashboard.listing.unsaved.unsavedChangesTitle', { - defaultMessage: 'You have unsaved changes in the following {dash}', + defaultMessage: 'You have unsaved changes in the following {dash}.', values: { dash: plural ? dashboardListingTable.getEntityNamePlural() From 0a46cbb8bfd32013376db7190f0a294f3171f6b4 Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Wed, 20 Jan 2021 18:04:03 -0500 Subject: [PATCH 17/30] Updated license headers, made both listing page create buttons use the same method. Removed functional tests which edit panels in URL, click start over in create confirm modal --- .../application/dashboard_state_manager.ts | 5 + .../lib/dashboard_panel_storage.ts | 21 +---- .../application/listing/confirm_overlays.tsx | 30 +++--- .../application/listing/dashboard_listing.tsx | 37 ++++---- .../dashboard_unsaved_listing.test.tsx | 21 +---- .../listing/dashboard_unsaved_listing.tsx | 21 +---- .../apps/dashboard/dashboard_state.ts | 92 ------------------- .../functional/page_objects/dashboard_page.ts | 9 +- 8 files changed, 57 insertions(+), 179 deletions(-) diff --git a/src/plugins/dashboard/public/application/dashboard_state_manager.ts b/src/plugins/dashboard/public/application/dashboard_state_manager.ts index 6eb80f888c23b..2ff3c99a251ca 100644 --- a/src/plugins/dashboard/public/application/dashboard_state_manager.ts +++ b/src/plugins/dashboard/public/application/dashboard_state_manager.ts @@ -174,6 +174,11 @@ export class DashboardStateManager { this.stateContainerChangeSub = this.stateContainer.state$.subscribe(() => { this.isDirty = this.checkIsDirty(); + + // Session storage keys seems to disappear in some functional tests. This will temporarily prevent that. + if (this.appState.panels.length && !this.getUnsavedPanelState().panels?.length) { + this.setUnsavedPanels(this.appState.panels); + } this.changeListeners.forEach((listener) => listener({ dirty: this.isDirty })); }); diff --git a/src/plugins/dashboard/public/application/lib/dashboard_panel_storage.ts b/src/plugins/dashboard/public/application/lib/dashboard_panel_storage.ts index 1cd66c0efb143..d7162b378ed80 100644 --- a/src/plugins/dashboard/public/application/lib/dashboard_panel_storage.ts +++ b/src/plugins/dashboard/public/application/lib/dashboard_panel_storage.ts @@ -1,20 +1,9 @@ /* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * and the Server Side Public License, v 1; you may not use this file except in + * compliance with, at your election, the Elastic License or the Server Side + * Public License, v 1. */ import { Storage } from '../../services/kibana_utils'; diff --git a/src/plugins/dashboard/public/application/listing/confirm_overlays.tsx b/src/plugins/dashboard/public/application/listing/confirm_overlays.tsx index a11e909559570..3539c08bc57f0 100644 --- a/src/plugins/dashboard/public/application/listing/confirm_overlays.tsx +++ b/src/plugins/dashboard/public/application/listing/confirm_overlays.tsx @@ -1,20 +1,9 @@ /* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * and the Server Side Public License, v 1; you may not use this file except in + * compliance with, at your election, the Elastic License or the Server Side + * Public License, v 1. */ import { @@ -56,7 +45,7 @@ export const confirmCreateWithUnsaved = ( const session = overlays.openModal( toMountPoint( session.close()}> - + {createConfirmStrings.getCreateTitle()} @@ -65,11 +54,15 @@ export const confirmCreateWithUnsaved = ( - session.close()}> + session.close()} + > {createConfirmStrings.getCancelButtonText()} { startBlankCallback(); session.close(); @@ -79,6 +72,7 @@ export const confirmCreateWithUnsaved = ( { contineCallback(); session.close(); diff --git a/src/plugins/dashboard/public/application/listing/dashboard_listing.tsx b/src/plugins/dashboard/public/application/listing/dashboard_listing.tsx index 994553e1de879..b8d369b9a93da 100644 --- a/src/plugins/dashboard/public/application/listing/dashboard_listing.tsx +++ b/src/plugins/dashboard/public/application/listing/dashboard_listing.tsx @@ -97,12 +97,24 @@ export const DashboardListing = ({ [savedObjectsTagging, redirectTo] ); + const createItem = useCallback(() => { + if (!dashboardPanelStorage.dashboardHasUnsavedEdits()) { + redirectTo({ destination: 'dashboard' }); + } else { + confirmCreateWithUnsaved( + core.overlays, + () => { + dashboardPanelStorage.clearPanels(); + redirectTo({ destination: 'dashboard' }); + }, + () => redirectTo({ destination: 'dashboard' }) + ); + } + }, [dashboardPanelStorage, redirectTo, core.overlays]); + const noItemsFragment = useMemo( - () => - getNoItemsMessage(hideWriteControls, core.application, () => - redirectTo({ destination: 'dashboard' }) - ), - [redirectTo, core.application, hideWriteControls] + () => getNoItemsMessage(hideWriteControls, core.application, createItem), + [createItem, core.application, hideWriteControls] ); const fetchItems = useCallback( @@ -137,21 +149,6 @@ export const DashboardListing = ({ [redirectTo] ); - const createItem = useCallback(() => { - if (!dashboardPanelStorage.dashboardHasUnsavedEdits()) { - redirectTo({ destination: 'dashboard' }); - } else { - confirmCreateWithUnsaved( - core.overlays, - () => { - dashboardPanelStorage.clearPanels(); - redirectTo({ destination: 'dashboard' }); - }, - () => redirectTo({ destination: 'dashboard' }) - ); - } - }, [dashboardPanelStorage, redirectTo, core.overlays]); - const searchFilters = useMemo(() => { return savedObjectsTagging ? [savedObjectsTagging.ui.getSearchBarFilter({ useName: true })] diff --git a/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.test.tsx b/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.test.tsx index 6c9201a6fa25c..7bf7b3ddcf34f 100644 --- a/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.test.tsx +++ b/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.test.tsx @@ -1,20 +1,9 @@ /* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * and the Server Side Public License, v 1; you may not use this file except in + * compliance with, at your election, the Elastic License or the Server Side + * Public License, v 1. */ import { I18nProvider } from '@kbn/i18n/react'; diff --git a/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.tsx b/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.tsx index f3526cf9d489a..05163406b22e0 100644 --- a/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.tsx +++ b/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.tsx @@ -1,20 +1,9 @@ /* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * and the Server Side Public License, v 1; you may not use this file except in + * compliance with, at your election, the Elastic License or the Server Side + * Public License, v 1. */ import { diff --git a/test/functional/apps/dashboard/dashboard_state.ts b/test/functional/apps/dashboard/dashboard_state.ts index 88823baf32a07..33bc0f2e7b169 100644 --- a/test/functional/apps/dashboard/dashboard_state.ts +++ b/test/functional/apps/dashboard/dashboard_state.ts @@ -206,98 +206,6 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { const newQuery = await queryBar.getQueryString(); expect(newQuery).to.equal('hi'); }); - - it('for panel size parameters', async function () { - await dashboardAddPanel.addVisualization(PIE_CHART_VIS_NAME); - const currentUrl = await browser.getCurrentUrl(); - const currentPanelDimensions = await PageObjects.dashboard.getPanelDimensions(); - const newUrl = currentUrl.replace( - `w:${DEFAULT_PANEL_WIDTH}`, - `w:${DEFAULT_PANEL_WIDTH * 2}` - ); - await browser.get(newUrl.toString(), false); - await retry.try(async () => { - const newPanelDimensions = await PageObjects.dashboard.getPanelDimensions(); - if (newPanelDimensions.length < 0) { - throw new Error('No panel dimensions...'); - } - - await PageObjects.dashboard.waitForRenderComplete(); - // Add a "margin" of error - because of page margins, it won't be a straight doubling of width. - const marginOfError = 10; - expect(newPanelDimensions[0].width).to.be.lessThan( - currentPanelDimensions[0].width * 2 + marginOfError - ); - expect(newPanelDimensions[0].width).to.be.greaterThan( - currentPanelDimensions[0].width * 2 - marginOfError - ); - }); - }); - - it('when removing a panel', async function () { - const currentUrl = await browser.getCurrentUrl(); - const newUrl = currentUrl.replace(/panels:\!\(.*\),query/, 'panels:!(),query'); - await browser.get(newUrl.toString(), false); - - await retry.try(async () => { - const newPanelCount = await PageObjects.dashboard.getPanelCount(); - expect(newPanelCount).to.be(0); - }); - }); - - describe('for embeddable config color parameters on a visualization', () => { - it('updates a pie slice color on a soft refresh', async function () { - await dashboardAddPanel.addVisualization(PIE_CHART_VIS_NAME); - await PageObjects.visChart.openLegendOptionColors( - '80,000', - `[data-title="${PIE_CHART_VIS_NAME}"]` - ); - await PageObjects.visChart.selectNewLegendColorChoice('#F9D9F9'); - const currentUrl = await browser.getCurrentUrl(); - const newUrl = currentUrl.replace('F9D9F9', 'FFFFFF'); - await browser.get(newUrl.toString(), false); - await PageObjects.header.waitUntilLoadingHasFinished(); - - await retry.try(async () => { - const allPieSlicesColor = await pieChart.getAllPieSliceStyles('80,000'); - let whitePieSliceCounts = 0; - allPieSlicesColor.forEach((style) => { - if (style.indexOf('rgb(255, 255, 255)') > 0) { - whitePieSliceCounts++; - } - }); - - expect(whitePieSliceCounts).to.be(1); - }); - }); - - it('and updates the pie slice legend color', async function () { - await retry.try(async () => { - const colorExists = await PageObjects.visChart.doesSelectedLegendColorExist('#FFFFFF'); - expect(colorExists).to.be(true); - }); - }); - - it('resets a pie slice color to the original when removed', async function () { - const currentUrl = await browser.getCurrentUrl(); - const newUrl = currentUrl.replace('vis:(colors:(%2780,000%27:%23FFFFFF))', ''); - await browser.get(newUrl.toString(), false); - await PageObjects.header.waitUntilLoadingHasFinished(); - - await retry.try(async () => { - const pieSliceStyle = await pieChart.getPieSliceStyle('80,000'); - // The default green color that was stored with the visualization before any dashboard overrides. - expect(pieSliceStyle.indexOf('rgb(87, 193, 123)')).to.be.greaterThan(0); - }); - }); - - it('resets the legend color as well', async function () { - await retry.try(async () => { - const colorExists = await PageObjects.visChart.doesSelectedLegendColorExist('#57c17b'); - expect(colorExists).to.be(true); - }); - }); - }); }); }); } diff --git a/test/functional/page_objects/dashboard_page.ts b/test/functional/page_objects/dashboard_page.ts index 8a31cc5ee3e63..2599882d6268a 100644 --- a/test/functional/page_objects/dashboard_page.ts +++ b/test/functional/page_objects/dashboard_page.ts @@ -221,8 +221,15 @@ export function DashboardPageProvider({ getService, getPageObjects }: FtrProvide await testSubjects.click('dashboardDiscardChanges'); } - public async clickNewDashboard() { + public async clickNewDashboard(continueEditing = false) { await listingTable.clickNewButton('createDashboardPromptButton'); + if (await testSubjects.exists('dashboardCreateConfirm')) { + if (continueEditing) { + await testSubjects.click('dashboardCreateConfirmContinue'); + } else { + await testSubjects.click('dashboardCreateConfirmStartOver'); + } + } // make sure the dashboard page is shown await this.waitForRenderComplete(); } From 0465fe3b2db8d9bd8d4a2a870d866ff12825ff6b Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Thu, 21 Jan 2021 13:21:40 -0500 Subject: [PATCH 18/30] worked around functional tests clearing session storage after each test suite --- .../application/dashboard_state_manager.ts | 5 -- .../apps/dashboard/dashboard_filtering.ts | 50 +++++++++++++------ 2 files changed, 35 insertions(+), 20 deletions(-) diff --git a/src/plugins/dashboard/public/application/dashboard_state_manager.ts b/src/plugins/dashboard/public/application/dashboard_state_manager.ts index 2ff3c99a251ca..6eb80f888c23b 100644 --- a/src/plugins/dashboard/public/application/dashboard_state_manager.ts +++ b/src/plugins/dashboard/public/application/dashboard_state_manager.ts @@ -174,11 +174,6 @@ export class DashboardStateManager { this.stateContainerChangeSub = this.stateContainer.state$.subscribe(() => { this.isDirty = this.checkIsDirty(); - - // Session storage keys seems to disappear in some functional tests. This will temporarily prevent that. - if (this.appState.panels.length && !this.getUnsavedPanelState().panels?.length) { - this.setUnsavedPanels(this.appState.panels); - } this.changeListeners.forEach((listener) => listener({ dirty: this.isDirty })); }); diff --git a/test/functional/apps/dashboard/dashboard_filtering.ts b/test/functional/apps/dashboard/dashboard_filtering.ts index 1dd84460314b9..cef3b11ab2cb0 100644 --- a/test/functional/apps/dashboard/dashboard_filtering.ts +++ b/test/functional/apps/dashboard/dashboard_filtering.ts @@ -31,6 +31,27 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { describe('dashboard filtering', function () { this.tags('includeFirefox'); + const populateDashboard = async () => { + await PageObjects.dashboard.clickNewDashboard(); + await PageObjects.timePicker.setDefaultDataRange(); + await dashboardAddPanel.addEveryVisualization('"Filter Bytes Test"'); + await dashboardAddPanel.addEverySavedSearch('"Filter Bytes Test"'); + + await dashboardAddPanel.closeAddPanel(); + }; + + const addFilterAndRefresh = async () => { + await PageObjects.header.waitUntilLoadingHasFinished(); + await PageObjects.dashboard.waitForRenderComplete(); + await filterBar.addFilter('bytes', 'is', '12345678'); + await PageObjects.header.waitUntilLoadingHasFinished(); + await PageObjects.dashboard.waitForRenderComplete(); + // first round of requests sometimes times out, refresh all visualizations to fetch again + await queryBar.clickQuerySubmitButton(); + await PageObjects.header.waitUntilLoadingHasFinished(); + await PageObjects.dashboard.waitForRenderComplete(); + }; + before(async () => { await esArchiver.load('dashboard/current/kibana'); await security.testUser.setRoles(['kibana_admin', 'test_logstash_reader', 'animals']); @@ -48,22 +69,12 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { describe('adding a filter that excludes all data', () => { before(async () => { - await PageObjects.dashboard.clickNewDashboard(); - await PageObjects.timePicker.setDefaultDataRange(); - await dashboardAddPanel.addEveryVisualization('"Filter Bytes Test"'); - await dashboardAddPanel.addEverySavedSearch('"Filter Bytes Test"'); - - await dashboardAddPanel.closeAddPanel(); + await populateDashboard(); + await addFilterAndRefresh(); + }); - await PageObjects.header.waitUntilLoadingHasFinished(); - await PageObjects.dashboard.waitForRenderComplete(); - await filterBar.addFilter('bytes', 'is', '12345678'); - await PageObjects.header.waitUntilLoadingHasFinished(); - await PageObjects.dashboard.waitForRenderComplete(); - // first round of requests sometimes times out, refresh all visualizations to fetch again - await queryBar.clickQuerySubmitButton(); - await PageObjects.header.waitUntilLoadingHasFinished(); - await PageObjects.dashboard.waitForRenderComplete(); + after(async () => { + await PageObjects.dashboard.gotoDashboardLandingPage(); }); it('filters on pie charts', async () => { @@ -118,6 +129,10 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { describe('using a pinned filter that excludes all data', () => { before(async () => { + // Functional tests clear session storage after each suite, so it is important to repopulate unsaved panels + await populateDashboard(); + await addFilterAndRefresh(); + await filterBar.toggleFilterPinned('bytes'); await PageObjects.header.waitUntilLoadingHasFinished(); await PageObjects.dashboard.waitForRenderComplete(); @@ -125,6 +140,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { after(async () => { await filterBar.toggleFilterPinned('bytes'); + await PageObjects.dashboard.gotoDashboardLandingPage(); }); it('filters on pie charts', async () => { @@ -175,6 +191,10 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { describe('disabling a filter unfilters the data on', function () { before(async () => { + // Functional tests clear session storage after each suite, so it is important to repopulate unsaved panels + await populateDashboard(); + await addFilterAndRefresh(); + await filterBar.toggleFilterEnabled('bytes'); await PageObjects.header.waitUntilLoadingHasFinished(); await PageObjects.dashboard.waitForRenderComplete(); From dafb75b4542bc0d97106b7c75a843dcfc2a4aac2 Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Thu, 21 Jan 2021 16:18:44 -0500 Subject: [PATCH 19/30] reimplemented functional tests which manually edit the URL by using the share feature --- .../apps/dashboard/dashboard_state.ts | 101 ++++++++++++++++++ 1 file changed, 101 insertions(+) diff --git a/test/functional/apps/dashboard/dashboard_state.ts b/test/functional/apps/dashboard/dashboard_state.ts index 33bc0f2e7b169..28a3ad41fa62d 100644 --- a/test/functional/apps/dashboard/dashboard_state.ts +++ b/test/functional/apps/dashboard/dashboard_state.ts @@ -20,6 +20,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { 'discover', 'tileMap', 'visChart', + 'share', 'timePicker', ]); const testSubjects = getService('testSubjects'); @@ -191,6 +192,13 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { expect(changedTileMapData.length).to.not.equal(tileMapData.length); }); + const getUrlFromShare = async () => { + await PageObjects.share.clickShareTopNavButton(); + const sharedUrl = await PageObjects.share.getSharedUrl(); + await PageObjects.share.clickShareTopNavButton(); + return sharedUrl; + }; + describe('Directly modifying url updates dashboard state', () => { it('for query parameter', async function () { await PageObjects.dashboard.gotoDashboardLandingPage(); @@ -206,6 +214,99 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { const newQuery = await queryBar.getQueryString(); expect(newQuery).to.equal('hi'); }); + + it('for panel size parameters', async function () { + await dashboardAddPanel.addVisualization(PIE_CHART_VIS_NAME); + const currentUrl = await getUrlFromShare(); + const currentPanelDimensions = await PageObjects.dashboard.getPanelDimensions(); + const newUrl = currentUrl.replace( + `w:${DEFAULT_PANEL_WIDTH}`, + `w:${DEFAULT_PANEL_WIDTH * 2}` + ); + await browser.get(newUrl.toString(), false); + await retry.try(async () => { + const newPanelDimensions = await PageObjects.dashboard.getPanelDimensions(); + if (newPanelDimensions.length < 0) { + throw new Error('No panel dimensions...'); + } + + await PageObjects.dashboard.waitForRenderComplete(); + // Add a "margin" of error - because of page margins, it won't be a straight doubling of width. + const marginOfError = 10; + expect(newPanelDimensions[0].width).to.be.lessThan( + currentPanelDimensions[0].width * 2 + marginOfError + ); + expect(newPanelDimensions[0].width).to.be.greaterThan( + currentPanelDimensions[0].width * 2 - marginOfError + ); + }); + }); + + it('when removing a panel', async function () { + await PageObjects.dashboard.waitForRenderComplete(); + const currentUrl = await getUrlFromShare(); + const newUrl = currentUrl.replace(/panels:\!\(.*\),query/, 'panels:!(),query'); + await browser.get(newUrl.toString(), false); + + await retry.try(async () => { + const newPanelCount = await PageObjects.dashboard.getPanelCount(); + expect(newPanelCount).to.be(0); + }); + }); + + describe('for embeddable config color parameters on a visualization', () => { + it('updates a pie slice color on a soft refresh', async function () { + await dashboardAddPanel.addVisualization(PIE_CHART_VIS_NAME); + await PageObjects.visChart.openLegendOptionColors( + '80,000', + `[data-title="${PIE_CHART_VIS_NAME}"]` + ); + await PageObjects.visChart.selectNewLegendColorChoice('#F9D9F9'); + const currentUrl = await getUrlFromShare(); + const newUrl = currentUrl.replace('F9D9F9', 'FFFFFF'); + await browser.get(newUrl.toString(), false); + await PageObjects.header.waitUntilLoadingHasFinished(); + + await retry.try(async () => { + const allPieSlicesColor = await pieChart.getAllPieSliceStyles('80,000'); + let whitePieSliceCounts = 0; + allPieSlicesColor.forEach((style) => { + if (style.indexOf('rgb(255, 255, 255)') > 0) { + whitePieSliceCounts++; + } + }); + + expect(whitePieSliceCounts).to.be(1); + }); + }); + + it('and updates the pie slice legend color', async function () { + await retry.try(async () => { + const colorExists = await PageObjects.visChart.doesSelectedLegendColorExist('#FFFFFF'); + expect(colorExists).to.be(true); + }); + }); + + it('resets a pie slice color to the original when removed', async function () { + const currentUrl = await getUrlFromShare(); + const newUrl = currentUrl.replace('vis:(colors:(%2780,000%27:%23FFFFFF))', ''); + await browser.get(newUrl.toString(), false); + await PageObjects.header.waitUntilLoadingHasFinished(); + + await retry.try(async () => { + const pieSliceStyle = await pieChart.getPieSliceStyle('80,000'); + // The default green color that was stored with the visualization before any dashboard overrides. + expect(pieSliceStyle.indexOf('rgb(87, 193, 123)')).to.be.greaterThan(0); + }); + }); + + it('resets the legend color as well', async function () { + await retry.try(async () => { + const colorExists = await PageObjects.visChart.doesSelectedLegendColorExist('#57c17b'); + expect(colorExists).to.be(true); + }); + }); + }); }); }); } From be357af51c068a00e35ceb6bfdbcf6bad01ae042 Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Thu, 21 Jan 2021 16:34:52 -0500 Subject: [PATCH 20/30] manually create new dashboard in panel_context_menu test --- test/functional/apps/dashboard/panel_context_menu.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/functional/apps/dashboard/panel_context_menu.ts b/test/functional/apps/dashboard/panel_context_menu.ts index 17072731555bd..e344d9421894e 100644 --- a/test/functional/apps/dashboard/panel_context_menu.ts +++ b/test/functional/apps/dashboard/panel_context_menu.ts @@ -105,6 +105,10 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await PageObjects.header.waitUntilLoadingHasFinished(); await PageObjects.header.clickDashboard(); + // The following tests require a fresh dashboard. + await PageObjects.dashboard.gotoDashboardLandingPage(); + await PageObjects.dashboard.clickNewDashboard(); + const inViewMode = await PageObjects.dashboard.getIsInViewMode(); if (inViewMode) await PageObjects.dashboard.switchToEditMode(); await dashboardAddPanel.addSavedSearch(searchName); From 634a8cc1fb7013121f11558e901ef9e2e1ffc4e2 Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Thu, 21 Jan 2021 16:42:18 -0500 Subject: [PATCH 21/30] revert cancel button name, remove panels retained toast --- .../application/top_nav/dashboard_top_nav.tsx | 14 +------------- .../application/top_nav/get_top_nav_config.ts | 8 ++++---- src/plugins/dashboard/public/dashboard_strings.ts | 14 -------------- .../plugins/translations/translations/ja-JP.json | 1 + .../plugins/translations/translations/zh-CN.json | 1 + 5 files changed, 7 insertions(+), 31 deletions(-) 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 6d6a99e86bb14..47aae123cc0c2 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 @@ -45,7 +45,7 @@ import { ShowShareModal } from './show_share_modal'; import { PanelToolbar } from './panel_toolbar'; import { confirmDiscardUnsavedChanges } from '../listing/confirm_overlays'; import { OverlayRef } from '../../../../../core/public'; -import { getNewDashboardTitle, unsavedChangesRetainedStrings } from '../../dashboard_strings'; +import { getNewDashboardTitle } from '../../dashboard_strings'; import { DASHBOARD_PANELS_UNSAVED_ID } from '../lib/dashboard_panel_storage'; import { DashboardContainer } from '..'; @@ -176,17 +176,6 @@ export function DashboardTopNav({ const { getFullEditPath, title, id } = savedDashboard; chrome.recentlyAccessed.add(getFullEditPath(newMode === ViewMode.EDIT), title, id); } - if ( - newMode === ViewMode.VIEW && - dashboardStateManager.getIsEditMode() && - dashboardStateManager.isDirty - ) { - core.notifications.toasts.addSuccess({ - title: unsavedChangesRetainedStrings.getTitle(), - text: unsavedChangesRetainedStrings.getText(), - }); - } - dashboardStateManager.switchViewMode(newMode); dashboardStateManager.restorePanels(); }, @@ -196,7 +185,6 @@ export function DashboardTopNav({ dashboardStateManager, allowByValueEmbeddables, chrome.recentlyAccessed, - core.notifications.toasts, ] ); 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 2d22232b893f6..2bbccdccd2eac 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 @@ -108,12 +108,12 @@ function getSaveConfig(action: NavAction) { */ function getViewConfig(action: NavAction) { return { - id: 'view', - label: i18n.translate('dashboard.topNave.viewButtonAriaLabel', { - defaultMessage: 'view', + id: 'cancel', + label: i18n.translate('dashboard.topNave.cancelButtonAriaLabel', { + defaultMessage: 'cancel', }), description: i18n.translate('dashboard.topNave.viewConfigDescription', { - defaultMessage: 'Return to view mode', + defaultMessage: 'Switch to view-only mode', }), testId: 'dashboardViewOnlyMode', run: action, diff --git a/src/plugins/dashboard/public/dashboard_strings.ts b/src/plugins/dashboard/public/dashboard_strings.ts index 99fa40b33c695..2a603673dc7f1 100644 --- a/src/plugins/dashboard/public/dashboard_strings.ts +++ b/src/plugins/dashboard/public/dashboard_strings.ts @@ -306,20 +306,6 @@ export const emptyScreenStrings = { }), }; -/* - Toasts -*/ -export const unsavedChangesRetainedStrings = { - getTitle: () => - i18n.translate('dashboard.changesRetainedToast.title', { - defaultMessage: 'Your changes are still here!', - }), - getText: () => - i18n.translate('dashboard.changesRetainedToast.text', { - defaultMessage: 'Return to edit mode to continue from where you left off.', - }), -}; - /* Dashboard Listing Page */ diff --git a/x-pack/plugins/translations/translations/ja-JP.json b/x-pack/plugins/translations/translations/ja-JP.json index eda6a2afaa825..6dd72d179b210 100644 --- a/x-pack/plugins/translations/translations/ja-JP.json +++ b/x-pack/plugins/translations/translations/ja-JP.json @@ -668,6 +668,7 @@ "dashboard.topNave.addConfigDescription": "既存のビジュアライゼーションをダッシュボードに追加", "dashboard.topNave.addNewButtonAriaLabel": "パネルの作成", "dashboard.topNave.addNewConfigDescription": "このダッシュボードに新規パネルを作成", + "dashboard.topNave.cancelButtonAriaLabel": "キャンセル", "dashboard.topNave.cloneButtonAriaLabel": "クローンを作成", "dashboard.topNave.cloneConfigDescription": "ダッシュボードのコピーを作成します", "dashboard.topNave.editButtonAriaLabel": "編集", diff --git a/x-pack/plugins/translations/translations/zh-CN.json b/x-pack/plugins/translations/translations/zh-CN.json index 02901686c0fb3..abc2867a8824a 100644 --- a/x-pack/plugins/translations/translations/zh-CN.json +++ b/x-pack/plugins/translations/translations/zh-CN.json @@ -666,6 +666,7 @@ "dashboard.topNav.showCloneModal.dashboardCopyTitle": "{title} 副本", "dashboard.topNave.addButtonAriaLabel": "库", "dashboard.topNave.addConfigDescription": "将现有可视化添加到仪表板", + "dashboard.topNave.cancelButtonAriaLabel": "取消", "dashboard.topNave.addNewButtonAriaLabel": "创建面板", "dashboard.topNave.addNewConfigDescription": "在此仪表板上创建新的面板", "dashboard.topNave.cloneButtonAriaLabel": "克隆", From 930087ce3f97c04454ef0bcf527775d5414dfef4 Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Mon, 25 Jan 2021 16:05:42 -0500 Subject: [PATCH 22/30] functional test fixes --- test/functional/apps/dashboard/dashboard_state.ts | 2 +- test/functional/apps/dashboard/panel_context_menu.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/functional/apps/dashboard/dashboard_state.ts b/test/functional/apps/dashboard/dashboard_state.ts index 28a3ad41fa62d..7a0b9fdc98f36 100644 --- a/test/functional/apps/dashboard/dashboard_state.ts +++ b/test/functional/apps/dashboard/dashboard_state.ts @@ -128,8 +128,8 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { }); it('Saved search with column changes will not update when the saved object changes', async () => { - await PageObjects.discover.removeHeaderColumn('bytes'); await PageObjects.dashboard.switchToEditMode(); + await PageObjects.discover.removeHeaderColumn('bytes'); await PageObjects.dashboard.saveDashboard('Has local edits'); await PageObjects.header.clickDiscover(); diff --git a/test/functional/apps/dashboard/panel_context_menu.ts b/test/functional/apps/dashboard/panel_context_menu.ts index e344d9421894e..0f1577ca49188 100644 --- a/test/functional/apps/dashboard/panel_context_menu.ts +++ b/test/functional/apps/dashboard/panel_context_menu.ts @@ -144,7 +144,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { before('and add one panel and save to put dashboard in "view" mode', async () => { await dashboardAddPanel.addVisualization(PIE_CHART_VIS_NAME); - await PageObjects.dashboard.saveDashboard(dashboardName); + await PageObjects.dashboard.saveDashboard(dashboardName + '2'); }); before('expand panel to "full screen"', async () => { From 3231d79d38e67759c64e430e862559b32e61572b Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Tue, 26 Jan 2021 15:32:12 -0500 Subject: [PATCH 23/30] restored default color --- .../public/application/dashboard_state_manager.ts | 4 ++-- test/functional/apps/dashboard/dashboard_state.ts | 4 ++-- .../apps/dashboard/edit_embeddable_redirects.ts | 13 +++++++++++++ 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/plugins/dashboard/public/application/dashboard_state_manager.ts b/src/plugins/dashboard/public/application/dashboard_state_manager.ts index 6eb80f888c23b..4e8bbce85a0d9 100644 --- a/src/plugins/dashboard/public/application/dashboard_state_manager.ts +++ b/src/plugins/dashboard/public/application/dashboard_state_manager.ts @@ -284,13 +284,13 @@ export class DashboardStateManager { this.stateContainer.transitions.set('panels', Object.values(convertedPanelStateMap)); if (dirtyBecauseOfInitialStateMigration) { this.saveState({ replace: true }); - } else { - this.setUnsavedPanels(this.getPanels()); } // If a panel has been changed, and the state is now equal to the state in the saved object, remove the unsaved panels if (!this.isDirty && this.getIsEditMode()) { this.clearUnsavedPanels(); + } else { + this.setUnsavedPanels(this.getPanels()); } } diff --git a/test/functional/apps/dashboard/dashboard_state.ts b/test/functional/apps/dashboard/dashboard_state.ts index 7a0b9fdc98f36..3c9b96d7277ef 100644 --- a/test/functional/apps/dashboard/dashboard_state.ts +++ b/test/functional/apps/dashboard/dashboard_state.ts @@ -289,12 +289,12 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { it('resets a pie slice color to the original when removed', async function () { const currentUrl = await getUrlFromShare(); - const newUrl = currentUrl.replace('vis:(colors:(%2780,000%27:%23FFFFFF))', ''); + const newUrl = currentUrl.replace(`vis:(colors:('80,000':%23FFFFFF))`, ''); await browser.get(newUrl.toString(), false); await PageObjects.header.waitUntilLoadingHasFinished(); await retry.try(async () => { - const pieSliceStyle = await pieChart.getPieSliceStyle('80,000'); + const pieSliceStyle = await pieChart.getPieSliceStyle(`80,000`); // The default green color that was stored with the visualization before any dashboard overrides. expect(pieSliceStyle.indexOf('rgb(87, 193, 123)')).to.be.greaterThan(0); }); diff --git a/test/functional/apps/dashboard/edit_embeddable_redirects.ts b/test/functional/apps/dashboard/edit_embeddable_redirects.ts index c1a2819e407b7..dfe7ec730aa10 100644 --- a/test/functional/apps/dashboard/edit_embeddable_redirects.ts +++ b/test/functional/apps/dashboard/edit_embeddable_redirects.ts @@ -70,6 +70,19 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { expect(titles.indexOf(newTitle)).to.not.be(-1); }); + it('retains unsaved panel count after navigating to listing page and back', async () => { + const originalPanelCount = await PageObjects.dashboard.getPanelCount(); + await PageObjects.header.waitUntilLoadingHasFinished(); + await PageObjects.dashboard.gotoDashboardLandingPage(); + // await PageObjects.visualize.gotoVisualizationLandingPage(); + await PageObjects.header.waitUntilLoadingHasFinished(); + // await PageObjects.common.navigateToApp('dashboards'); + await PageObjects.dashboard.loadSavedDashboard('few panels'); + await PageObjects.dashboard.switchToEditMode(); + const newPanelCount = await PageObjects.dashboard.getPanelCount(); + expect(newPanelCount).to.eql(originalPanelCount); + }); + it('loses originatingApp connection after save as when redirectToOrigin is false', async () => { const newTitle = 'wowee, my title just got cooler again'; await PageObjects.header.waitUntilLoadingHasFinished(); From 2aab904c4d9d8612407c9fbba34248da4a4f25b5 Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Tue, 26 Jan 2021 21:31:27 -0500 Subject: [PATCH 24/30] Added functional tests --- .../listing/dashboard_unsaved_listing.tsx | 4 +- .../dashboard/dashboard_unsaved_listing.ts | 160 ++++++++++++++++++ .../apps/dashboard/dashboard_unsaved_state.ts | 86 ++++++++++ .../dashboard/edit_embeddable_redirects.ts | 13 -- test/functional/apps/dashboard/index.ts | 2 + .../functional/page_objects/dashboard_page.ts | 39 +++++ .../services/dashboard/panel_actions.ts | 10 +- 7 files changed, 293 insertions(+), 21 deletions(-) create mode 100644 test/functional/apps/dashboard/dashboard_unsaved_listing.ts create mode 100644 test/functional/apps/dashboard/dashboard_unsaved_state.ts diff --git a/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.tsx b/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.tsx index 05163406b22e0..e8f7b3f241975 100644 --- a/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.tsx +++ b/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.tsx @@ -59,7 +59,7 @@ const DashboardUnsavedItem = ({ size="s" color="primary" onClick={onOpenClick} - data-test-subj={`edit-unsaved-${dashboard?.id ?? DASHBOARD_PANELS_UNSAVED_ID}`} + data-test-subj={`edit-unsaved-${title.split(' ').join('-')}`} aria-label={dashboardUnsavedListingStrings.getEditAriaLabel(title)} > {dashboardUnsavedListingStrings.getEditTitle()} @@ -70,7 +70,7 @@ const DashboardUnsavedItem = ({ size="s" color="danger" onClick={onDiscardClick} - data-test-subj={`discard-unsaved-${dashboard?.id ?? DASHBOARD_PANELS_UNSAVED_ID}`} + data-test-subj={`discard-unsaved-${title.split(' ').join('-')}`} aria-label={dashboardUnsavedListingStrings.getDiscardAriaLabel(title)} > {dashboardUnsavedListingStrings.getDiscardTitle()} diff --git a/test/functional/apps/dashboard/dashboard_unsaved_listing.ts b/test/functional/apps/dashboard/dashboard_unsaved_listing.ts new file mode 100644 index 0000000000000..19e532f0af213 --- /dev/null +++ b/test/functional/apps/dashboard/dashboard_unsaved_listing.ts @@ -0,0 +1,160 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * and the Server Side Public License, v 1; you may not use this file except in + * compliance with, at your election, the Elastic License 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 PageObjects = getPageObjects(['dashboard', 'header', 'visualize', 'settings', 'common']); + const esArchiver = getService('esArchiver'); + const kibanaServer = getService('kibanaServer'); + const dashboardAddPanel = getService('dashboardAddPanel'); + const dashboardPanelActions = getService('dashboardPanelActions'); + + let existingDashboardPanelCount = 0; + const dashboardTitle = 'few panels'; + const unsavedDashboardTitle = 'New Dashboard'; + const newDashboartTitle = 'A Wild Dashboard'; + + describe('dashboard unsaved listing', () => { + const addSomePanels = async () => { + // add an area chart by value + await dashboardAddPanel.clickCreateNewLink(); + await PageObjects.visualize.clickAggBasedVisualizations(); + await PageObjects.visualize.clickAreaChart(); + await PageObjects.visualize.clickNewSearch(); + await PageObjects.visualize.saveVisualizationAndReturn(); + + // add a metric by reference + await dashboardAddPanel.addVisualization('Rendering-Test: metric'); + }; + + before(async () => { + await esArchiver.load('dashboard/current/kibana'); + await kibanaServer.uiSettings.replace({ + defaultIndex: '0bf35f60-3dc9-11e8-8660-4d65aa086b3c', + }); + await PageObjects.common.navigateToApp('dashboard'); + await PageObjects.dashboard.preserveCrossAppState(); + }); + + it('lists unsaved changes to existing dashboards', async () => { + await PageObjects.dashboard.loadSavedDashboard(dashboardTitle); + await PageObjects.dashboard.switchToEditMode(); + await addSomePanels(); + existingDashboardPanelCount = await PageObjects.dashboard.getPanelCount(); + await PageObjects.dashboard.gotoDashboardLandingPage(); + await PageObjects.header.waitUntilLoadingHasFinished(); + await PageObjects.dashboard.expectUnsavedChangesListingExists(dashboardTitle); + }); + + it('restores unsaved changes to existing dashboards', async () => { + await PageObjects.dashboard.clickUnsavedChangesContinueEditing(dashboardTitle); + await PageObjects.header.waitUntilLoadingHasFinished(); + const currentPanelCount = await PageObjects.dashboard.getPanelCount(); + expect(currentPanelCount).to.eql(existingDashboardPanelCount); + await PageObjects.dashboard.gotoDashboardLandingPage(); + await PageObjects.header.waitUntilLoadingHasFinished(); + }); + + it('lists unsaved changes to new dashboards', async () => { + await PageObjects.dashboard.clickNewDashboard(); + await addSomePanels(); + await PageObjects.dashboard.gotoDashboardLandingPage(); + await PageObjects.header.waitUntilLoadingHasFinished(); + await PageObjects.dashboard.expectUnsavedChangesListingExists(unsavedDashboardTitle); + }); + + it('restores unsaved changes to new dashboards', async () => { + await PageObjects.dashboard.clickUnsavedChangesContinueEditing(unsavedDashboardTitle); + await PageObjects.header.waitUntilLoadingHasFinished(); + expect(await PageObjects.dashboard.getPanelCount()).to.eql(2); + await PageObjects.dashboard.gotoDashboardLandingPage(); + await PageObjects.header.waitUntilLoadingHasFinished(); + }); + + it('shows a warning on create new, and restores panels if continue is selected', async () => { + await PageObjects.dashboard.clickNewDashboardExpectWarning(true); + await PageObjects.header.waitUntilLoadingHasFinished(); + expect(await PageObjects.dashboard.getPanelCount()).to.eql(2); + await PageObjects.dashboard.gotoDashboardLandingPage(); + await PageObjects.header.waitUntilLoadingHasFinished(); + }); + + it('shows a warning on create new, and clears unsaved panels if discard is selected', async () => { + await PageObjects.dashboard.clickNewDashboardExpectWarning(); + await PageObjects.header.waitUntilLoadingHasFinished(); + expect(await PageObjects.dashboard.getPanelCount()).to.eql(0); + await PageObjects.dashboard.gotoDashboardLandingPage(); + await PageObjects.header.waitUntilLoadingHasFinished(); + }); + + it('does not show unsaved changes on new dashboard when no panels have been added', async () => { + await PageObjects.dashboard.expectUnsavedChangesDoesNotExist(unsavedDashboardTitle); + }); + + it('can discard unsaved changes using the discard link', async () => { + await PageObjects.dashboard.clickUnsavedChangesDiscard(dashboardTitle); + await PageObjects.dashboard.expectUnsavedChangesDoesNotExist(dashboardTitle); + await PageObjects.dashboard.loadSavedDashboard(dashboardTitle); + await PageObjects.dashboard.switchToEditMode(); + const currentPanelCount = await PageObjects.dashboard.getPanelCount(); + expect(currentPanelCount).to.eql(existingDashboardPanelCount - 2); + await PageObjects.dashboard.gotoDashboardLandingPage(); + await PageObjects.header.waitUntilLoadingHasFinished(); + }); + + it('loses unsaved changes to new dashboard upon saving', async () => { + await PageObjects.dashboard.clickNewDashboard(); + await addSomePanels(); + + // ensure that the unsaved listing exists first + await PageObjects.dashboard.gotoDashboardLandingPage(); + await PageObjects.header.waitUntilLoadingHasFinished(); + await PageObjects.dashboard.clickUnsavedChangesContinueEditing(unsavedDashboardTitle); + await PageObjects.header.waitUntilLoadingHasFinished(); + + // Save the dashboard, and check that it now does not exist + await PageObjects.dashboard.saveDashboard(newDashboartTitle); + await PageObjects.header.waitUntilLoadingHasFinished(); + await PageObjects.dashboard.gotoDashboardLandingPage(); + await PageObjects.dashboard.expectUnsavedChangesDoesNotExist(unsavedDashboardTitle); + }); + + it('does not list unsaved changes when unsaved version of the dashboard is the same', async () => { + await PageObjects.dashboard.loadSavedDashboard(newDashboartTitle); + await PageObjects.dashboard.switchToEditMode(); + + // add another panel so we can delete it later + await dashboardAddPanel.clickCreateNewLink(); + await PageObjects.visualize.clickAggBasedVisualizations(); + await PageObjects.visualize.clickAreaChart(); + await PageObjects.visualize.clickNewSearch(); + await PageObjects.visualize.saveVisualizationExpectSuccess('Wildvis', { + redirectToOrigin: true, + }); + + // ensure that the unsaved listing exists + await PageObjects.dashboard.gotoDashboardLandingPage(); + await PageObjects.header.waitUntilLoadingHasFinished(); + await PageObjects.dashboard.expectUnsavedChangesListingExists(newDashboartTitle); + await PageObjects.dashboard.clickUnsavedChangesContinueEditing(newDashboartTitle); + await PageObjects.header.waitUntilLoadingHasFinished(); + + // Remove the panel that was just added + await dashboardPanelActions.removePanelByTitle('Wildvis'); + await PageObjects.header.waitUntilLoadingHasFinished(); + + // Check that it now does not exist + await PageObjects.dashboard.gotoDashboardLandingPage(); + await PageObjects.header.waitUntilLoadingHasFinished(); + await PageObjects.dashboard.expectUnsavedChangesDoesNotExist(newDashboartTitle); + }); + }); +} diff --git a/test/functional/apps/dashboard/dashboard_unsaved_state.ts b/test/functional/apps/dashboard/dashboard_unsaved_state.ts new file mode 100644 index 0000000000000..21ff5be925db5 --- /dev/null +++ b/test/functional/apps/dashboard/dashboard_unsaved_state.ts @@ -0,0 +1,86 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * and the Server Side Public License, v 1; you may not use this file except in + * compliance with, at your election, the Elastic License 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 PageObjects = getPageObjects(['dashboard', 'header', 'visualize', 'settings', 'common']); + const esArchiver = getService('esArchiver'); + const kibanaServer = getService('kibanaServer'); + const dashboardAddPanel = getService('dashboardAddPanel'); + + let originalPanelCount = 0; + let unsavedPanelCount = 0; + + describe('dashboard unsaved panels', () => { + before(async () => { + await esArchiver.load('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(); + + originalPanelCount = await PageObjects.dashboard.getPanelCount(); + + // add an area chart by value + await dashboardAddPanel.clickCreateNewLink(); + await PageObjects.visualize.clickAggBasedVisualizations(); + await PageObjects.visualize.clickAreaChart(); + await PageObjects.visualize.clickNewSearch(); + await PageObjects.visualize.saveVisualizationAndReturn(); + + // add a metric by reference + await dashboardAddPanel.addVisualization('Rendering-Test: metric'); + }); + + it('has correct number of panels', async () => { + unsavedPanelCount = await PageObjects.dashboard.getPanelCount(); + expect(unsavedPanelCount).to.eql(originalPanelCount + 2); + }); + + it('retains unsaved panel count after navigating to listing page and back', async () => { + await PageObjects.header.waitUntilLoadingHasFinished(); + await PageObjects.dashboard.gotoDashboardLandingPage(); + await PageObjects.header.waitUntilLoadingHasFinished(); + await PageObjects.dashboard.loadSavedDashboard('few panels'); + await PageObjects.dashboard.switchToEditMode(); + const currentPanelCount = await PageObjects.dashboard.getPanelCount(); + expect(currentPanelCount).to.eql(unsavedPanelCount); + }); + + it('retains unsaved panel count after navigating to another app and back', async () => { + await PageObjects.header.waitUntilLoadingHasFinished(); + await PageObjects.visualize.gotoVisualizationLandingPage(); + await PageObjects.header.waitUntilLoadingHasFinished(); + await PageObjects.common.navigateToApp('dashboards'); + await PageObjects.dashboard.loadSavedDashboard('few panels'); + await PageObjects.dashboard.switchToEditMode(); + const currentPanelCount = await PageObjects.dashboard.getPanelCount(); + expect(currentPanelCount).to.eql(unsavedPanelCount); + }); + + it('resets to original panel count upon entering view mode', async () => { + await PageObjects.header.waitUntilLoadingHasFinished(); + await PageObjects.dashboard.clickCancelOutOfEditMode(); + const currentPanelCount = await PageObjects.dashboard.getPanelCount(); + expect(currentPanelCount).to.eql(originalPanelCount); + }); + + it('retains unsaved panel count after returning to edit mode', async () => { + await PageObjects.header.waitUntilLoadingHasFinished(); + await PageObjects.dashboard.switchToEditMode(); + const currentPanelCount = await PageObjects.dashboard.getPanelCount(); + expect(currentPanelCount).to.eql(unsavedPanelCount); + }); + }); +} diff --git a/test/functional/apps/dashboard/edit_embeddable_redirects.ts b/test/functional/apps/dashboard/edit_embeddable_redirects.ts index dfe7ec730aa10..c1a2819e407b7 100644 --- a/test/functional/apps/dashboard/edit_embeddable_redirects.ts +++ b/test/functional/apps/dashboard/edit_embeddable_redirects.ts @@ -70,19 +70,6 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { expect(titles.indexOf(newTitle)).to.not.be(-1); }); - it('retains unsaved panel count after navigating to listing page and back', async () => { - const originalPanelCount = await PageObjects.dashboard.getPanelCount(); - await PageObjects.header.waitUntilLoadingHasFinished(); - await PageObjects.dashboard.gotoDashboardLandingPage(); - // await PageObjects.visualize.gotoVisualizationLandingPage(); - await PageObjects.header.waitUntilLoadingHasFinished(); - // await PageObjects.common.navigateToApp('dashboards'); - await PageObjects.dashboard.loadSavedDashboard('few panels'); - await PageObjects.dashboard.switchToEditMode(); - const newPanelCount = await PageObjects.dashboard.getPanelCount(); - expect(newPanelCount).to.eql(originalPanelCount); - }); - it('loses originatingApp connection after save as when redirectToOrigin is false', async () => { const newTitle = 'wowee, my title just got cooler again'; await PageObjects.header.waitUntilLoadingHasFinished(); diff --git a/test/functional/apps/dashboard/index.ts b/test/functional/apps/dashboard/index.ts index 86b12da791752..29f4180019b11 100644 --- a/test/functional/apps/dashboard/index.ts +++ b/test/functional/apps/dashboard/index.ts @@ -46,6 +46,8 @@ export default function ({ getService, loadTestFile }: FtrProviderContext) { loadTestFile(require.resolve('./embeddable_data_grid')); loadTestFile(require.resolve('./create_and_add_embeddables')); loadTestFile(require.resolve('./edit_embeddable_redirects')); + loadTestFile(require.resolve('./dashboard_unsaved_state')); + loadTestFile(require.resolve('./dashboard_unsaved_listing')); loadTestFile(require.resolve('./edit_visualizations')); loadTestFile(require.resolve('./time_zones')); loadTestFile(require.resolve('./dashboard_options')); diff --git a/test/functional/page_objects/dashboard_page.ts b/test/functional/page_objects/dashboard_page.ts index 2599882d6268a..c115100653729 100644 --- a/test/functional/page_objects/dashboard_page.ts +++ b/test/functional/page_objects/dashboard_page.ts @@ -111,6 +111,33 @@ export function DashboardPageProvider({ getService, getPageObjects }: FtrProvide return id; } + public async expectUnsavedChangesListingExists(title: string) { + log.debug(`Expect Unsaved Changes Listing Exists for `, title); + await testSubjects.existOrFail(`edit-unsaved-${title.split(' ').join('-')}`); + } + + public async expectUnsavedChangesDoesNotExist(title: string) { + log.debug(`Expect Unsaved Changes Listing Does Not Exist for `, title); + await testSubjects.missingOrFail(`edit-unsaved-${title.split(' ').join('-')}`); + } + + public async clickUnsavedChangesContinueEditing(title: string) { + log.debug(`Click Unsaved Changes Continue Editing `, title); + await testSubjects.existOrFail(`edit-unsaved-${title.split(' ').join('-')}`); + await testSubjects.click(`edit-unsaved-${title.split(' ').join('-')}`); + } + + public async clickUnsavedChangesDiscard(title: string, confirmDiscard = true) { + log.debug(`Click Unsaved Changes Discard for `, title); + await testSubjects.existOrFail(`discard-unsaved-${title.split(' ').join('-')}`); + await testSubjects.click(`discard-unsaved-${title.split(' ').join('-')}`); + if (confirmDiscard) { + await PageObjects.common.clickConfirmOnModal(); + } else { + await PageObjects.common.clickCancelOnModal(); + } + } + /** * Returns true if already on the dashboard landing page (that page doesn't have a link to itself). * @returns {Promise} @@ -234,6 +261,18 @@ export function DashboardPageProvider({ getService, getPageObjects }: FtrProvide await this.waitForRenderComplete(); } + public async clickNewDashboardExpectWarning(continueEditing = false) { + await listingTable.clickNewButton('createDashboardPromptButton'); + await testSubjects.existOrFail('dashboardCreateConfirm'); + if (continueEditing) { + await testSubjects.click('dashboardCreateConfirmContinue'); + } else { + await testSubjects.click('dashboardCreateConfirmStartOver'); + } + // make sure the dashboard page is shown + await this.waitForRenderComplete(); + } + public async clickCreateDashboardPrompt() { await testSubjects.click('createDashboardPromptButton'); } diff --git a/test/functional/services/dashboard/panel_actions.ts b/test/functional/services/dashboard/panel_actions.ts index d5bd4ab566883..ad6a70c2c97e5 100644 --- a/test/functional/services/dashboard/panel_actions.ts +++ b/test/functional/services/dashboard/panel_actions.ts @@ -99,9 +99,9 @@ export function DashboardPanelActionsProvider({ getService, getPageObjects }: Ft await testSubjects.click(TOGGLE_EXPAND_PANEL_DATA_TEST_SUBJ); } - async removePanel() { + async removePanel(parent?: WebElementWrapper) { log.debug('removePanel'); - await this.openContextMenu(); + await this.openContextMenu(parent); const isActionVisible = await testSubjects.exists(REMOVE_PANEL_DATA_TEST_SUBJ); if (!isActionVisible) await this.clickContextMenuMoreItem(); const isPanelActionVisible = await testSubjects.exists(REMOVE_PANEL_DATA_TEST_SUBJ); @@ -111,10 +111,8 @@ export function DashboardPanelActionsProvider({ getService, getPageObjects }: Ft async removePanelByTitle(title: string) { const header = await this.getPanelHeading(title); - await this.openContextMenu(header); - const isActionVisible = await testSubjects.exists(REMOVE_PANEL_DATA_TEST_SUBJ); - if (!isActionVisible) await this.clickContextMenuMoreItem(); - await testSubjects.click(REMOVE_PANEL_DATA_TEST_SUBJ); + log.debug('found header? ', Boolean(header)); + await this.removePanel(header); } async customizePanel(parent?: WebElementWrapper) { From 53b20654bfc54ad5376e1faaba733716210679b2 Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Wed, 27 Jan 2021 10:59:34 -0500 Subject: [PATCH 25/30] fixed jest --- .../application/listing/dashboard_unsaved_listing.test.tsx | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.test.tsx b/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.test.tsx index 7bf7b3ddcf34f..3717ea79dc060 100644 --- a/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.test.tsx +++ b/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.test.tsx @@ -101,7 +101,7 @@ describe('Unsaved listing', () => { it('Redirects to the requested dashboard in edit mode when continue editing clicked', async () => { const { props, component } = mountWith({}); - const getEditButton = () => findTestSubject(component, 'edit-unsaved-dashboardUnsavedOne'); + const getEditButton = () => findTestSubject(component, 'edit-unsaved-Dashboard-Unsaved-One'); await waitFor(() => { component.update(); expect(getEditButton().length).toEqual(1); @@ -120,8 +120,7 @@ describe('Unsaved listing', () => { .fn() .mockImplementation(() => [DASHBOARD_PANELS_UNSAVED_ID]); const { props, component } = mountWith({ services }); - const getEditButton = () => - findTestSubject(component, `edit-unsaved-${DASHBOARD_PANELS_UNSAVED_ID}`); + const getEditButton = () => findTestSubject(component, `edit-unsaved-New-Dashboard`); await waitFor(() => { component.update(); expect(getEditButton().length).toBe(1); @@ -137,7 +136,7 @@ describe('Unsaved listing', () => { it('Shows a warning then clears changes when delete unsaved changes is pressed', async () => { const { services, component } = mountWith({}); const getDiscardButton = () => - findTestSubject(component, 'discard-unsaved-dashboardUnsavedOne'); + findTestSubject(component, 'discard-unsaved-Dashboard-Unsaved-One'); await waitFor(() => { component.update(); expect(getDiscardButton().length).toBe(1); From 09de50728c9d50e2dcc785d93bcd260c370e8dc4 Mon Sep 17 00:00:00 2001 From: Ryan Keairns Date: Thu, 28 Jan 2021 09:15:32 -0600 Subject: [PATCH 26/30] Add responsive styles --- .../public/application/_dashboard_app.scss | 14 ++++++++++++++ .../listing/dashboard_unsaved_listing.tsx | 11 +++++++++-- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/src/plugins/dashboard/public/application/_dashboard_app.scss b/src/plugins/dashboard/public/application/_dashboard_app.scss index 63a73d6f80c8f..ca956948c006c 100644 --- a/src/plugins/dashboard/public/application/_dashboard_app.scss +++ b/src/plugins/dashboard/public/application/_dashboard_app.scss @@ -49,3 +49,17 @@ .dshUnsavedListingItem__actions { margin-left: $euiSizeL + $euiSizeXS; } + +@include euiBreakpoint('xs', 's') { + .dshUnsavedListingItem { + margin-top: $euiSize; + } + + .dshUnsavedListingItem__heading { + margin-bottom: $euiSizeXS; + } + + .dshUnsavedListingItem__actions { + flex-direction: column; + } +} \ No newline at end of file diff --git a/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.tsx b/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.tsx index 05163406b22e0..8378376caf0e7 100644 --- a/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.tsx +++ b/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.tsx @@ -36,7 +36,12 @@ const DashboardUnsavedItem = ({ const title = dashboard?.title ?? getNewDashboardTitle(); return (
- + @@ -49,9 +54,10 @@ const DashboardUnsavedItem = ({ Date: Thu, 28 Jan 2021 12:28:24 -0500 Subject: [PATCH 27/30] error handling for dashboard panel storage. --- .../public/application/dashboard_router.tsx | 2 +- .../lib/dashboard_panel_storage.ts | 51 +++++++++++++++---- .../application/listing/confirm_overlays.tsx | 8 ++- .../listing/dashboard_unsaved_listing.tsx | 18 +++++-- .../dashboard/public/dashboard_strings.ts | 18 +++++++ 5 files changed, 79 insertions(+), 18 deletions(-) diff --git a/src/plugins/dashboard/public/application/dashboard_router.tsx b/src/plugins/dashboard/public/application/dashboard_router.tsx index 41a8f97820526..d49871b853731 100644 --- a/src/plugins/dashboard/public/application/dashboard_router.tsx +++ b/src/plugins/dashboard/public/application/dashboard_router.tsx @@ -99,7 +99,7 @@ export async function mountApp({ indexPatterns: dataStart.indexPatterns, savedQueryService: dataStart.query.savedQueries, savedObjectsClient: coreStart.savedObjects.client, - dashboardPanelStorage: new DashboardPanelStorage(), + dashboardPanelStorage: new DashboardPanelStorage(core.notifications.toasts), savedDashboards: dashboardStart.getSavedDashboardLoader(), savedObjectsTagging: savedObjectsTaggingOss?.getTaggingApi(), allowByValueEmbeddables: initializerContext.config.get() diff --git a/src/plugins/dashboard/public/application/lib/dashboard_panel_storage.ts b/src/plugins/dashboard/public/application/lib/dashboard_panel_storage.ts index d7162b378ed80..ab1842048abd0 100644 --- a/src/plugins/dashboard/public/application/lib/dashboard_panel_storage.ts +++ b/src/plugins/dashboard/public/application/lib/dashboard_panel_storage.ts @@ -7,6 +7,8 @@ */ import { Storage } from '../../services/kibana_utils'; +import { NotificationsStart } from '../../services/core'; +import { panelStorageErrorStrings } from '../../dashboard_strings'; import { SavedDashboardPanel } from '..'; export const DASHBOARD_PANELS_UNSAVED_ID = 'unsavedDashboard'; @@ -15,30 +17,59 @@ const DASHBOARD_PANELS_SESSION_KEY = 'dashboardStateManagerPanels'; export class DashboardPanelStorage { private sessionStorage: Storage; - constructor() { + constructor(private toasts: NotificationsStart['toasts']) { this.sessionStorage = new Storage(sessionStorage); } public clearPanels(id = DASHBOARD_PANELS_UNSAVED_ID) { - const sessionStoragePanels = this.sessionStorage.get(DASHBOARD_PANELS_SESSION_KEY) || {}; - if (sessionStoragePanels[id]) { - delete sessionStoragePanels[id]; - this.sessionStorage.set(DASHBOARD_PANELS_SESSION_KEY, sessionStoragePanels); + try { + const sessionStoragePanels = this.sessionStorage.get(DASHBOARD_PANELS_SESSION_KEY) || {}; + if (sessionStoragePanels[id]) { + delete sessionStoragePanels[id]; + this.sessionStorage.set(DASHBOARD_PANELS_SESSION_KEY, sessionStoragePanels); + } + } catch (e) { + this.toasts.addDanger({ + title: panelStorageErrorStrings.getPanelsClearError(e.message), + 'data-test-subj': 'dashboardPanelsClearFailure', + }); } } public getPanels(id = DASHBOARD_PANELS_UNSAVED_ID): SavedDashboardPanel[] | undefined { - return this.sessionStorage.get(DASHBOARD_PANELS_SESSION_KEY)?.[id]; + try { + return this.sessionStorage.get(DASHBOARD_PANELS_SESSION_KEY)?.[id]; + } catch (e) { + this.toasts.addDanger({ + title: panelStorageErrorStrings.getPanelsGetError(e.message), + 'data-test-subj': 'dashboardPanelsGetFailure', + }); + } } public setPanels(id = DASHBOARD_PANELS_UNSAVED_ID, newPanels: SavedDashboardPanel[]) { - const sessionStoragePanels = this.sessionStorage.get(DASHBOARD_PANELS_SESSION_KEY) || {}; - sessionStoragePanels[id] = newPanels; - this.sessionStorage.set(DASHBOARD_PANELS_SESSION_KEY, sessionStoragePanels); + try { + const sessionStoragePanels = this.sessionStorage.get(DASHBOARD_PANELS_SESSION_KEY) || {}; + sessionStoragePanels[id] = newPanels; + this.sessionStorage.set(DASHBOARD_PANELS_SESSION_KEY, sessionStoragePanels); + } catch (e) { + this.toasts.addDanger({ + title: panelStorageErrorStrings.getPanelsSetError(e.message), + 'data-test-subj': 'dashboardPanelsSetFailure', + }); + } } public getDashboardIdsWithUnsavedChanges() { - return Object.keys(this.sessionStorage.get(DASHBOARD_PANELS_SESSION_KEY) || {}); + try { + return Object.keys(this.sessionStorage.get(DASHBOARD_PANELS_SESSION_KEY) || {}); + } catch (e) { + this.toasts.addDanger({ + title: panelStorageErrorStrings.getPanelsGetError(e.message), + 'data-test-subj': 'dashboardPanelsGetFailure', + }); + return []; + } } public dashboardHasUnsavedEdits(id = DASHBOARD_PANELS_UNSAVED_ID) { diff --git a/src/plugins/dashboard/public/application/listing/confirm_overlays.tsx b/src/plugins/dashboard/public/application/listing/confirm_overlays.tsx index 3539c08bc57f0..519978741dedf 100644 --- a/src/plugins/dashboard/public/application/listing/confirm_overlays.tsx +++ b/src/plugins/dashboard/public/application/listing/confirm_overlays.tsx @@ -22,11 +22,15 @@ import { OverlayStart } from '../../../../../core/public'; import { createConfirmStrings, leaveConfirmStrings } from '../../dashboard_strings'; import { toMountPoint } from '../../services/kibana_react'; -export const confirmDiscardUnsavedChanges = (overlays: OverlayStart, discardCallback: () => void) => +export const confirmDiscardUnsavedChanges = ( + overlays: OverlayStart, + discardCallback: () => void, + cancelButtonText = leaveConfirmStrings.getCancelButtonText() +) => overlays .openConfirm(leaveConfirmStrings.getDiscardSubtitle(), { confirmButtonText: leaveConfirmStrings.getConfirmButtonText(), - cancelButtonText: leaveConfirmStrings.getCancelButtonText(), + cancelButtonText, buttonColor: 'danger', defaultFocusedButton: EUI_MODAL_CANCEL_BUTTON, title: leaveConfirmStrings.getDiscardTitle(), diff --git a/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.tsx b/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.tsx index d596c74b5b14f..1d8928f8b3e6a 100644 --- a/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.tsx +++ b/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.tsx @@ -18,7 +18,11 @@ import { import React, { useCallback, useEffect, useState } from 'react'; import useMount from 'react-use/lib/useMount'; import { DashboardSavedObject } from '../..'; -import { dashboardUnsavedListingStrings, getNewDashboardTitle } from '../../dashboard_strings'; +import { + createConfirmStrings, + dashboardUnsavedListingStrings, + getNewDashboardTitle, +} from '../../dashboard_strings'; import { useKibana } from '../../services/kibana_react'; import { DASHBOARD_PANELS_UNSAVED_ID } from '../lib/dashboard_panel_storage'; import { DashboardAppServices, DashboardRedirect } from '../types'; @@ -109,10 +113,14 @@ export const DashboardUnsavedListing = ({ redirectTo }: { redirectTo: DashboardR const onDiscard = useCallback( (id?: string) => { - confirmDiscardUnsavedChanges(overlays, () => { - dashboardPanelStorage.clearPanels(id); - setDashboardIds(dashboardPanelStorage.getDashboardIdsWithUnsavedChanges()); - }); + confirmDiscardUnsavedChanges( + overlays, + () => { + dashboardPanelStorage.clearPanels(id); + setDashboardIds(dashboardPanelStorage.getDashboardIdsWithUnsavedChanges()); + }, + createConfirmStrings.getCancelButtonText() + ); }, [overlays, dashboardPanelStorage] ); diff --git a/src/plugins/dashboard/public/dashboard_strings.ts b/src/plugins/dashboard/public/dashboard_strings.ts index 2a603673dc7f1..44ebad659d6d4 100644 --- a/src/plugins/dashboard/public/dashboard_strings.ts +++ b/src/plugins/dashboard/public/dashboard_strings.ts @@ -264,6 +264,24 @@ export const createConfirmStrings = { }), }; +export const panelStorageErrorStrings = { + getPanelsGetError: (message: string) => + i18n.translate('dashboard.panelStorageError.getError', { + defaultMessage: 'Error encountered while fetching unsaved changes: {message}', + values: { message }, + }), + getPanelsSetError: (message: string) => + i18n.translate('dashboard.panelStorageError.setError', { + defaultMessage: 'Error encountered while setting unsaved changes: {message}', + values: { message }, + }), + getPanelsClearError: (message: string) => + i18n.translate('dashboard.panelStorageError.clearError', { + defaultMessage: 'Error encountered while clearing unsaved changes: {message}', + values: { message }, + }), +}; + /* Empty Screen */ From efd5b3f07c1947679e81f9c7b232a23a937f7041 Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Fri, 29 Jan 2021 15:19:50 -0500 Subject: [PATCH 28/30] refactor dashboard unsaved listing to show entries when loading --- .../public/application/_dashboard_app.scss | 4 + .../application/dashboard_state_manager.ts | 1 - .../listing/dashboard_unsaved_listing.tsx | 107 +++++++++++------- .../dashboard/public/dashboard_constants.ts | 20 +++- .../dashboard/public/dashboard_strings.ts | 4 + 5 files changed, 87 insertions(+), 49 deletions(-) diff --git a/src/plugins/dashboard/public/application/_dashboard_app.scss b/src/plugins/dashboard/public/application/_dashboard_app.scss index ca956948c006c..30253afff391f 100644 --- a/src/plugins/dashboard/public/application/_dashboard_app.scss +++ b/src/plugins/dashboard/public/application/_dashboard_app.scss @@ -46,6 +46,10 @@ margin-bottom: 0 !important; } +.dshUnsavedListingItem__loading { + color: $euiTextSubduedColor !important; +} + .dshUnsavedListingItem__actions { margin-left: $euiSizeL + $euiSizeXS; } diff --git a/src/plugins/dashboard/public/application/dashboard_state_manager.ts b/src/plugins/dashboard/public/application/dashboard_state_manager.ts index a3f2e0395435b..562fbc7aab3ba 100644 --- a/src/plugins/dashboard/public/application/dashboard_state_manager.ts +++ b/src/plugins/dashboard/public/application/dashboard_state_manager.ts @@ -78,7 +78,6 @@ export class DashboardStateManager { >; private readonly stateContainerChangeSub: Subscription; private readonly dashboardPanelStorage?: DashboardPanelStorage; - private readonly STATE_STORAGE_KEY = '_a'; public readonly kbnUrlStateStorage: IKbnUrlStateStorage; private readonly stateSyncRef: ISyncStateRef; private readonly allowByValueEmbeddables: boolean; diff --git a/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.tsx b/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.tsx index 1d8928f8b3e6a..5af2fa6be6120 100644 --- a/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.tsx +++ b/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.tsx @@ -16,7 +16,6 @@ import { EuiTitle, } from '@elastic/eui'; import React, { useCallback, useEffect, useState } from 'react'; -import useMount from 'react-use/lib/useMount'; import { DashboardSavedObject } from '../..'; import { createConfirmStrings, @@ -29,15 +28,16 @@ import { DashboardAppServices, DashboardRedirect } from '../types'; import { confirmDiscardUnsavedChanges } from './confirm_overlays'; const DashboardUnsavedItem = ({ - dashboard, + id, + title, onOpenClick, onDiscardClick, }: { - dashboard?: DashboardSavedObject; + id: string; + title?: string; onOpenClick: () => void; onDiscardClick: () => void; }) => { - const title = dashboard?.title ?? getNewDashboardTitle(); return (
- + -

- {dashboard?.title ?? getNewDashboardTitle()} +

+ {title || dashboardUnsavedListingStrings.getLoadingTitle()}

@@ -68,9 +76,10 @@ const DashboardUnsavedItem = ({ flush="left" size="s" color="primary" + disabled={!title} onClick={onOpenClick} - data-test-subj={`edit-unsaved-${title.split(' ').join('-')}`} - aria-label={dashboardUnsavedListingStrings.getEditAriaLabel(title)} + data-test-subj={title ? `edit-unsaved-${title.split(' ').join('-')}` : undefined} + aria-label={dashboardUnsavedListingStrings.getEditAriaLabel(title ?? id)} > {dashboardUnsavedListingStrings.getEditTitle()} @@ -80,9 +89,10 @@ const DashboardUnsavedItem = ({ flush="left" size="s" color="danger" + disabled={!title} onClick={onDiscardClick} - data-test-subj={`discard-unsaved-${title.split(' ').join('-')}`} - aria-label={dashboardUnsavedListingStrings.getDiscardAriaLabel(title)} + data-test-subj={title ? `discard-unsaved-${title.split(' ').join('-')}` : undefined} + aria-label={dashboardUnsavedListingStrings.getDiscardAriaLabel(title ?? id)} > {dashboardUnsavedListingStrings.getDiscardTitle()} @@ -92,6 +102,10 @@ const DashboardUnsavedItem = ({ ); }; +interface UnsavedItemMap { + [key: string]: DashboardSavedObject; +} + export const DashboardUnsavedListing = ({ redirectTo }: { redirectTo: DashboardRedirect }) => { const { services: { @@ -101,8 +115,11 @@ export const DashboardUnsavedListing = ({ redirectTo }: { redirectTo: DashboardR }, } = useKibana(); - const [items, setItems] = useState([]); - const [dashboardIds, setDashboardIds] = useState([]); + const [items, setItems] = useState({}); + const [mounted, setMounted] = useState(true); + const [dashboardIds, setDashboardIds] = useState( + dashboardPanelStorage.getDashboardIdsWithUnsavedChanges() + ); const onOpen = useCallback( (id?: string) => { @@ -125,50 +142,52 @@ export const DashboardUnsavedListing = ({ redirectTo }: { redirectTo: DashboardR [overlays, dashboardPanelStorage] ); - useMount(() => { - setDashboardIds(dashboardPanelStorage.getDashboardIdsWithUnsavedChanges()); + useEffect(() => { + return () => setMounted(false); }); useEffect(() => { - let hasNewDashboard = false; const dashPromises = dashboardIds - .filter((id) => { - if (id !== DASHBOARD_PANELS_UNSAVED_ID) { - return true; - } - hasNewDashboard = true; - return false; - }) + .filter((id) => id !== DASHBOARD_PANELS_UNSAVED_ID) .map((dashboardId) => savedDashboards.get(dashboardId)); Promise.all(dashPromises).then((dashboards: DashboardSavedObject[]) => { - const newItems = dashboards.map((dashboard) => ( - onOpen(dashboard.id)} - onDiscardClick={() => onDiscard(dashboard.id)} - /> - )); - if (hasNewDashboard) { - newItems.unshift( - onOpen()} - onDiscardClick={() => onDiscard()} - /> - ); + const dashboardMap = {}; + if (!mounted) { + return; } - setItems(newItems); + setItems( + dashboards.reduce((map, dashboard) => { + return { + ...map, + [dashboard.id || DASHBOARD_PANELS_UNSAVED_ID]: dashboard, + }; + }, dashboardMap) + ); }); - }, [dashboardIds, onOpen, onDiscard, savedDashboards]); + }, [dashboardIds, savedDashboards, mounted]); - return items.length === 0 ? null : ( + return dashboardIds.length === 0 ? null : ( <> 1)} + title={dashboardUnsavedListingStrings.getUnsavedChangesTitle(dashboardIds.length > 1)} > - {items} + {dashboardIds.map((dashboardId: string) => { + const title: string | undefined = + dashboardId === DASHBOARD_PANELS_UNSAVED_ID + ? getNewDashboardTitle() + : items[dashboardId]?.title; + const redirectId = dashboardId === DASHBOARD_PANELS_UNSAVED_ID ? undefined : dashboardId; + return ( + onOpen(redirectId)} + onDiscardClick={() => onDiscard(redirectId)} + /> + ); + })} diff --git a/src/plugins/dashboard/public/dashboard_constants.ts b/src/plugins/dashboard/public/dashboard_constants.ts index 56b9820184268..26590422d079c 100644 --- a/src/plugins/dashboard/public/dashboard_constants.ts +++ b/src/plugins/dashboard/public/dashboard_constants.ts @@ -6,6 +6,9 @@ * Public License, v 1. */ +import { ViewMode } from './services/embeddable'; +import { setStateToKbnUrl } from './services/kibana_utils'; + const DASHBOARD_STATE_STORAGE_KEY = '_a'; export const DashboardConstants = { @@ -20,11 +23,20 @@ export const DashboardConstants = { }; export function createDashboardEditUrl(id?: string, editMode?: boolean) { - const edit = editMode ? `?${DASHBOARD_STATE_STORAGE_KEY}=(viewMode:edit)` : ''; - if (id) { - return `${DashboardConstants.VIEW_DASHBOARD_URL}/${id}${edit}`; + if (!id) { + return `${DashboardConstants.CREATE_NEW_DASHBOARD_URL}`; + } + const viewUrl = `${DashboardConstants.VIEW_DASHBOARD_URL}/${id}`; + if (editMode) { + const editUrl = setStateToKbnUrl( + DASHBOARD_STATE_STORAGE_KEY, + { viewMode: ViewMode.EDIT }, + { useHash: false, storeInHashQuery: false }, + viewUrl + ); + return editUrl; } - return `${DashboardConstants.CREATE_NEW_DASHBOARD_URL}${edit}`; + return viewUrl; } export function createDashboardListingFilterUrl(filter: string | undefined) { diff --git a/src/plugins/dashboard/public/dashboard_strings.ts b/src/plugins/dashboard/public/dashboard_strings.ts index 44ebad659d6d4..68681826c27fb 100644 --- a/src/plugins/dashboard/public/dashboard_strings.ts +++ b/src/plugins/dashboard/public/dashboard_strings.ts @@ -358,6 +358,10 @@ export const dashboardUnsavedListingStrings = { : dashboardListingTable.getEntityName(), }, }), + getLoadingTitle: () => + i18n.translate('dashboard.listing.unsaved.loading', { + defaultMessage: 'Loading', + }), getEditAriaLabel: (title: string) => i18n.translate('dashboard.listing.unsaved.editAria', { defaultMessage: 'Continue editing {title}', From 0bf8e68177365a1cbb8134fb93e29f4ee13c3ba6 Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Mon, 1 Feb 2021 15:04:15 -0500 Subject: [PATCH 29/30] revert import in dashboard_constants --- .../dashboard/public/dashboard_constants.ts | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/src/plugins/dashboard/public/dashboard_constants.ts b/src/plugins/dashboard/public/dashboard_constants.ts index 26590422d079c..f809eaa2914ed 100644 --- a/src/plugins/dashboard/public/dashboard_constants.ts +++ b/src/plugins/dashboard/public/dashboard_constants.ts @@ -6,9 +6,6 @@ * Public License, v 1. */ -import { ViewMode } from './services/embeddable'; -import { setStateToKbnUrl } from './services/kibana_utils'; - const DASHBOARD_STATE_STORAGE_KEY = '_a'; export const DashboardConstants = { @@ -26,17 +23,8 @@ export function createDashboardEditUrl(id?: string, editMode?: boolean) { if (!id) { return `${DashboardConstants.CREATE_NEW_DASHBOARD_URL}`; } - const viewUrl = `${DashboardConstants.VIEW_DASHBOARD_URL}/${id}`; - if (editMode) { - const editUrl = setStateToKbnUrl( - DASHBOARD_STATE_STORAGE_KEY, - { viewMode: ViewMode.EDIT }, - { useHash: false, storeInHashQuery: false }, - viewUrl - ); - return editUrl; - } - return viewUrl; + const edit = editMode ? `?${DASHBOARD_STATE_STORAGE_KEY}=(viewMode:edit)` : ''; + return `${DashboardConstants.VIEW_DASHBOARD_URL}/${id}${edit}`; } export function createDashboardListingFilterUrl(filter: string | undefined) { From b4355d9bafeb53eb2169c187fb24568e8730d0a8 Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Wed, 3 Feb 2021 11:44:21 -0500 Subject: [PATCH 30/30] changed unsaved listing cancelation logic --- .../listing/dashboard_unsaved_listing.tsx | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.tsx b/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.tsx index 5af2fa6be6120..9b78f290acbeb 100644 --- a/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.tsx +++ b/src/plugins/dashboard/public/application/listing/dashboard_unsaved_listing.tsx @@ -116,7 +116,6 @@ export const DashboardUnsavedListing = ({ redirectTo }: { redirectTo: DashboardR } = useKibana(); const [items, setItems] = useState({}); - const [mounted, setMounted] = useState(true); const [dashboardIds, setDashboardIds] = useState( dashboardPanelStorage.getDashboardIdsWithUnsavedChanges() ); @@ -143,16 +142,16 @@ export const DashboardUnsavedListing = ({ redirectTo }: { redirectTo: DashboardR ); useEffect(() => { - return () => setMounted(false); - }); - - useEffect(() => { + if (dashboardIds?.length === 0) { + return; + } + let canceled = false; const dashPromises = dashboardIds .filter((id) => id !== DASHBOARD_PANELS_UNSAVED_ID) .map((dashboardId) => savedDashboards.get(dashboardId)); Promise.all(dashPromises).then((dashboards: DashboardSavedObject[]) => { const dashboardMap = {}; - if (!mounted) { + if (canceled) { return; } setItems( @@ -164,7 +163,10 @@ export const DashboardUnsavedListing = ({ redirectTo }: { redirectTo: DashboardR }, dashboardMap) ); }); - }, [dashboardIds, savedDashboards, mounted]); + return () => { + canceled = true; + }; + }, [dashboardIds, savedDashboards]); return dashboardIds.length === 0 ? null : ( <>