[Image with text] Fix ambient movement and zoom in on placeholder #2744
Conversation
|
Looks good to me https://hllvkum3xlhy2x07-45840990230.shopifypreview.com |
| @@ -74,7 +74,7 @@ | |||
| fill: currentColor; | |||
| } | |||
|
|
|||
| .image-with-text__media--placeholder.animate--ambient > svg { | |||
| .image-with-text__media--placeholder:is(.animate--ambient, .animate--zoom-in) > svg { | |||
| top: 0; | |||
There was a problem hiding this comment.
Asking question so I can enhance my learning: why did you use :is() vs. :where() ?
There was a problem hiding this comment.
The main difference is in specificity.
:is() will use the selectors within it to count towards the specificity of the overall selector.
:where() will default to a specificity of 0.
So in our case, because the selector used to be .class.anotherClass then it would have a specificity count of 2. By using :is() I make sure I keep that same specificity.
If I had used :where() then the specificity would be lower.
There was a problem hiding this comment.
Oh wow! This is a very helpful diagram! Thank you so much for taking the time to explain the difference to me !
There was a problem hiding this comment.
This PR is working as expected in the editor and preview!
Here's all that I tested for this PR
- Tested in mobile, tablet and desktop view
- Tested in safari and chrome
- Applied color scheme on all of the images with text variations (placeholder w/ ambient, placeholder w/zoom, image w/ambient, image with zoom, and no animation)
- Tests changing typography in global theme settings
- Tested changing button styles in global theme settings
- Tested media global theme settings by applying border, paddings and shadows
| .image-with-text__media--placeholder:is(.animate--ambient, .animate--zoom-in) > svg { | ||
| top: 0; | ||
| left: 0; | ||
| transform: translate(0); |
There was a problem hiding this comment.
Did you find you needed to set the initial translate to 0?
There was a problem hiding this comment.
Needed specifically for the zoom in effect and doesn't do any harm for the ambient basically 👍
…opify#2744) * Fix ambient movement on placeholder * fix position for zoom-in animation too
PR Summary:
Fixes the ambient background animation on the placeholder image
Why are these changes introduced?
Fixes #2742
What approach did you take?
Changed the left, top and transform values when the ambient and zoom in animation is enabled.
Other considerations
Visual impact on existing themes
Testing steps/scenarios
Demo links
Checklist