-
Notifications
You must be signed in to change notification settings - Fork 35
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
feat(gallery): add thumbnail - FRONT-4565 #3553
Conversation
emeryro
commented
Aug 8, 2024
- add possibility to have a thumbnail, different from the full picture
- this thumbnail is optional; if not provided, the main image will be used as it was the case before
mmh..i understand that this new feature might actually come from the request to fix a "wrong" behavior in the overlay, but this way we are not really doing much to improve the situation we had regarding the amount and the heaviness of the images loaded, actually we are doing worst, since now we will have two picture objects per items, both images we will be downloaded by the browser. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-
We cannot use the same picture element for the thumbnail and the overlay, if sources are provided nothing is ensuring that the "thumbnail" image will be shown in the gallery list.
-
The previous implementation seemed ok, we probably don't need to have a picture element for the thumbnail itself, a simple img could be fine for that, but in any case the problem was that the second picture element had a src attribute defined, and with that the image would have been downloaded at page load.
So the idea was just to set an attribute in the second (hidden) picture element instead of the src, for then replacing this via js when needed.
- there is a problematic css at line 96, with multiple sources defined each src would take some space and the rendered image would not really "cover" the available space, this currently happens both in the list and in the overlay
So, my proposal is:
-
make the thumbnail a simple img or keeping it as a picture, but separated from the overlay one
-
having a picture object for the overlay image hidden at page load
-
rendering the img in the overlay picture element without the src attribute, like with a data-src attribute
-
replacing the data-src attribute with src when the item is clicked
-
introducing some examples with multiple sources, we have none at the moment and this is probably the reason why certain issues cannot be identified in the current demo
This reverts commit 7e3605a.
src/implementations/twig/components/gallery/gallery-item.html.twig
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor things, i think we are ok with this, we should only ensure that any "attribute" they could pass before is still going to work, if none was available before, then we are safe..