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

[Imagery] Images cannot be saved #3756

Closed
akhenry opened this issue Mar 16, 2021 · 12 comments · Fixed by #3857 or #5192
Closed

[Imagery] Images cannot be saved #3756

akhenry opened this issue Mar 16, 2021 · 12 comments · Fixed by #3857 or #5192

Comments

@akhenry
Copy link
Contributor

akhenry commented Mar 16, 2021

Users need a way to be able to save images from the imagery view.

Right-click and "Save Image" would be perfectly sufficient except that it doesn't work right now because it's blocked by a div that overlays the image which is part of the compass implementation.

If the compass can't be achieved without blocking the browser's build in "Save Image" functionality, then we should implement an action for doing this.

@xshifty
Copy link

xshifty commented Apr 14, 2021

Hello @akhenry. I added a new action to imagery plugin in order to make it possible to download the image.
Hope it solves this problem.

If you have any suggestion, let me know.

@charlesh88
Copy link
Contributor

Testing Notes

Ideally we test this in a variety of browsers and OS's, especially Ubuntu if possible.

  1. Navigate to an imagery endpoint, ideally real imagery (vs. Example Imagery) so that you can see the compass rose.
  2. Context-click anywhere in the main image (other than the compass rose) and verify that the browser-based context menu includes an action to save the image.
  3. Save the image, then view the resulting file and confirm the image was saved correctly.
  4. Put the image in a Display Layout and repeat steps 1 - 3.
  5. Put the image in a Flexible Layout and repeat steps 1 - 3.

@charlesh88
Copy link
Contributor

Verified fixed Testathon 6/2/2021

@nikhilmandlik
Copy link
Contributor

nikhilmandlik commented Jun 2, 2021

Verified Fixed.

right clicking, in middle of image does not give 'save image as' option

@jvigliotta
Copy link
Contributor

Still seeing an issue when right clicking in certain areas.

@jvigliotta jvigliotta reopened this Jun 2, 2021
@unlikelyzero unlikelyzero added this to Needs triage in Bug Tracker via automation Jun 9, 2021
@unlikelyzero unlikelyzero pinned this issue Jun 9, 2021
@unlikelyzero unlikelyzero added the bug:regression It used to work. Now it doesn't :( label Jun 9, 2021
@charlesh88 charlesh88 unpinned this issue Jun 23, 2021
@unlikelyzero
Copy link
Collaborator

@charlesh88 can you verify that this was not already fixed?

Bug Tracker automation moved this from Needs triage to Done Apr 19, 2022
@unlikelyzero unlikelyzero reopened this Apr 19, 2022
Bug Tracker automation moved this from Done to Needs triage Apr 19, 2022
@unlikelyzero unlikelyzero added this to Looked at in BC via automation Apr 19, 2022
@unlikelyzero unlikelyzero added the bug:retest Retest without a specific milestone label Apr 19, 2022
@charlesh88
Copy link
Contributor

Verified NOT fixed Bug Bash 04-21-22. Context-click on image does not give user a Save Image... option. This is due to a code change to display imagery as a div with background.

image

@unlikelyzero unlikelyzero removed the bug:retest Retest without a specific milestone label Apr 25, 2022
@unlikelyzero unlikelyzero moved this from Looked at to Keep in BC Apr 25, 2022
@unlikelyzero unlikelyzero moved this from Needs triage to Will Do in Bug Tracker Apr 25, 2022
@akhenry
Copy link
Contributor Author

akhenry commented May 2, 2022

We need to revisit this. If the image does indeed need to be displayed as the background of a div then we need to provide the user with some additional affordance for downloading the image. @xshifty proposed an approach using a new local control, which seems like a reasonable alternative if we can't use browser native behavior here.

Unless there's some trick we can use to trigger the correct browser native behavior? Like an aria attribute or something?

@charlesh88
Copy link
Contributor

charlesh88 commented May 3, 2022

Solution here: https://codepen.io/charlesh88/pen/bGLNyzv?editors=1100

Basically, layer an actual <img> tag (with opacity: 0) over the zoomable/pan-able background-image div.

charlesh88 added a commit that referenced this issue May 12, 2022
- Tweaks to image CSS to allow context click access.
@charlesh88
Copy link
Contributor

Testing Notes

  1. View Example Imagery or an image data product.
  2. Context (right) click on the image itself and verify that the OS-provided context menu includes "Save Image As...". Note that the image itself must be clicked; clicking outside the image, the Prev or Next Image buttons or the compass rose will not (and is not expected to) provide an image context.
  3. Choose that menu option, then verify that the downloaded image is the same one that was displayed.

image

@unlikelyzero unlikelyzero added the needs:e2e Needs an e2e test label May 12, 2022
@unlikelyzero unlikelyzero added this to To triage in Improve Test Coverage via automation May 12, 2022
Bug Tracker automation moved this from Will Do to Done Jun 8, 2022
BC automation moved this from Keep to Close Jun 8, 2022
akhenry pushed a commit that referenced this issue Jun 8, 2022
- Tweaks to image CSS to allow context click access.
Co-authored-by: Andrew Henry <akhenry@gmail.com>
@michaelrogers
Copy link
Contributor

michaelrogers commented Jun 9, 2022

Fix verified locally on release/2.0.5 branch during testathon. User is able to successfully save images directly and when composed in layouts.

@khalidadil
Copy link
Contributor

Verified fixed - testathon on 06/09/2022

@unlikelyzero unlikelyzero removed this from Close in BC Jun 14, 2022
@unlikelyzero unlikelyzero added this to Needs triage in Build 6 Blockers via automation Jul 17, 2022
@unlikelyzero unlikelyzero added this to the Sprint:2.0.7 milestone Jul 17, 2022
@unlikelyzero unlikelyzero moved this from Needs triage to Needs an automated test in Build 6 Blockers Jul 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug:regression It used to work. Now it doesn't :( needs:e2e Needs an e2e test severity:critical type:bug
Projects
Build 6 Blockers
Needs an automated test
9 participants