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

Close the browser tab when clicking on "Close and Shut Down Notebook" #6937

Merged
merged 12 commits into from
Jun 22, 2023
8 changes: 8 additions & 0 deletions packages/application-extension/schema/menus.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,14 @@
"command": "notebook:trust",
"rank": 20
},
{
"type": "separator",
"rank": 30
},
{
"command": "filemenu:close-and-cleanup",
"rank": 40
},
{
"command": "application:close",
"disabled": true
Expand Down
39 changes: 38 additions & 1 deletion packages/notebook-extension/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import { Text, Time } from '@jupyterlab/coreutils';

import { IDocumentManager } from '@jupyterlab/docmanager';

import { IMainMenu } from '@jupyterlab/mainmenu';

import {
NotebookPanel,
INotebookTracker,
Expand All @@ -26,7 +28,7 @@ import {

import { ISettingRegistry } from '@jupyterlab/settingregistry';

import { ITranslator } from '@jupyterlab/translation';
import { ITranslator, nullTranslator } from '@jupyterlab/translation';

import { INotebookShell } from '@jupyter-notebook/application';

Expand Down Expand Up @@ -126,6 +128,40 @@ const checkpoints: JupyterFrontEndPlugin<void> = {
},
};

/**
* Add a command to close the browser tab when clicking on "Close and Shut Down"
*/
const closeTab: JupyterFrontEndPlugin<void> = {
id: '@jupyter-notebook/notebook-extension:close-tab',
autoStart: true,
requires: [IMainMenu],
optional: [ITranslator],
activate: (
app: JupyterFrontEnd,
menu: IMainMenu,
translator: ITranslator | null
) => {
const { commands } = app;
translator = translator ?? nullTranslator;
const trans = translator.load('notebook');

const id = 'notebook:close-and-halt';
commands.addCommand(id, {
label: trans.__('Close and Halt'),
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
label: trans.__('Close and Halt'),
label: trans.__('Close Notebook and Shutdown Kernel'),

Copy link
Member Author

Choose a reason for hiding this comment

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

If going with this suggestion then we'll likely need to use Shut Down instead of Shutdown for consistency with the other menu entries.

execute: async () => {
await commands.execute('notebook:close-and-shutdown');
window.close();
},
});
menu.fileMenu.closeAndCleaners.add({
id,
// use a small rank to it takes precedence over the default
// shut down action for the notebook
rank: 0,
});
},
};

/**
* The kernel logo plugin.
*/
Expand Down Expand Up @@ -402,6 +438,7 @@ const trusted: JupyterFrontEndPlugin<void> = {
*/
const plugins: JupyterFrontEndPlugin<any>[] = [
checkpoints,
closeTab,
kernelLogo,
kernelStatus,
scrollOutput,
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
20 changes: 20 additions & 0 deletions ui-tests/test/notebook.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,4 +155,24 @@ test.describe('Notebook', () => {
const imageName = 'notebooktools-right-panel.png';
expect(await panel.screenshot()).toMatchSnapshot(imageName);
});

test('Clicking on "Close and Halt" should close the browser tab', async ({
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
test('Clicking on "Close and Halt" should close the browser tab', async ({
test('Clicking on "Close Notebook and Shutdown Kernel" should close the browser tab', async ({

page,
tmpPath,
}) => {
const notebook = 'simple.ipynb';
await page.contents.uploadFile(
path.resolve(__dirname, `./notebooks/${notebook}`),
`${tmpPath}/${notebook}`
);
await page.goto(`notebooks/${tmpPath}/${notebook}`);

const menuPath = 'File>Close and Halt';
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const menuPath = 'File>Close and Halt';
const menuPath = 'File>Close Notebook and Shutdown Kernel';

await page.menu.clickMenuItem(menuPath);

// Press Enter to confirm the dialog
await page.keyboard.press('Enter');

expect(page.isClosed());
});
});