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

[3.x] Fix layout shift on thumbnails & quantity #710

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

Jade-GG
Copy link
Collaborator

@Jade-GG Jade-GG commented Jan 22, 2025

This is an updated version of #675 without any duplication. I took the untested code I posted there, tested and fixed it, and made some adjustments to make the code slightly nicer.

Some notes on this:

  • We cannot use the resizedPath helper here currently, as the URL constructor doesn't like paths that are not full URLs. This means we cannot pass something like /catalog/product/m/p/mp07-blue_main_1.jpg.webp (which is what we have here) into the function, it will throw an error. Should we change/fix this?
  • I used @unless to make the v-cloak+v-if combination as clear as I could, making sure both checks have the same conditional. At the end of the day, we're making a small code quality sacrifice here to allow us to fully eliminate any content jumps.
  • I minimized DOM usage a little bit for the +3 overlay by only adding it on actual breakpoint positions (this is the in_array($imageId + 1, $breakpoints) check). This does, however, complicate the code further. I figured it's worth moving the overlay to a separate partial, but I would love to hear opinions on this.
  • The breakpoints were weird. I've made them more in line to the page spacing available, however it's still not perfect and the thumbnails don't always line up with the big image. Ideally, someone with actual frontend skills should have a look at how the breakpoints behave and make it cleaner. (For example, on a window width between 1280px and 1536px, having more than 6 thumbnails makes the thumbnails themselves shrink)

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.

2 participants