[Motion] No animation on added/edited section #2502

ludoboludo · 2023-04-07T16:20:15Z

PR Summary:

Prevent having animation while being in the editor.

Why are these changes introduced?

Fixes https://github.com/Shopify/themes/issues/400

What approach did you take?

I added some logic that only runs in the DOMContentLoaded listener, shopify:section:reorder, and shopify:section:load. So it should run when the page loads, when you're re ordering a section or when you're editing a section in the editor.
It adds a new class that overwrite the other ones.

Other considerations

At first I was thinking of removing the existing classes we had but I though it was cleaner/clearer to have a specific class to prevent fade in and slide in animations.

Decision log

#	Decision	Alternatives	Rationale	Downsides
1	~~Do not apply animation on elements in the viewport on page load~~	Animate on page load all elements	It can be a bit jarring to always have animated things show up.	🤷
2	Do not apply animation on elements being edited in the theme editor	Animate elements everytime they reload	it is not necessary to show the animation every time a merchant edit their section	Merchant could wonder why the animation isn't applied when a section reloads

Visual impact on existing themes

In the editor, prevent a section from being animated anytime it's re rendered due to setting changes.

Testing steps/scenarios

Test in the editor while tweaking a section and/or its blocks

Demo links

Editor

Checklist

Added PR summary for release notes
Requested review from UX (Only for changes that are affecting the experience or perceivable visual details)
Created a ticket for the help.shopify.com documentation team about updates to theme settings. (Internal-only task)
Followed theme code principles
Linted with Theme Check
Tested on mobile
Tested on multiple browsers
Tested for accessibility

…ones

kjellr · 2023-04-07T17:55:45Z

This feels pretty good! @ludoboludo, how do you feel about the experience with these changes?

I do sort of miss the initial slide-in of some above-the-fold elements, but maybe that's because I've been seeing them slide in for so long now. Let's all test this and let it sit for a few days before we merge in.

ludoboludo · 2023-04-11T20:12:12Z

After tweaking our animations which are feeling much nicer now, we agreed not to prevent them from happening for elements in the viewport on load. So this PR (description updated) is more about preventing the animation to happen when not necessary in the theme editor.

kjellr · 2023-04-12T12:38:41Z

Thanks for revising the PR, @ludoboludo! This feels pretty good. Is it possible to turn off all the animations during this zoomed-out reorder state?

12-37-lgsho-g0iuh.mp4

ludoboludo · 2023-04-13T16:21:26Z

assets/animations.js

-document.addEventListener('shopify:section:load', (event) => initializeScrollAnimationTrigger(event.target));
+if (Shopify.designMode) {
+  document.addEventListener('shopify:section:load', (event) => initializeScrollAnimationTrigger(event, event.target));
+  document.addEventListener('shopify:section:reorder', (event) =>


@kjellr I added something for the section being re ordered. One thing to note, when I checked, I can only prevent the animation on the section I'm currently trying to re order. Any other section has the animation re running.

When I checked, section:load event wasn't triggered on other sections, so it must be a full page reload. I think it's ok though as long as the section we're currently moving around isn't animating every time we're moving it around.

Here is a little video of how it looks

Hmm ok. I'd definitely prefer to have the other animations stop on reorder too. Can we add a body class with some !important rules or something to disable all animations when you're in that mode or something? 😅

Either way, what we have here is an improvement, so happy to get this merged in and tackle improvements separately. I'll add an approval for now, with the caveat that if we come up with a quick fix to stop all animation on reorder, we should take it.

Ok so I can actually do it by making it remove the animation from all present sections that usually do animate. One of the potential issue with this, is that if you're at the top of a page and you haven't fully scrolled down yet and triggered the animations and you're reordering one of your first section on the page, nothing will be animated on further navigation on this page until page reload.

Here is a video cause it's seem hard to explain without visual aid.

Ah, nice work! This seems like a pretty good solution. From watching the video, I'm not concerned about the subsequent sections not animating. That seems like a reasonable tradeoff in this case. If you're happy with the dev solution, let's push it up here and test it out.

kjellr

This generally looks good. Add a few additional notes in #2502 (comment).

assets/animations.js

stufreen · 2023-04-17T18:10:55Z

assets/animations.js

-document.addEventListener('shopify:section:load', (event) => initializeScrollAnimationTrigger(event.target));
+if (Shopify.designMode) {
+  document.addEventListener('shopify:section:load', (event) => initializeScrollAnimationTrigger(event, event.target));
+  document.addEventListener('shopify:section:reorder', (event) => initializeScrollAnimationTrigger(event));


If you don't pass in an element to initializeScrollAnimationTrigger it will create a new intersection observer for the entire document, which already has one from line 34. Can we just create the new observer for the section(s) that was reordered?

I had it for the specific section in the first place but it worked in a way that only the re ordered section was not getting animated while every other section was (video).

What I did just now was adding an early return within my if statement. So that if the event triggering the initialization is coming from the editor ((re)load or reorder) so we're not observing more element as it's unnecessary since the classes applied prevent the animation from happening anyway.

e89ed02

assets/animations.js

LucasLacerdaUX · 2023-04-17T18:56:58Z

assets/animations.js

  const animationTriggerElements = Array.from(rootEl.getElementsByClassName(SCROLL_ANIMATION_TRIGGER_CLASSNAME));
  if (animationTriggerElements.length === 0) return;

+  if (isDesignMode) {


Is isDesignMode simply Shopify.designMode or is there another use case where these won't be equal?

If they're the same thing, do we need this another variable/parameter instead of simply checking for Shopify.designMode?

I just changed the name based on your comment. It's not exactly Shopify.designMode but theme editor triggered events.

Cause for example you can be in the editor (Shopify.designMode = true) but we want to run some logic specifically for the section:load and section:reorder events.

LucasLacerdaUX

Looks good to me now :D

* [Motion] Use rootMargin instread of threshold to trigger slide in animations (#2517) * Use rootMargin instread of threshold to trigger slide in animations * address review comment, remove threshold * Add conditional around data-cascade attribute (#2532) * [Motion] No animation on added/edited section (#2502) --------- Co-authored-by: Ludo <ludo.segura@shopify.com>

ludoboludo added 4 commits April 6, 2023 15:35

Prevent animation on elements within the viewport on page load

3c691c2

remove animation when editing a section in the admin

5dd131b

remove console log

c0bb4db

change approach with adding a new class instead of removing existing …

34e4c7b

…ones

kjellr self-requested a review April 7, 2023 17:11

ludoboludo added 2 commits April 11, 2023 15:14

only prevent the animation in the editor while adding/editing section

a8fee54

Merge branch 'main' into no-animation-on-elements-in-viewport

c97f1f9

ludoboludo changed the title ~~[Motion] No animation on elements in viewport~~ [Motion] No animation on added/edited section Apr 11, 2023

ludoboludo added 2 commits April 13, 2023 11:39

prevent the section from re running the animation on section reorder

cbdbd98

only run check when in the editor

2d4ef75

ludoboludo commented Apr 13, 2023

View reviewed changes

kjellr approved these changes Apr 13, 2023

View reviewed changes

ludoboludo added 2 commits April 13, 2023 15:41

remove animations on section when reordering a section

acdfa96

remove unnecessary code

bc6ba19

ludoboludo requested a review from LucasLacerdaUX April 13, 2023 19:53

ludoboludo self-assigned this Apr 17, 2023

stufreen reviewed Apr 17, 2023

View reviewed changes

metamoni reviewed Apr 17, 2023

View reviewed changes

assets/animations.js Outdated Show resolved Hide resolved

ludoboludo added 2 commits April 17, 2023 14:44

address some of the reviewer's comments

06efe8d

simplify conditions for if statement

3785afb

LucasLacerdaUX reviewed Apr 17, 2023

View reviewed changes

ludoboludo added 2 commits April 17, 2023 15:13

Add early return

e89ed02

change variable name

9762be4

ludoboludo requested review from metamoni, stufreen and LucasLacerdaUX April 17, 2023 19:17

stufreen approved these changes Apr 17, 2023

View reviewed changes

LucasLacerdaUX approved these changes Apr 17, 2023

View reviewed changes

ludoboludo merged commit d032879 into main Apr 17, 2023

ludoboludo deleted the no-animation-on-elements-in-viewport branch April 17, 2023 20:46

phapsidesGT pushed a commit to Gravytrain-UK/gt-shopify-dawn-theme that referenced this pull request Sep 3, 2024

[Motion] No animation on added/edited section (Shopify#2502)

03fc5b9

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Motion] No animation on added/edited section #2502

[Motion] No animation on added/edited section #2502

ludoboludo commented Apr 7, 2023 •

edited

Loading

kjellr commented Apr 7, 2023

ludoboludo commented Apr 11, 2023

kjellr commented Apr 12, 2023

ludoboludo Apr 13, 2023 •

edited

Loading

kjellr Apr 13, 2023

ludoboludo Apr 13, 2023

kjellr Apr 13, 2023

kjellr left a comment

stufreen Apr 17, 2023

ludoboludo Apr 17, 2023

LucasLacerdaUX Apr 17, 2023

ludoboludo Apr 17, 2023

LucasLacerdaUX left a comment

[Motion] No animation on added/edited section #2502

[Motion] No animation on added/edited section #2502

Conversation

ludoboludo commented Apr 7, 2023 • edited Loading

PR Summary:

Why are these changes introduced?

What approach did you take?

Other considerations

Decision log

Visual impact on existing themes

Testing steps/scenarios

Demo links

Checklist

kjellr commented Apr 7, 2023

ludoboludo commented Apr 11, 2023

kjellr commented Apr 12, 2023

ludoboludo Apr 13, 2023 • edited Loading

Choose a reason for hiding this comment

kjellr Apr 13, 2023

Choose a reason for hiding this comment

ludoboludo Apr 13, 2023

Choose a reason for hiding this comment

kjellr Apr 13, 2023

Choose a reason for hiding this comment

kjellr left a comment

Choose a reason for hiding this comment

stufreen Apr 17, 2023

Choose a reason for hiding this comment

ludoboludo Apr 17, 2023

Choose a reason for hiding this comment

LucasLacerdaUX Apr 17, 2023

Choose a reason for hiding this comment

ludoboludo Apr 17, 2023

Choose a reason for hiding this comment

LucasLacerdaUX left a comment

Choose a reason for hiding this comment

ludoboludo commented Apr 7, 2023 •

edited

Loading

ludoboludo Apr 13, 2023 •

edited

Loading