fix(#7552): Fix notebook snapshot image annotations (#7555)

* fix: painterro import

* test(snapshotAnnotation): add minimal e2e test

* chore: add e2e test annotation

* fix: notebook snapshot test

* refactor: put `v-else` on template

* small changes to the test and a visual one

* additional a11y

* fix: html structure

* test(e2e): fix notebook snapshot tests

* Update documentation for file download and JSON testing

* Update stubs and add jpg/png export

* refactor(TimelistComponent): tidy up

---------

Co-authored-by: John Hill <john.c.hill@nasa.gov>

e2e/README.md

@@ -516,6 +516,30 @@ test.describe('foo test suite', () => {
 - Working with multiple pages
 There are instances where multiple browser pages will needed to verify multi-page or multi-tab application behavior. Make sure to use the `@2p` annotation as well as name each page appropriately: i.e. `page1` and `page2` or `tab1` and `tab2` depending on the intended use case. Generally pages should be used unless testing `sharedWorker` code, specifically.
 
+- Working with file downloads and JSON data
+Open MCT has the capability of exporting certain objects in the form of a JSON file handled by the chrome browser. The best example of this type of test can be found in the exportAsJson test.
+
+```js
+const [download] = await Promise.all([
+  page.waitForEvent('download'), // Waits for the download event
+  page.getByLabel('Export as JSON').click() // Triggers the download
+]);
+
+// Wait for the download process to complete
+const path = await download.path();
+
+// Read the contents of the downloaded file using readFile from fs/promises
+const fileContents = await fs.readFile(path, 'utf8');
+const jsonData = JSON.parse(fileContents);
+
+// Use the function to retrieve the key
+const key = getFirstKeyFromOpenMctJson(jsonData);
+
+// Verify the contents of the JSON file
+expect(jsonData.openmct[key]).toHaveProperty('name', 'e2e folder');
+```
+
+
 ### Reporting
 
 Test Reporting is done through official Playwright reporters and the CI Systems which execute them.

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

@@ -289,7 +289,7 @@ test.describe('Flexible Layout Toolbar Actions @localStorage', () => {
     await page.getByTitle('Add Container').click();
     expect(await containerHandles.count()).toEqual(3);
     await page.getByTitle('Remove Container').click();
-    await expect(page.getByRole('dialog', { name: 'Overlay' })).toHaveText(
+    await expect(page.getByRole('dialog', { name: 'Overlay' })).toContainText(
       'This action will permanently delete this container from this Flexible Layout. Do you want to continue?'
     );
     await page.getByRole('button', { name: 'OK', exact: true }).click();
@@ -299,7 +299,7 @@ test.describe('Flexible Layout Toolbar Actions @localStorage', () => {
     expect(await page.getByRole('group', { name: 'Frame' }).count()).toEqual(2);
     await page.getByRole('group', { name: 'Child Layout 1' }).click();
     await page.getByTitle('Remove Frame').click();
-    await expect(page.getByRole('dialog', { name: 'Overlay' })).toHaveText(
+    await expect(page.getByRole('dialog', { name: 'Overlay' })).toContainText(
       'This action will remove this frame from this Flexible Layout. Do you want to continue?'
     );
     await page.getByRole('button', { name: 'OK', exact: true }).click();

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

@@ -71,42 +71,89 @@ test.describe('Snapshot Container tests', () => {
   test.beforeEach(async ({ page }) => {
     //Navigate to baseURL
     await page.goto('./', { waitUntil: 'domcontentloaded' });
-
-    // Create Notebook
-    // const notebook = await createDomainObjectWithDefaults(page, {
-    //     type: 'Notebook',
-    //     name: "Test Notebook"
-    // });
-    // // Create Overlay Plot
-    // const snapShotObject = await createDomainObjectWithDefaults(page, {
-    //     type: 'Overlay Plot',
-    //     name: "Dropped Overlay Plot"
-    // });
-
     await page.getByLabel('Open the Notebook Snapshot Menu').click();
     await page.getByRole('menuitem', { name: 'Save to Notebook Snapshots' }).click();
     await page.getByLabel('Show Snapshots').click();
   });
   test('A snapshot can be Quick Viewed from Container with 3 dot action menu', async ({ page }) => {
-    await page.locator('.c-snapshot.c-ne__embed').first().getByTitle('More actions').click();
+    await page.getByLabel('My Items Notebook Embed').getByLabel('More actions').click();
     await page.getByRole('menuitem', { name: 'Quick View' }).click();
-    await expect(page.locator('.c-overlay__outer')).toBeVisible();
+    await expect(page.getByLabel('Modal Overlay')).toBeVisible();
+    await expect(page.getByLabel('Preview Container')).toBeVisible();
   });
-  test.fixme(
+  test('A snapshot can be Viewed, Annotated, display deleted, and saved from Container with 3 dot action menu', async ({
-    'A snapshot can be Viewed, Annotated, display deleted, and saved from Container with 3 dot action menu',
+    page
-    async ({ page }) => {
+  }) => {
-      await page.locator('.c-snapshot.c-ne__embed').first().getByTitle('More actions').click();
+    test.info().annotations.push({
-      await page.getByRole('menuitem', { name: ' View Snapshot' }).click();
+      type: 'issue',
-      await expect(page.locator('.c-overlay__outer')).toBeVisible();
+      description: 'https://github.com/nasa/openmct/issues/7552'
-      await page.getByTitle('Annotate').click();
+    });
+    //Open Snapshot Object View
+    await page.getByLabel('My Items Notebook Embed').getByLabel('More actions').click();
+    await page.getByRole('menuitem', { name: 'View Snapshot' }).click();
+    await expect(page.getByRole('dialog', { name: 'Modal Overlay' })).toBeVisible();
+    await expect(page.locator('#snapshotDescriptor')).toHaveText(
+      /SNAPSHOT \d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}/
+    );
+    // Open Annotation Editor with Painterro
+    await page.getByLabel('Annotate this snapshot').click();
     await expect(page.locator('#snap-annotation-canvas')).toBeVisible();
-      await page.getByRole('button', { name: '' }).click();
+    // Clear the canvas
-      // await expect(page.locator('#snap-annotation-canvas')).not.toBeVisible();
+    await page.getByRole('button', { name: 'Put text [T]' }).click();
+    // Click in the Painterro canvas to add a text annotation
+    await page.locator('.ptro-crp-el').click();
+    await page.locator('.ptro-text-tool-input').fill('...is there life on mars?');
+    // When working with Painterro, we need to check that the Apply button is hidden after clicking
+    await page.getByTitle('Apply').click();
+    await expect(page.getByTitle('Apply')).toBeHidden();
+
+    // Save and exit annotation window
     await page.getByRole('button', { name: 'Save' }).click();
     await page.getByRole('button', { name: 'Done' }).click();
-      //await expect(await page.locator)
+
-    }
+    // Open up annotation again
-  );
+    await page.getByRole('img', { name: 'My Items thumbnail' }).click();
+    await expect(page.getByLabel('Modal Overlay').getByRole('img')).toBeVisible();
+  });
+  test('A snapshot can be Annotated and saved as a JPG and PNG', async ({ page }) => {
+    //Open Snapshot Object View
+    await page.getByLabel('My Items Notebook Embed').getByLabel('More actions').click();
+    await page.getByRole('menuitem', { name: 'View Snapshot' }).click();
+    await expect(page.getByRole('dialog', { name: 'Modal Overlay' })).toBeVisible();
+
+    // Open Annotation Editor with Painterro
+    await page.getByLabel('Annotate this snapshot').click();
+    await expect(page.locator('#snap-annotation-canvas')).toBeVisible();
+    // Clear the canvas
+    await page.getByRole('button', { name: 'Put text [T]' }).click();
+    // Click in the Painterro canvas to add a text annotation
+    await page.locator('.ptro-crp-el').click();
+    await page.locator('.ptro-text-tool-input').fill('...is there life on mars?');
+    // When working with Painterro, we need to check that the Apply button is hidden after clicking
+    await page.getByTitle('Apply').click();
+    await expect(page.getByTitle('Apply')).toBeHidden();
+
+    // Save and exit annotation window
+    await page.getByRole('button', { name: 'Save' }).click();
+    await page.getByRole('button', { name: 'Done' }).click();
+
+    // Open up annotation again
+    await page.getByRole('img', { name: 'My Items thumbnail' }).click();
+    await expect(page.getByLabel('Modal Overlay').getByRole('img')).toBeVisible();
+
+    // Save as JPG
+    await Promise.all([
+      page.waitForEvent('download'), // Waits for the download event
+      page.getByLabel('Export as JPG').click() // Triggers the download
+    ]);
+
+    // Save as PNG
+    await expect(page.getByLabel('Modal Overlay').getByRole('img')).toBeVisible();
+    await Promise.all([
+      page.waitForEvent('download'), // Waits for the download event
+      page.getByLabel('Export as PNG').click() // Triggers the download
+    ]);
+  });
   test.fixme('5 Snapshots can be added to a container', async ({ page }) => {});
   test.fixme(
     '5 Snapshots can be added to a container and Deleted with Delete All action',
@@ -116,10 +163,6 @@ test.describe('Snapshot Container tests', () => {
     'A snapshot can be Deleted from Container with 3 dot action menu',
     async ({ page }) => {}
   );
-  test.fixme(
-    'A snapshot can be Navigated To from Container with 3 dot action menu',
-    async ({ page }) => {}
-  );
   test.fixme(
     'A snapshot can be Navigated To Item in Time from Container with 3 dot action menu',
     async ({ page }) => {}
@@ -151,11 +194,4 @@ test.describe('Snapshot Container tests', () => {
       //Snapshot removed from container?
     }
   );
-  test.fixme(
-    'Verify Embedded options for PNG, JPG, and Annotate work correctly',
-    async ({ page }) => {
-      //Add snapshot to container
-      //Verify PNG, JPG, and Annotate buttons work correctly
-    }
-  );
 });

e2e/tests/visual-a11y/notebook.visual.spec.js

@@ -23,7 +23,7 @@
 import percySnapshot from '@percy/playwright';
 
 import { createDomainObjectWithDefaults, expandTreePaneItemByName } from '../../appActions.js';
-import { test } from '../../avpFixtures.js';
+import { expect, test } from '../../avpFixtures.js';
 import { VISUAL_URL } from '../../constants.js';
 import { enterTextEntry, startAndAddRestrictedNotebookObject } from '../../helper/notebookUtils.js';
 
@@ -39,6 +39,44 @@ test.describe('Visual - Restricted Notebook @a11y', () => {
   });
 });
 
+test.describe('Visual - Notebook Snapshot @a11y', () => {
+  test.beforeEach(async ({ page }) => {
+    await page.goto('./?hideTree=true&hideInspector=true', { waitUntil: 'domcontentloaded' });
+  });
+  test('Visual check for Snapshot Annotation', async ({ page, theme }) => {
+    await page.getByLabel('Open the Notebook Snapshot Menu').click();
+    await page.getByRole('menuitem', { name: 'Save to Notebook Snapshots' }).click();
+    await page.getByLabel('Show Snapshots').click();
+
+    await page.getByLabel('My Items Notebook Embed').getByLabel('More actions').click();
+    await page.getByRole('menuitem', { name: 'View Snapshot' }).click();
+
+    await page.getByLabel('Annotate this snapshot').click();
+    await expect(page.locator('#snap-annotation-canvas')).toBeVisible();
+    // Clear the canvas
+    await page.getByRole('button', { name: 'Put text [T]' }).click();
+    // Click in the Painterro canvas to add a text annotation
+    await page.locator('.ptro-crp-el').click();
+    await page.locator('.ptro-text-tool-input').fill('...is there life on mars?');
+    await percySnapshot(page, `Notebook Snapshot with text entry open (theme: '${theme}')`);
+
+    // When working with Painterro, we need to check that the Apply button is hidden after clicking
+    await page.getByTitle('Apply').click();
+    await expect(page.getByTitle('Apply')).toBeHidden();
+
+    // Save and exit annotation window
+    await page.getByRole('button', { name: 'Save' }).click();
+    await page.getByRole('button', { name: 'Done' }).click();
+
+    // Open up annotation again
+    await page.getByRole('img', { name: 'My Items thumbnail' }).click();
+    await expect(page.getByLabel('Modal Overlay').getByRole('img')).toBeVisible();
+
+    // Take a snapshot
+    await percySnapshot(page, `Notebook Snapshot with annotation (theme: '${theme}')`);
+  });
+});
+
 test.describe('Visual - Notebook @a11y', () => {
   let notebook;
   test.beforeEach(async ({ page }) => {

src/api/overlays/components/OverlayComponent.vue

@@ -20,7 +20,7 @@
  at runtime from the About dialog for additional information.
 -->
 <template>
-  <div class="c-overlay js-overlay">
+  <div class="c-overlay js-overlay" role="dialog" aria-modal="true" aria-label="Modal Overlay">
     <div class="c-overlay__blocker" @click="destroy"></div>
     <div class="c-overlay__outer">
       <button
@@ -34,9 +34,6 @@
           ref="element"
           class="c-overlay__contents js-notebook-snapshot-item-wrapper"
           tabindex="0"
-          aria-modal="true"
-          aria-label="Overlay"
-          role="dialog"
         ></div>
         <div v-if="buttons" class="c-overlay__button-bar">
           <button
@@ -61,7 +58,7 @@
 export default {
   inject: ['dismiss', 'element', 'buttons', 'dismissable'],
   emits: ['destroy'],
-  data: function () {
+  data() {
     return {
       focusIndex: -1
     };

src/plugins/notebook/components/NotebookEmbed.vue

@@ -23,6 +23,7 @@
   <div
     ref="notebookEmbed"
     class="c-snapshot c-ne__embed"
+    :aria-label="`${embed.name} Notebook Embed`"
     @mouseover.ctrl="showToolTip"
     @mouseleave="hideToolTip"
   >

src/plugins/notebook/components/snapshot-template.html

@@ -1,36 +1,32 @@
 <div class="c-notebook-snapshot">
-  <!-- parent container sets up this for flex column layout -->
   <div class="c-notebook-snapshot__header l-browse-bar">
     <div class="l-browse-bar__start">
       <div class="l-browse-bar__object-name--w">
         <span class="c-object-label l-browse-bar__object-name">
-          <span class="c-object-label__type-icon" v-bind:class="cssClass"></span>
+          <span class="c-object-label__type-icon" :class="cssClass"></span>
           <span class="c-object-label__name">{{ name }}</span>
         </span>
       </div>
     </div>
-
+    <div id="snapshotDescriptor" class="l-browse-bar__snapshot-datetime">
-    <div class="l-browse-bar__end">
+      SNAPSHOT {{ createdOn }}
-      <div class="l-browse-bar__snapshot-datetime">SNAPSHOT {{ createdOn }}</div>
+    </div>
-      <span class="c-button-set c-button-set--strip-h">
+    <div class="c-button-set c-button-set--strip-h" role="toolbar">
-        <button
+      <button class="c-button icon-download" aria-label="Export as PNG" @click="exportImage('png')">
-          class="c-button icon-download"
-          title="Export This View's Data as PNG"
-          @click="exportImage('png')"
-        >
         <span class="c-button__label">PNG</span>
       </button>
-        <button class="c-button" title="Export This View's Data as JPG" @click="exportImage('jpg')">
+      <button class="c-button icon-download" aria-label="Export as JPG" @click="exportImage('jpg')">
         <span class="c-button__label">JPG</span>
       </button>
-      </span>
+    </div>
-      <a
+    <div class="l-browse-bar__end">
+      <button
         class="l-browse-bar__annotate-button c-button icon-pencil"
-        title="Annotate"
+        aria-label="Annotate this snapshot"
         @click="annotateSnapshot"
       >
         <span class="title-label">Annotate</span>
-      </a>
+      </button>
     </div>
   </div>
 
@@ -38,5 +34,7 @@
     ref="snapshot-image"
     class="c-notebook-snapshot__image"
     :style="{ backgroundImage: 'url(' + src + ')' }"
+    role="img"
+    alt="Annotatable Snapshot"
   ></div>
 </div>

src/plugins/notebook/utils/painterroInstance.js

@@ -46,7 +46,7 @@ export default class PainterroInstance {
     this.config.id = this.elementId;
     this.config.saveHandler = this.saveHandler.bind(this);
 
-    this.painterro = Painterro(this.config);
+    this.painterro = Painterro.default(this.config);
   }
 
   save(callback) {

src/plugins/timelist/TimelistComponent.vue

@@ -35,21 +35,17 @@
         :item-properties="itemProperties"
         :execution-state="persistedActivityStates[item.id]"
         @click.stop="setSelectionForActivity(item, $event.currentTarget)"
-      >
+      />
-      </expanded-view-item>
     </template>
-    <div v-else class="c-table c-table--sortable c-list-view c-list-view--sticky-header sticky">
+    <template v-else>
+      <div class="c-table c-table--sortable c-list-view c-list-view--sticky-header sticky">
         <table class="c-table__body js-table__body">
           <thead class="c-table__header">
             <tr>
               <list-header
                 v-for="headerItem in headerItems"
                 :key="headerItem.property"
-              :direction="
+                :direction="getSortDirection(headerItem)"
-                defaultSort.property === headerItem.property
-                  ? defaultSort.defaultDirection
-                  : headerItem.defaultDirection
-              "
                 :is-sortable="headerItem.isSortable"
                 :aria-label="headerItem.name"
                 :title="headerItem.name"
@@ -71,6 +67,7 @@
           </tbody>
         </table>
       </div>
+    </template>
   </div>
 </template>
 
@@ -526,20 +523,16 @@ export default {
       return activities.map(this.styleActivity);
     },
     setSort() {
-      const sortOrder = SORT_ORDER_OPTIONS[this.domainObject.configuration.sortOrderIndex];
+      const { property, direction } =
-      const property = sortOrder.property;
+        SORT_ORDER_OPTIONS[this.domainObject.configuration.sortOrderIndex];
-      const direction = sortOrder.direction.toLowerCase() === 'asc';
       this.defaultSort = {
         property,
-        defaultDirection: direction
+        defaultDirection: direction.toLowerCase() === 'asc'
       };
     },
     sortItems(activities) {
-      let sortedItems = _.sortBy(activities, this.defaultSort.property);
+      const sortedItems = _.sortBy(activities, this.defaultSort.property);
-      if (!this.defaultSort.defaultDirection) {
+      return this.defaultSort.defaultDirection ? sortedItems : sortedItems.reverse();
-        sortedItems = sortedItems.reverse();
-      }
-      return sortedItems;
     },
     setStatus(status) {
       this.status = status;
@@ -548,10 +541,7 @@ export default {
       this.isEditing = isEditing;
       this.setViewFromConfig(this.domainObject.configuration);
     },
-    sort(data) {
+    sort({ property, direction }) {
-      const property = data.property;
-      const direction = data.direction;
-
       if (this.defaultSort.property === property) {
         this.defaultSort.defaultDirection = !this.defaultSort.defaultDirection;
       } else {
@@ -565,10 +555,10 @@ export default {
       this.openmct.selection.select(
         [
           {
-            element: element,
+            element,
             context: {
               type: 'activity',
-              activity: activity
+              activity
             }
           },
           {
@@ -581,6 +571,11 @@ export default {
         ],
         multiSelect
       );
+    },
+    getSortDirection(headerItem) {
+      return this.defaultSort.property === headerItem.property
+        ? this.defaultSort.defaultDirection
+        : headerItem.defaultDirection;
     }
   }
 };