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

Duplicate images not showing on Next / Back preview.(v6.3.0) #171

Closed
2 of 8 tasks
ghost opened this issue Nov 30, 2018 · 7 comments
Closed
2 of 8 tasks

Duplicate images not showing on Next / Back preview.(v6.3.0) #171

ghost opened this issue Nov 30, 2018 · 7 comments

Comments

@ghost
Copy link

ghost commented Nov 30, 2018

I'm submitting a...


[ ] Regression (a behavior that used to work and stopped working in a new release)
[x] Bug report  
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request

Current behavior

Hello!
I'm trying to add a no_preview image for files that are not considered image.
The problem that I'm facing is that when I'm adding 2 or more of the same image src (remote / local) the preview is not working on Next / Back button.

I also tried on your demo app :
https://stackblitz.com/edit/angular-modal-gallery-v5/?file=app%2Fapp.component.ts
And for example if I'm changing the image array like this:
mages: Image[] = [
new Image(
0,
{ // modal
img: 'https://raw.githubusercontent.com/Ks89/angular-modal-gallery/v4/examples/systemjs/assets/images/gallery/img1.jpg',
extUrl: 'http://www.google.com'
}
),
new Image(
1,
{ // modal
img: 'https://raw.githubusercontent.com/Ks89/angular-modal-gallery/v4/examples/systemjs/assets/images/gallery/img1.jpg',
description: 'Description 2'
}
)
];

You will notice that when you want to navigate from image 0 to 1 is staying in a loading loop.
This is happening only first time.

Expected behavior

I would expect that even if you have duplicate images to be able to navigate without any problems.

Minimal reproduction of the problem with instructions

  • add 2 image object with same src in the image array

Browser:

  • Chrome (desktop) version 70
  • Chrome (Android) version XX
  • Chrome (iOS) version XX
  • Firefox version XX
  • Safari (desktop) version XX
  • Safari (iOS) version XX
  • IE version 11
  • Edge version XX
@ghost ghost changed the title Duplicate images not showing on Next / Back preview.(v 6.3.0) Duplicate images not showing on Next / Back preview.(v6.3.0) Nov 30, 2018
@Ks89
Copy link
Owner

Ks89 commented Nov 30, 2018

Thank u I'll investigate

@ghost
Copy link
Author

ghost commented Nov 30, 2018

I've made a temp fix . When I'm pushing the images from my response into image[].
I'm adding to img: url+"?"index .(The url will look somethinkg like : urltomyimage.png?1)
I think this has something to do with caching.

@Ks89
Copy link
Owner

Ks89 commented Nov 30, 2018

Oh yes you are right I already saw this. It's about caching and browsers don't emit the load event for cached images. I can't do anything to fix this, however your idea is interesting. If it is really working I can suggest it in the doc as a workaround.

@Ks89 Ks89 self-assigned this Nov 30, 2018
@Ks89 Ks89 added this to the 7.1.0 milestone Dec 6, 2018
@Ks89
Copy link
Owner

Ks89 commented Dec 7, 2018

I made some quick experiments and your idea (...?1) is working very well. I'll add it for sure in the doc website also with a dedicated demo in version 7.1.0 or 7.2.0 this month.

I'm still thinking if it could be good to implement a logic that automatically append '?index' to all images when you provide the images array to the library. What do you think?

Thank u man 👍

@Ks89
Copy link
Owner

Ks89 commented Dec 7, 2018

I'm still thinking if it could be good to implement a logic that automatically append '?index' to all images when you provide the images array to the library. What do you think?

Ok no, I cannot do this because if users want base64 images this will break paths.
I'll add an example to the official doc. Probably in future releases I can change the Image interface to specify the type of the path (url, base64...). But at the moment it's not feasible without adding frustrating breaking changes.

@ghost
Copy link
Author

ghost commented Dec 7, 2018

I was thinking about that ...., because I saw support for base64, and you are right for now I think only worth mentioning in documentation as a workaround for people who are using img paths.

@Ks89
Copy link
Owner

Ks89 commented Dec 8, 2018

All demos has been updated in 7.2.0 on a dedicated branch (called 720) in this repo. I'll merge the branch into master this month (I don't know when exactly, probably at the end of December).
Stackblitz example is already updated and published.
Doc website will be updated at the same time of 7.2.0

Issue closed.
thank u.

@Ks89 Ks89 closed this as completed Dec 8, 2018
@Ks89 Ks89 modified the milestones: 7.1.0, 7.2.0 Dec 8, 2018
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

1 participant