From 00353cdccf220ed01d71175fdcc356770b9e2795 Mon Sep 17 00:00:00 2001
From: Scott Bell <scott@traclabs.com>
Date: Wed, 22 Mar 2023 00:53:24 +0100
Subject: [PATCH] Cancel annotation selections if you click outside the plot
 component (#6476)

* resolve conflicts

* resolve conflicts

* more selectively add listeners

* add and fix tests

* address PR review comments

* test(e2e): stabilize flaky overlayPlot test

---------

Co-authored-by: Jesse Mazzella <ozyx@users.noreply.github.com>
Co-authored-by: Jesse Mazzella <jesse.d.mazzella@nasa.gov>
---
 .../plugins/plot/overlayPlot.e2e.spec.js      |  3 +
 .../plugins/plot/tagging.e2e.spec.js          | 63 ++++++++++++++-----
 .../components/controls/AutoCompleteField.vue |  2 +-
 src/plugins/plot/MctPlot.vue                  | 21 +++++++
 src/ui/inspector/Inspector.vue                |  2 +-
 5 files changed, 72 insertions(+), 19 deletions(-)

diff --git a/e2e/tests/functional/plugins/plot/overlayPlot.e2e.spec.js b/e2e/tests/functional/plugins/plot/overlayPlot.e2e.spec.js
index 14106066c3..be8edca1ec 100644
--- a/e2e/tests/functional/plugins/plot/overlayPlot.e2e.spec.js
+++ b/e2e/tests/functional/plugins/plot/overlayPlot.e2e.spec.js
@@ -268,6 +268,9 @@ async function getCanvasPixelsWithData(page) {
  * @param {import('@playwright/test').Page} page
  */
 async function assertLimitLinesExistAndAreVisible(page) {
+    // Wait for plot series data to load
+    await expect(page.locator('.js-series-data-loaded')).toBeVisible();
+    // Wait for limit lines to be created
     await page.waitForSelector('.js-limit-area', { state: 'attached' });
     const limitLineCount = await page.locator('.c-plot-limit-line').count();
     // There should be 10 limit lines created by default
diff --git a/e2e/tests/functional/plugins/plot/tagging.e2e.spec.js b/e2e/tests/functional/plugins/plot/tagging.e2e.spec.js
index 81e0f8653b..877abf9860 100644
--- a/e2e/tests/functional/plugins/plot/tagging.e2e.spec.js
+++ b/e2e/tests/functional/plugins/plot/tagging.e2e.spec.js
@@ -28,6 +28,14 @@ const { test, expect } = require('../../../../pluginFixtures');
 const { createDomainObjectWithDefaults, setRealTimeMode, setFixedTimeMode } = require('../../../../appActions');
 
 test.describe('Plot Tagging', () => {
+    /**
+     * Given a canvas and a set of points, tags the points on the canvas.
+     * @param {import('@playwright/test').Page} page
+     * @param {HTMLCanvasElement} canvas a telemetry item with a plot
+     * @param {Number} xEnd a telemetry item with a plot
+     * @param {Number} yEnd a telemetry item with a plot
+     * @returns {Promise}
+     */
     async function createTags({page, canvas, xEnd, yEnd}) {
         await canvas.hover({trial: true});
 
@@ -64,12 +72,20 @@ test.describe('Plot Tagging', () => {
         await page.getByText('Science').click();
     }
 
-    async function testTelemetryItem(page, canvas, telemetryItem) {
+    /**
+     * Given a telemetry item (e.g., a Sine Wave Generator) with a plot, tests that the plot can be tagged.
+     * @param {import('@playwright/test').Page} page
+     * @param {import('../../../../appActions').CreatedObjectInfo} telemetryItem a telemetry item with a plot
+     * @returns {Promise}
+     */
+    async function testTelemetryItem(page, telemetryItem) {
         // Check that telemetry item also received the tag
         await page.goto(telemetryItem.url);
 
         await expect(page.getByText('No tags to display for this item')).toBeVisible();
 
+        const canvas = page.locator('canvas').nth(1);
+
         //Wait for canvas to stablize.
         await canvas.hover({trial: true});
 
@@ -85,19 +101,31 @@ test.describe('Plot Tagging', () => {
         await expect(page.getByText('Driving')).toBeHidden();
     }
 
-    async function basicTagsTests(page, canvas) {
-        // Search for Science
+    /**
+     * Given a page, tests that tags are searchable, deletable, and persist across reloads.
+     * @param {import('@playwright/test').Page} page
+     * @returns {Promise}
+     */
+    async function basicTagsTests(page) {
+        // Search for Driving
         await page.locator('[aria-label="OpenMCT Search"] input[type="search"]').click();
-        await page.locator('[aria-label="OpenMCT Search"] input[type="search"]').fill('sc');
-        await expect(page.locator('[aria-label="Search Result"]').nth(0)).toContainText("Science");
-        await expect(page.locator('[aria-label="Search Result"]').nth(0)).not.toContainText("Drilling");
+
+        // Clicking elsewhere should cause annotation selection to be cleared
+        await expect(page.getByText('No tags to display for this item')).toBeVisible();
+
+        await page.locator('[aria-label="OpenMCT Search"] input[type="search"]').fill('driv');
+        // click on the search result
+        await page.getByRole('searchbox', { name: 'OpenMCT Search' }).getByText(/Sine Wave/).first().click();
 
         // Delete Driving
         await page.hover('[aria-label="Tag"]:has-text("Driving")');
         await page.locator('[aria-label="Remove tag Driving"]').click();
 
-        await expect(page.locator('[aria-label="Tags Inspector"]')).toContainText("Science");
-        await expect(page.locator('[aria-label="Tags Inspector"]')).not.toContainText("Driving");
+        // Search for Science
+        await page.locator('[aria-label="OpenMCT Search"] input[type="search"]').click();
+        await page.locator('[aria-label="OpenMCT Search"] input[type="search"]').fill('sc');
+        await expect(page.locator('[aria-label="Search Result"]').nth(0)).toContainText("Science");
+        await expect(page.locator('[aria-label="Search Result"]').nth(0)).not.toContainText("Drilling");
 
         // Search for Driving
         await page.locator('[aria-label="OpenMCT Search"] input[type="search"]').click();
@@ -109,12 +137,13 @@ test.describe('Plot Tagging', () => {
             page.reload(),
             page.waitForLoadState('networkidle')
         ]);
-        // wait for plot progress bar to disappear
-        await page.locator('.l-view-section.c-progress-bar').waitFor({ state: 'detached' });
+        // wait for plots to load
+        await expect(page.locator('.js-series-data-loaded')).toBeVisible();
 
         await page.getByText('Annotations').click();
         await expect(page.getByText('No tags to display for this item')).toBeVisible();
 
+        const canvas = page.locator('canvas').nth(1);
         // click on the tagged plot point
         await canvas.click({
             position: {
@@ -171,8 +200,8 @@ test.describe('Plot Tagging', () => {
         // changing to fixed time mode rebuilds canvas?
         canvas = page.locator('canvas').nth(1);
 
-        await basicTagsTests(page, canvas);
-        await testTelemetryItem(page, canvas, alphaSineWave);
+        await basicTagsTests(page);
+        await testTelemetryItem(page, alphaSineWave);
 
         // set to real time mode
         await setRealTimeMode(page);
@@ -182,8 +211,8 @@ test.describe('Plot Tagging', () => {
         await page.locator('[aria-label="OpenMCT Search"] input[type="search"]').fill('sc');
         // click on the search result
         await page.getByRole('searchbox', { name: 'OpenMCT Search' }).getByText('Alpha Sine Wave').first().click();
-        // wait for plot progress bar to disappear
-        await page.locator('.l-view-section.c-progress-bar').waitFor({ state: 'detached' });
+        // wait for plots to load
+        await expect(page.locator('.js-series-data-loaded')).toBeVisible();
         // expect plot to be paused
         await expect(page.locator('[title="Resume displaying real-time data"]')).toBeVisible();
 
@@ -202,7 +231,7 @@ test.describe('Plot Tagging', () => {
             xEnd: 700,
             yEnd: 480
         });
-        await basicTagsTests(page, canvas);
+        await basicTagsTests(page);
     });
 
     test('Tags work with Stacked Plots', async ({ page }) => {
@@ -232,7 +261,7 @@ test.describe('Plot Tagging', () => {
             xEnd: 700,
             yEnd: 215
         });
-        await basicTagsTests(page, canvas);
-        await testTelemetryItem(page, canvas, alphaSineWave);
+        await basicTagsTests(page);
+        await testTelemetryItem(page, alphaSineWave);
     });
 });
diff --git a/src/api/forms/components/controls/AutoCompleteField.vue b/src/api/forms/components/controls/AutoCompleteField.vue
index 2be6d7e65e..08d907d2e5 100644
--- a/src/api/forms/components/controls/AutoCompleteField.vue
+++ b/src/api/forms/components/controls/AutoCompleteField.vue
@@ -43,7 +43,7 @@
     </div>
     <div
         v-if="!hideOptions && filteredOptions.length > 0"
-        class="c-menu c-input--autocomplete__options"
+        class="c-menu c-input--autocomplete__options js-autocomplete-options"
         aria-label="Autocomplete Options"
         @blur="hideOptions = true"
     >
diff --git a/src/plugins/plot/MctPlot.vue b/src/plugins/plot/MctPlot.vue
index 410e5be1e2..8227ba044e 100644
--- a/src/plugins/plot/MctPlot.vue
+++ b/src/plugins/plot/MctPlot.vue
@@ -22,7 +22,9 @@
 <template>
 <div
     v-if="loaded"
+    ref="plot"
     class="gl-plot"
+    :class="{ 'js-series-data-loaded' : seriesDataLoaded }"
 >
     <slot></slot>
     <div class="plot-wrapper-axis-and-display-area flex-elem grows">
@@ -347,6 +349,9 @@ export default {
             const parentLeftTickWidth = this.parentYTickWidth.leftTickWidth;
 
             return parentLeftTickWidth || leftTickWidth;
+        },
+        seriesDataLoaded() {
+            return ((this.pending === 0) && this.loaded);
         }
     },
     watch: {
@@ -412,6 +417,7 @@ export default {
         this.openmct.selection.off('change', this.updateSelection);
         document.removeEventListener('keydown', this.handleKeyDown);
         document.removeEventListener('keyup', this.handleKeyUp);
+        document.body.removeEventListener('click', this.cancelSelection);
         this.destroy();
     },
     methods: {
@@ -444,6 +450,19 @@ export default {
             //This section is common to all entry points for annotation display
             this.prepareExistingAnnotationSelection(selectedAnnotations);
         },
+        cancelSelection(event) {
+            if (this.$refs?.plot) {
+                const clickedInsidePlot = this.$refs.plot.contains(event.target);
+                const clickedInsideInspector = event.target.closest('.js-inspector') !== null;
+                const clickedOption = event.target.closest('.js-autocomplete-options') !== null;
+                if (!clickedInsidePlot && !clickedInsideInspector && !clickedOption) {
+                    this.rectangles = [];
+                    this.annotationSelections = [];
+                    this.selectPlot();
+                    document.body.removeEventListener('click', this.cancelSelection);
+                }
+            }
+        },
         waitForAxesToLoad() {
             return new Promise(resolve => {
                 // When there is no plot data, the ranges can be undefined
@@ -1276,6 +1295,8 @@ export default {
             }
 
             this.openmct.selection.select(selection, true);
+
+            document.body.addEventListener('click', this.cancelSelection);
         },
         selectNewPlotAnnotations(boundingBoxPerYAxis, pointsInBox, event) {
             let targetDomainObjects = {};
diff --git a/src/ui/inspector/Inspector.vue b/src/ui/inspector/Inspector.vue
index 1bcf7ec999..fedff094f4 100644
--- a/src/ui/inspector/Inspector.vue
+++ b/src/ui/inspector/Inspector.vue
@@ -21,7 +21,7 @@
  *****************************************************************************/
 
 <template>
-<div class="c-inspector">
+<div class="c-inspector js-inspector">
     <object-name />
     <InspectorTabs
         :selection="selection"