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

Fix visualization of large images on angular #15644

Merged
merged 1 commit into from
Jul 22, 2021
Merged

Fix visualization of large images on angular #15644

merged 1 commit into from
Jul 22, 2021

Conversation

CGarces
Copy link
Contributor

@CGarces CGarces commented Jul 13, 2021

Big images not are loaded after click on it on chrome 91.0.4472.124 or Microsoft Edge 91.0.864.67 under windows 10. Works fine on Firefox.

The error can be reproduced with this project.
https://github.com/CGarces/testimgJHipster
Fails with the bigger image at fake data, the others works fine.
https://github.com/CGarces/testimgJHipster/tree/main/src/main/resources/config/liquibase/fake-data/blob
And can be fixed with this patch
https://github.com/CGarces/testimgJHipster/compare/fix_img?expand=1

The error only happens if is loaded by the openFile function.

The proposed solution just works for me on angular, I haven't test the issue on other web frameworks. I don't know why the iframe is needed, please give me some background about that to improve the PR.


Please make sure the below checklist is followed for Pull Requests.

When you are still working on the PR, consider converting it to Draft (bellow reviewers) and adding skip-ci label, you can still see CI build result at your branch.

@mshima
Copy link
Member

mshima commented Jul 14, 2021

Your solution is for images only. iFrame was added at 6da973f
Back in the time it seems to work with pdf, not sure now.

@CGarces
Copy link
Contributor Author

CGarces commented Jul 15, 2021

Thanks @mshima for you feedback.
I have changed the patch to modify the behaviour only on images.

If the solution is good enough, I can do the same for all front-ends.

@CGarces
Copy link
Contributor Author

CGarces commented Jul 15, 2021

I think that I have found the correct way, reusing the existing base64 to blob code.

      const fileURL = window.URL.createObjectURL(blob);
      const win = window.open();
      win!.location.href = fileURL; 

Tested on video, images and pdf.

Copy link
Member

@mshima mshima left a comment

Choose a reason for hiding this comment

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

The solution looks ok to me.

@CGarces
Copy link
Contributor Author

CGarces commented Jul 16, 2021

Thanks @mshima for the review.
I have fixed the Jest tests but i'm not sure if is the correct way.
Should I modify also the other frameworks?

@Tcharl Tcharl merged commit fa8b61d into jhipster:main Jul 22, 2021
@CGarces CGarces deleted the fix_big_images branch July 22, 2021 18:34
@mshima
Copy link
Member

mshima commented Jul 22, 2021

Just for reference, IFAIK current implementation blocks the ui, there is a new async api for converting base64.
https://stackoverflow.com/a/36183085

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants