Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Dashboard] Add reset button #154872

Merged
merged 16 commits into from
Apr 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,14 @@ export const topNavStrings = {
defaultMessage: 'Save as a new dashboard',
}),
},
resetChanges: {
label: i18n.translate('dashboard.topNave.resetChangesButtonAriaLabel', {
defaultMessage: 'Reset',
}),
description: i18n.translate('dashboard.topNave.resetChangesConfigDescription', {
defaultMessage: 'Reset changes to dashboard',
}),
},
switchToViewMode: {
label: i18n.translate('dashboard.topNave.cancelButtonAriaLabel', {
defaultMessage: 'Switch to view mode',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ export const useDashboardMenuItems = ({
const hasOverlays = dashboard.select((state) => state.componentState.hasOverlays);
const lastSavedId = dashboard.select((state) => state.componentState.lastSavedId);
const dashboardTitle = dashboard.select((state) => state.explicitInput.title);
const viewMode = dashboard.select((state) => state.explicitInput.viewMode);

/**
* Show the Dashboard app's share menu
Expand Down Expand Up @@ -109,21 +110,26 @@ export const useDashboardMenuItems = ({
}, [maybeRedirect, dashboard]);

/**
* Returns to view mode. If the dashboard has unsaved changes shows a warning and resets to last saved state.
* Show the dashboard's "Confirm reset changes" modal. If confirmed:
* (1) reset the dashboard to the last saved state, and
* (2) if `switchToViewMode` is `true`, set the dashboard to view mode.
*/
const returnToViewMode = useCallback(() => {
dashboard.clearOverlays();
if (hasUnsavedChanges) {
confirmDiscardUnsavedChanges(() => {
batch(() => {
dashboard.resetToLastSavedState();
dashboard.dispatch.setViewMode(ViewMode.VIEW);
});
});
return;
}
dashboard.dispatch.setViewMode(ViewMode.VIEW);
}, [dashboard, hasUnsavedChanges]);
const resetChanges = useCallback(
(switchToViewMode: boolean = false) => {
dashboard.clearOverlays();
if (hasUnsavedChanges) {
confirmDiscardUnsavedChanges(() => {
batch(() => {
dashboard.resetToLastSavedState();
if (switchToViewMode) dashboard.dispatch.setViewMode(ViewMode.VIEW);
});
}, viewMode);
} else {
if (switchToViewMode) dashboard.dispatch.setViewMode(ViewMode.VIEW);
}
},
[dashboard, hasUnsavedChanges, viewMode]
);

/**
* Register all of the top nav configs that can be used by dashboard.
Expand Down Expand Up @@ -184,7 +190,7 @@ export const useDashboardMenuItems = ({
id: 'cancel',
disableButton: isSaveInProgress || !lastSavedId || hasOverlays,
testId: 'dashboardViewOnlyMode',
run: () => returnToViewMode(),
run: () => resetChanges(true),
} as TopNavMenuData,

share: {
Expand Down Expand Up @@ -215,9 +221,9 @@ export const useDashboardMenuItems = ({
quickSaveDashboard,
hasUnsavedChanges,
isSaveInProgress,
returnToViewMode,
saveDashboardAs,
setIsLabsShown,
resetChanges,
hasOverlays,
lastSavedId,
isLabsShown,
Expand All @@ -226,27 +232,53 @@ export const useDashboardMenuItems = ({
clone,
]);

const resetChangesMenuItem = useMemo(() => {
return {
...topNavStrings.resetChanges,
id: 'reset',
testId: 'dashboardDiscardChangesMenuItem',
disableButton:
!hasUnsavedChanges ||
hasOverlays ||
(viewMode === ViewMode.EDIT && (isSaveInProgress || !lastSavedId)),
run: () => resetChanges(),
};
}, [hasOverlays, lastSavedId, resetChanges, viewMode, isSaveInProgress, hasUnsavedChanges]);

/**
* Build ordered menus for view and edit mode.
*/
const viewModeTopNavConfig = useMemo(() => {
const labsMenuItem = isLabsEnabled ? [menuItems.labs] : [];
const shareMenuItem = share ? [menuItems.share] : [];
const writePermissionsMenuItems = showWriteControls ? [menuItems.clone, menuItems.edit] : [];
return [...labsMenuItem, menuItems.fullScreen, ...shareMenuItem, ...writePermissionsMenuItems];
}, [menuItems, share, showWriteControls, isLabsEnabled]);
const cloneMenuItem = showWriteControls ? [menuItems.clone] : [];
const editMenuItem = showWriteControls ? [menuItems.edit] : [];
return [
...labsMenuItem,
menuItems.fullScreen,
...shareMenuItem,
...cloneMenuItem,
resetChangesMenuItem,
...editMenuItem,
];
}, [menuItems, share, showWriteControls, resetChangesMenuItem, isLabsEnabled]);

const editModeTopNavConfig = useMemo(() => {
const labsMenuItem = isLabsEnabled ? [menuItems.labs] : [];
const shareMenuItem = share ? [menuItems.share] : [];
const editModeItems: TopNavMenuData[] = [];
if (lastSavedId) {
editModeItems.push(menuItems.saveAs, menuItems.switchToViewMode, menuItems.quickSave);
editModeItems.push(
menuItems.saveAs,
menuItems.switchToViewMode,
resetChangesMenuItem,
menuItems.quickSave
);
} else {
editModeItems.push(menuItems.switchToViewMode, menuItems.saveAs);
}
return [...labsMenuItem, menuItems.settings, ...shareMenuItem, ...editModeItems];
}, [lastSavedId, menuItems, share, isLabsEnabled]);
}, [lastSavedId, menuItems, share, resetChangesMenuItem, isLabsEnabled]);

return { viewModeTopNavConfig, editModeTopNavConfig };
};
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,10 @@ export const dashboardContainerReducers = {
},

resetToLastSavedInput: (state: DashboardReduxState) => {
state.explicitInput = state.componentState.lastSavedInput;
state.explicitInput = {
...state.componentState.lastSavedInput,
viewMode: state.explicitInput.viewMode, // keep current view mode when resetting
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is possible for a view view mode to be included in a dashboard's saved input - for example, in the sample dashboards, their imported state is in view mode which meant that hitting the "reset" would actually flip them from edit mode into view mode (since this is included as part of the last saved input). This line ensures that view mode is ignored when resetting to the last saved input.

};
},

// ------------------------------------------------------------------------------
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
* Side Public License, v 1.
*/

import { ViewMode } from '@kbn/embeddable-plugin/public';
import { i18n } from '@kbn/i18n';

export const dashboardListingErrorStrings = {
Expand Down Expand Up @@ -91,32 +92,32 @@ export const dashboardUnsavedListingStrings = {
defaultMessage: 'Continue editing',
}),
getDiscardAriaLabel: (title: string) =>
i18n.translate('dashboard.listing.unsaved.discardAria', {
defaultMessage: 'Discard changes to {title}',
i18n.translate('dashboard.listing.unsaved.resetAria', {
defaultMessage: 'Reset changes to {title}',
values: { title },
}),
getDiscardTitle: () =>
i18n.translate('dashboard.listing.unsaved.discardTitle', {
defaultMessage: 'Discard changes',
i18n.translate('dashboard.listing.unsaved.resetTitle', {
defaultMessage: 'Reset changes',
}),
};

export const discardConfirmStrings = {
getDiscardTitle: () =>
i18n.translate('dashboard.discardChangesConfirmModal.discardChangesTitle', {
defaultMessage: 'Discard changes to dashboard?',
}),
getDiscardSubtitle: () =>
i18n.translate('dashboard.discardChangesConfirmModal.discardChangesDescription', {
defaultMessage: `Once you discard your changes, there's no getting them back.`,
}),
getDiscardConfirmButtonText: () =>
i18n.translate('dashboard.discardChangesConfirmModal.confirmButtonLabel', {
defaultMessage: 'Discard changes',
}),
getDiscardCancelButtonText: () =>
i18n.translate('dashboard.discardChangesConfirmModal.cancelButtonLabel', {
defaultMessage: 'Cancel',
export const resetConfirmStrings = {
getResetTitle: () =>
i18n.translate('dashboard.resetChangesConfirmModal.resetChangesTitle', {
defaultMessage: 'Reset dashboard?',
}),
getResetSubtitle: (viewMode: ViewMode) =>
viewMode === ViewMode.EDIT
? i18n.translate('dashboard.discardChangesConfirmModal.discardChangesDescription', {
defaultMessage: `All unsaved changes will be lost.`,
})
: i18n.translate('dashboard.resetChangesConfirmModal.resetChangesDescription', {
defaultMessage: `This dashboard will return to its last saved state. You might lose changes to filters and queries.`,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cqliu1 Brought up a good point that this could be rephrased to be something like "This dashboard will return to its last saved state. You might lose changes to things like filters and queries." to make it less explicit - after all, you could lose changes to control selections (which kinda fall in to the "filters" bucket) or maps zoom/positioning, but it's hard to call out those things specifically without potentially causing confusion.

Not sure we want to hold up this PR on a small copy change, but throwing it out there 💃

}),
getResetConfirmButtonText: () =>
i18n.translate('dashboard.resetChangesConfirmModal.confirmButtonLabel', {
defaultMessage: 'Reset dashboard',
}),
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,24 +20,28 @@ import {
EuiText,
EUI_MODAL_CANCEL_BUTTON,
} from '@elastic/eui';
import { ViewMode } from '@kbn/embeddable-plugin/public';
import { toMountPoint } from '@kbn/kibana-react-plugin/public';

import { pluginServices } from '../services/plugin_services';
import { createConfirmStrings, discardConfirmStrings } from './_dashboard_listing_strings';
import { createConfirmStrings, resetConfirmStrings } from './_dashboard_listing_strings';

export type DiscardOrKeepSelection = 'cancel' | 'discard' | 'keep';

export const confirmDiscardUnsavedChanges = (discardCallback: () => void) => {
export const confirmDiscardUnsavedChanges = (
discardCallback: () => void,
viewMode: ViewMode = ViewMode.EDIT // we want to show the danger modal on the listing page
) => {
const {
overlays: { openConfirm },
} = pluginServices.getServices();

openConfirm(discardConfirmStrings.getDiscardSubtitle(), {
confirmButtonText: discardConfirmStrings.getDiscardConfirmButtonText(),
cancelButtonText: discardConfirmStrings.getDiscardCancelButtonText(),
buttonColor: 'danger',
openConfirm(resetConfirmStrings.getResetSubtitle(viewMode), {
confirmButtonText: resetConfirmStrings.getResetConfirmButtonText(),
buttonColor: viewMode === ViewMode.EDIT ? 'danger' : 'primary',
maxWidth: 500,
defaultFocusedButton: EUI_MODAL_CANCEL_BUTTON,
title: discardConfirmStrings.getDiscardTitle(),
title: resetConfirmStrings.getResetTitle(),
}).then((isConfirmed) => {
if (isConfirmed) {
discardCallback();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,10 @@ export class CustomizePanelAction implements Action<CustomizePanelActionContext>
{
size: 's',
'data-test-subj': 'customizePanel',
onClose: (overlayRef) => {
if (overlayTracker) overlayTracker.clearOverlays();
overlayRef.close();
},
Comment on lines +132 to +135
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixes a small bug I discovered with the panel settings flyout where, if you closed the flyout by clicking outside of the flyout rather than via the "Close" buttons, hasOverlays remains true and so a bunch of items in the top nav bar stayed disabled.

Before

Apr-13-2023 15-28-16

After

Apr-13-2023 15-27-09

}
);
overlayTracker?.openOverlay(handle);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,14 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
await validateQueryAndFilter();
});

after(async () => {
// discard changes made in view mode
await PageObjects.dashboard.switchToEditMode();
await PageObjects.dashboard.clickCancelOutOfEditMode();
it('can discard changes', async () => {
await PageObjects.dashboard.clickDiscardChanges();
await PageObjects.dashboard.waitForRenderComplete();

const query = await queryBar.getQueryString();
expect(query).to.eql('');
const filterCount = await filterBar.getFilterCount();
expect(filterCount).to.eql(0);
});
});

Expand Down Expand Up @@ -140,8 +144,21 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
expect(currentPanelCount).to.eql(unsavedPanelCount);
});

it('resets to original panel count after discarding changes', async () => {
it('can discard changes', async () => {
unsavedPanelCount = await PageObjects.dashboard.getPanelCount();
expect(unsavedPanelCount).to.eql(originalPanelCount + 2);

await PageObjects.dashboard.clickDiscardChanges();
const currentPanelCount = await PageObjects.dashboard.getPanelCount();
expect(currentPanelCount).to.eql(originalPanelCount);
});

it('resets to original panel count after switching to view mode and discarding changes', async () => {
await addPanels();
await PageObjects.header.waitUntilLoadingHasFinished();
unsavedPanelCount = await PageObjects.dashboard.getPanelCount();
expect(unsavedPanelCount).to.eql(originalPanelCount + 2);

await PageObjects.dashboard.clickCancelOutOfEditMode();
await PageObjects.header.waitUntilLoadingHasFinished();
const currentPanelCount = await PageObjects.dashboard.getPanelCount();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -257,21 +257,36 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
await dashboard.clearUnsavedChanges();
});

it('changes to selections can be discarded', async () => {
await dashboardControls.optionsListOpenPopover(controlId);
await dashboardControls.optionsListPopoverSelectOption('bark');
await dashboardControls.optionsListEnsurePopoverIsClosed(controlId);
let selections = await dashboardControls.optionsListGetSelectionsString(controlId);
expect(selections).to.equal('hiss, grr, bark');
describe('discarding changes', async () => {
describe('changes can be discarded', async () => {
let selections = '';

beforeEach(async () => {
await dashboardControls.optionsListOpenPopover(controlId);
await dashboardControls.optionsListPopoverSelectOption('bark');
await dashboardControls.optionsListEnsurePopoverIsClosed(controlId);
selections = await dashboardControls.optionsListGetSelectionsString(controlId);
expect(selections).to.equal('hiss, grr, bark');
});

await dashboard.clickCancelOutOfEditMode();
selections = await dashboardControls.optionsListGetSelectionsString(controlId);
expect(selections).to.equal('hiss, grr');
});
afterEach(async () => {
selections = await dashboardControls.optionsListGetSelectionsString(controlId);
expect(selections).to.equal('hiss, grr');
});

it('dashboard does not load with unsaved changes when changes are discarded', async () => {
await dashboard.switchToEditMode();
await testSubjects.missingOrFail('dashboardUnsavedChangesBadge');
it('by clicking the discard changes button', async () => {
await dashboard.clickDiscardChanges();
});

it('by switching to view mode', async () => {
await dashboard.clickCancelOutOfEditMode();
});
});

it('dashboard does not load with unsaved changes when changes are discarded', async () => {
await dashboard.switchToEditMode();
await testSubjects.missingOrFail('dashboardUnsavedChangesBadge');
});
});
});

Expand Down
21 changes: 21 additions & 0 deletions test/functional/page_objects/dashboard_page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,18 @@ export class DashboardPageObject extends FtrService {
}
}

public async clickDiscardChanges(accept = true) {
await this.retry.try(async () => {
await this.expectDiscardChangesButtonEnabled();
this.log.debug('clickDiscardChanges');
await this.testSubjects.click('dashboardDiscardChangesMenuItem');
});
await this.common.expectConfirmModalOpenState(true);
if (accept) {
await this.common.clickConfirmOnModal();
}
}

public async clickQuickSave() {
await this.retry.try(async () => {
await this.expectQuickSaveButtonEnabled();
Expand Down Expand Up @@ -734,6 +746,15 @@ export class DashboardPageObject extends FtrService {
await this.testSubjects.existOrFail('dashboardQuickSaveMenuItem');
}

public async expectDiscardChangesButtonEnabled() {
this.log.debug('expectDiscardChangesButtonEnabled');
const quickSaveButton = await this.testSubjects.find('dashboardDiscardChangesMenuItem');
const isDisabled = await quickSaveButton.getAttribute('disabled');
if (isDisabled) {
throw new Error('Discard changes button disabled');
}
}

public async expectQuickSaveButtonEnabled() {
this.log.debug('expectQuickSaveButtonEnabled');
const quickSaveButton = await this.testSubjects.find('dashboardQuickSaveMenuItem');
Expand Down
Loading