fix(#7055): Expand on image with double click #7308

marcelo-earth · 2023-12-17T20:59:44Z

Describe your changes:

I added the expand function to dblclick function, allowing users to double-click on an image to enlarge it. I made a modification to the Imagery tests to verify this feature.

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?
Has this been smoke tested?
Have you associated this PR with a type: label? Note: this is not necessarily the same as the original issue.
Have you associated a milestone with this PR? Note: leave blank if unsure.
Is this a breaking change to be called out in the release notes?
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?

button

deploysentinel · 2023-12-17T21:05:27Z

Current Playwright Test Results Summary

✅ 176 Passing - ⚠️ 3 Flaky

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

(Last updated on 01/10/2024 03:30:41am UTC)

Run Details

Running Job e2e-stable on CircleCI

Commit: bc04900

Started: 01/10/2024 03:25:26am UTC

⚠️ Flakes

📄 functional/plugins/imagery/exampleImagery.e2e.spec.js • 1 Flake

Test Case Results

Test Case	Last 7 days Failures	Last 7 days Flakes
Example Imagery in Display Layout Resizing the layout changes thumbnail visibility and size Retry 1 • Initial Attempt	0% (0) 0 / 52 runs failed over last 7 days	5.77% (3) 3 / 52 runs flaked over last 7 days

📄 functional/plugins/tabs/tabs.e2e.spec.js • 1 Flake

Test Case Results

Test Case	Last 7 days Failures	Last 7 days Flakes
Tabs View Renders tabbed elements Retry 1 • Initial Attempt	5% (3) 3 / 60 runs failed over last 7 days	35% (21) 21 / 60 runs flaked over last 7 days

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

Test Case Results

Test Case	Last 7 days Failures	Last 7 days Flakes
Restricted Notebook with a page locked and with an embed @addinit Disallows embeds to be deleted if page locked @addinit Retry 1 • Initial Attempt	4.84% (3) 3 / 62 runs failed over last 7 days	53.23% (33) 33 / 62 runs flaked over last 7 days

View Detailed Build Results

codecov · 2023-12-17T21:19:09Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (6ce340c) 55.85% compared to head (bc04900) 55.87%.

❗ Current head bc04900 differs from pull request most recent head f0dbf70. Consider uploading reports for the commit f0dbf70 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7308      +/-   ##
==========================================
+ Coverage   55.85%   55.87%   +0.02%     
==========================================
  Files         659      657       -2     
  Lines       26243    26198      -45     
  Branches     2549     2548       -1     
==========================================
- Hits        14658    14639      -19     
+ Misses      10880    10854      -26     
  Partials      705      705

Flag	Coverage Δ
e2e-full	`41.06% <ø> (-0.08%)`	⬇️
e2e-stable	`59.09% <ø> (+0.04%)`	⬆️
unit	`48.86% <100.00%> (-0.03%)`	⬇️

Files	Coverage Δ
src/plugins/imagery/components/ImageryView.vue	`40.19% <100.00%> (ø)`

... and 13 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 6ce340c...f0dbf70. Read the comment docs.

e2e/tests/performance/contract/imagery.contract.perf.spec.js

ozyx

~~I can't seem to get this to work. Am I doing something wrong?~~ Yep, I am. Need to embed the imagery in a layout first.

ozyx

Looks good overall! Just had one small suggestion for consistency. And I agree with @unlikelyzero that we should have a functional test for this instead of the perf test.

src/plugins/imagery/components/ImageryView.vue

…lity

ozyx

Looking good, just have a few comments for the e2e.

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

ozyx · 2023-12-19T20:32:57Z

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

+
+    test('Can double-click on the image to view large image', async ({ page }) => {
+      // Double-click on the image to open large view
+      await page.locator('.image-wrapper').dblclick();


Same thing here. Is there a way we could avoid .locator() ?

Sure! But I'm not sure if I did it right, would waitForSelector('.image-wrapper') work? If not, what would you recommend me to do (or use as the method to get the element)?

We're moving away from page.locator() in general and we now prefer to use ARIA based methods such as page.getByRole(). One way we could improve this could be by adding an accessible name (such as an aria-label to the img element. We could then update the locator to something like this:

await page.getByRole('img', { name: 'Focused Image' });

Thank you! I have replaced the pageSelector of the image wrapper and the background image with getByRole.

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

large view in imagery.contract.per.spec.js

ozyx

Nice work! Just a couple small comments but otherwise LGTM. Thanks for this!

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

src/plugins/imagery/components/ImageryView.vue

marcelo-earth · 2024-01-10T03:01:40Z

Nice work! Just a couple small comments but otherwise LGTM. Thanks for this!

Thank you! Done.

marcelo-earth added 2 commits December 16, 2023 09:07

Add double click event to expand image view

59d3573

Add performance tests for large view and close

8bede7b

button

ozyx added the source:community Community contribution or request label Dec 18, 2023

unlikelyzero reviewed Dec 18, 2023

View reviewed changes

e2e/tests/performance/contract/imagery.contract.perf.spec.js Outdated Show resolved Hide resolved

unlikelyzero changed the title ~~Expand on image with double click~~ [CLA Approved] Expand on image with double click Dec 18, 2023

ozyx self-requested a review December 18, 2023 18:58

ozyx reviewed Dec 18, 2023

View reviewed changes

ozyx added the type:bug label Dec 18, 2023

ozyx added this to the Target:3.3.0 milestone Dec 18, 2023

ozyx suggested changes Dec 18, 2023

View reviewed changes

src/plugins/imagery/components/ImageryView.vue Show resolved Hide resolved

marcelo-earth added 2 commits December 18, 2023 20:20

Add example imagery in flexible layout to test double-click functiona…

444e504

…lity

Remove click event listener in ImageryView.vue

3d3227f

ozyx suggested changes Dec 19, 2023

View reviewed changes

Move test to 'Example Imagery in Flexible layout'

f0a25e5

ozyx reviewed Dec 20, 2023

View reviewed changes

e2e/tests/functional/plugins/imagery/exampleImagery.e2e.spec.js Outdated Show resolved Hide resolved

marcelo-earth added 4 commits December 20, 2023 13:39

Refactor exampleImagery.e2e.spec.js

9171c89

Remove unnecessary code for opening and closing

a52482e

large view in imagery.contract.per.spec.js

Replace pageSelector with getByRole on double-click image test

c4a286b

Add role and aria-label to focused image

12d6726

ozyx self-requested a review January 8, 2024 18:21

ozyx added the pr:e2e:couchdb npm run test:e2e:couchdb label Jan 8, 2024

Merge branch 'master' into 7055-add-expand-with-double-click

c2270ab

ozyx added pr:e2e:couchdb npm run test:e2e:couchdb and removed pr:e2e:couchdb npm run test:e2e:couchdb labels Jan 8, 2024

ozyx approved these changes Jan 8, 2024

View reviewed changes

e2e/tests/functional/plugins/imagery/exampleImagery.e2e.spec.js Outdated Show resolved Hide resolved

src/plugins/imagery/components/ImageryView.vue Outdated Show resolved Hide resolved

ozyx changed the title ~~[CLA Approved] Expand on image with double click~~ Expand on image with double click Jan 8, 2024

ozyx changed the title ~~Expand on image with double click~~ fix(#7055): Expand on image with double click Jan 8, 2024

Remove unnecessary code and role attribute from ImageryView.vue

590d94e

Merge branch 'master' into 7055-add-expand-with-double-click

bc04900

ozyx added pr:e2e:couchdb npm run test:e2e:couchdb and removed pr:e2e:couchdb npm run test:e2e:couchdb labels Jan 10, 2024

ozyx enabled auto-merge (squash) January 14, 2024 23:40

Merge branch 'master' into 7055-add-expand-with-double-click

f0dbf70

ozyx added pr:e2e:couchdb npm run test:e2e:couchdb and removed pr:e2e:couchdb npm run test:e2e:couchdb labels Jan 14, 2024

ozyx merged commit 11f3ce9 into nasa:master Jan 14, 2024
21 of 22 checks passed

unlikelyzero modified the milestones: Target:3.3.0, Target:4.0.0 Jan 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(#7055): Expand on image with double click #7308

fix(#7055): Expand on image with double click #7308

marcelo-earth commented Dec 17, 2023 •

edited by ozyx

Loading

deploysentinel bot commented Dec 17, 2023 •

edited

Loading

Test Case Results

Test Case Results

Test Case Results

codecov bot commented Dec 17, 2023 •

edited

Loading

ozyx left a comment •

edited

Loading

ozyx left a comment

ozyx left a comment

ozyx Dec 19, 2023

marcelo-earth Dec 20, 2023

ozyx Jan 2, 2024

marcelo-earth Jan 8, 2024

ozyx left a comment

marcelo-earth commented Jan 10, 2024

fix(#7055): Expand on image with double click #7308

fix(#7055): Expand on image with double click #7308

Conversation

marcelo-earth commented Dec 17, 2023 • edited by ozyx Loading

Describe your changes:

All Submissions:

Author Checklist

Reviewer Checklist

deploysentinel bot commented Dec 17, 2023 • edited Loading

Current Playwright Test Results Summary

⚠️ Flakes

Test Case Results

Test Case Results

Test Case Results

codecov bot commented Dec 17, 2023 • edited Loading

Codecov Report

ozyx left a comment • edited Loading

Choose a reason for hiding this comment

ozyx left a comment

Choose a reason for hiding this comment

ozyx left a comment

Choose a reason for hiding this comment

ozyx Dec 19, 2023

Choose a reason for hiding this comment

marcelo-earth Dec 20, 2023

Choose a reason for hiding this comment

ozyx Jan 2, 2024

Choose a reason for hiding this comment

marcelo-earth Jan 8, 2024

Choose a reason for hiding this comment

ozyx left a comment

Choose a reason for hiding this comment

marcelo-earth commented Jan 10, 2024

marcelo-earth commented Dec 17, 2023 •

edited by ozyx

Loading

deploysentinel bot commented Dec 17, 2023 •

edited

Loading

codecov bot commented Dec 17, 2023 •

edited

Loading

ozyx left a comment •

edited

Loading