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.

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

Clicking "View Large" on Imagery leaves it paused #3647

Closed
charlesh88 opened this issue Jan 14, 2021 · 8 comments
Closed

Clicking "View Large" on Imagery leaves it paused #3647

charlesh88 opened this issue Jan 14, 2021 · 8 comments

Comments

@charlesh88
Copy link
Contributor

Seen on hulk/viper 01/14/21. When operating in real-time mode, clicking "View Large" on an Image in a layout pauses it in the overlay, which is desirable. But the image should then be unpaused when dismissing the overlay, unless the user actively unpaused and paused the image via the Pause button, or clicked a past image in the thumb strip.

Reproduce

  1. Put Time Conductor in real-time mode.
  2. Click "View Large" on an Image view in a layout.
  3. Observe: the image in the overlay is paused. This is Ok.
  4. Dismiss the overlay.
  5. Observe: the image in the layout continues to be paused, but instead should be returned to the initial auto-updating mode.
@akhenry
Copy link
Contributor

akhenry commented Jan 14, 2021

Agreed. I'm not sure we need to put the imagery view into a paused state at all, the image shown in the overlay should just be the one that the user embiggened. The imagery view can carry on doing its thing in the background because the user can't see it. It would be great if we could avoid pausing and unpausing the imagery view in the process (just sounds like code complexity that we don't need).

@charlesh88
Copy link
Contributor Author

Agreed. I'm not sure we need to put the imagery view into a paused state at all, the image shown in the overlay should just be the one that the user embiggened. The imagery view can carry on doing its thing in the background because the user can't see it. It would be great if we could avoid pausing and unpausing the imagery view in the process (just sounds like code complexity that we don't need).

Initially, I thought we shouldn't be pausing the image either. But, typically the user is embiggening because they're studying a particular image in order to make a decision, and the thinking is that if that gets replaced by a new, incoming image that would be quite disruptive. I will discuss this with stakeholders and feed back into this issue.

We should also consider a notification of new imagery, that when paused, lets the user know a new image has come in and gives an affordance to see it and/or switch back to RT mode.

@akhenry
Copy link
Contributor

akhenry commented Jan 15, 2021

I should have checked how view large works with imagery before commenting. It enlarges the whole imagery view, and not just the image itself. In that case, yes, it should go into pause mode while it's embiggened.

I thought that it just showed the image, in which case I was hoping we could avoid the code spaghettification involved in adding logic to pause it while it's in view large, and then unpause it when it's not.

@unlikelyzero unlikelyzero added this to Needs triage in Bug Tracker via automation Jun 16, 2021
@shefalijoshi
Copy link
Contributor

shefalijoshi commented Aug 13, 2021

The code for imagery view for time strips now emits a 'destroy' event when viewLarge closes. We could key off this to unpause the imagery when view large exits.
https://github.com/nasa/openmct/pull/4114/files#diff-0d80c6e0d05f5412837707bd2d546fe635064e6127670878997805c59eb81cffR531

@unlikelyzero unlikelyzero added the bug:retest Retest without a specific milestone label Mar 23, 2022
@unlikelyzero unlikelyzero added this to Looked at in BC via automation Mar 23, 2022
@michaelrogers michaelrogers self-assigned this May 25, 2022
@michaelrogers
Copy link
Contributor

michaelrogers commented Oct 31, 2022

How to test

Clicking the image directly vs activating the view large action from the context menu activates different code paths, so the testing instructions are separated below.

Route 1

  1. Create a display layout with at least one imagery object
  2. Ensure clock is realtime and not paused
  3. Click on the imagery to activate the view large action
  4. Observe the imagery is paused (orange outline around image)
  5. Exit overlay
  6. Observe that the imagery pause/unpause status is restored

Route 2

  1. Create a display layout with at least one imagery object
  2. Ensure clock is realtime and not paused
  3. Expand the context menu of an image
  4. Click on the View Large or View action from the context menu
  5. Observe the imagery is paused (orange outline around image)
  6. Exit overlay
  7. Observe that the imagery pause/unpause status is restored

@unlikelyzero unlikelyzero added this to the Target:2.1.4 milestone Nov 17, 2022
Bug Tracker automation moved this from Needs triage to Done Jan 20, 2023
BC automation moved this from Looked at to Close Jan 20, 2023
@jvigliotta
Copy link
Contributor

Verified Fixed: Testathon - 1/24/23

@unlikelyzero unlikelyzero added needs:e2e Needs an e2e test and removed unverified labels Jan 27, 2023
@unlikelyzero unlikelyzero added this to To triage in Improve Test Coverage via automation Jan 27, 2023
@rukmini-bose
Copy link
Contributor

rukmini-bose commented Mar 20, 2024

Testathon Verified Fixed 3/20/2024

@charlesh88
Copy link
Contributor Author

Testathon 2024-03-20 cannot repro, fixed.

@charlesh88 charlesh88 removed the bug:retest Retest without a specific milestone label Mar 20, 2024
@unlikelyzero unlikelyzero added the verified Tested or intentionally closed label Apr 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs:e2e Needs an e2e test severity:critical type:bug verified Tested or intentionally closed
Projects
BC
Close
Development

No branches or pull requests

7 participants