feat: add sources to improve LCP + add loading and fetchpriority attributes

[luca-peruzzo]

With this pull request, I've added loading and fetchpriority attributes to img elements. they are both configurable, but I set defaults to 'lazy' and 'auto'.
I also added the property sources to ModalImage interface letting users set source tags in picture elements at desired media breakpoints (for improving LCP).
Last I removed id="current-image" from img cause if I use two carousels on the same page I will receive "id must be unique" error from the console.
All my changes should be safe to merge without any breaking changes, but I suggest you to check carefully 😅.

[Ks89]

Thanks you for the contribution. I need some time to review all changes. 👍

[Ks89]

Hi @luca-peruzzo

I'm so sorry for the huge delay, but I finally found some time to review your PR.
Your changes are breaking Modal gallery in some way. For instance A1bis - (id=401). The image become huge moving dots and previews outside of the screen.

This is what I expect (v11.0.0):

but in your PR is:

Also, I found a weird behaviour about loading. It seems that I cannot open modal gallery to show an image without opening carousel first. Very strange. Did you find the same behaviour.

I'll try to investigate a bit today on a train :)

[luca-peruzzo]

Hi @luca-peruzzo

I'm so sorry for the huge delay, but I finally found some time to review your PR. Your changes are breaking Modal gallery in some way. For instance A1bis - (id=401). The image become huge moving dots and previews outside of the screen.

This is what I expect (v11.0.0):

but in your PR is:

Also, I found a weird behaviour about loading. It seems that I cannot open modal gallery to show an image without opening carousel first. Very strange. Did you find the same behaviour.

I'll try to investigate a bit today on a train :)

I think I miss some css for the first problem. Could you tell me more about the second? how to try it?

[luca-peruzzo]

@Ks89 for the second problem, if you mean about the loading never hide is because the image is lazy loaded and the figure container of current-image.component has display: none while loading. So the browser never download image because it isn't on screen. we could fix changing to opacity: 0|1 to hide img while loading

[luca-peruzzo]

@Ks89 I've updated the pull request, let me know if it solves the problems.

[Ks89]

Thank you for the update, now it seems to be ok.

[Ks89]

Missing things:

• update all examples in ./examples folder with the same changes made for ./src (assets and app/carousel)

I tested your changes and it seems to be backward compatible without issues. I could be released as 11.1.0.

However, we need to update also the documentation website HERE at least with the new APIs and an example for carousel and modal.

[luca-peruzzo]

Missing things:

• update all examples in ./examples folder with the same changes made for ./src (assets and app/carousel)

I tested your changes and it seems to be backward compatible without issues. I could be released as 11.1.0.

However, we need to update also the documentation website HERE at least with the new APIs and an example for carousel and modal.

@Ks89 All done. here my pull request for the docs page: Ks89/angular-modal-gallery-2023-v11.github.io#1

[Ks89]

Thanks again. I was reviewing before merging, but I found another bug.
This happens on Safari (tested on Version 16.5.2 (17615.2.9.101.1, 17615))

If you resize Safari (vertically), current image is stretched
Safari 😡😡😡😡

It happens also without resizing with vertical images:

[Ks89]

Thanks again. I was reviewing before merging, but I found another bug. This happens on Safari (tested on Version 16.5.2 (17615.2.9.101.1, 17615))

If you resize Safari (vertically), current image is stretched Safari 😡😡😡😡

It happens also without resizing with vertical images:

Fixed on Safari using max-width: 100% instead of width: 100% in current-image.scss:

> img {
        // display: block;
        // don't use this
        // Bootstrap 4 img-fluid class uses max-width: 100% and height: auto
        max-width: 100%; <---------------------------------- changed this
        height: auto;
        display: block;
        &.unclickable {
          cursor: not-allowed;
        }
      }

[luca-peruzzo]

@Ks89 any updates?

[Ks89]

@Ks89 any updates?

this is ready to merge applying the max-width: 100% fix.
If you want to update your PR with this, feel free, otherwise I'll apply it by myself.

About the doc website, we need to apply changes based on my comments. However I didn't find enough spare time to do that. Next week, I will merge everything applying changes on top of your commits. If you want to help and speed up the process, you can also update your PR with that changes.