From 5076bafed86a784b42e3ea10eb653a3145834323 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Thu, 11 Feb 2021 11:05:27 +0100 Subject: [PATCH 1/8] implements ScopedHistory.block --- .../application/application_service.tsx | 15 +- .../application/navigation_confirm.test.ts | 96 +++++++++++ .../public/application/navigation_confirm.ts | 60 +++++++ .../public/application/scoped_history.test.ts | 152 +++++++++++++++++- src/core/public/application/scoped_history.ts | 28 +++- 5 files changed, 340 insertions(+), 11 deletions(-) create mode 100644 src/core/public/application/navigation_confirm.test.ts create mode 100644 src/core/public/application/navigation_confirm.ts diff --git a/src/core/public/application/application_service.tsx b/src/core/public/application/application_service.tsx index 0d977d104951ac..5e999ff94b9ce9 100644 --- a/src/core/public/application/application_service.tsx +++ b/src/core/public/application/application_service.tsx @@ -8,7 +8,7 @@ import React from 'react'; import { BehaviorSubject, Observable, Subject, Subscription } from 'rxjs'; -import { map, shareReplay, takeUntil, distinctUntilChanged, filter } from 'rxjs/operators'; +import { map, shareReplay, takeUntil, distinctUntilChanged, filter, take } from 'rxjs/operators'; import { createBrowserHistory, History } from 'history'; import { MountPoint } from '../types'; @@ -31,6 +31,7 @@ import { NavigateToAppOptions, } from './types'; import { getLeaveAction, isConfirmAction } from './application_leave'; +import { getUserConfirmationHandler } from './navigation_confirm'; import { appendAppPath, parseAppUrl, relativeToAbsolute, getAppInfo } from './utils'; interface SetupDeps { @@ -92,6 +93,7 @@ export class ApplicationService { private history?: History; private navigate?: (url: string, state: unknown, replace: boolean) => void; private redirectTo?: (url: string) => void; + private overlayStart$ = new Subject(); public setup({ http: { basePath }, @@ -101,7 +103,14 @@ export class ApplicationService { history, }: SetupDeps): InternalApplicationSetup { const basename = basePath.get(); - this.history = history || createBrowserHistory({ basename }); + this.history = + history || + createBrowserHistory({ + basename, + getUserConfirmation: getUserConfirmationHandler({ + overlayPromise: this.overlayStart$.pipe(take(1)).toPromise(), + }), + }); this.navigate = (url, state, replace) => { // basePath not needed here because `history` is configured with basename @@ -173,6 +182,8 @@ export class ApplicationService { throw new Error('ApplicationService#setup() must be invoked before start.'); } + this.overlayStart$.next(overlays); + const httpLoadingCount$ = new BehaviorSubject(0); http.addLoadingCountSource(httpLoadingCount$); diff --git a/src/core/public/application/navigation_confirm.test.ts b/src/core/public/application/navigation_confirm.test.ts new file mode 100644 index 00000000000000..d31f25fd94c934 --- /dev/null +++ b/src/core/public/application/navigation_confirm.test.ts @@ -0,0 +1,96 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { OverlayStart } from '../overlays'; +import { overlayServiceMock } from '../overlays/overlay_service.mock'; +import { getUserConfirmationHandler, ConfirmHandler } from './navigation_confirm'; + +const nextTick = () => new Promise((resolve) => setImmediate(resolve)); + +describe('getUserConfirmationHandler', () => { + let overlayStart: ReturnType; + let overlayPromise: Promise; + let resolvePromise: Function; + let rejectPromise: Function; + let fallbackHandler: jest.MockedFunction; + let handler: ConfirmHandler; + + beforeEach(() => { + overlayStart = overlayServiceMock.createStartContract(); + overlayPromise = new Promise((resolve, reject) => { + resolvePromise = () => resolve(overlayStart); + rejectPromise = () => reject('some error'); + }); + fallbackHandler = jest.fn().mockImplementation((message, callback) => { + callback(true); + }); + + handler = getUserConfirmationHandler({ + overlayPromise, + fallbackHandler, + }); + }); + + it('uses the fallback handler if the promise is not resolved yet', () => { + const callback = jest.fn(); + handler('foo', callback); + + expect(fallbackHandler).toHaveBeenCalledTimes(1); + expect(fallbackHandler).toHaveBeenCalledWith('foo', callback); + }); + + it('calls the callback with the value returned by the fallback handler', async () => { + const callback = jest.fn(); + handler('foo', callback); + + expect(fallbackHandler).toHaveBeenCalledTimes(1); + expect(fallbackHandler).toHaveBeenCalledWith('foo', callback); + + expect(callback).toHaveBeenCalledTimes(1); + expect(callback).toHaveBeenCalledWith(true); + }); + + it('uses the overlay handler once the promise is resolved', async () => { + resolvePromise(); + await nextTick(); + + const callback = jest.fn(); + handler('foo', callback); + + expect(fallbackHandler).not.toHaveBeenCalled(); + + expect(overlayStart.openConfirm).toHaveBeenCalledTimes(1); + expect(overlayStart.openConfirm).toHaveBeenCalledWith('foo', expect.any(Object)); + }); + + it('calls the callback with the value returned by `openConfirm`', async () => { + overlayStart.openConfirm.mockResolvedValue(true); + + resolvePromise(); + await nextTick(); + + const callback = jest.fn(); + handler('foo', callback); + + await nextTick(); + + expect(callback).toHaveBeenCalledTimes(1); + expect(callback).toHaveBeenCalledWith(true); + }); + + it('uses the fallback handler if the promise rejects', async () => { + rejectPromise(); + await nextTick(); + + const callback = jest.fn(); + handler('foo', callback); + + expect(fallbackHandler).toHaveBeenCalledTimes(1); + expect(overlayStart.openConfirm).not.toHaveBeenCalled(); + }); +}); diff --git a/src/core/public/application/navigation_confirm.ts b/src/core/public/application/navigation_confirm.ts new file mode 100644 index 00000000000000..4f68a48eb8fc99 --- /dev/null +++ b/src/core/public/application/navigation_confirm.ts @@ -0,0 +1,60 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { OverlayStart } from 'kibana/public'; + +export type ConfirmHandlerCallback = (result: boolean) => void; +export type ConfirmHandler = (message: string, callback: ConfirmHandlerCallback) => void; + +interface GetUserConfirmationHandlerParams { + overlayPromise: Promise; + fallbackHandler?: ConfirmHandler; +} + +export const getUserConfirmationHandler = ({ + overlayPromise, + fallbackHandler = windowConfirm, +}: GetUserConfirmationHandlerParams): ConfirmHandler => { + let overlayConfirm: ConfirmHandler; + + overlayPromise.then( + (overlay) => { + overlayConfirm = getOverlayConfirmHandler(overlay); + }, + () => { + // should never append, but even if it does, we don't need to do anything, + // and will just use the default window confirm instead + } + ); + + return (message: string, callback: ConfirmHandlerCallback) => { + if (overlayConfirm) { + overlayConfirm(message, callback); + } else { + fallbackHandler(message, callback); + } + }; +}; + +const windowConfirm: ConfirmHandler = (message: string, callback: ConfirmHandlerCallback) => { + const confirmed = window.confirm(message); + callback(confirmed); +}; + +const getOverlayConfirmHandler = (overlay: OverlayStart): ConfirmHandler => { + return (message: string, callback: ConfirmHandlerCallback) => { + overlay.openConfirm(message, { title: ' ' }).then( + (confirmed) => { + callback(confirmed); + }, + () => { + callback(false); + } + ); + }; +}; diff --git a/src/core/public/application/scoped_history.test.ts b/src/core/public/application/scoped_history.test.ts index 9e25809d670079..2c8c66d447c5f7 100644 --- a/src/core/public/application/scoped_history.test.ts +++ b/src/core/public/application/scoped_history.test.ts @@ -7,7 +7,8 @@ */ import { ScopedHistory } from './scoped_history'; -import { createMemoryHistory } from 'history'; +import { createMemoryHistory, History } from 'history'; +import type { ConfirmHandler } from './navigation_confirm'; describe('ScopedHistory', () => { describe('construction', () => { @@ -336,4 +337,153 @@ describe('ScopedHistory', () => { expect(gh.length).toBe(4); }); }); + + describe('block', () => { + let gh: History; + let h: ScopedHistory; + + const initHistory = ({ + initialPath = '/app/wow', + scopedHistoryPath = '/app/wow', + confirmHandler, + }: { + initialPath?: string; + scopedHistoryPath?: string; + confirmHandler?: ConfirmHandler; + } = {}) => { + gh = createMemoryHistory({ + getUserConfirmation: confirmHandler, + }); + gh.push(initialPath); + h = new ScopedHistory(gh, scopedHistoryPath); + }; + + it('calls block on the global history', () => { + initHistory(); + + const blockSpy = jest.spyOn(gh, 'block'); + h.block('confirm'); + + expect(blockSpy).toHaveBeenCalledTimes(1); + expect(blockSpy).toHaveBeenCalledWith('confirm'); + }); + + it('returns a wrapped unregister function', () => { + initHistory(); + + const blockSpy = jest.spyOn(gh, 'block'); + const unregister = jest.fn(); + blockSpy.mockReturnValue(unregister); + + const wrapperUnregister = h.block('confirm'); + + expect(unregister).not.toHaveBeenCalled(); + + wrapperUnregister(); + + expect(unregister).toHaveBeenCalledTimes(1); + }); + + it('calls the block handler when navigating to another app', () => { + initHistory(); + + const blockHandler = jest.fn().mockReturnValue(true); + + h.block(blockHandler); + + gh.push('/app/other'); + + expect(blockHandler).toHaveBeenCalledTimes(1); + expect(gh.location.pathname).toEqual('/app/other'); + }); + + it('calls the block handler when navigating inside the current app', () => { + initHistory(); + + const blockHandler = jest.fn().mockReturnValue(true); + + h.block(blockHandler); + + gh.push('/app/wow/another-page'); + + expect(blockHandler).toHaveBeenCalledTimes(1); + expect(gh.location.pathname).toEqual('/app/wow/another-page'); + }); + + it('can block the navigation', () => { + initHistory(); + + const blockHandler = jest.fn().mockReturnValue(false); + + h.block(blockHandler); + + gh.push('/app/other'); + + expect(blockHandler).toHaveBeenCalledTimes(1); + expect(gh.location.pathname).toEqual('/app/wow'); + }); + + it('no longer blocks the navigation when unregistered', () => { + initHistory(); + + const blockHandler = jest.fn().mockReturnValue(false); + + const unregister = h.block(blockHandler); + + gh.push('/app/other'); + + expect(gh.location.pathname).toEqual('/app/wow'); + + unregister(); + + gh.push('/app/other'); + + expect(gh.location.pathname).toEqual('/app/other'); + }); + + it('throws if the history is no longer active', () => { + initHistory(); + + gh.push('/app/other'); + + expect(() => h.block()).toThrowErrorMatchingInlineSnapshot( + `"ScopedHistory instance has fell out of navigation scope for basePath: /app/wow"` + ); + }); + + it('unregisters the block handler when the history is no longer active', () => { + initHistory(); + + const blockSpy = jest.spyOn(gh, 'block'); + const unregister = jest.fn(); + blockSpy.mockReturnValue(unregister); + + h.block('confirm'); + + expect(unregister).not.toHaveBeenCalled(); + + gh.push('/app/other'); + + expect(unregister).toHaveBeenCalledTimes(1); + }); + + it('calls the defined global history confirm handler', () => { + const confirmHandler: jest.MockedFunction = jest + .fn() + .mockImplementation((message, callback) => { + callback(true); + }); + + initHistory({ + confirmHandler, + }); + + h.block('are you sure'); + + gh.push('/app/other'); + + expect(confirmHandler).toHaveBeenCalledTimes(1); + expect(gh.location.pathname).toEqual('/app/other'); + }); + }); }); diff --git a/src/core/public/application/scoped_history.ts b/src/core/public/application/scoped_history.ts index daf0aee7921814..b932465f800cd2 100644 --- a/src/core/public/application/scoped_history.ts +++ b/src/core/public/application/scoped_history.ts @@ -51,6 +51,10 @@ export class ScopedHistory * The key of the current position of the window in the history stack. */ private currentLocationKeyIndex: number = 0; + /** + * Array of the current {@link block} unregister callbacks + */ + private blockUnregisterCallbacks: Set = new Set(); constructor(private readonly parentHistory: History, private readonly basePath: string) { const parentPath = this.parentHistory.location.pathname; @@ -176,18 +180,20 @@ export class ScopedHistory }; /** - * Not supported. Use {@link AppMountParameters.onAppLeave}. - * - * @remarks - * We prefer that applications use the `onAppLeave` API because it supports a more graceful experience that prefers - * a modal when possible, falling back to a confirm dialog box in the beforeunload case. + * Add a block prompt requesting user confirmation when navigating away from the current page. */ public block = ( prompt?: boolean | string | TransitionPromptHook ): UnregisterCallback => { - throw new Error( - `history.block is not supported. Please use the AppMountParameters.onAppLeave API.` - ); + this.verifyActive(); + + const unregisterCallback = this.parentHistory.block(prompt); + this.blockUnregisterCallbacks.add(unregisterCallback); + + return () => { + this.blockUnregisterCallbacks.delete(unregisterCallback); + unregisterCallback(); + }; }; /** @@ -290,6 +296,12 @@ export class ScopedHistory if (!location.pathname.startsWith(this.basePath)) { unlisten(); this.isActive = false; + + for (const unregisterBlock of this.blockUnregisterCallbacks) { + unregisterBlock(); + } + this.blockUnregisterCallbacks.clear(); + return; } From d4a9e6996f9cdde6df4a0c2306c9756626838799 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Thu, 11 Feb 2021 11:32:06 +0100 Subject: [PATCH 2/8] add FTR tests --- .../public/application/navigation_confirm.ts | 18 ++-- .../plugins/core_history_block/kibana.json | 8 ++ .../plugins/core_history_block/package.json | 14 ++++ .../plugins/core_history_block/public/app.tsx | 83 +++++++++++++++++++ .../core_history_block/public/index.ts | 13 +++ .../core_history_block/public/plugin.ts | 40 +++++++++ .../plugins/core_history_block/tsconfig.json | 13 +++ .../test_suites/core_plugins/history_block.ts | 57 +++++++++++++ .../test_suites/core_plugins/index.ts | 1 + 9 files changed, 239 insertions(+), 8 deletions(-) create mode 100644 test/plugin_functional/plugins/core_history_block/kibana.json create mode 100644 test/plugin_functional/plugins/core_history_block/package.json create mode 100644 test/plugin_functional/plugins/core_history_block/public/app.tsx create mode 100644 test/plugin_functional/plugins/core_history_block/public/index.ts create mode 100644 test/plugin_functional/plugins/core_history_block/public/plugin.ts create mode 100644 test/plugin_functional/plugins/core_history_block/tsconfig.json create mode 100644 test/plugin_functional/test_suites/core_plugins/history_block.ts diff --git a/src/core/public/application/navigation_confirm.ts b/src/core/public/application/navigation_confirm.ts index 4f68a48eb8fc99..6fa5bcb2c5bb1a 100644 --- a/src/core/public/application/navigation_confirm.ts +++ b/src/core/public/application/navigation_confirm.ts @@ -48,13 +48,15 @@ const windowConfirm: ConfirmHandler = (message: string, callback: ConfirmHandler const getOverlayConfirmHandler = (overlay: OverlayStart): ConfirmHandler => { return (message: string, callback: ConfirmHandlerCallback) => { - overlay.openConfirm(message, { title: ' ' }).then( - (confirmed) => { - callback(confirmed); - }, - () => { - callback(false); - } - ); + overlay + .openConfirm(message, { title: ' ', 'data-test-subj': 'navigationBlockConfirmModAl' }) + .then( + (confirmed) => { + callback(confirmed); + }, + () => { + callback(false); + } + ); }; }; diff --git a/test/plugin_functional/plugins/core_history_block/kibana.json b/test/plugin_functional/plugins/core_history_block/kibana.json new file mode 100644 index 00000000000000..27c0a9eab52da2 --- /dev/null +++ b/test/plugin_functional/plugins/core_history_block/kibana.json @@ -0,0 +1,8 @@ +{ + "id": "core_history_block", + "version": "0.0.1", + "kibanaVersion": "kibana", + "server": false, + "ui": true, + "requiredBundles": ["kibanaReact"] +} diff --git a/test/plugin_functional/plugins/core_history_block/package.json b/test/plugin_functional/plugins/core_history_block/package.json new file mode 100644 index 00000000000000..f5590e33e6ac01 --- /dev/null +++ b/test/plugin_functional/plugins/core_history_block/package.json @@ -0,0 +1,14 @@ +{ + "name": "core_history_block", + "version": "1.0.0", + "main": "target/test/plugin_functional/plugins/core_history_block", + "kibana": { + "version": "kibana", + "templateVersion": "1.0.0" + }, + "license": "SSPL-1.0 OR Elastic License 2.0", + "scripts": { + "kbn": "node ../../../../scripts/kbn.js", + "build": "rm -rf './target' && ../../../../node_modules/.bin/tsc" + } +} \ No newline at end of file diff --git a/test/plugin_functional/plugins/core_history_block/public/app.tsx b/test/plugin_functional/plugins/core_history_block/public/app.tsx new file mode 100644 index 00000000000000..2f253ab31500c8 --- /dev/null +++ b/test/plugin_functional/plugins/core_history_block/public/app.tsx @@ -0,0 +1,83 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import React from 'react'; +import ReactDOM from 'react-dom'; +import { Router, Switch, Route, Prompt } from 'react-router-dom'; +import type { AppMountParameters, IBasePath, ApplicationStart } from 'kibana/public'; +import { RedirectAppLinks } from '../../../../../src/plugins/kibana_react/public'; + +const HomePage = ({ + basePath, + application, +}: { + basePath: IBasePath; + application: ApplicationStart; +}) => ( + +); + +const FooPage = ({ + basePath, + application, +}: { + basePath: IBasePath; + application: ApplicationStart; +}) => ( + +); + +interface AppOptions { + basePath: IBasePath; + application: ApplicationStart; +} + +export const renderApp = ( + { basePath, application }: AppOptions, + { element, history }: AppMountParameters +) => { + ReactDOM.render( + + + + + + + + + + , + element + ); + return () => ReactDOM.unmountComponentAtNode(element); +}; diff --git a/test/plugin_functional/plugins/core_history_block/public/index.ts b/test/plugin_functional/plugins/core_history_block/public/index.ts new file mode 100644 index 00000000000000..deec3d61a0d647 --- /dev/null +++ b/test/plugin_functional/plugins/core_history_block/public/index.ts @@ -0,0 +1,13 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { PluginInitializer } from 'kibana/public'; +import { CoreAppLinkPlugin, CoreAppLinkPluginSetup, CoreAppLinkPluginStart } from './plugin'; + +export const plugin: PluginInitializer = () => + new CoreAppLinkPlugin(); diff --git a/test/plugin_functional/plugins/core_history_block/public/plugin.ts b/test/plugin_functional/plugins/core_history_block/public/plugin.ts new file mode 100644 index 00000000000000..3483d8dfee513d --- /dev/null +++ b/test/plugin_functional/plugins/core_history_block/public/plugin.ts @@ -0,0 +1,40 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { Plugin, CoreSetup, AppMountParameters } from 'kibana/public'; +import { renderApp } from './app'; + +export class CoreAppLinkPlugin implements Plugin { + public setup(core: CoreSetup, deps: {}) { + core.application.register({ + id: 'core_history_block', + title: 'History block test', + mount: async (params: AppMountParameters) => { + const [{ application }] = await core.getStartServices(); + return renderApp( + { + basePath: core.http.basePath, + application, + }, + params + ); + }, + }); + + return {}; + } + + public start() { + return {}; + } + + public stop() {} +} + +export type CoreAppLinkPluginSetup = ReturnType; +export type CoreAppLinkPluginStart = ReturnType; diff --git a/test/plugin_functional/plugins/core_history_block/tsconfig.json b/test/plugin_functional/plugins/core_history_block/tsconfig.json new file mode 100644 index 00000000000000..ffd2cd261ab23b --- /dev/null +++ b/test/plugin_functional/plugins/core_history_block/tsconfig.json @@ -0,0 +1,13 @@ +{ + "extends": "../../../../tsconfig.base.json", + "compilerOptions": { + "outDir": "./target", + "skipLibCheck": true + }, + "include": ["index.ts", "public/**/*.ts", "public/**/*.tsx", "../../../../typings/**/*"], + "exclude": [], + "references": [ + { "path": "../../../../src/core/tsconfig.json" }, + { "path": "../../../../src/plugins/kibana_react/tsconfig.json" } + ] +} diff --git a/test/plugin_functional/test_suites/core_plugins/history_block.ts b/test/plugin_functional/test_suites/core_plugins/history_block.ts new file mode 100644 index 00000000000000..58bdd3cbea716b --- /dev/null +++ b/test/plugin_functional/test_suites/core_plugins/history_block.ts @@ -0,0 +1,57 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import expect from '@kbn/expect'; +import { PluginFunctionalProviderContext } from '../../services'; + +export default function ({ getService, getPageObjects }: PluginFunctionalProviderContext) { + const PageObjects = getPageObjects(['common']); + const browser = getService('browser'); + const testSubjects = getService('testSubjects'); + + describe('application using `ScopedHistory.block`', () => { + beforeEach(async () => { + await PageObjects.common.navigateToApp('core_history_block'); + }); + + describe('when navigating to another app', () => { + it('prevents navigation if user click cancel on the confirmation dialog', async () => { + await testSubjects.click('applink-external-test'); + + await testSubjects.existOrFail('navigationBlockConfirmModAl'); + await PageObjects.common.clickCancelOnModal(false); + expect(await browser.getCurrentUrl()).to.contain('/app/core_history_block'); + }); + it('allows navigation if user click confirm on the confirmation dialog', async () => { + await testSubjects.click('applink-external-test'); + + await testSubjects.existOrFail('navigationBlockConfirmModAl'); + await PageObjects.common.clickConfirmOnModal(); + expect(await browser.getCurrentUrl()).to.contain('/app/home'); + }); + }); + + describe('when navigating to another app', () => { + it('prevents navigation if user click cancel on the confirmation dialog', async () => { + await testSubjects.click('applink-intra-test'); + + await testSubjects.existOrFail('navigationBlockConfirmModAl'); + await PageObjects.common.clickCancelOnModal(false); + expect(await browser.getCurrentUrl()).to.contain('/app/core_history_block'); + expect(await browser.getCurrentUrl()).not.to.contain('/foo'); + }); + it('allows navigation if user click confirm on the confirmation dialog', async () => { + await testSubjects.click('applink-intra-test'); + + await testSubjects.existOrFail('navigationBlockConfirmModAl'); + await PageObjects.common.clickConfirmOnModal(); + expect(await browser.getCurrentUrl()).to.contain('/app/core_history_block/foo'); + }); + }); + }); +} diff --git a/test/plugin_functional/test_suites/core_plugins/index.ts b/test/plugin_functional/test_suites/core_plugins/index.ts index 0770bd9774dca8..3f26b317b81edc 100644 --- a/test/plugin_functional/test_suites/core_plugins/index.ts +++ b/test/plugin_functional/test_suites/core_plugins/index.ts @@ -20,5 +20,6 @@ export default function ({ loadTestFile }: PluginFunctionalProviderContext) { loadTestFile(require.resolve('./application_status')); loadTestFile(require.resolve('./rendering')); loadTestFile(require.resolve('./chrome_help_menu_links')); + loadTestFile(require.resolve('./history_block')); }); } From 22e3c859765a8d6c3cae86c5f23904e203067d87 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Mon, 22 Feb 2021 08:39:47 +0100 Subject: [PATCH 3/8] fix test plugin id --- test/plugin_functional/plugins/core_history_block/kibana.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/plugin_functional/plugins/core_history_block/kibana.json b/test/plugin_functional/plugins/core_history_block/kibana.json index 27c0a9eab52da2..6d2dda2b13225c 100644 --- a/test/plugin_functional/plugins/core_history_block/kibana.json +++ b/test/plugin_functional/plugins/core_history_block/kibana.json @@ -1,5 +1,5 @@ { - "id": "core_history_block", + "id": "coreHistoryBlock", "version": "0.0.1", "kibanaVersion": "kibana", "server": false, From d341f6a21ce0ccaada719775ccda7df332c0035f Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Mon, 22 Feb 2021 08:39:59 +0100 Subject: [PATCH 4/8] update generated doc --- .../kibana-plugin-core-public.scopedhistory.block.md | 7 +------ .../core/public/kibana-plugin-core-public.scopedhistory.md | 2 +- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/docs/development/core/public/kibana-plugin-core-public.scopedhistory.block.md b/docs/development/core/public/kibana-plugin-core-public.scopedhistory.block.md index 922cab9ef3769d..eb632465e46990 100644 --- a/docs/development/core/public/kibana-plugin-core-public.scopedhistory.block.md +++ b/docs/development/core/public/kibana-plugin-core-public.scopedhistory.block.md @@ -4,15 +4,10 @@ ## ScopedHistory.block property -Not supported. Use [AppMountParameters.onAppLeave](./kibana-plugin-core-public.appmountparameters.onappleave.md). +Add a block prompt requesting user confirmation when navigating away from the current page. Signature: ```typescript block: (prompt?: string | boolean | History.TransitionPromptHook | undefined) => UnregisterCallback; ``` - -## Remarks - -We prefer that applications use the `onAppLeave` API because it supports a more graceful experience that prefers a modal when possible, falling back to a confirm dialog box in the beforeunload case. - diff --git a/docs/development/core/public/kibana-plugin-core-public.scopedhistory.md b/docs/development/core/public/kibana-plugin-core-public.scopedhistory.md index 1818d2bc0851db..15ed4e74c4dc5f 100644 --- a/docs/development/core/public/kibana-plugin-core-public.scopedhistory.md +++ b/docs/development/core/public/kibana-plugin-core-public.scopedhistory.md @@ -27,7 +27,7 @@ export declare class ScopedHistory implements Hi | Property | Modifiers | Type | Description | | --- | --- | --- | --- | | [action](./kibana-plugin-core-public.scopedhistory.action.md) | | Action | The last action dispatched on the history stack. | -| [block](./kibana-plugin-core-public.scopedhistory.block.md) | | (prompt?: string | boolean | History.TransitionPromptHook<HistoryLocationState> | undefined) => UnregisterCallback | Not supported. Use [AppMountParameters.onAppLeave](./kibana-plugin-core-public.appmountparameters.onappleave.md). | +| [block](./kibana-plugin-core-public.scopedhistory.block.md) | | (prompt?: string | boolean | History.TransitionPromptHook<HistoryLocationState> | undefined) => UnregisterCallback | Add a block prompt requesting user confirmation when navigating away from the current page. | | [createHref](./kibana-plugin-core-public.scopedhistory.createhref.md) | | (location: LocationDescriptorObject<HistoryLocationState>, { prependBasePath }?: {
prependBasePath?: boolean | undefined;
}) => Href | Creates an href (string) to the location. If prependBasePath is true (default), it will prepend the location's path with the scoped history basePath. | | [createSubHistory](./kibana-plugin-core-public.scopedhistory.createsubhistory.md) | | <SubHistoryLocationState = unknown>(basePath: string) => ScopedHistory<SubHistoryLocationState> | Creates a ScopedHistory for a subpath of this ScopedHistory. Useful for applications that may have sub-apps that do not need access to the containing application's history. | | [go](./kibana-plugin-core-public.scopedhistory.go.md) | | (n: number) => void | Send the user forward or backwards in the history stack. | From 6c0bd0a32ca56c45251f20ffd5a85e60ac6e7a15 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Mon, 22 Feb 2021 09:15:26 +0100 Subject: [PATCH 5/8] deprecates AppMountParameters.onAppLeave --- .../core/public/kibana-plugin-core-public.appleavehandler.md | 5 +++++ ...ibana-plugin-core-public.appmountparameters.onappleave.md | 5 +++++ src/core/public/application/types.ts | 4 ++++ src/core/public/public.api.md | 3 ++- 4 files changed, 16 insertions(+), 1 deletion(-) diff --git a/docs/development/core/public/kibana-plugin-core-public.appleavehandler.md b/docs/development/core/public/kibana-plugin-core-public.appleavehandler.md index d86f7b7a1a5f9e..2eacdd811f438a 100644 --- a/docs/development/core/public/kibana-plugin-core-public.appleavehandler.md +++ b/docs/development/core/public/kibana-plugin-core-public.appleavehandler.md @@ -4,6 +4,11 @@ ## AppLeaveHandler type +> Warning: This API is now obsolete. +> +> [AppMountParameters.onAppLeave](./kibana-plugin-core-public.appmountparameters.onappleave.md) has been deprecated in favor of [ScopedHistory.block](./kibana-plugin-core-public.scopedhistory.block.md) +> + A handler that will be executed before leaving the application, either when going to another application or when closing the browser tab or manually changing the url. Should return `confirm` to to prompt a message to the user before leaving the page, or `default` to keep the default behavior (doing nothing). See [AppMountParameters](./kibana-plugin-core-public.appmountparameters.md) for detailed usage examples. diff --git a/docs/development/core/public/kibana-plugin-core-public.appmountparameters.onappleave.md b/docs/development/core/public/kibana-plugin-core-public.appmountparameters.onappleave.md index e898126a553e2d..e64e40a49e44e0 100644 --- a/docs/development/core/public/kibana-plugin-core-public.appmountparameters.onappleave.md +++ b/docs/development/core/public/kibana-plugin-core-public.appmountparameters.onappleave.md @@ -4,6 +4,11 @@ ## AppMountParameters.onAppLeave property +> Warning: This API is now obsolete. +> +> [ScopedHistory.block](./kibana-plugin-core-public.scopedhistory.block.md) should be used instead. +> + A function that can be used to register a handler that will be called when the user is leaving the current application, allowing to prompt a confirmation message before actually changing the page. This will be called either when the user goes to another application, or when trying to close the tab or manually changing the url. diff --git a/src/core/public/application/types.ts b/src/core/public/application/types.ts index a94f96e48ba6ca..0643b9070d9c69 100644 --- a/src/core/public/application/types.ts +++ b/src/core/public/application/types.ts @@ -478,6 +478,8 @@ export interface AppMountParameters { * return renderApp({ element, history }); * } * ``` + * + * @deprecated {@link ScopedHistory.block} should be used instead. */ onAppLeave: (handler: AppLeaveHandler) => void; @@ -523,6 +525,7 @@ export interface AppMountParameters { * See {@link AppMountParameters} for detailed usage examples. * * @public + * @deprecated {@link AppMountParameters.onAppLeave} has been deprecated in favor of {@link ScopedHistory.block} */ export type AppLeaveHandler = ( factory: AppLeaveActionFactory, @@ -590,6 +593,7 @@ export interface AppLeaveActionFactory { * so we can show to the user the right UX for him to saved his/her/their changes */ confirm(text: string, title?: string, callback?: () => void): AppLeaveConfirmAction; + /** * Returns a default action, resulting on executing the default behavior when * the user tries to leave an application diff --git a/src/core/public/public.api.md b/src/core/public/public.api.md index b068606b880472..d79cba5346a73f 100644 --- a/src/core/public/public.api.md +++ b/src/core/public/public.api.md @@ -116,7 +116,7 @@ export interface AppLeaveDefaultAction { // Warning: (ae-forgotten-export) The symbol "AppLeaveActionFactory" needs to be exported by the entry point index.d.ts // -// @public +// @public @deprecated export type AppLeaveHandler = (factory: AppLeaveActionFactory, nextAppId?: string) => AppLeaveAction; // @public (undocumented) @@ -153,6 +153,7 @@ export interface AppMountParameters { appBasePath: string; element: HTMLElement; history: ScopedHistory; + // @deprecated onAppLeave: (handler: AppLeaveHandler) => void; setHeaderActionMenu: (menuMount: MountPoint | undefined) => void; } From d360bb54b009999ec39e287729e6d134a6115c74 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Mon, 22 Feb 2021 09:17:47 +0100 Subject: [PATCH 6/8] typo fix --- src/core/public/application/navigation_confirm.ts | 2 +- .../test_suites/core_plugins/history_block.ts | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/core/public/application/navigation_confirm.ts b/src/core/public/application/navigation_confirm.ts index 6fa5bcb2c5bb1a..9bae41c71e2d00 100644 --- a/src/core/public/application/navigation_confirm.ts +++ b/src/core/public/application/navigation_confirm.ts @@ -49,7 +49,7 @@ const windowConfirm: ConfirmHandler = (message: string, callback: ConfirmHandler const getOverlayConfirmHandler = (overlay: OverlayStart): ConfirmHandler => { return (message: string, callback: ConfirmHandlerCallback) => { overlay - .openConfirm(message, { title: ' ', 'data-test-subj': 'navigationBlockConfirmModAl' }) + .openConfirm(message, { title: ' ', 'data-test-subj': 'navigationBlockConfirmModal' }) .then( (confirmed) => { callback(confirmed); diff --git a/test/plugin_functional/test_suites/core_plugins/history_block.ts b/test/plugin_functional/test_suites/core_plugins/history_block.ts index 58bdd3cbea716b..96145f6ce3da79 100644 --- a/test/plugin_functional/test_suites/core_plugins/history_block.ts +++ b/test/plugin_functional/test_suites/core_plugins/history_block.ts @@ -23,24 +23,24 @@ export default function ({ getService, getPageObjects }: PluginFunctionalProvide it('prevents navigation if user click cancel on the confirmation dialog', async () => { await testSubjects.click('applink-external-test'); - await testSubjects.existOrFail('navigationBlockConfirmModAl'); + await testSubjects.existOrFail('navigationBlockConfirmModal'); await PageObjects.common.clickCancelOnModal(false); expect(await browser.getCurrentUrl()).to.contain('/app/core_history_block'); }); it('allows navigation if user click confirm on the confirmation dialog', async () => { await testSubjects.click('applink-external-test'); - await testSubjects.existOrFail('navigationBlockConfirmModAl'); + await testSubjects.existOrFail('navigationBlockConfirmModal'); await PageObjects.common.clickConfirmOnModal(); expect(await browser.getCurrentUrl()).to.contain('/app/home'); }); }); - describe('when navigating to another app', () => { + describe('when navigating to the same app', () => { it('prevents navigation if user click cancel on the confirmation dialog', async () => { await testSubjects.click('applink-intra-test'); - await testSubjects.existOrFail('navigationBlockConfirmModAl'); + await testSubjects.existOrFail('navigationBlockConfirmModal'); await PageObjects.common.clickCancelOnModal(false); expect(await browser.getCurrentUrl()).to.contain('/app/core_history_block'); expect(await browser.getCurrentUrl()).not.to.contain('/foo'); @@ -48,7 +48,7 @@ export default function ({ getService, getPageObjects }: PluginFunctionalProvide it('allows navigation if user click confirm on the confirmation dialog', async () => { await testSubjects.click('applink-intra-test'); - await testSubjects.existOrFail('navigationBlockConfirmModAl'); + await testSubjects.existOrFail('navigationBlockConfirmModal'); await PageObjects.common.clickConfirmOnModal(); expect(await browser.getCurrentUrl()).to.contain('/app/core_history_block/foo'); }); From d1ed614467ee771b6ec0453834d3976418916b7b Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Mon, 22 Feb 2021 09:38:39 +0100 Subject: [PATCH 7/8] add new FTR test --- .../test_suites/core_plugins/history_block.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/plugin_functional/test_suites/core_plugins/history_block.ts b/test/plugin_functional/test_suites/core_plugins/history_block.ts index 96145f6ce3da79..61eea8be2d204e 100644 --- a/test/plugin_functional/test_suites/core_plugins/history_block.ts +++ b/test/plugin_functional/test_suites/core_plugins/history_block.ts @@ -52,6 +52,15 @@ export default function ({ getService, getPageObjects }: PluginFunctionalProvide await PageObjects.common.clickConfirmOnModal(); expect(await browser.getCurrentUrl()).to.contain('/app/core_history_block/foo'); }); + it('allows navigating back without prompt once the block handler has been disposed', async () => { + await testSubjects.click('applink-intra-test'); + await PageObjects.common.clickConfirmOnModal(); + expect(await browser.getCurrentUrl()).to.contain('/app/core_history_block/foo'); + + await testSubjects.click('applink-intra-test'); + expect(await browser.getCurrentUrl()).to.contain('/app/core_history_block'); + expect(await browser.getCurrentUrl()).not.to.contain('/foo'); + }); }); }); } From 6c4a505d088edc1e72a398a3476b8cdd67fcda1d Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Mon, 22 Feb 2021 10:16:47 +0100 Subject: [PATCH 8/8] fix added test --- .../plugin_functional/plugins/core_history_block/public/app.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/plugin_functional/plugins/core_history_block/public/app.tsx b/test/plugin_functional/plugins/core_history_block/public/app.tsx index 2f253ab31500c8..28866426f281b1 100644 --- a/test/plugin_functional/plugins/core_history_block/public/app.tsx +++ b/test/plugin_functional/plugins/core_history_block/public/app.tsx @@ -46,7 +46,7 @@ const FooPage = ({

FOO PAGE



- + Link to home on the same app