Rework leave confirmation dialogue to use "save/leave" text in buttons and add a cancel button (#233)

* Change text in buttons

* Add styles

* Update tests

* Fix strict mode violation for button selection

* Update Playwright Snapshots

* Add padding only for leave dialogue, not others

* Override Jupyter colours for "X" button SVG

Co-authored-by: Michał Krassowski <[email protected]>

* Subclass `Dialog` into `LeaveDialog` and add a container

* Update src/ui-components/LeaveConfirmation.tsx

Co-authored-by: Michał Krassowski <[email protected]>

---------

Co-authored-by: Michał Krassowski <[email protected]>

Author: agriyakhetarpal

src/sidebar.ts

@@ -1,10 +1,14 @@
 import { TabBar, Widget } from '@lumino/widgets';
 import { ILabShell, JupyterFrontEnd, JupyterFrontEndPlugin } from '@jupyterlab/application';
-import { Dialog, showDialog } from '@jupyterlab/apputils';
+import { Dialog } from '@jupyterlab/apputils';
 
 import { SidebarIcon } from './ui-components/SidebarIcon';
 import { EverywhereIcons } from './icons';
-import { LEAVE_CONFIRMATION_TITLE, LeaveConfirmation } from './ui-components/LeaveConfirmation';
+import {
+  LEAVE_CONFIRMATION_TITLE,
+  LeaveConfirmation,
+  LeaveDialog
+} from './ui-components/LeaveConfirmation';
 import { Commands } from './commands';
 
 import { INotebookTracker } from '@jupyterlab/notebook';
@@ -87,17 +91,32 @@ export const customSidebar: JupyterFrontEndPlugin<void> = {
               }
 
               // Non-empty regular notebook -> confirm and optionally save/share
-              const result = await showDialog({
+              const dialog = new LeaveDialog({
                 title: LEAVE_CONFIRMATION_TITLE,
                 body: new LeaveConfirmation(),
+                hasClose: true,
                 buttons: [
-                  Dialog.cancelButton({ label: 'Cancel' }),
-                  Dialog.okButton({ label: 'Yes' })
+                  Dialog.createButton({
+                    label: "Don't save and leave",
+                    accept: true,
+                    actions: ['leave-nosave']
+                  }),
+                  Dialog.okButton({
+                    label: 'Save and leave',
+                    actions: ['leave-save']
+                  })
                 ],
-                defaultButton: 0
+                defaultButton: 1
               });
 
-              if (result.button.label === 'Yes') {
+              const result = await dialog.launch();
+              const actions = result.button.actions ?? [];
+
+              if (actions.includes('leave-nosave')) {
+                window.location.href = '/index.html';
+                return;
+              }
+              if (actions.includes('leave-save')) {
                 try {
                   await app.commands.execute(Commands.shareNotebookCommand);
                 } catch (error) {

src/ui-components/LeaveConfirmation.tsx

@@ -1,4 +1,4 @@
-import { ReactWidget } from '@jupyterlab/apputils';
+import { Dialog, ReactWidget } from '@jupyterlab/apputils';
 import React from 'react';
 
 export const LEAVE_CONFIRMATION_TITLE =
@@ -26,3 +26,20 @@ export class LeaveConfirmation extends ReactWidget {
     );
   }
 }
+
+/**
+ * A dialog with a "close" button in the top-right corner, currently here
+ * because we only use hasClose in the leave confirmation dialog.
+ */
+export class LeaveDialog extends Dialog<void> {
+  constructor(
+    options: Partial<Dialog.IOptions<void>> & Pick<Dialog.IOptions<void>, 'title' | 'body'>
+  ) {
+    const normalized: Partial<Dialog.IOptions<void>> = {
+      host: document.body,
+      ...options
+    };
+    super(normalized);
+    this.addClass('je-LeaveDialog-container');
+  }
+}

style/base.css

@@ -68,6 +68,7 @@
 
 .jp-Dialog-footerButtons {
   text-align: center;
+  cursor: pointer;
 }
 
 .jp-Dialog {
@@ -86,6 +87,7 @@
   font-style: normal;
   font-weight: 500;
   line-height: 150%;
+  cursor: pointer;
 }
 
 .jp-Dialog-header {
@@ -360,3 +362,22 @@ consistent with code cells */
 .je-LeaveDialog.jp-Dialog-body {
   order: 2;
 }
+
+/* we reserve space for the close button */
+@supports selector(:has(*)) {
+  .jp-Dialog-content:has(> .je-LeaveDialog.jp-Dialog-body) > .jp-Dialog-header {
+    padding-right: 28px;
+  }
+}
+
+.jp-Dialog.je-LeaveDialog-container .jp-Dialog-close-button,
+.jp-Dialog-content.je-LeaveDialog-container .jp-Dialog-close-button {
+  position: absolute;
+  top: -16px;
+  right: -8px;
+  transform: scale(calc(var(--je-scale) * 1.5));
+}
+
+.jp-Dialog .jp-Dialog-close-button .jp-Icon {
+  --jp-inverse-layout-color3: var(--je-slate-blue);
+}

ui-tests/tests/jupytereverywhere.spec.ts

@@ -622,7 +622,7 @@ test.describe('Leave confirmation', () => {
     const dialog = page.locator('.jp-Dialog');
     await expect(dialog).toBeVisible();
 
-    await dialog.getByRole('button', { name: 'Cancel' }).click();
+    await dialog.locator('.jp-Dialog-close-button').click();
 
     await expect(page.locator('.jp-NotebookPanel')).toBeVisible();
     await expect(dialog).toHaveCount(0);
@@ -644,7 +644,7 @@ test.describe('Leave confirmation', () => {
     const dialog = page.locator('.jp-Dialog');
     await expect(dialog).toBeVisible();
 
-    await dialog.getByRole('button', { name: 'Yes' }).click();
+    await dialog.getByRole('button', { name: 'Save and leave', exact: true }).click();
 
     const shareDialog = page.locator('.jp-Dialog-content');
     await expect(shareDialog).toBeVisible();