Fix controls scope to only the current image #6710

davetsay · 2023-06-03T22:57:48Z

Closes #6709

Describe your changes:

Removed ids from dom elements and scoped labels to the inputs by wrapping inputs in label elements.

All Submissions:

Have you followed the guidelines in our Contributing document?
Have you checked to ensure there aren't other open Pull Requests for the same update/change?
Is this change backwards compatible? For example, developers won't need to change how they are calling the API or how they've extended core plugins such as Tables or Plots.

Author Checklist

Changes address original issue?
Tests included and/or updated with changes?
Command line build passes?
Has this been smoke tested?
Testing instructions included in associated issue OR is this a dependency/testcase change?

Reviewer Checklist

Changes appear to address issue?
Reviewer has tested changes by following the provided instructions?
Changes appear not to be breaking changes?
Appropriate automated tests included?
Code style and in-line documentation are appropriate?
Has associated issue been labelled unverified? (only applicable if this PR closes the issue)
Has associated issue been labelled bug? (only applicable if this PR is for a bug fix)

deploysentinel · 2023-06-03T23:02:21Z

Current Playwright Test Results Summary

✅ 13 Passing - ⚠️ 2 Flaky

Run may still be in progress, this comment will be updated as current testing workflow or job completes...

(Last updated on 06/16/2023 10:14:21pm UTC)

Run Details

Running Workflow e2e-couchdb on Github Actions

Commit: 472757a

Started: 06/16/2023 10:10:25pm UTC

⚠️ Flakes

📄 functional/plugins/notebook/notebookWithCouchDB.e2e.spec.js • 1 Flake

Test Case Results

Test Case	Last 7 days Failures	Last 7 days Flakes
Notebook Tests with CouchDB @couchdb Inspect Notebook Entry Network Requests Retry 1 • Initial Attempt	3.70% (1) 1 / 27 run failed over last 7 days	40.74% (11) 11 / 27 runs flaked over last 7 days

📄 functional/couchdb.e2e.spec.js • 1 Flake

Test Case Results

Test Case	Last 7 days Failures	Last 7 days Flakes
CouchDB Status Indicator with mocked responses @couchdb Shows red if not connected Retry 1 • Initial Attempt	0% (0) 0 / 27 runs failed over last 7 days	7.41% (2) 2 / 27 runs flaked over last 7 days

View Detailed Build Results

codecov · 2023-06-03T23:06:31Z

Codecov Report

Merging #6710 (d57ee75) into master (022dffd) will decrease coverage by 0.43%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #6710      +/-   ##
==========================================
- Coverage   54.18%   53.76%   -0.43%     
==========================================
  Files         623      623              
  Lines       24656    24656              
  Branches     2469     2469              
==========================================
- Hits        13361    13257     -104     
- Misses      10631    10734     +103     
- Partials      664      665       +1

Flag	Coverage Δ		*Carryforward flag
e2e-full	`42.21% <ø> (ø)`		Carriedforward from 0821bb8
e2e-stable	`51.43% <ø> (-4.12%)`	⬇️
unit	`48.82% <100.00%> (-0.03%)`	⬇️

*This pull request uses carry forward flags. Click here to find out more.

Impacted Files	Coverage Δ
src/plugins/imagery/components/LayerSettings.vue	`0.00% <ø> (ø)`
src/plugins/imagery/components/ImageControls.vue	`36.92% <100.00%> (ø)`

... and 28 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 022dffd...d57ee75. Read the comment docs.

akhenry · 2023-06-05T13:29:27Z

Removed ids from dom elements and scoped labels to the inputs by wrapping inputs in label elements.

Oh that's what was going on. I saw the behavior and assumed state was being tracked on a shared object. Nice one, thanks for fixing.

ozyx

Smoke tested locally and this is working great. I have some small suggestions to improve the e2e test, otherwise LGTM!

e2e/tests/functional/plugins/imagery/exampleImagery.e2e.spec.js

ozyx · 2023-06-06T16:34:38Z

e2e/tests/functional/plugins/imagery/exampleImagery.e2e.spec.js

+
+        await expect(imageElements).toHaveCount(2);
+
+        const imageOne = await page.locator('.c-imagery__main-image-wrapper').nth(0);


Nice work with these locators! Small nitpick, .locator() is synchronous so we can remove the await from each of these calls.

ozyx · 2023-06-06T16:40:57Z

src/plugins/imagery/components/LayerSettings.vue

+                v-for="(layer, index) in layers"
+                :key="index"
+            >
+                <label>


👏👏👏

Co-authored-by: Jesse Mazzella <[email protected]>

davetsay added 7 commits May 17, 2023 11:19

de-dupe method

ae5b75a

there can be only one... input per label

9a189b0

there can be only one... id but we need none

9c64c3e

there can be only one... input

80974c8

create test and add multiple images to display

1d37277

WIP test written but not passing

078cb1c

fix test

dfed8c5

Merge branch 'master' into image-grid

8512fef

ozyx self-requested a review June 5, 2023 22:39

ozyx suggested changes Jun 6, 2023

View reviewed changes

davetsay and others added 4 commits June 16, 2023 13:52

Update e2e/tests/functional/plugins/imagery/exampleImagery.e2e.spec.js

a8b2d2b

Co-authored-by: Jesse Mazzella <[email protected]>

Update e2e/tests/functional/plugins/imagery/exampleImagery.e2e.spec.js

b2c4c97

Co-authored-by: Jesse Mazzella <[email protected]>

Merge branch 'master' into image-grid

acd80c3

remove await from synchronous code

0821bb8

davetsay requested a review from ozyx June 16, 2023 21:06

ozyx added this to the Target:2.2.5 milestone Jun 16, 2023

ozyx approved these changes Jun 16, 2023

View reviewed changes

linting

d57ee75

ozyx enabled auto-merge (squash) June 16, 2023 22:05

ozyx added the pr:e2e:couchdb npm run test:e2e:couchdb label Jun 16, 2023

github-actions bot removed the pr:e2e:couchdb npm run test:e2e:couchdb label Jun 16, 2023

ozyx merged commit bd5cb81 into master Jun 16, 2023

ozyx deleted the image-grid branch June 16, 2023 22:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix controls scope to only the current image #6710

Fix controls scope to only the current image #6710

davetsay commented Jun 3, 2023 •

edited by ozyx

Loading

deploysentinel bot commented Jun 3, 2023 •

edited

Loading

Test Case Results

Test Case Results

codecov bot commented Jun 3, 2023 •

edited

Loading

akhenry commented Jun 5, 2023 •

edited

Loading

ozyx left a comment

ozyx Jun 6, 2023

ozyx Jun 6, 2023


		await expect(imageElements).toHaveCount(2);

		const imageOne = await page.locator('.c-imagery__main-image-wrapper').nth(0);

Fix controls scope to only the current image #6710

Fix controls scope to only the current image #6710

Conversation

davetsay commented Jun 3, 2023 • edited by ozyx Loading

Describe your changes:

All Submissions:

Author Checklist

Reviewer Checklist

deploysentinel bot commented Jun 3, 2023 • edited Loading

Current Playwright Test Results Summary

⚠️ Flakes

Test Case Results

Test Case Results

codecov bot commented Jun 3, 2023 • edited Loading

Codecov Report

akhenry commented Jun 5, 2023 • edited Loading

ozyx left a comment

Choose a reason for hiding this comment

ozyx Jun 6, 2023

Choose a reason for hiding this comment

ozyx Jun 6, 2023

Choose a reason for hiding this comment

davetsay commented Jun 3, 2023 •

edited by ozyx

Loading

deploysentinel bot commented Jun 3, 2023 •

edited

Loading

codecov bot commented Jun 3, 2023 •

edited

Loading

akhenry commented Jun 5, 2023 •

edited

Loading