Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

[DisplayLayout] Object removal from object tree doesn't consistently remove object from Display Layout in main window #3117

Closed
johnriedel opened this issue Jun 25, 2020 · 7 comments · Fixed by #5941
Closed

[DisplayLayout] Object removal from object tree doesn't consistently remove object from Display Layout in main window #3117

johnriedel opened this issue Jun 25, 2020 · 7 comments · Fixed by #5941
Assignees
@ozyx
Labels
bug:regression It used to work. Now it doesn't :( severity:critical type:bug
Milestone
Target:3.0.0

Comments

@johnriedel
Copy link
Contributor

johnriedel commented Jun 25, 2020 •
edited
Loading

Overview to reproduce error:

  1. Add an item to display layout, then "Save and Finish Editing"
  2. Select the child item in the object tree so that it's active in the main window
  3. Right-click and remove object. Notice it was removed from object tree but not display layout.

Detailed steps to reproduce error:

  1. Add a display layout to "My Items" folder
  2. In Display Layout main window, click "Save and Finish Editing"
  3. Add a "State Generator" object to the Display Layout in the object tree
  4. Select the Dispay Layout in the object tree so it's active in the main window
  5. Note that you can see the State Generator in the display layout in the upper left corner of the main window
  6. Select the State Generator in the object tree so it's active in the main window
  7. Right-click the State Generator and select Remove from the drop-down menu
  8. Note that the State Generator is removed from the object tree, which is expected
  9. Select the Display Layout in the object tree so it's active in the main window
  10. Note that the State Generator was not removed from the display layout in the main window, which is unexpected behavior.

Expected behavior: When the state generator is removed from the display layout in the object tree, it should also be removed from the display layout in the main window.

Actual behavior: When the state generator is removed from the display layout in the object tree, it's not removed from the display layout in the main window.

(Ubuntu 16.04 - Firefox 77.0.1 64-bit, Chrome Version 70.0.3538.102 64-bit)
@johnriedel johnriedel changed the title [Layout] Object not removed from Display Layout when it's removed from Display Layout's object tree [Layout] Object removal from object tree doesn't consistently remove object from Display Layout Jun 25, 2020
@johnriedel johnriedel changed the title [Layout] Object removal from object tree doesn't consistently remove object from Display Layout [Layout] Object removal from object tree doesn't consistently remove object from Display Layout in main window Jun 25, 2020
johnriedel added a commit to johnriedel/openmct that referenced this issue Jun 26, 2020
@johnriedel
Display layout bug fix for display layout object removal (nasa#3117)
9f86c7e 
* remove object tree objects that weren't being removed
@johnriedel johnriedel changed the title [Layout] Object removal from object tree doesn't consistently remove object from Display Layout in main window [DisplayLayout] Object removal from object tree doesn't consistently remove object from Display Layout in main window Jun 27, 2020
johnriedel added a commit to johnriedel/openmct that referenced this issue Jun 27, 2020
@johnriedel
fix display layout object removal from DisplayLayout code (nasa#3117)
76c7f48 
* removes orphans from display layout layoutItems

* reverts changes made to RemoveAction.js
@ozyx
Copy link
Contributor

ozyx commented Nov 2, 2022 •
edited
Loading

This is still an issue. The object is currently removed from the displayLayout's composition correctly, but stays behind in its configuration (configuration.items).

Also worth noting that this also affects Flexible Layouts

@ozyx ozyx added the bug:regression It used to work. Now it doesn't :( label Nov 2, 2022
@ozyx
Copy link
Contributor

ozyx commented Nov 2, 2022

regression caused by changes in #5827

ozyx added a commit that referenced this issue Nov 2, 2022
@ozyx
fix(#3117): init layoutItems within transaction
5ecd625
ozyx added a commit that referenced this issue Nov 2, 2022
@ozyx
fix(#3117): init frames and use transactions
8d15c5f 
- fixes #3117 for flexible layouts by syncing frames and composition

- also uses transactions now to avoid race condition
@ozyx ozyx mentioned this issue Nov 2, 2022
Merged
15 tasks
@ozyx
Copy link
Contributor

ozyx commented Nov 10, 2022

Testing Notes

Deleting an object in a layout using the RemoveAction while viewing another object

  1. Create an object (any)
  2. Create a Display Layout
  3. Drag the object into the display layout and save it
  4. Navigate to the parent object
  5. Right-click on the child object of the display layout in the object tree and remove it
  6. Verify that the object has been removed in the display layout and in the tree
  7. navigate away and back, verify it's still removed

Repeat the steps above for Flexible Layout

Deleting an object in a layout using the RemoveAction while viewing the layout

  1. Same steps as above, but remove the child object while viewing the layout itself

Repeat the steps above for Flexible Layout

@ozyx ozyx added this to the 2.1.3 milestone Nov 10, 2022
@ozyx ozyx closed this as completed in 7bb4a13 Nov 10, 2022
@khalidadil
Copy link
Contributor

Verified Fixed in Testathon on 11/14/22

@ozyx
Copy link
Contributor

ozyx commented Jul 24, 2023

Testing Notes

Deleting an object in a layout using the RemoveAction while viewing another object

  1. Create an object (any)
  2. Create a Display Layout
  3. Drag the object into the display layout and save it
  4. Navigate to the parent object
  5. Right-click on the child object of the display layout in the object tree and remove it
  6. Verify that the object has been removed in the display layout and in the tree
  7. navigate away and back, verify it's still removed

Repeat the steps above for Flexible Layout

Deleting an object in a layout using the RemoveAction while viewing the layout

  1. Same steps as above, but remove the child object while viewing the layout itself

Repeat the steps above for Flexible Layout

This regressed with the Vue 3 upgrade. Re-opening, this should be verified with the instructions above

@ozyx ozyx reopened this Jul 24, 2023
@ozyx ozyx self-assigned this Jul 24, 2023
@ozyx ozyx modified the milestones: Target:2.1.3, Target:3.0.0 Jul 24, 2023
ozyx added a commit that referenced this issue Jul 24, 2023
@ozyx
fix(#3117): can remove item from displayLayout via tree context menu …
23a0ddb 
…while viewing another item
@ozyx ozyx closed this as completed Jul 24, 2023
@shefalijoshi
Copy link
Contributor

Verified Fixed. Tried removing items from the tree, from the main display layout and from the elements pool.

@ozyx ozyx removed the unverified label Jul 24, 2023
@rukmini-bose
Copy link
Contributor

Testathon 6/24/2023: Removing child objects from a layout does not update the tree.

shefalijoshi added a commit that referenced this issue Jul 28, 2023
@ozyx @shefalijoshi
@khalidadil @unlikelyzero @davetsay @jvigliotta
Fixes for e2e tests following the Vue 3 compat upgrade (#6837)
16e1ac2 
* clock, timeConductor and appActions fixes

* Ensure realtime uses upstream context when available
Eliminate ambiguity when looking for time conductor locator

* Fix log plot e2e tests

* Fix displayLayout e2e tests

* Specify global time conductor to fix issues with duplicate selectors with independent time contexts

* a11y: ARIA for conductor and independent time conductor

* a11y: fix label collisions, specify 'Menu' in label

* Add watch mode

* fix(e2e): update appActions and tests to use a11y locators for ITC

* Don't remove the itc popup from the DOM. Just show/hide it once it's added the first time.

* test(e2e): disable one imagery test due to known bug

* Add fixme to tagging tests, issue described in 6822

* Fix locator for time conductor popups

* Improve how time bounds are set in independent time conductor.
Fix tests for flexible layout and timestrip

* Fix some tests for itc for display layouts

* Fix Inspector tabs remounting on change

* fix autoscale test and snapshot

* Fix telemetry table test

* Fix timestrip test

* e2e: move test info annotations to within test

* 6826: Fixes padStart error due to using it on a number rather than a string

* fix(e2e): update snapshots

* fix(e2e): fix restricted notebook locator

* fix(restrictedNotebook): fix issue causing sections not to update on lock

* fix(restrictedNotebook): fix issue causing snapshots to not be able to be deleted from a locked page

- Using `this.$delete(arr, index)` does not update the `length` property on the underlying target object, so it can lead to bizarre issues where your array is of length 4 but it has 3 objects in it.

* fix: replace all instances of `$delete` with `Array.splice()` or `delete`

* fix(e2e): fix grand search test

* fix(#3117): can remove item from displayLayout via tree context menu while viewing another item

* fix: remove typo 

* Wait for background image to load

* fix(#6832): timelist events can tick down

* fix: ensure that menuitems have the raw objects so emits work

* fix: assign new arrays instead of editing state in-place

* refactor(timelist): use `getClock()` instead of `clock()`

* Revert "refactor(timelist): use `getClock()` instead of `clock()`"

This reverts commit d888553.

* refactor(timelist): use new timeAPI

* Stop ticking when the independent time context is disabled (#6833)

* Turn off the clock ticket for independent time conductor when it is disabled

* Fix linting issues

---------

Co-authored-by: Khalid Adil <[email protected]>

* test: update couchdb notebook test

* fix: codeQL warnings

* fix(tree-item): infinite spinner issue

- Using `indexOf()` with an object was failing due to some items in the tree being Proxy-wrapped and others not. So instead, use `findIndex()` with a predicate that compares the navigationPaths of both objects

* [Timer] Remove "refresh" call, it is not needed (#6841)

* removing an unneccessary refresh that waas causing many get requests
* lets just pretend this never happened

* fix(mct-tree): maintain reactivity of all tree items

* Hide change role button in the indicator in cases where there is only… (#6840)

Hide change role button in the indicator in cases where there is only a single role available for the current user

---------

Co-authored-by: Shefali <[email protected]>
Co-authored-by: Khalid Adil <[email protected]>
Co-authored-by: John Hill <[email protected]>
Co-authored-by: David Tsay <[email protected]>
Co-authored-by: Jamie V <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ozyx ozyx

Labels
bug:regression It used to work. Now it doesn't :( severity:critical type:bug
Projects
None yet
Milestone
Target:3.0.0
Development

Successfully merging a pull request may close this issue.

Use Composition API to add/remove from composition nasa/openmct
7 participants
@khalidadil @shefalijoshi @ozyx @scottbell @unlikelyzero @rukmini-bose @johnriedel