Only show marquee for selected item #7180

davetsay · 2023-10-26T18:22:43Z

Closes #7179

Describe your changes:

Added check to see if selection is in the currently selected layout

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-26T18:27:58Z

Current Playwright Test Results Summary

✅ 159 Passing - ⚠️ 2 Flaky

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

(Last updated on 11/16/2023 06:03:28pm UTC)

Run Details

Running Job e2e-stable on CircleCI

Commit: 5bbf4a6

Started: 11/16/2023 05:56:48pm UTC

⚠️ Flakes

📄 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	1.19% (1) 1 / 84 run failed over last 7 days	54.76% (46) 46 / 84 runs flaked over last 7 days

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

Test Case Results

Test Case	Last 7 days Failures	Last 7 days Flakes
Snapshot image tests Can drop an image onto a notebook and create a new entry Retry 1 • Initial Attempt	0% (0) 0 / 52 runs failed over last 7 days	73.08% (38) 38 / 52 runs flaked over last 7 days

View Detailed Build Results

Current Playwright Test Results Summary

✅ 14 Passing - ⚠️ 1 Flaky

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

(Last updated on 11/16/2023 06:03:28pm UTC)

Run Details

Running Workflow e2e-couchdb on Github Actions

Commit: 5bbf4a6

Started: 11/16/2023 05:57:16pm UTC

⚠️ Flakes

📄 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 / 30 runs failed over last 7 days	20% (6) 6 / 30 runs flaked over last 7 days

View Detailed Build Results

davetsay · 2023-10-26T18:51:21Z

Was scratching my head on this one for a bit. Was unsure why this regressed when the code didn't change much. It looks like regardless of showMarquee check, the marquee is further controlled by css. Something with selectors, the html structure, must have changed as a result of the Vue 3 upgrade, since Vue 2 components mounted by replacing the element, while Vue 3 does not.

@charlesh88 , do you want to look at the css, or this JS fix should work to fix.

charlesh88 · 2023-10-26T19:08:05Z

Seeing a new problem: the selection marquee is now hidden after the first manipulation of an object, via the cursor or numeric input position and size inputs in the toolbar. The object remains selected and it can be manipulated (moved with drag and changed via toolbar inputs), but the marquee isn't displayed. Another object can't be selected unless the first is deselected, or by clicking a new object twice. This is the case for all elements with 4 control handles: drawing objects, alphanumerics, nested layouts - everything but the line drawing object.

Display.Layout.7179.selection.marquee.lost.after.manipulation.mp4

unlikelyzero

Sync with me on how to write a test for this

davetsay · 2023-10-27T19:54:48Z

Seeing a new problem: the selection marquee is now hidden after the first manipulation of an object, via the cursor or numeric input position and size inputs in the toolbar. The object remains selected and it can be manipulated (moved with drag and changed via toolbar inputs), but the marquee isn't displayed. Another object can't be selected unless the first is deselected, or by clicking a new object twice. This is the case for all elements with 4 control handles: drawing objects, alphanumerics, nested layouts - everything but the line drawing object.

Display.Layout.7179.selection.marquee.lost.after.manipulation.mp4

Weird but I see it too. Will look into it. What do you think about the css? Worth fixing?

davetsay · 2023-10-27T19:56:49Z

Sync with me on how to write a test for this

e2e or visual? I should be good on e2e. I should put this into draft until we decide what the proper fix is.

charlesh88 · 2023-10-30T16:26:00Z

Weird but I see it too. Will look into it. What do you think about the css? Worth fixing?

I will take a further look.

ozyx · 2023-11-06T17:52:17Z

Sync with me on how to write a test for this

e2e or visual? I should be good on e2e. I should put this into draft until we decide what the proper fix is.

probably a visual i'd think

This reverts commit d17af21.

codecov · 2023-11-07T00:13:21Z

Codecov Report

Merging #7180 (5bbf4a6) into master (cdd772a) will decrease coverage by 0.07%.
The diff coverage is n/a.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7180      +/-   ##
==========================================
- Coverage   55.96%   55.89%   -0.07%     
==========================================
  Files         653      653              
  Lines       26213    26213              
  Branches     2525     2525              
==========================================
- Hits        14669    14653      -16     
- Misses      10839    10853      +14     
- Partials      705      707       +2

Flag	Coverage Δ		*Carryforward flag
e2e-full	`41.92% <ø> (ø)`		Carriedforward from 6218c85
e2e-stable	`58.22% <ø> (-0.03%)`	⬇️
unit	`49.18% <ø> (-0.09%)`	⬇️

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

Files	Coverage Δ
...plugins/displayLayout/components/DisplayLayout.vue	`12.47% <ø> (ø)`

... and 8 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 cdd772a...5bbf4a6. Read the comment docs.

e2e/tests/visual/displayLayout.visual.spec.js

davetsay · 2023-11-15T01:14:24Z

Sync with me on how to write a test for this

How'd I do?

davetsay · 2023-11-15T01:15:44Z

@charlesh88 , somewhere I think grids also regressed. Please verify if grids are as expected.
Marquee only for selected. Grid only on parent of selected.

ozyx · 2023-11-15T21:40:52Z

e2e/tests/visual/displayLayout.visual.spec.js

+  });
+});
+
+async function setupBaseline(page, theme, openmctConfig) {


Let's add some JSDocs here with valid types

ozyx

Nice work!

e2e/tests/visual/displayLayout.visual.spec.js

unlikelyzero

I'm not seeing the test results. Let's sync first thing tomorrow

e2e/tests/visual/displayLayout.visual.spec.js

unlikelyzero · 2023-11-16T01:08:38Z

e2e/tests/visual/displayLayout.visual.spec.js

+    name: 'Main Tree'
+  });
+
+  const objectViewLocator = page.locator('.c-object-view');


generally, the practice would be to improve the locator such that we don't reference this by classname

only show marquee for selected item

d17af21

davetsay requested review from shefalijoshi and ozyx October 26, 2023 18:22

davetsay requested a review from charlesh88 October 26, 2023 18:47

unlikelyzero self-requested a review October 27, 2023 13:41

unlikelyzero added this to the Target:3.2.1 milestone Oct 27, 2023

unlikelyzero suggested changes Oct 27, 2023

View reviewed changes

davetsay marked this pull request as draft October 27, 2023 19:57

shefalijoshi removed their request for review November 2, 2023 16:01

davetsay added 2 commits November 6, 2023 16:11

Revert "only show marquee for selected item"

de99517

This reverts commit d17af21.

revert change made in #6767

14f2fcf

create framework for displayLayout visual test

b2a91eb

github-advanced-security bot found potential problems Nov 7, 2023

View reviewed changes

e2e/tests/visual/displayLayout.visual.spec.js Fixed Show fixed Hide fixed

e2e/tests/visual/displayLayout.visual.spec.js Fixed Show fixed Hide fixed

e2e/tests/visual/displayLayout.visual.spec.js Fixed Show fixed Hide fixed

WIP create display layout for test

3f157ba

github-advanced-security bot found potential problems Nov 7, 2023

View reviewed changes

e2e/tests/visual/displayLayout.visual.spec.js Fixed Show fixed Hide fixed

davetsay added 3 commits November 7, 2023 16:55

only show marquee for selected

4cb146e

fix selection of object in nested layout

207d225

fix grid and code cleanup

9721d00

unlikelyzero modified the milestones: Target:3.2.1, Target:3.3.0 Nov 13, 2023

davetsay added 2 commits November 14, 2023 12:20

add child layouts side by side

904e1d0

code cleanup

15df590

davetsay requested a review from unlikelyzero November 15, 2023 01:15

davetsay added 2 commits November 15, 2023 13:01

fix nested layout selector

14431ea

add aria label to layouts

949d6fa

ozyx suggested changes Nov 15, 2023

View reviewed changes

davetsay added 2 commits November 15, 2023 15:01

fix layout locator

9fca586

add jsdoc for test setup function

22c7064

davetsay requested a review from ozyx November 15, 2023 23:02

davetsay added the pr:e2e:couchdb npm run test:e2e:couchdb label Nov 15, 2023

ozyx approved these changes Nov 15, 2023

View reviewed changes

github-advanced-security bot found potential problems Nov 15, 2023

View reviewed changes

e2e/tests/visual/displayLayout.visual.spec.js Fixed Show fixed Hide fixed

e2e/tests/visual/displayLayout.visual.spec.js Fixed Show fixed Hide fixed

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

unlikelyzero suggested changes Nov 15, 2023

View reviewed changes

make test more efficient

ef83f61

github-advanced-security bot found potential problems Nov 15, 2023

View reviewed changes

e2e/tests/visual/displayLayout.visual.spec.js Fixed Show fixed Hide fixed

unlikelyzero reviewed Nov 16, 2023

View reviewed changes

davetsay and others added 3 commits November 15, 2023 17:22

cleanup and linting

3985d22

update locator

a8a7368

update locators

6218c85

unlikelyzero modified the milestones: Target:3.3.0, Target:3.2.0 Nov 16, 2023

Merge branch 'master' into fix/7179

5bbf4a6

unlikelyzero enabled auto-merge (squash) November 16, 2023 17:54

unlikelyzero added the pr:e2e:couchdb npm run test:e2e:couchdb label Nov 16, 2023

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

davetsay requested a review from unlikelyzero November 16, 2023 18:20

unlikelyzero approved these changes Nov 16, 2023

View reviewed changes

unlikelyzero merged commit a914e4f into master Nov 16, 2023

unlikelyzero deleted the fix/7179 branch November 16, 2023 18:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Only show marquee for selected item #7180

Only show marquee for selected item #7180

davetsay commented Oct 26, 2023 •

edited

Loading

deploysentinel bot commented Oct 26, 2023 •

edited

Loading

Test Case Results

Test Case Results

Test Case Results

davetsay commented Oct 26, 2023

charlesh88 commented Oct 26, 2023 •

edited

Loading

unlikelyzero left a comment

davetsay commented Oct 27, 2023

davetsay commented Oct 27, 2023

charlesh88 commented Oct 30, 2023

ozyx commented Nov 6, 2023

codecov bot commented Nov 7, 2023 •

edited

Loading

davetsay commented Nov 15, 2023

davetsay commented Nov 15, 2023

ozyx Nov 15, 2023

ozyx left a comment

unlikelyzero left a comment

unlikelyzero Nov 16, 2023

Only show marquee for selected item #7180

Only show marquee for selected item #7180

Conversation

davetsay commented Oct 26, 2023 • edited Loading

Describe your changes:

All Submissions:

Author Checklist

Reviewer Checklist

deploysentinel bot commented Oct 26, 2023 • edited Loading

Current Playwright Test Results Summary

⚠️ Flakes

Test Case Results

Test Case Results

Current Playwright Test Results Summary

⚠️ Flakes

Test Case Results

davetsay commented Oct 26, 2023

charlesh88 commented Oct 26, 2023 • edited Loading

unlikelyzero left a comment

Choose a reason for hiding this comment

davetsay commented Oct 27, 2023

davetsay commented Oct 27, 2023

charlesh88 commented Oct 30, 2023

ozyx commented Nov 6, 2023

codecov bot commented Nov 7, 2023 • edited Loading

Codecov Report

davetsay commented Nov 15, 2023

davetsay commented Nov 15, 2023

ozyx Nov 15, 2023

Choose a reason for hiding this comment

ozyx left a comment

Choose a reason for hiding this comment

unlikelyzero left a comment

Choose a reason for hiding this comment

unlikelyzero Nov 16, 2023

Choose a reason for hiding this comment

davetsay commented Oct 26, 2023 •

edited

Loading

deploysentinel bot commented Oct 26, 2023 •

edited

Loading

charlesh88 commented Oct 26, 2023 •

edited

Loading

codecov bot commented Nov 7, 2023 •

edited

Loading