[Notebook] hide trashcan during edit; add shortcut keys

e2e/helper/notebookUtils.js

@@ -73,7 +73,25 @@ async function dragAndDropEmbed(page, notebookObject) {
  */
 async function commitEntry(page) {
   //Click the Commit Entry button
-  await page.locator('.c-ne__save-button > button').click();
+  await page.getByLabel('Save notebook entry').click();
+}
+
+/**
+ * @private
+ * @param {import('@playwright/test').Page} page
+ */
+async function commitEntryViaShortcutKey(page) {
+  //Click the Commit Entry button
+  await page.keyboard.press('Control+Enter');
+}
+
+/**
+ * @private
+ * @param {import('@playwright/test').Page} page
+ */
+async function cancelEntry(page) {
+  //Press the Escape key to cancel editing entry
+  await page.keyboard.press('Escape');
 }
 
 /**
@@ -153,7 +171,9 @@ async function createNotebookEntryAndTags(page, iterations = 1) {
 
 export {
   addNotebookEntry,
+  cancelEntry,
   commitEntry,
+  commitEntryViaShortcutKey,
   createNotebookAndEntry,
   createNotebookEntryAndTags,
   dragAndDropEmbed,

e2e/tests/functional/plugins/notebook/notebook.e2e.spec.js

@@ -298,7 +298,133 @@ test.describe('Notebook entry tests', () => {
     await page.locator('.c-notebook__drag-area').click();
     await expect(page.getByLabel('Notebook Entry Input')).toBeVisible();
     await expect(page.getByLabel('Notebook Entry', { exact: true })).toHaveClass(/is-selected/);
+    await expect(page.getByLabel('Save notebook entry')).toBeVisible();
   });
+
+  test("A new entry that's being created can be saved via ctrl+enter shortcut", async ({
+    page
+  }) => {
+    // Navigate to the notebook object
+    await page.goto(notebookObject.url);
+
+    // Click .c-notebook__drag-area to create a new notebook entry
+    await page.locator('.c-notebook__drag-area').click();
+
+    //verify visible elements
+    await expect(page.getByLabel('Notebook Entry Input')).toBeVisible();
+    await expect(page.getByLabel('Delete this entry')).toBeHidden();
+    await expect(page.getByLabel('Notebook Entry', { exact: true })).toHaveClass(/is-selected/);
+    await expect(page.getByLabel('Save notebook entry')).toBeVisible();
+
+    //add text to textarea
+    const testText = 'test text';
+    await page.getByLabel('Notebook Entry Input').fill(testText);
+
+    const textareaVal = await page.getByLabel('Notebook Entry Input').inputValue();
+    await expect(textareaVal).toBe(testText);
+
+    //press "Control+Enter" to save
+    await nbUtils.commitEntryViaShortcutKey(page);
+
+    //verify notebook entry is saved
+    await expect(page.locator('.c-ne__input')).toBeHidden();
+    await expect(page.getByLabel('Notebook Entry Input')).toBeHidden();
+    await expect(page.getByLabel('Save notebook entry')).toBeHidden();
+
+    await expect(page.getByLabel('Delete this entry')).toBeVisible();
+    await expect(page.getByLabel('Notebook Entry Display')).toBeVisible();
+    await expect(page.getByLabel('Notebook Entry Display')).toContainText(testText);
+  });
+
+  test("A new entry that's being created can be canceled via the escape key", async ({ page }) => {
+    // Navigate to the notebook object
+    await page.goto(notebookObject.url);
+
+    // Click .c-notebook__drag-area to create a new notebook entry
+    await page.locator('.c-notebook__drag-area').click();
+
+    //verify visible elements
+    await expect(page.getByLabel('Notebook Entry Input')).toBeVisible();
+    await expect(page.getByLabel('Delete this entry')).toBeHidden();
+    await expect(page.getByLabel('Notebook Entry', { exact: true })).toHaveClass(/is-selected/);
+    await expect(page.getByLabel('Save notebook entry')).toBeVisible();
+
+    //add text to textarea
+    const testText = 'test text';
+    await page.getByLabel('Notebook Entry Input').fill(testText);
+
+    const textareaVal = await page.getByLabel('Notebook Entry Input').inputValue();
+    await expect(textareaVal).toBe(testText);
+
+    //press "Escape" key to cancel
+    await nbUtils.cancelEntry(page);
+
+    //verify notebook entry is gone
+    await expect(page.locator('.c-ne__input')).toBeHidden();
+    await expect(page.getByLabel('Notebook Entry Input')).toBeHidden();
+    await expect(page.getByLabel('Delete this entry')).toBeHidden();
+    await expect(page.getByLabel('Save notebook entry')).toBeHidden();
+  });
+
+  test("An existing entry that's being edited can be canceled via the escape key", async ({
+    page
+  }) => {
+    // Navigate to the notebook object
+    await page.goto(notebookObject.url);
+
+    const testText = 'test text';
+    const otherText = 'other text';
+
+    //create notebook entry
+    await nbUtils.enterTextEntry(page, testText);
+
+    //verify entry
+    await expect(page.getByLabel('Notebook Entry', { exact: true })).toBeVisible();
+    await expect(page.getByLabel('Notebook Entry Display')).toContainText(testText);
+
+    //make entry the active selection
+    await page.getByLabel('Notebook Entry', { exact: true }).click();
+
+    //edit entry (make textarea visible for editing)
+    await page.getByLabel('Notebook Entry Display', { exact: true }).click();
+    await expect(page.getByLabel('Notebook Entry Input')).toBeVisible();
+
+    //edit textarea value
+    await page.getByLabel('Notebook Entry Input').fill(otherText);
+    const textareaVal = await page.getByLabel('Notebook Entry Input').inputValue();
+    await expect(textareaVal).toBe(otherText);
+
+    //press "Escape" key
+    await nbUtils.cancelEntry(page);
+
+    //verify entry reverts back to the original value
+    await expect(page.getByLabel('Notebook Entry Input')).toBeHidden();
+    await expect(page.getByLabel('Notebook Entry Display')).toContainText(testText);
+    await expect(page.getByLabel('Notebook Entry Display')).not.toContainText(otherText);
+  });
+
+  test('The trashcan is not visible when notebook entry is in edit mode', async ({ page }) => {
+    // Navigate to the notebook object
+    await page.goto(notebookObject.url);
+
+    const testText = 'test text';
+
+    //create notebook entry
+    await nbUtils.enterTextEntry(page, testText);
+
+    //verify entry
+    await expect(page.getByLabel('Notebook Entry', { exact: true })).toBeVisible();
+    await expect(page.getByLabel('Delete this entry', { exact: true })).toBeVisible();
+
+    //make entry the active selection
+    await page.getByLabel('Notebook Entry', { exact: true }).click();
+
+    //edit entry (make textarea visible for editing)
+    await page.getByLabel('Notebook Entry Display', { exact: true }).click();
+    await expect(page.getByLabel('Notebook Entry Input')).toBeVisible();
+    await expect(page.getByLabel('Delete this entry', { exact: true })).toBeHidden();
+  });
+
   test('When an object is dropped into a notebook, a new entry is created and it should be focused', async ({
     page
   }) => {
@@ -351,7 +477,38 @@ test.describe('Notebook entry tests', () => {
     await expect(embed).toHaveClass(/icon-plot-overlay/);
     expect(embedName).toBe(overlayPlot.name);
   });
-  test.fixme('new entries persist through navigation events without save', async ({ page }) => {});
+
+  test('new entries persist through navigation events without save', async ({ page }) => {
+    // Navigate to the notebook object
+    await page.goto(notebookObject.url);
+
+    // Click .c-notebook__drag-area to create a new notebook entry
+    await page.locator('.c-notebook__drag-area').click();
+
+    //verify visible elements
+    await expect(page.getByLabel('Notebook Entry', { exact: true })).toBeVisible();
+    await expect(page.getByLabel('Notebook Entry Input')).toBeVisible();
+
+    //add text to textarea
+    const testText = 'test text';
+    await page.getByLabel('Notebook Entry Input').fill(testText);
+
+    const textareaVal = await page.getByLabel('Notebook Entry Input').inputValue();
+    await expect(textareaVal).toBe(testText);
+
+    //navigate away from notebook without saving the new notebook entry
+    await page.getByLabel('Navigate up to parent').click();
+    await page.goto('./', { waitUntil: 'domcontentloaded' });
+
+    await expect(page.getByLabel('Notebook Entry', { exact: true })).toBeHidden();
+
+    //navigate back to notebook
+    await page.goto(notebookObject.url);
+
+    await expect(page.getByLabel('Notebook Entry', { exact: true })).toBeVisible();
+    await expect(page.getByLabel('Notebook Entry Display')).toContainText(testText);
+  });
+
   test('previous and new entries can be deleted', async ({ page }) => {
     // Navigate to the notebook object
     await page.goto(notebookObject.url);

e2e/tests/functional/plugins/notebook/notebookSnapshotImage.e2e.spec.js

@@ -60,7 +60,7 @@ test.describe('Snapshot image tests', () => {
     }, fileData);
 
     await page.dispatchEvent('.c-notebook__drag-area', 'drop', { dataTransfer: dropTransfer });
-    await page.locator('.c-ne__save-button > button').click();
+    await page.getByLabel('Save notebook entry').click();
     // be sure that entry was created
     await expect(page.getByText('favicon-96x96.png')).toBeVisible();
 

e2e/tests/functional/plugins/notebook/notebookTags.e2e.spec.js

@@ -163,7 +163,7 @@ test.describe('Tagging in Notebooks @addInit', () => {
 
     await page.locator('text=To start a new entry, click here or drag and drop any object').click();
     await page.getByLabel('Notebook Entry Input').fill(`An entry without tags`);
-    await page.locator('.c-ne__save-button > button').click();
+    await page.getByLabel('Save notebook entry').click();
 
     await page.hover('[aria-label="Notebook Entry Display"] >> nth=1');
     await page.locator('button[title="Delete this entry"]').last().click();

src/plugins/notebook/components/NotebookEntry.vue

@@ -45,7 +45,7 @@
             }}
           </span>
         </div>
-        <span v-if="!readOnly && !isLocked" class="c-ne__local-controls--hidden">
+        <span v-if="!readOnly && !isLocked && !editMode" class="c-ne__local-controls--hidden">
           <button
             class="c-ne__remove c-icon-button c-icon-button--major icon-trash"
             title="Delete this entry"
@@ -80,15 +80,21 @@
             v-else
             :id="entry.id"
             ref="entryInput"
-            v-model="entry.text"
+            v-model="entryTextVal"
             class="c-ne__input"
             aria-label="Notebook Entry Input"
             tabindex="-1"
             @mouseleave="canEdit = true"
+            @keydown.esc="cancelEntry()"
+            @keydown.ctrl.enter="updateEntryValue($event)"
             @blur="updateEntryValue($event)"
           ></textarea>
           <div v-if="editMode" class="c-ne__save-button">
-            <button class="c-button c-button--major icon-check"></button>
+            <button
+              class="c-button c-button--major icon-check"
+              title="Save notebook entry (Ctrl-Enter)"
+              aria-label="Save notebook entry"
+            ></button>
           </div>
         </template>
 
@@ -271,6 +277,7 @@
     }
   },
   emits: [
+    'cancel-edit',
     'delete-entry',
     'change-section-page',
     'update-entry',
@@ -283,7 +290,8 @@
       editMode: false,
       canEdit: true,
       enableEmbedsWrapperScroll: false,
-      urlWhitelist: []
+      urlWhitelist: [],
+      entryTextVal: null
     };
   },
   computed: {
@@ -347,6 +355,7 @@
     this.renderer = new this.marked.Renderer();
   },
   mounted() {
+    this.entryTextVal = this.entry.text;
     const originalLinkRenderer = this.renderer.link;
     this.renderer.link = this.validateLink.bind(this, originalLinkRenderer);
 
@@ -492,6 +501,11 @@
         this.canEdit = false;
       }
     },
+    cancelEntry() {
+      this.editMode = false;
+      this.entryTextVal = this.entry.text;
+      this.$emit('cancel-edit');
+    },
     deleteEntry() {
       this.$emit('delete-entry', this.entry.id);
     },
@@ -657,7 +671,7 @@
     },
     updateEntryValue($event) {
       this.editMode = false;
-      const rawEntryValue = $event.target.value;
+      const rawEntryValue = this.entryTextVal;
       const sanitizeInput = sanitizeHtml(rawEntryValue, { allowedAttributes: [], allowedTags: [] });
       // change &gt back to > for markdown to do blockquotes
       const restoredQuoteBrackets = sanitizeInput.replace(/&gt;/g, '>');