Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add check to hide any old popup items when showing a new one #5680

Merged
merged 40 commits into from
Aug 31, 2022

Conversation

khalidadil
Copy link
Contributor

@khalidadil khalidadil commented Aug 18, 2022

Closes #5679

Describe your changes:

Added a check to hide any old popup items when showing a new one

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?
  • Changes appear not to be breaking changes?
  • Appropriate unit tests included?
  • Code style and in-line documentation are appropriate?
  • Commit messages meet standards?
  • 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)

michaelrogers and others added 18 commits August 2, 2022 13:44
* Add an active class to thumbnail to indicate current focused image

* Differentiate bg color between real-time and fixed

* scrollIntoView inline: center

* Added watcher for bounds change to trigger thumbnail scroll

* Resolve merge conflict with requestHistory change to telemetry collection

* Split thumbnail into sub component

* Monitor isFixed value to unpause playback status

Co-authored-by: Khalid Adil <khalidadil29@gmail.com>
* update selectors to use aria labels

* Update appActions

- Create new function `getHashUrlToDomainObject` to get the browse url to a given object given its uuid

- Create new function `getFocusedObjectUuid`... self explanatory :)

- Update `createDomainObjectWIthDefaults` to make use of the new url generation

- Update `createDomainObject...`'s arguments to be more organized, and accept a parent object

- Update some docs, still need to clarify some

* Update appActions e2e tests

- Refactor for organization

- Test our new appActions in one go

* Update existing usages of `createDomainObject...` to match the new API

* fix accidental renamed export

* Fix jsdoc return types

* refactor telemetryTable test to use appActions

* Improve selectors

* Refactor test

* improve selector

* add clock mode appActions

* lint

* Fix jsdoc

* Code review comments

* mark failing visual tests as fixme temporarily
…aned results (#5599)

* no matching result implemented

* now filtering annotations that are orphaned

* filter object results without valid paths

* add progress bar

* added e2e tests

* removed extraneous click

* fix typos

* fix unit tests

* lint

* address pr comments

* fix tests

* fix tests, centralize logic to object api, check for root instead

* remove debug statement

* lint

* fix documentation

* lint

* fix doc

* made some optimizations after talking with akhenry

* fix test

* update docs

* fix docs
* need to remove tags and objects on composition removal
* had to separate out emits from load as it was causing memory indexer to loop upon itself
* Add parsing util to identifier for ID comparison

* Moved firstIdentifier to top of function

* Lint fix

Co-authored-by: Andrew Henry <akhenry@gmail.com>
* check for circular references

* add test

* fix test

* address PR comments by making comments better

* fix docs...again
* do not create circular refs

* add negative validation test

* move to plugin

* add link test too

* fix docs

* refactored per john request

* fix path

* use appAction lib

Co-authored-by: Jesse Mazzella <ozyx@users.noreply.github.com>
* added unit tests for fault management plugin

* modified the example fault provider to work out of the box

* updating for new e2e folder structure

* part of the e2e tests

* WIP

* Imagery thumbnail regression fixes - 5327 (#5569)

* Add an active class to thumbnail to indicate current focused image

* Differentiate bg color between real-time and fixed

* scrollIntoView inline: center

* Added watcher for bounds change to trigger thumbnail scroll

* Resolve merge conflict with requestHistory change to telemetry collection

* Split thumbnail into sub component

* Monitor isFixed value to unpause playback status

* updated search to include name, namespace and description added some more e2e tests

* added rest of e2e tests

* fixed my init script, had to disable lint for no-force because it was not working without it, saw online this may be a pw bug

* fix: removing maelstrom theme from application (#5600)

* added some tests for no faults

* visual tests

* added visual tests for fault management

* created utils file for shared functionality between function and visual tests

* updating to 2.0.8

* tryin to remove imagery changes from master

* trying to trigger a refresh

* tryin to refresh

* updated search to include name, namespace and description added some more e2e tests

* added rest of e2e tests

* fix: removing maelstrom theme from application (#5600)

* fixed my init script, had to disable lint for no-force because it was not working without it, saw online this may be a pw bug

* added some tests for no faults

* visual tests

* added visual tests for fault management

* created utils file for shared functionality between function and visual tests

* updating to 2.0.8

* no clue

* still no clue

* removing imports and chaning to requires

* updating utils file to work with require

* fixing paths

* fixing a test I had messed up when adding static exmaple faults

* ONE LAST PATH FIX... hopefully

* typo in files fix

* fix folder typo

* thought I got this one, but apparently not, well I did now! who is laughing now!?

Co-authored-by: Michael Rogers <contact@mhrogers.com>
Co-authored-by: Vitor Henckel <vitor@henckel.com.br>
* fix typo

* Sort the tree items locally on object rename

* Use the navigationPath as a key

- This ensures that objects AND linked objects will be sorted

* add 'tree' and 'treeitem' roles to mct-tree

* WIP tree item reordering test

* Select the first object that matches

* Test that all object links are also reordered

* Get the final uuid before queryParams as notebook sections have uuids

* Make `openObjectTreeContextMenu` more deterministic and update usage

* Add `expandPathToTreeItem` and `expandTreeItemByName` appActions

* add `#tree-pane` id for the tree view

* Add tree visual component test suite and bump percy-cli

* Remove tree appActions

* Better variable name

Co-authored-by: Scott Bell <scott@traclabs.com>
@codecov
Copy link

codecov bot commented Aug 18, 2022

Codecov Report

Merging #5680 (5a14fb5) into master (0c9ea26) will decrease coverage by 0.00%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5680      +/-   ##
==========================================
- Coverage   54.26%   54.26%   -0.01%     
==========================================
  Files         597      597              
  Lines       22451    22453       +2     
  Branches     2062     2063       +1     
==========================================
+ Hits        12183    12184       +1     
- Misses       9676     9677       +1     
  Partials      592      592              
Flag Coverage Δ *Carryforward flag
e2e-ci 39.49% <ø> (ø) Carriedforward from 0c9ea26
e2e-full 49.77% <ø> (ø) Carriedforward from 0c9ea26
e2e-stable 49.41% <ø> (ø)
unit 50.64% <0.00%> (-0.02%) ⬇️

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

Impacted Files Coverage Δ
src/plugins/notebook/components/PopupMenu.vue 6.66% <0.00%> (-0.48%) ⬇️
...c/plugins/persistence/couch/CouchObjectProvider.js 57.52% <0.00%> (-0.25%) ⬇️
src/plugins/imagery/components/ImageryView.vue 40.41% <0.00%> (+0.45%) ⬆️

Continue to review full report at Codecov.

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

@unlikelyzero
Copy link
Collaborator

@khalidadil would you mind adding a simple e2e test to capture this behavior in the regular notebook suite?

@khalidadil khalidadil marked this pull request as draft August 18, 2022 15:43
@khalidadil
Copy link
Contributor Author

@khalidadil would you mind adding a simple e2e test to capture this behavior in the regular notebook suite?

yup, it looks like the notebook tests are a bit light, so I was planning on writing an e2e test to make sure there's not a regression on this fix.

ozyx and others added 3 commits August 18, 2022 21:23
* Use more deterministic selector

* Hover first to "slow down" e2e actions while in headless mode
@khalidadil khalidadil marked this pull request as ready for review August 18, 2022 22:18
@khalidadil khalidadil requested a review from ozyx August 18, 2022 22:18
@khalidadil khalidadil linked an issue Aug 18, 2022 that may be closed by this pull request
7 tasks
Copy link
Contributor

@ozyx ozyx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small suggestion on the e2e but looks great overall! Nice tests!

name: "Test Notebook"
});
});
test('Default sections are automatically named Unnamed Section with Unnamed Page', async ({ page }) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two tests could probably be combined into one. That would save us a tiny bit of runtime on the e2e tests.

You could even use test.step() to separate out the different test cases if you wanted to

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense! I just combined them into a single test

Copy link
Contributor

@ozyx ozyx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nicely done!

shefalijoshi and others added 5 commits August 19, 2022 15:47
…- 5632 (#5664)

* Set focused image when timestamp prop is passed in

* Unused var

* Create timestrip with imagery child

* Add equality check for hovered image and view large image url

* Cleanup
* Changes to Time List view. Closes #5534.
- Compacted table row spacing.
- Set all timeframes to display by default when creating a new Time List.
- Removed 'Upload plan' file button from properties.

* Changes to Time List view. Closes #5534.
- Better hint text for editing Timeframe Inspector section.

Co-authored-by: Andrew Henry <akhenry@gmail.com>
…ed. (#5654)

* Fix tests for interceptors
* [e2e] Add test for 'mine' folder initialization
* [e2e] don't fail on expected console errors

Co-authored-by: Andrew Henry <akhenry@gmail.com>
Co-authored-by: Scott Bell <scott@traclabs.com>
Co-authored-by: John Hill <john.c.hill@nasa.gov>
Co-authored-by: Jesse Mazzella <jesse.d.mazzella@nasa.gov>
Copy link
Collaborator

@unlikelyzero unlikelyzero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing

unlikelyzero and others added 6 commits August 22, 2022 13:03
* Update local CouchDB install docs to include docker workflow

* reformat to source configuration scripts where possible

* correct couchdb case

Co-authored-by: John Hill <john.c.hill@nasa.gov>
* the check for fixed time vs realtime was not updating, have fixed this

* merging in related changes from master pr #4414

* lint fixes

* Update src/plugins/timeConductor/ConductorHistory.vue

Co-authored-by: Jesse Mazzella <ozyx@users.noreply.github.com>

* setting time mode directly on load

* fixing issue where realtime history was being wiped on reloads while viewing fixed time

* formatting

* stubbed in some tests

Co-authored-by: Jesse Mazzella <ozyx@users.noreply.github.com>
@ozyx ozyx requested review from unlikelyzero and ozyx August 23, 2022 21:31
@ozyx ozyx changed the base branch from release/2.0.8 to master August 24, 2022 20:14
@ozyx
Copy link
Contributor

ozyx commented Aug 25, 2022

#2994

Might be fixed by this as well

@unlikelyzero
Copy link
Collaborator

@khalidadil there are merge conflicts. This is all that needs to be changed in the e2e test for now:

  //Test will need to be implemented after a refactor in #5713
    // eslint-disable-next-line playwright/no-skipped-test
    test.skip('Delete page popup is removed properly on clicking dropdown again', async ({ page }) => {
        test.info().annotations.push({
            type: 'issue',
            description: 'https://github.com/nasa/openmct/issues/5713'
        });

@ozyx
Copy link
Contributor

ozyx commented Aug 26, 2022

@khalidadil there are merge conflicts.

Went ahead and resolved those for you 🧚 ✨

@khalidadil
Copy link
Contributor Author

@khalidadil there are merge conflicts.

Went ahead and resolved those for you 🧚 ✨

Thank you!! 🙏🏽

@ozyx ozyx enabled auto-merge (squash) August 31, 2022 16:52
@ozyx ozyx merged commit 79d1df3 into master Aug 31, 2022
@ozyx ozyx deleted the fix/VIPEROMCT-160 branch August 13, 2024 22:20
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Notebook] Delete Page PopUp Does Not Go Away
10 participants