Skip to content

Commit

Permalink
fix(workbench/view): prevent closing views with a pending CanClose
Browse files Browse the repository at this point in the history
…guard

The router no longer invokes a view's `CanClose` guard if the view is already pending confirmation from a previous close attempt.
  • Loading branch information
danielwiehl authored and Marcarrian committed Nov 25, 2024
1 parent ecd52b3 commit 4326a63
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 4 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/*
* Copyright (c) 2018-2024 Swiss Federal Railways
*
* This program and the accompanying materials are made
* available under the terms of the Eclipse Public License 2.0
* which is available at https://www.eclipse.org/legal/epl-2.0/
*
* SPDX-License-Identifier: EPL-2.0
*/

import {Component, inject} from '@angular/core';
import {WorkbenchView} from '@scion/workbench';
import {noop} from 'rxjs';

@Component({
selector: 'app-blocking-can-close-test-page',
template: 'Blocking CanClose Test Page',
standalone: true,
})
export default class BlockingCanCloseTestPageComponent {

constructor() {
inject(WorkbenchView).canClose(() => {
console.debug('[BlockingCanCloseTestPageComponent] BLOCKING');
return new Promise<boolean>(noop);
});
}
}
4 changes: 4 additions & 0 deletions apps/workbench-testing-app/src/app/test-pages/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,10 @@ export default [
path: 'size-test-page',
loadComponent: () => import('./size-test-page/size-test-page.component'),
},
{
path: 'blocking-can-close-test-page',
loadComponent: () => import('./blocking-can-close-test-page/blocking-can-close-test-page.component'),
},
] satisfies Routes;

/**
Expand Down
32 changes: 32 additions & 0 deletions projects/scion/e2e-testing/src/workbench/view.e2e-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -830,6 +830,38 @@ test.describe('Workbench View', () => {
await expectView(testViewPage).not.toBeAttached();
});

test('should not invoke `CanClose` guard while blocking', async ({appPO, workbenchNavigator, consoleLogs}) => {
await appPO.navigateTo({microfrontendSupport: false});

// Open view.
const routerPage = await workbenchNavigator.openInNewTab(RouterPagePO);
await routerPage.navigate(['test-pages/blocking-can-close-test-page'], {target: 'view.100'});
const testView = appPO.view({viewId: 'view.100'});

// Close view.
await testView.tab.close();

// Expect `CanClose` guard to be invoked.
await expect.poll(() => consoleLogs.get()).toContain('[BlockingCanCloseTestPageComponent] BLOCKING');

// Clear log.
consoleLogs.clear();

// Expect view not to be closable until `CanClose` guard is released.
await expect(testView.tab.closeButton).not.toBeVisible();

// Close all views.
await routerPage.view.tab.click();
const contextMenu = await routerPage.view.tab.openContextMenu();
await contextMenu.menuItems.closeAll.click();

// Expect `CanClose` guard not to be invoked again.
await expect.poll(() => consoleLogs.get()).not.toContain('[BlockingCanCloseTestPageComponent] BLOCKING');

// Expect view not to be closable until `CanClose` guard is released.
await expect(testView.tab.closeButton).not.toBeVisible();
});

test(`should disable context menu 'Close tab' for 'non-closable' view`, async ({appPO, workbenchNavigator}) => {
await appPO.navigateTo({microfrontendSupport: false});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -198,11 +198,18 @@ export class ɵWorkbenchRouter implements WorkbenchRouter {
.map(async view => {
// Capture current navigation id to not proceed closing if navigated in the meantime.
const navigationId = view.navigationId;
const close = await view.canCloseGuard!();
if (close && view.navigationId === navigationId) {
return this.navigate(layout => layout.removeView(view.id, {force: true}));
// Make view non-closable to prevent closing the view again while awaiting closing confirmation.
view.closable = false;
try {
const close = await view.canCloseGuard!();
if (close && view.navigationId === navigationId) {
return this.navigate(layout => layout.removeView(view.id, {force: true}));
}
return true;
}
finally {
view.closable = true;
}
return true;
});

// Wait for all `CanClose` guards to resolve and subsequent navigations to complete.
Expand Down

0 comments on commit 4326a63

Please sign in to comment.