Handle negative height & width in image annotations #7116

scottbell · 2023-10-05T19:08:41Z

Describe your changes:

If the height or width are negative due the user dragging back and to the left, or forward and up, handle this properly when adding to the flatbush index.

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-10-05T19:14:03Z

Current Playwright Test Results Summary

✅ 14 Passing

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

(Last updated on 10/06/2023 05:04:38pm UTC)

Run Details

Running Workflow e2e-couchdb on Github Actions

Commit: 3084032

Started: 10/06/2023 04:59:41pm UTC

View Detailed Build Results

Current Playwright Test Results Summary

✅ 141 Passing - ⚠️ 3 Flaky

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

(Last updated on 10/06/2023 05:04:38pm UTC)

Run Details

Running Job e2e-stable on CircleCI

Commit: 3084032

Started: 10/06/2023 04:57:38pm UTC

⚠️ Flakes

📄 functional/plugins/notebook/restrictedNotebook.e2e.spec.js • 2 Flakes

Top 1 Common Error Messages

`null`	2 Test Cases Affected

Test Case Results

Test Case	Last 7 days Failures	Last 7 days Flakes
Restricted Notebook with a page locked and with an embed @addinit Allows embeds to be deleted if page unlocked @addinit Retry 1 • Initial Attempt	8.33% (7) 7 / 84 runs failed over last 7 days	45.24% (38) 38 / 84 runs flaked over last 7 days
Restricted Notebook with a page locked and with an embed @addinit Disallows embeds to be deleted if page locked @addinit Retry 1 • Initial Attempt	0% (0) 0 / 75 runs failed over last 7 days	33.33% (25) 25 / 75 runs flaked over last 7 days

📄 functional/planning/timelist.e2e.spec.js • 1 Flake

Test Case Results

Test Case	Last 7 days Failures	Last 7 days Flakes
Time List Create a Time List, add a single Plan to it and verify all the activities are displayed with no milliseconds Retry 1 • Initial Attempt	0% (0) 0 / 125 runs failed over last 7 days	54.40% (68) 68 / 125 runs flaked over last 7 days

View Detailed Build Results

codecov · 2023-10-05T19:17:21Z

Codecov Report

Merging #7116 (3084032) into master (734a8dd) will decrease coverage by 0.10%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master    #7116      +/-   ##
==========================================
- Coverage   55.85%   55.76%   -0.10%     
==========================================
  Files         650      650              
  Lines       26092    26103      +11     
  Branches     2519     2521       +2     
==========================================
- Hits        14574    14556      -18     
- Misses      10823    10843      +20     
- Partials      695      704       +9

Flag	Coverage Δ		*Carryforward flag
e2e-full	`41.85% <ø> (-0.02%)`	⬇️	Carriedforward from 5fcd118
e2e-stable	`57.28% <ø> (+0.03%)`	⬆️
unit	`49.32% <0.00%> (-0.13%)`	⬇️

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

Files	Coverage Δ
...c/plugins/imagery/components/AnnotationsCanvas.vue	`2.43% <0.00%> (-0.18%)`	⬇️

... and 12 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 734a8dd...3084032. Read the comment docs.

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

ozyx

Awesome! Just a couple minor suggestions before we merge

src/plugins/imagery/components/AnnotationsCanvas.vue

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

ozyx · 2023-10-06T16:59:02Z

src/plugins/imagery/components/AnnotationsCanvas.vue

@@ -124,6 +122,23 @@ export default {
      this.selectedAnnotations = annotations;
      this.$emit('annotations-changed', annotations);
    },
+    transformAnnotationRectangleToFlatbushRectangle(annotationRectangle) {


ozyx

🤌

add fix

21e0cce

scottbell linked an issue Oct 5, 2023 that may be closed by this pull request

It is possible to create image annotations that cannot be selected #7083

Closed

7 tasks

remove debug code

8cf7cf8

add test for unholy rectangles

be65d29

scottbell commented Oct 6, 2023

View reviewed changes

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

make test a bit more forgiving

5fcd118

scottbell added the pr:e2e:couchdb npm run test:e2e:couchdb label Oct 6, 2023

scottbell marked this pull request as ready for review October 6, 2023 09:06

scottbell requested a review from ozyx October 6, 2023 09:06

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

ozyx requested changes Oct 6, 2023

View reviewed changes

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

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

address pr review

3084032

scottbell requested a review from ozyx October 6, 2023 16:56

scottbell added the pr:e2e:couchdb npm run test:e2e:couchdb label Oct 6, 2023

ozyx reviewed Oct 6, 2023

View reviewed changes

ozyx approved these changes Oct 6, 2023

View reviewed changes

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

scottbell merged commit 084784a into master Oct 6, 2023
17 checks passed

scottbell deleted the 7083-it-is-possible-to-create-image-annotations-that-cannot-be-selected branch October 6, 2023 17:08

unlikelyzero added this to the Target:3.2.0 milestone Nov 13, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Handle negative height & width in image annotations #7116

Handle negative height & width in image annotations #7116

scottbell commented Oct 5, 2023 •

edited

Loading

deploysentinel bot commented Oct 5, 2023 •

edited

Loading

Top 1 Common Error Messages

Test Case Results

Test Case Results

codecov bot commented Oct 5, 2023 •

edited

Loading

ozyx left a comment

ozyx Oct 6, 2023

ozyx left a comment

Handle negative height & width in image annotations #7116

Handle negative height & width in image annotations #7116

Conversation

scottbell commented Oct 5, 2023 • edited Loading

Describe your changes:

All Submissions:

Author Checklist

Reviewer Checklist

deploysentinel bot commented Oct 5, 2023 • edited Loading

Current Playwright Test Results Summary

Current Playwright Test Results Summary

⚠️ Flakes

Top 1 Common Error Messages

Test Case Results

Test Case Results

codecov bot commented Oct 5, 2023 • edited Loading

Codecov Report

ozyx left a comment

Choose a reason for hiding this comment

ozyx Oct 6, 2023

Choose a reason for hiding this comment

ozyx left a comment

Choose a reason for hiding this comment

scottbell commented Oct 5, 2023 •

edited

Loading

deploysentinel bot commented Oct 5, 2023 •

edited

Loading

codecov bot commented Oct 5, 2023 •

edited

Loading