Skip to content

Commit

Permalink
another refactor
Browse files Browse the repository at this point in the history
  • Loading branch information
rshen91 committed Nov 4, 2024
1 parent efe2233 commit 609c9c1
Show file tree
Hide file tree
Showing 5 changed files with 9 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import {
type AppLeaveConfirmAction,
type AppLeaveHandler,
} from '@kbn/core-application-browser';
import { AppLeaveSkipAction } from '@kbn/core-application-browser/src/app_leave';

const appLeaveActionFactory: AppLeaveActionFactory = {
confirm(
Expand All @@ -37,19 +36,12 @@ const appLeaveActionFactory: AppLeaveActionFactory = {
default() {
return { type: AppLeaveActionType.default };
},
skip() {
return { type: AppLeaveActionType.skip };
},
};

export function isConfirmAction(action: AppLeaveAction): action is AppLeaveConfirmAction {
return action.type === AppLeaveActionType.confirm;
}

export function isSkipAction(action: AppLeaveAction): action is AppLeaveSkipAction {
return action.type === AppLeaveActionType.skip;
}

export function getLeaveAction(handler?: AppLeaveHandler, nextAppId?: string): AppLeaveAction {
if (!handler) {
return appLeaveActionFactory.default();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import type { CustomBrandingStart } from '@kbn/core-custom-branding-browser';
import { AppRouter } from './ui';
import type { InternalApplicationSetup, InternalApplicationStart, Mounter } from './types';

import { getLeaveAction, isConfirmAction, isSkipAction } from './application_leave';
import { getLeaveAction, isConfirmAction } from './application_leave';
import { getUserConfirmationHandler } from './navigation_confirm';
import {
appendAppPath,
Expand Down Expand Up @@ -409,14 +409,10 @@ export class ApplicationService {
if (currentAppId === undefined) {
return true;
}

const action = getLeaveAction(
this.appInternalStates.get(currentAppId)?.leaveHandler,
nextAppId
);
if (isSkipAction(action)) {
return true;
}
if (isConfirmAction(action)) {
const confirmed = await overlays.openConfirm(action.text, {
title: action.title,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ export type AppLeaveHandler = (
export enum AppLeaveActionType {
confirm = 'confirm',
default = 'default',
skip = 'skip',
}

/**
Expand All @@ -50,10 +49,6 @@ export interface AppLeaveDefaultAction {
type: AppLeaveActionType.default;
}

export interface AppLeaveSkipAction {
type: AppLeaveActionType.skip;
}

/**
* Action to return from a {@link AppLeaveHandler} to show a confirmation
* message when trying to leave an application.
Expand All @@ -78,13 +73,12 @@ export interface AppLeaveConfirmAction {
*
* @public
* */
export type AppLeaveAction = AppLeaveDefaultAction | AppLeaveConfirmAction | AppLeaveSkipAction;
export type AppLeaveAction = AppLeaveDefaultAction | AppLeaveConfirmAction;

/**
* Factory provided when invoking a {@link AppLeaveHandler} to retrieve the {@link AppLeaveAction} to execute.
*/
export interface AppLeaveActionFactory {
skip(): AppLeaveAction;
/**
* Returns a confirm action, resulting on prompting a message to the user before leaving the
* application, allowing him to choose if he wants to stay on the app or confirm that he
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ import { getDashboardRecentlyAccessedService } from '../services/dashboard_recen
import {
coreServices,
dataService,
embeddableService,
navigationService,
serverlessService,
} from '../services/kibana_services';
Expand Down Expand Up @@ -194,13 +193,6 @@ export function InternalDashboardTopNav({
*/
useEffect(() => {
onAppLeave((actions) => {
if (
viewMode === 'edit' &&
hasUnsavedChanges &&
!embeddableService.getStateTransfer().isTransferInProgress
) {
return actions.skip();
}
return actions.default();
});
return () => {
Expand Down
16 changes: 7 additions & 9 deletions x-pack/plugins/lens/public/app_plugin/app.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1493,18 +1493,16 @@ describe('Lens App', () => {
describe('showing a confirm message when leaving', () => {
let defaultLeave: jest.Mock;
let confirmLeave: jest.Mock;
let skipLeave: jest.Mock;

beforeEach(() => {
defaultLeave = jest.fn();
confirmLeave = jest.fn();
skipLeave = jest.fn();
});

it('should not show a confirm message if there is no expression to save', async () => {
const { props } = await mountWith({});
const lastCall = props.onAppLeave.mock.calls[props.onAppLeave.mock.calls.length - 1][0];
lastCall({ default: defaultLeave, confirm: confirmLeave, skip: skipLeave });
lastCall({ default: defaultLeave, confirm: confirmLeave });
expect(defaultLeave).toHaveBeenCalled();
expect(confirmLeave).not.toHaveBeenCalled();
});
Expand All @@ -1520,7 +1518,7 @@ describe('Lens App', () => {
};
const { props } = await mountWith({ services, preloadedState: { isSaveable: true } });
const lastCall = props.onAppLeave.mock.calls[props.onAppLeave.mock.calls.length - 1][0];
lastCall({ default: defaultLeave, confirm: confirmLeave, skip: skipLeave });
lastCall({ default: defaultLeave, confirm: confirmLeave });
expect(defaultLeave).toHaveBeenCalled();
expect(confirmLeave).not.toHaveBeenCalled();
});
Expand All @@ -1536,7 +1534,7 @@ describe('Lens App', () => {
},
});
const lastCall = props.onAppLeave.mock.calls[props.onAppLeave.mock.calls.length - 1][0];
lastCall({ default: defaultLeave, confirm: confirmLeave, skip: skipLeave });
lastCall({ default: defaultLeave, confirm: confirmLeave });
expect(confirmLeave).toHaveBeenCalled();
expect(defaultLeave).not.toHaveBeenCalled();
});
Expand All @@ -1553,7 +1551,7 @@ describe('Lens App', () => {
},
});
const lastCall = props.onAppLeave.mock.calls[props.onAppLeave.mock.calls.length - 1][0];
lastCall({ default: defaultLeave, confirm: confirmLeave, skip: skipLeave });
lastCall({ default: defaultLeave, confirm: confirmLeave });
expect(confirmLeave).toHaveBeenCalled();
expect(defaultLeave).not.toHaveBeenCalled();
});
Expand Down Expand Up @@ -1630,7 +1628,7 @@ describe('Lens App', () => {
mountedApp.props.onAppLeave.mock.calls[
mountedApp.props.onAppLeave.mock.calls.length - 1
][0];
lastCall({ default: defaultLeave, confirm: confirmLeave, skip: skipLeave });
lastCall({ default: defaultLeave, confirm: confirmLeave });
expect(defaultLeave).not.toHaveBeenCalled();
expect(confirmLeave).toHaveBeenCalled();
});
Expand Down Expand Up @@ -1659,7 +1657,7 @@ describe('Lens App', () => {
const { props } = await mountWith({ preloadedState, props: customProps });

const lastCall = props.onAppLeave.mock.calls[props.onAppLeave.mock.calls.length - 1][0];
lastCall({ default: defaultLeave, confirm: confirmLeave, skip: skipLeave });
lastCall({ default: defaultLeave, confirm: confirmLeave });
expect(defaultLeave).toHaveBeenCalled();
expect(confirmLeave).not.toHaveBeenCalled();
});
Expand All @@ -1676,7 +1674,7 @@ describe('Lens App', () => {
);
});
const lastCall = props.onAppLeave.mock.calls[props.onAppLeave.mock.calls.length - 1][0];
lastCall({ default: defaultLeave, confirm: confirmLeave, skip: skipLeave });
lastCall({ default: defaultLeave, confirm: confirmLeave });
expect(confirmLeave).toHaveBeenCalled();
expect(defaultLeave).not.toHaveBeenCalled();
});
Expand Down

0 comments on commit 609c9c1

Please sign in to comment.