Animate the page section #2512
Animate the page section #2512
Conversation
kjellr
requested review from
stufreen,
metamoni,
melissaperreault and
ludoboludo
| {%- if section.settings.page.title != blank -%} | ||
| {{ section.settings.page.title | escape }} | ||
| {%- else -%} | ||
| {{ 'sections.page.title' | t }} | ||
| {%- endif -%} | ||
| </h2> | ||
| <div class="rte"> | ||
| <div class="rte{% if settings.animations_reveal_on_scroll %} scroll-trigger animate--slide-in{% endif %}"> |
There was a problem hiding this comment.
If the page is very long then the Intersection Observer never fires. This is the same bug @metamoni encountered on the product details page. This is because 10% of the element is never in the viewport. Not sure how to fix it...
I'm starting to wonder if maybe instead of a threshold of 0.1 we should have a threshold of 0 but add a small delay to the animations. It would hopefully have a similar effect but be more reliable.
There was a problem hiding this comment.
You can also pass in an array of thresholds when creating the intersection observer: https://developer.mozilla.org/en-US/docs/Web/API/Intersection_Observer_API#thresholds
So perhaps we could do something like
const observer = new IntersectionObserver(onIntersection, {
threshold: [0, 0.1],
});Then when we check element.isIntersecting in the handler we could look at the element height to handle these super large elements. Basically like this solution: w3c/IntersectionObserver#124 (comment)
cc @metamoni
There was a problem hiding this comment.
I think we could actually do threshold: 0 and rootMargin: 0px 0px -50px 0px
This way it triggers the intersection 50px above the bottom of the viewport 👍 🤔
I can make the change in another PR if needed
There was a problem hiding this comment.
Yeah that sounds promising!
There was a problem hiding this comment.
Yeah, that does sound promising! I'd like to see that — we can fine-tune the rootMargin value until we find something that feels good.
There was a problem hiding this comment.
K, ill get something up, 50px seemed pretty good when I just tested 👍
There was a problem hiding this comment.
👍 I'll merge this one, since we're tackling that elsewhere.
PR Summary:
Adds scroll animation to the Page section.
Why are these changes introduced?
Fixes #2510
Visual impact on existing themes
Will animate the Page section when Reveal sections on scroll is enabled.
page-section.mp4
Testing steps/scenarios
Demo links
Checklist