From be38c3e6546e9d2217b1062cc12a419780e28a3c Mon Sep 17 00:00:00 2001 From: Shefali Joshi Date: Fri, 3 Feb 2023 15:56:50 -0800 Subject: [PATCH] Fix stacked plot child selection (#6275) * Fix selections for different scenarios * Ensure plot selection in stacked plots works when there are no selected or found annotations * Adds e2e test for stacked plot selection and fixes the old e2e test which was testing overlay plots instead. * Fix selection of plots while in Edit mode * Improve tests for stacked plots * refactor: remove unnecessary `await`s * a11y: move aria-label to StackedPlotItem * refactor(e2e): combine like tests, unique object names - Use unique object names in `text=` selectors - Combine like tests to reduce execution time - Use `getByRole` selectors where able * docs(e2e): add comments to test * fix: add class back for unit test selector --------- Co-authored-by: Scott Bell Co-authored-by: Jesse Mazzella --- .../plugins/plot/stackedPlot.e2e.spec.js | 124 +++++++++++++----- src/plugins/plot/MctPlot.vue | 40 ++++-- .../plot/inspector/PlotOptionsBrowse.vue | 2 + .../plot/inspector/PlotOptionsEdit.vue | 1 + .../plot/inspector/forms/YAxisForm.vue | 5 +- .../plot/stackedPlot/StackedPlotItem.vue | 39 +++++- src/ui/inspector/InspectorViews.vue | 2 +- 7 files changed, 165 insertions(+), 48 deletions(-) diff --git a/e2e/tests/functional/plugins/plot/stackedPlot.e2e.spec.js b/e2e/tests/functional/plugins/plot/stackedPlot.e2e.spec.js index 5d6c47f65c..509fe267eb 100644 --- a/e2e/tests/functional/plugins/plot/stackedPlot.e2e.spec.js +++ b/e2e/tests/functional/plugins/plot/stackedPlot.e2e.spec.js @@ -29,29 +29,39 @@ const { test, expect } = require('../../../../pluginFixtures'); const { createDomainObjectWithDefaults } = require('../../../../appActions'); test.describe('Stacked Plot', () => { + let stackedPlot; + let swgA; + let swgB; + let swgC; + + test.beforeEach(async ({ page }) => { + //Open a browser, navigate to the main page, and wait until all networkevents to resolve + await page.goto('/', { waitUntil: 'networkidle' }); + stackedPlot = await createDomainObjectWithDefaults(page, { + type: "Stacked Plot" + }); + + swgA = await createDomainObjectWithDefaults(page, { + type: "Sine Wave Generator", + parent: stackedPlot.uuid + }); + swgB = await createDomainObjectWithDefaults(page, { + type: "Sine Wave Generator", + parent: stackedPlot.uuid + }); + swgC = await createDomainObjectWithDefaults(page, { + type: "Sine Wave Generator", + parent: stackedPlot.uuid + }); + }); test('Using the remove action removes the correct plot', async ({ page }) => { - await page.goto('/', { waitUntil: 'networkidle' }); - const overlayPlot = await createDomainObjectWithDefaults(page, { - type: "Overlay Plot" - }); + const swgAElementsPoolItem = page.locator('#inspector-elements-tree').locator('.c-object-label', { hasText: swgA.name }); + const swgBElementsPoolItem = page.locator('#inspector-elements-tree').locator('.c-object-label', { hasText: swgB.name }); + const swgCElementsPoolItem = page.locator('#inspector-elements-tree').locator('.c-object-label', { hasText: swgC.name }); + + await page.goto(stackedPlot.url); - await createDomainObjectWithDefaults(page, { - type: "Sine Wave Generator", - name: 'swg a', - parent: overlayPlot.uuid - }); - await createDomainObjectWithDefaults(page, { - type: "Sine Wave Generator", - name: 'swg b', - parent: overlayPlot.uuid - }); - await createDomainObjectWithDefaults(page, { - type: "Sine Wave Generator", - name: 'swg c', - parent: overlayPlot.uuid - }); - await page.goto(overlayPlot.url); await page.click('button[title="Edit"]'); // Expand the elements pool vertically @@ -60,20 +70,70 @@ test.describe('Stacked Plot', () => { await page.mouse.move(0, 100); await page.mouse.up(); - await page.locator('.js-elements-pool__tree >> text=swg b').click({ button: 'right' }); - await page.locator('li[role="menuitem"]:has-text("Remove")').click(); - await page.locator('.js-overlay .js-overlay__button >> text=OK').click(); + await swgBElementsPoolItem.click({ button: 'right' }); + await page.getByRole('menuitem').filter({ hasText: /Remove/ }).click(); + await page.getByRole('button').filter({ hasText: "OK" }).click(); - // Wait until the number of elements in the elements pool has changed, and then confirm that the correct children were retained - // await page.waitForFunction(() => { - // return Array.from(document.querySelectorAll('.js-elements-pool__tree .js-elements-pool__item')).length === 2; - // }); - // Wait until there are only two items in the elements pool (ie the remove action has completed) - await expect(page.locator('.js-elements-pool__tree .js-elements-pool__item')).toHaveCount(2); + await expect(page.locator('#inspector-elements-tree .js-elements-pool__item')).toHaveCount(2); // Confirm that the elements pool contains the items we expect - await expect(page.locator('.js-elements-pool__tree >> text=swg a')).toHaveCount(1); - await expect(page.locator('.js-elements-pool__tree >> text=swg b')).toHaveCount(0); - await expect(page.locator('.js-elements-pool__tree >> text=swg c')).toHaveCount(1); + await expect(swgAElementsPoolItem).toHaveCount(1); + await expect(swgBElementsPoolItem).toHaveCount(0); + await expect(swgCElementsPoolItem).toHaveCount(1); + }); + + test('Selecting a child plot while in browse and edit modes shows its properties in the inspector', async ({ page }) => { + await page.goto(stackedPlot.url); + + // Click on the 1st plot + await page.locator(`[aria-label="Stacked Plot Item ${swgA.name}"] canvas`).nth(1).click(); + + // Assert that the inspector shows the Y Axis properties for swgA + await expect(page.locator('[aria-label="Plot Series Properties"] >> h2')).toContainText("Plot Series"); + await expect(page.getByRole('list', { name: "Y Axis Properties" }).locator("h2")).toContainText("Y Axis"); + await expect(page.locator('[aria-label="Plot Series Properties"] .c-object-label')).toContainText(swgA.name); + + // Click on the 2nd plot + await page.locator(`[aria-label="Stacked Plot Item ${swgB.name}"] canvas`).nth(1).click(); + + // Assert that the inspector shows the Y Axis properties for swgB + await expect(page.locator('[aria-label="Plot Series Properties"] >> h2')).toContainText("Plot Series"); + await expect(page.getByRole('list', { name: "Y Axis Properties" }).locator("h2")).toContainText("Y Axis"); + await expect(page.locator('[aria-label="Plot Series Properties"] .c-object-label')).toContainText(swgB.name); + + // Click on the 3rd plot + await page.locator(`[aria-label="Stacked Plot Item ${swgC.name}"] canvas`).nth(1).click(); + + // Assert that the inspector shows the Y Axis properties for swgC + await expect(page.locator('[aria-label="Plot Series Properties"] >> h2')).toContainText("Plot Series"); + await expect(page.getByRole('list', { name: "Y Axis Properties" }).locator("h2")).toContainText("Y Axis"); + await expect(page.locator('[aria-label="Plot Series Properties"] .c-object-label')).toContainText(swgC.name); + + // Go into edit mode + await page.click('button[title="Edit"]'); + + // Click on canvas for the 1st plot + await page.locator(`[aria-label="Stacked Plot Item ${swgA.name}"]`).click(); + + // Assert that the inspector shows the Y Axis properties for swgA + await expect(page.locator('[aria-label="Plot Series Properties"] >> h2')).toContainText("Plot Series"); + await expect(page.getByRole('list', { name: "Y Axis Properties" }).locator("h2")).toContainText("Y Axis"); + await expect(page.locator('[aria-label="Plot Series Properties"] .c-object-label')).toContainText(swgA.name); + + //Click on canvas for the 2nd plot + await page.locator(`[aria-label="Stacked Plot Item ${swgB.name}"]`).click(); + + // Assert that the inspector shows the Y Axis properties for swgB + await expect(page.locator('[aria-label="Plot Series Properties"] >> h2')).toContainText("Plot Series"); + await expect(page.getByRole('list', { name: "Y Axis Properties" }).locator("h2")).toContainText("Y Axis"); + await expect(page.locator('[aria-label="Plot Series Properties"] .c-object-label')).toContainText(swgB.name); + + //Click on canvas for the 3rd plot + await page.locator(`[aria-label="Stacked Plot Item ${swgC.name}"]`).click(); + + // Assert that the inspector shows the Y Axis properties for swgC + await expect(page.locator('[aria-label="Plot Series Properties"] >> h2')).toContainText("Plot Series"); + await expect(page.getByRole('list', { name: "Y Axis Properties" }).locator("h2")).toContainText("Y Axis"); + await expect(page.locator('[aria-label="Plot Series Properties"] .c-object-label')).toContainText(swgC.name); }); }); diff --git a/src/plugins/plot/MctPlot.vue b/src/plugins/plot/MctPlot.vue index 49fc537c88..ecdee217ff 100644 --- a/src/plugins/plot/MctPlot.vue +++ b/src/plugins/plot/MctPlot.vue @@ -1190,28 +1190,42 @@ export default { selectNearbyAnnotations(event) { // need to stop propagation right away to prevent selecting the plot itself event.stopPropagation(); - if (!this.annotationViewingAndEditingAllowed || this.annotationSelections.length) { - return; - } const nearbyAnnotations = this.gatherNearbyAnnotations(); - if (!nearbyAnnotations.length) { - const emptySelection = this.createPathSelection(); - this.openmct.selection.select(emptySelection, true); - // should show plot itself if we didn't find any annotations + + if (this.annotationViewingAndEditingAllowed && this.annotationSelections.length) { + //no annotations were found, but we are adding some now + return; + } + + if (this.annotationViewingAndEditingAllowed && nearbyAnnotations.length) { + //show annotations if some were found + const { targetDomainObjects, targetDetails } = this.prepareExistingAnnotationSelection(nearbyAnnotations); + this.selectPlotAnnotations({ + targetDetails, + targetDomainObjects, + annotations: nearbyAnnotations + }); return; } - const { targetDomainObjects, targetDetails } = this.prepareExistingAnnotationSelection(nearbyAnnotations); - this.selectPlotAnnotations({ - targetDetails, - targetDomainObjects, - annotations: nearbyAnnotations - }); + //Fall through to here if either there is no new selection add tags or no existing annotations were retrieved + this.selectPlot(); + }, + selectPlot() { + // should show plot itself if we didn't find any annotations + const selection = this.createPathSelection(); + this.openmct.selection.select(selection, true); }, createPathSelection() { let selection = []; + selection.unshift({ + element: this.$el, + context: { + item: this.domainObject + } + }); this.path.forEach((pathObject, index) => { selection.push({ element: this.openmct.layout.$refs.browseObject.$el, diff --git a/src/plugins/plot/inspector/PlotOptionsBrowse.vue b/src/plugins/plot/inspector/PlotOptionsBrowse.vue index 9707f8a001..c99004d74a 100644 --- a/src/plugins/plot/inspector/PlotOptionsBrowse.vue +++ b/src/plugins/plot/inspector/PlotOptionsBrowse.vue @@ -27,6 +27,7 @@

    Plot Series

    Y Axis {{ yAxesWithSeries.length > 1 ? yAxis.id : '' }}

  • diff --git a/src/plugins/plot/inspector/PlotOptionsEdit.vue b/src/plugins/plot/inspector/PlotOptionsEdit.vue index ee83dd3081..a67baeec22 100644 --- a/src/plugins/plot/inspector/PlotOptionsEdit.vue +++ b/src/plugins/plot/inspector/PlotOptionsEdit.vue @@ -27,6 +27,7 @@

      Plot Series

    • -
        +

          Y Axis {{ id > 1 ? id : '' }}