Minor fixes and improvements on frontend extension (mwouts#1178)

* refactor: Improve settings readability Signed-off-by: mahendrapaipuri <[email protected]> * refactor: Remove utility command from palette Signed-off-by: mahendrapaipuri <[email protected]> * test: Update test snapshot Signed-off-by: mahendrapaipuri <[email protected]> * refactor: Move New Text Notebook to File menu * Remove Category settings option Signed-off-by: mahendrapaipuri <[email protected]> * test: Update UI tests * Save full page screenshot so that we can use them in docs Signed-off-by: mahendrapaipuri <[email protected]> * test: Add new UI test for launcher Signed-off-by: mahendrapaipuri <[email protected]> * docs: Use UI test screenshots in docs Signed-off-by: mahendrapaipuri <[email protected]> * refactor: Improve naming for better readability Signed-off-by: mahendrapaipuri <[email protected]> * refactor: Remove desc in settings * Titles are already self explanatory Signed-off-by: mahendrapaipuri <[email protected]> * test: Add opt dep for ui tests * Install Calysto bash kernel for UI tests * To ensure non standard kernels are working as well Signed-off-by: mahendrapaipuri <[email protected]> * refactor: Use generic language name for items * Avoid using kernelspec display names Signed-off-by: mahendrapaipuri <[email protected]> * test: Add bash kernel UI test Signed-off-by: mahendrapaipuri <[email protected]> * test: Update test snapshots Signed-off-by: mahendrapaipuri <[email protected]> * refactor: Move jupytext menu into Filemenu * See [discussion](mwouts#1178 (comment)) Signed-off-by: mahendrapaipuri <[email protected]> * test: Update ui tests and snapshots Signed-off-by: mahendrapaipuri <[email protected]> --------- Signed-off-by: mahendrapaipuri <[email protected]>
mahendrapaipuri · Dec 2, 2023 · 7223622 · 7223622
1 parent 2bf03e6
commit 7223622
Show file tree

Hide file tree

Showing 27 changed files with 139 additions and 118 deletions.
diff --git a/.github/workflows/step_tests-ui.yml b/.github/workflows/step_tests-ui.yml
@@ -21,7 +21,7 @@ jobs:
         uses: jupyterlab/maintainer-tools/.github/actions/base-setup@v1
 
       - name: Install from source
-        run: python -m pip install -e .
+        run: python -m pip install -e '.[test-ui]'
 
       - name: Install galata
         working-directory: jupyterlab/packages/jupyterlab-jupytext-extension/ui-tests

diff --git a/docs/jupyterlab-extension.md b/docs/jupyterlab-extension.md
@@ -11,21 +11,24 @@ notebooks as they launch a regular notebook from JupyterLab launcher.
 After installing Jupytext extension, users will have a new category in the launcher
 called Jupytext as shown below:
 
-![](images/jupyterlab_launcher.png)
+![](../jupyterlab/packages/jupyterlab-jupytext-extension/ui-tests/tests/jupytext-launcher.spec.ts-snapshots/launcher-category-jupytext-linux.png)
 
 Users can remove and/or add new formats to the Jupytext section _via_ `Settings>Jupytext`.
 
-![](images/jupyterlab_settings.png)
+![](../jupyterlab/packages/jupyterlab-jupytext-extension/ui-tests/tests/jupytext-settings.spec.ts-snapshots/jupytext-settings-panel-jupytext-linux.png)
 
-By clicking `Add` and adding a new format, say `qmd`, will
-add the Quatro Text Notebook to the launcher. **Note** that users need to refresh the current browser tab when they modify the settings for them to take effect.
-
-Besides, users can also change the category of Text Notebook launcher icons using
-`Category` field in the Settings. For example, by using `Notebook` as category, all
-Text Notebook launchers will be moved into `Notebook` category.
+By (un)selecting different formats, users can filter the items in launcher and main menu. **Note** that users need to refresh the current browser tab when they modify the settings for them to take effect.
 
 ## Main menu
 
-It is also possible to launch text notebooks and/or pair existing notebooks from the dedicated Jupytext main menu as shown below.
+It is also possible to create new text notebooks and/or pair existing notebooks from main menu and dedicated Jupytext main menu, respectively.
+
+Following screenshot shows the `New Text Notebook` submenu available in `File` menu.
+
+![](../jupyterlab/packages/jupyterlab-jupytext-extension/ui-tests/tests/jupytext-menu.spec.ts-snapshots/opened-jupytext-menu-file-new-text-notebook-jupytext-linux.png)
+
+Similarly, to pair existing notebooks, users can go to `Jupytext` menu on main menu as shown below:
+
+![](../jupyterlab/packages/jupyterlab-jupytext-extension/ui-tests/tests/jupytext-menu.spec.ts-snapshots/opened-jupytext-menu-jupytext-pair-notebook-jupytext-linux.png)
 
-![](images/jupyterlab_main_menu.png)
+All the options are greyed out in the above screenshot as there is no notebook widget currently active. Once the user opens a new notebook, the options will become available.
diff --git a/jupyterlab/packages/jupyterlab-jupytext-extension/schema/plugin.json b/jupyterlab/packages/jupyterlab-jupytext-extension/schema/plugin.json
@@ -1,6 +1,6 @@
 {
   "title": "Jupytext",
-  "description": "List of Jupytext Text Notebook formats that will be added to launcher.\n\nWhen any of percent, light, hydrogen and nomarker formats is selected, all the available kernel variants will be added to launcher. For instance, if Python and R kernels are installed, percent format will add launcher items to create Python and R Text notebooks with percent format.\n\nRefresh the current browser tab for the changes to take effect.",
+  "description": "List of Jupytext Text Notebook formats that will be added to launcher and jupytext menu.",
   "properties": {
     "auto:percent": {
       "type": "boolean",
@@ -42,11 +42,9 @@
       "title": "Quarto Markdown Notebook",
       "default": false
     },
-    "category": {
-      "title": "Category",
-      "description": "Category under which Jupytext Text Notebooks will be placed.",
-      "type": "string",
-      "default": "Jupytext"
+    "NOTE": {
+      "description": "Refresh the current browser tab for the changes to take effect.",
+      "type": "null"
     }
   },
   "additionalProperties": false,

diff --git a/jupyterlab/packages/jupyterlab-jupytext-extension/src/index.ts b/jupyterlab/packages/jupyterlab-jupytext-extension/src/index.ts
@@ -70,7 +70,7 @@ import { registerFileTypes } from './registry';
 import { createFactory } from './factory';
 
 import {
-  getAvailableKernelFileTypes,
+  getAvailableKernelLanguages,
   getAvailableCreateTextNotebookCommands,
   createNewTextNotebook,
 } from './utils';
@@ -123,7 +123,6 @@ const extension: JupyterFrontEndPlugin<void> = {
 
     // Load settings
     const includeFormats = TEXT_NOTEBOOKS_LAUNCHER_ICONS;
-    let launcherItemsCategory = 'Jupytext';
     if (settingRegistry) {
       const settings = await settingRegistry.load(extension.id);
       for (const format of JUPYTEXT_FORMATS) {
@@ -134,40 +133,30 @@ const extension: JupyterFrontEndPlugin<void> = {
           includeFormats.splice(includeFormats.indexOf(format), 1);
         }
       }
-      launcherItemsCategory = settings.get('category').composite as string;
     }
 
     // Unpack necessary components
     const { commands, serviceManager, docRegistry } = app;
 
-    // Initialise Jupytext menu and add it to main menu
-    const jupytextMenu = new Menu({ commands: app.commands });
-    mainmenu.addMenu(jupytextMenu, true, { rank: 40 });
-    jupytextMenu.id = 'jp-mainmenu-jupytext-menu';
-    jupytextMenu.title.label = trans.__('Jupytext');
-
-    // Initialise Jupytext create notebook submenu and add it to Jupytext menu
+    // Initialise Jupytext create notebook submenu and add it to File menu
     const jupytextCreateMenu = new Menu({ commands: app.commands });
     jupytextCreateMenu.id = 'jp-mainmenu-jupytext-new-menu';
     jupytextCreateMenu.title.label = trans.__('New Text Notebook');
-    jupytextMenu.addItem({
+    mainmenu.fileMenu.addItem({
+      rank: 0.97,
       type: 'submenu',
       submenu: jupytextCreateMenu,
     });
 
-    // Initialise Jupytext pair notebook submenu and add it to Jupytext menu
-    const jupytextPairMenu = new Menu({ commands: app.commands });
-    jupytextPairMenu.id = 'jp-mainmenu-jupytext-pair-menu';
-    jupytextPairMenu.title.label = trans.__('Pair Notebook');
-    jupytextMenu.addItem({
+    // Initialise Jupytext menu and add it to main menu
+    const jupytextMenu = new Menu({ commands: app.commands });
+    mainmenu.fileMenu.addItem({
+      rank: 0.98,
       type: 'submenu',
-      submenu: jupytextPairMenu,
-    });
-
-    // Add a separator
-    jupytextMenu.addItem({
-      type: 'separator',
+      submenu: jupytextMenu,
     });
+    jupytextMenu.id = 'jp-mainmenu-jupytext-menu';
+    jupytextMenu.title.label = trans.__('Jupytext');
 
     // Get all Jupytext formats
     let rank = 0;
@@ -224,7 +213,7 @@ const extension: JupyterFrontEndPlugin<void> = {
             category: 'Jupytext',
           });
           // Add to jupytext pair menu
-          jupytextPairMenu.addItem({
+          jupytextMenu.addItem({
             command: command,
           });
           if (fileType.separator) {
@@ -237,7 +226,7 @@ const extension: JupyterFrontEndPlugin<void> = {
 
     // Add separators in jupytext pair menu
     separatorIndex.map((index, idx) => {
-      jupytextPairMenu.insertItem(index + idx + 1, {
+      jupytextMenu.insertItem(index + idx + 1, {
         type: 'separator',
       });
     });
@@ -274,6 +263,9 @@ const extension: JupyterFrontEndPlugin<void> = {
     jupytextMenu.addItem({
       command: CommandIDs.metadata,
     });
+    jupytextMenu.addItem({
+      type: 'separator',
+    });
 
     // Register Jupytext FAQ command
     commands.addCommand(CommandIDs.faq, {
@@ -349,14 +341,16 @@ const extension: JupyterFrontEndPlugin<void> = {
       label: (args) =>
         (args['label'] as string) || `New ${args['type'] as string}`,
     });
-    palette?.addItem({
-      command: CommandIDs.newUntitled,
-      rank: 50,
-      category: 'Jupytext',
-    });
-
-    // Get a map of available kernels in current widget
-    const availableKernels = await getAvailableKernelFileTypes(
+    // We dont need to add this command to palettte as it is a utility one
+    // which does not have direct usage
+    // palette?.addItem({
+    //   command: CommandIDs.newUntitled,
+    //   rank: 50,
+    //   category: 'Jupytext',
+    // });
+
+    // Get a map of available kernel languages in current widget
+    const availableKernelLanguages = await getAvailableKernelLanguages(
       languages,
       serviceManager
     );
@@ -365,22 +359,22 @@ const extension: JupyterFrontEndPlugin<void> = {
     const createTextNotebookCommands =
       await getAvailableCreateTextNotebookCommands(
         includeFormats,
-        availableKernels
+        availableKernelLanguages
       );
 
     // Register Jupytext text notebooks file types
     registerFileTypes(docRegistry, trans);
 
     // Get all kernel file types to add to Jupytext factory
-    const kernelFileTypeNames = [];
-    for (const kernelFileTypes of availableKernels.values()) {
-      for (const kernelFileType of kernelFileTypes) {
-        kernelFileTypeNames.push(kernelFileType.kernelName);
+    const kernelLanguageNames = [];
+    for (const kernelLanguages of availableKernelLanguages.values()) {
+      for (const kernelLanguage of kernelLanguages) {
+        kernelLanguageNames.push(kernelLanguage.kernelName);
       }
     }
     // Create a factory for Jupytext
     createFactory(
-      kernelFileTypeNames,
+      kernelLanguageNames,
       toolbarRegistry,
       settingRegistry,
       docRegistry,
@@ -489,7 +483,7 @@ const extension: JupyterFrontEndPlugin<void> = {
                 launcher.add({
                   command: command,
                   args: { isLauncher: true, kernelName: fileType.kernelName },
-                  category: trans.__(launcherItemsCategory),
+                  category: trans.__('Jupytext'),
                   rank: rank++,
                   kernelIconUrl,
                   metadata: {

diff --git a/jupyterlab/packages/jupyterlab-jupytext-extension/src/utils.ts b/jupyterlab/packages/jupyterlab-jupytext-extension/src/utils.ts
@@ -97,10 +97,10 @@ async function getKernelIcon(
 }
 
 /**
- * Get all available kernel file types so that we replace auto format
- * with these file extensions
+ * Get all available kernel languages so that we replace auto format
+ * with these language file extensions
  */
-export async function getAvailableKernelFileTypes(
+export async function getAvailableKernelLanguages(
   languages: IEditorLanguageRegistry,
   serviceManager: ServiceManager.IManager
 ): Promise<Map<string, IFileTypeData[]>> {
@@ -124,13 +124,15 @@ export async function getAvailableKernelFileTypes(
           // We attempt to get kernelIcon here for specModel.resources
           // If none provided, we return generic kernel icon
           const kernelIcon = await getKernelIcon(specModel);
+          const displayName =
+            languageInfo.displayName || specModel.display_name;
           const exts: IFileTypeData[] = [
             {
               fileExt: languageInfo.extensions[0],
-              paletteLabel: `New ${languageInfo.displayName} Text Notebook`,
-              caption: `Create a new ${languageInfo.displayName} Text Notebook`,
+              paletteLabel: `New ${displayName} Text Notebook`,
+              caption: `Create a new ${displayName} Text Notebook`,
               kernelIcon: kernelIcon,
-              launcherLabel: specModel.display_name || languageInfo.displayName,
+              launcherLabel: displayName,
               kernelName: spec,
             },
           ];
@@ -147,10 +149,10 @@ export async function getAvailableKernelFileTypes(
  * formats and available kernels
  */
 export async function getAvailableCreateTextNotebookCommands(
-  launcherItems: string[],
-  availableKernels: Map<string, IFileTypeData[]>
+  includeFormats: string[],
+  availableKernelLanguages: Map<string, IFileTypeData[]>
 ): Promise<Map<string, IFileTypeData[]>> {
-  const numKernels = availableKernels.size;
+  const numKernels = availableKernelLanguages.size;
 
   // Initialise a map of 'Create New Text Notebook' command filetypes
   const createTextNotebookCommands = new Map<string, IFileTypeData[]>();
@@ -165,27 +167,31 @@ export async function getAvailableCreateTextNotebookCommands(
         if (format.startsWith('auto')) {
           const formatType = format.split(':')[1];
           let mapIndex = 0;
-          availableKernels.forEach(
-            (kernelFileTypes: IFileTypeData[], kernelKey: string) => {
+          availableKernelLanguages.forEach(
+            (kernelLanguages: IFileTypeData[], kernelKey: string) => {
               const updatedKernelKey = `${kernelKey}:${formatType}`;
               createTextNotebookCommands.set(updatedKernelKey, []);
               mapIndex += 1;
-              kernelFileTypes.map((kernelFileType) => {
+              kernelLanguages.map((kernelLanguageFileType) => {
                 // Merge fileType object from kernel and Jupytext format and push
                 // it to createTextNotebookCommands
-                const updatedKernelFileType = { ...kernelFileType };
-                updatedKernelFileType.fileExt = `${updatedKernelFileType.fileExt}:${formatType}`;
-                updatedKernelFileType.paletteLabel = `${updatedKernelFileType.paletteLabel} with ${fileType.paletteLabel}`;
-                updatedKernelFileType.caption = `${updatedKernelFileType.caption} with ${fileType.caption}`;
-                updatedKernelFileType.launcherLabel = `${updatedKernelFileType.launcherLabel} - ${fileType.launcherLabel}`;
+                const updatedKernelLanguageFileType = {
+                  ...kernelLanguageFileType,
+                };
+                updatedKernelLanguageFileType.fileExt = `${updatedKernelLanguageFileType.fileExt}:${formatType}`;
+                updatedKernelLanguageFileType.paletteLabel = `${updatedKernelLanguageFileType.paletteLabel} with ${fileType.paletteLabel}`;
+                updatedKernelLanguageFileType.caption = `${updatedKernelLanguageFileType.caption} with ${fileType.caption}`;
+                updatedKernelLanguageFileType.launcherLabel = `${updatedKernelLanguageFileType.launcherLabel} - ${fileType.launcherLabel}`;
                 if (numKernels === mapIndex) {
-                  updatedKernelFileType.separator = true;
+                  updatedKernelLanguageFileType.separator = true;
                 }
                 createTextNotebookCommands
                   .get(updatedKernelKey)
-                  .push(updatedKernelFileType);
-                if (launcherItems.includes(format)) {
-                  launcherItems.push(updatedKernelFileType.fileExt);
+                  .push(updatedKernelLanguageFileType);
+                // Update includeFormats with the language specific formats
+                // Effectiviely we will add formats like py:light, js:light here
+                if (includeFormats.includes(format)) {
+                  includeFormats.push(updatedKernelLanguageFileType.fileExt);
                 }
               });
             }

diff --git a/jupyterlab/packages/jupyterlab-jupytext-extension/ui-tests/tests/jupytext-launcher.spec.ts b/jupyterlab/packages/jupyterlab-jupytext-extension/ui-tests/tests/jupytext-launcher.spec.ts
@@ -0,0 +1,12 @@
+import { expect, test } from '@jupyterlab/galata';
+
+test.describe('Jupytext Launcher Category', () => {
+  test.use({ autoGoto: false, viewport: { height: 1020, width: 1280 } });
+  test('should have Jupytext category in launcher', async ({ page }) => {
+    await page.goto();
+    await page.waitForSelector('.jp-LauncherCard-label');
+
+    const imageName = 'launcher-category.png';
+    expect(await page.screenshot()).toMatchSnapshot(imageName.toLowerCase());
+  });
+});
diff --git a/.../tests/jupytext-launcher.spec.ts-snapshots/launcher-category-jupytext-linux.png b/.../tests/jupytext-launcher.spec.ts-snapshots/launcher-category-jupytext-linux.png
diff --git a/jupyterlab/packages/jupyterlab-jupytext-extension/ui-tests/tests/jupytext-menu.spec.ts b/jupyterlab/packages/jupyterlab-jupytext-extension/ui-tests/tests/jupytext-menu.spec.ts
@@ -1,11 +1,7 @@
 import { expect, test } from '@jupyterlab/galata';
 
 // Main Jupytext menu
-const jupytextMenu = [
-  'Jupytext',
-  'Jupytext>New Text Notebook',
-  'Jupytext>Pair Notebook',
-];
+const jupytextMenu = ['File>New Text Notebook', 'File>Jupytext'];
 
 test.describe('Jupytext Menu Tests', () => {
   test.use({ autoGoto: false });
@@ -20,8 +16,8 @@ test.describe('Jupytext Menu Tests', () => {
         />/g,
         '-'
       )}.png`;
-      const menu = await page.menu.getOpenMenu();
-      expect(await menu!.screenshot()).toMatchSnapshot(imageName.toLowerCase());
+      // const menu = await page.menu.getOpenMenu();
+      expect(await page!.screenshot()).toMatchSnapshot(imageName.toLowerCase());
     });
   });
 });
diff --git a/...xt-menu.spec.ts-snapshots/opened-jupytext-menu-file-jupytext-jupytext-linux.png b/...xt-menu.spec.ts-snapshots/opened-jupytext-menu-file-jupytext-jupytext-linux.png
diff --git a/...pec.ts-snapshots/opened-jupytext-menu-file-new-text-notebook-jupytext-linux.png b/...pec.ts-snapshots/opened-jupytext-menu-file-new-text-notebook-jupytext-linux.png
diff --git a/...upytext-menu.spec.ts-snapshots/opened-jupytext-menu-jupytext-jupytext-linux.png b/...upytext-menu.spec.ts-snapshots/opened-jupytext-menu-jupytext-jupytext-linux.png
diff --git a/...ts-snapshots/opened-jupytext-menu-jupytext-new-text-notebook-jupytext-linux.png b/...ts-snapshots/opened-jupytext-menu-jupytext-new-text-notebook-jupytext-linux.png
diff --git a/...pec.ts-snapshots/opened-jupytext-menu-jupytext-pair-notebook-jupytext-linux.png b/...pec.ts-snapshots/opened-jupytext-menu-jupytext-pair-notebook-jupytext-linux.png