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

feat(gallery): add thumbnail - FRONT-4565 #3553

Merged
merged 11 commits into from
Aug 14, 2024
Merged

Conversation

emeryro
Copy link
Contributor

@emeryro 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

Copy link

github-actions bot commented Aug 8, 2024

@github-actions github-actions bot temporarily deployed to pull request August 8, 2024 09:19 Inactive
@planctus
Copy link
Collaborator

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.
To me we should do more or less what we have been doing in BCL, where we set the "src" attribute via js when the overlay opens, so that it will only be downloaded when it is requested in the overlay.

@github-actions github-actions bot temporarily deployed to pull request August 12, 2024 12:23 Inactive
@github-actions github-actions bot temporarily deployed to pull request August 13, 2024 07:11 Inactive
@planctus planctus self-requested a review August 13, 2024 08:30
Copy link
Collaborator

@planctus planctus left a 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
    image

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

@github-actions github-actions bot temporarily deployed to pull request August 13, 2024 09:54 Inactive
@github-actions github-actions bot temporarily deployed to pull request August 13, 2024 10:07 Inactive
@github-actions github-actions bot temporarily deployed to pull request August 13, 2024 12:30 Inactive
Copy link
Collaborator

@planctus planctus left a 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..

@github-actions github-actions bot temporarily deployed to pull request August 13, 2024 14:36 Inactive
@github-actions github-actions bot temporarily deployed to pull request August 14, 2024 07:33 Inactive
@github-actions github-actions bot temporarily deployed to pull request August 14, 2024 07:44 Inactive
@planctus planctus merged commit 4448442 into v4-dev Aug 14, 2024
6 of 7 checks passed
@planctus planctus deleted the FRONT-4565-gallery-thumbnail branch August 14, 2024 07:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants