-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Animate search template #2534
Animate search template #2534
Conversation
sections/main-search.liquid
Outdated
@@ -242,7 +245,7 @@ | |||
lazy_load: lazy_load | |||
%} | |||
{%- when 'page' -%} | |||
<div class="article-card-wrapper card-wrapper underline-links-hover"> | |||
<div class="article-card-wrapper card-wrapper underline-links-hover{% if settings.animations_reveal_on_scroll %} scroll-trigger animate--slide-in{% endif %}"> |
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.
I don't think we need this instance since we already have the class applied to the parent li
🤔
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.
Yeah, I think I got confused there. Removed in ea377de.
I was looking around to catch up, is it expected to not have the filter component not animate? |
@melissaperreault Yep! The filter component is a shared one that's handled by #2533. Once that's merged in, it will animate in search results too. |
* Animate search template. * Remove unnecessary class.
PR Summary:
This adds animation to the search template.
Why are these changes introduced?
Part of #2381.
What approach did you take?
This follows the same general template animation strategy set forth in #2381 (comment):
Visual impact on existing themes
When "Reveal sections on scroll" is enabled, this will animate elements on the search results page template.
search.mp4
Testing steps/scenarios
Demo links
Checklist