[Collapsible content] add animation to the section and its blocks #2529
Conversation
sections/collapsible-content.liquid
Outdated
| @@ -55,8 +57,11 @@ | |||
| <div class="grid__item"> | |||
| {%- for block in section.blocks -%} | |||
| <div | |||
| class="accordion{% if section.settings.layout == 'row' %} content-container color-{{ section.settings.container_color_scheme }} gradient{% endif %}" | |||
| class="accordion{% if section.settings.layout == 'row' %} content-container color-{{ section.settings.container_color_scheme }} gradient{% endif %} {% if settings.animations_reveal_on_scroll %} scroll-trigger animate--slide-in{% endif %}" | |||
There was a problem hiding this comment.
I know @kjellr has approved this, but are we sure we want the animation on each collapsible row, rather than just their container? For many other sections we decided to keep it minimal and only slide in the container rather than every single individual element. And with the animation duration being shorter now, I think this doesn't look so smooth anymore:
collapsible-row.mp4
There was a problem hiding this comment.
One thing I'd like to add on that topic. The more granular we go about the elements we choose to animate the more we impact performance we will notice 🤔
It might be better to use with caution vs apply on most elements separately
There was a problem hiding this comment.
If this can improve performance overall, I can see this not being in cascade. We can rely on smoother animation as we interact with the rows instead. That would be a good trade off to me considering the points you raised. 👍
This also aligns for now with the Collapsible row blocks under the product page. It would harmonize how they reveal together.
It does give a smoother feel though when displayed alone without an image beside it instead of revealing a larger chunk, but I am fine without cascading it too!
There was a problem hiding this comment.
On the Product Page we decided to only slide in the Product Info block and fade in the image. The individual blocks within the Product Info don't cascade. Same for the Featured Product. Similarly, in the Slideshow section we don't cascade in the heading, subheading, button. So for consistency, it makes more sense to me to not cascade each individual collapsible row.
So I'm confused, because from the comments above I understand that we can get rid of the cascading effect, but the code hasn't changed to reflect that. What's the final decision here?
There was a problem hiding this comment.
I'm gonna tweak this today 👍 to remove the cascade on each row 🙂
There was a problem hiding this comment.
Thank you! Sorry for the confusion!
There was a problem hiding this comment.
Code LGTM pending a response to Monica's point. @melissaperreault do you have a strong opinion on the cascading here?
PR Summary:
Add animation to this section and its blocks
Why are these changes introduced?
Fixes #2387
What approach did you take?
Added the necessary classes and data attributes
Other considerations
Visual impact on existing themes
Adds animation effect when enabled from the general settings
Testing steps/scenarios
Demo links
Checklist