Skip to content
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

[a11y] Changing logic of how reduce motion options are set to match it with lottie iOS #2536

Merged
merged 4 commits into from
Aug 21, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions lottie/src/main/java/com/airbnb/lottie/L.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
import androidx.annotation.Nullable;
import androidx.annotation.RestrictTo;

import com.airbnb.lottie.configurations.reducemotion.ReducedMotionOption;
import com.airbnb.lottie.configurations.reducemotion.SystemReducedMotionOption;
import com.airbnb.lottie.network.DefaultLottieNetworkFetcher;
import com.airbnb.lottie.network.LottieNetworkCacheProvider;
import com.airbnb.lottie.network.LottieNetworkFetcher;
Expand All @@ -32,6 +34,7 @@ public class L {
private static volatile NetworkFetcher networkFetcher;
private static volatile NetworkCache networkCache;
private static ThreadLocal<LottieTrace> lottieTrace;
private static ReducedMotionOption reducedMotionOption = new SystemReducedMotionOption();

private L() {
}
Expand Down Expand Up @@ -143,4 +146,10 @@ public static void setDefaultAsyncUpdates(AsyncUpdates asyncUpdates) {
public static AsyncUpdates getDefaultAsyncUpdates() {
return L.defaultAsyncUpdates;
}

public static void setReducedMotionOption(ReducedMotionOption reducedMotionOption){
L.reducedMotionOption = reducedMotionOption;
}

public static ReducedMotionOption getReducedMotionOption(){return reducedMotionOption;}
}
1 change: 1 addition & 0 deletions lottie/src/main/java/com/airbnb/lottie/Lottie.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,6 @@ public static void initialize(@NonNull final LottieConfig lottieConfig) {
L.setNetworkCacheEnabled(lottieConfig.enableNetworkCache);
L.setDisablePathInterpolatorCache(lottieConfig.disablePathInterpolatorCache);
L.setDefaultAsyncUpdates(lottieConfig.defaultAsyncUpdates);
L.setReducedMotionOption(lottieConfig.reducedMotionOption);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -260,8 +260,6 @@ private void init(@Nullable AttributeSet attrs, @AttrRes int defStyleAttr) {
}

ta.recycle();

lottieDrawable.setSystemAnimationsAreEnabled(Utils.getAnimationScale(getContext()) != 0f);
}

@Override public void setImageResource(int resId) {
Expand Down Expand Up @@ -377,7 +375,10 @@ private void init(@Nullable AttributeSet attrs, @AttrRes int defStyleAttr) {
* Defaults to false.
*
* @param ignore if true animations will run even when they are disabled in the system settings.
* @deprecated Use {@link com.airbnb.lottie.configurations.reducemotion.IgnoreDisabledSystemAnimationsOption}
* instead and set them on the {@link LottieConfig}
*/
@Deprecated
public void setIgnoreDisabledSystemAnimations(boolean ignore) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing an existing public API is tricky. At the very least, we'll need to deprecate this and provide a graceful fallback. Ideally, its behavior would remain the same (scope IgnoreDisabledSystemAnimations() to this view if possible).

It can be fully removed after a few releases.

Copy link
Contributor Author

@pranayairan pranayairan Aug 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gpeal make sense, added the original API back. I also marked it as deprecated so folks can switch to new API

lottieDrawable.setIgnoreDisabledSystemAnimations(ignore);
}
Expand Down
29 changes: 25 additions & 4 deletions lottie/src/main/java/com/airbnb/lottie/LottieConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@

import androidx.annotation.NonNull;
import androidx.annotation.Nullable;

import com.airbnb.lottie.configurations.reducemotion.ReducedMotionOption;
import com.airbnb.lottie.configurations.reducemotion.IgnoreDisabledSystemAnimationsOption;
import com.airbnb.lottie.configurations.reducemotion.SystemReducedMotionOption;
import com.airbnb.lottie.network.LottieNetworkCacheProvider;
import com.airbnb.lottie.network.LottieNetworkFetcher;

Expand All @@ -21,16 +23,18 @@ public class LottieConfig {
final boolean enableNetworkCache;
final boolean disablePathInterpolatorCache;
final AsyncUpdates defaultAsyncUpdates;
final ReducedMotionOption reducedMotionOption;

private LottieConfig(@Nullable LottieNetworkFetcher networkFetcher, @Nullable LottieNetworkCacheProvider cacheProvider,
boolean enableSystraceMarkers, boolean enableNetworkCache, boolean disablePathInterpolatorCache,
AsyncUpdates defaultAsyncUpdates) {
AsyncUpdates defaultAsyncUpdates, ReducedMotionOption reducedMotionOption) {
this.networkFetcher = networkFetcher;
this.cacheProvider = cacheProvider;
this.enableSystraceMarkers = enableSystraceMarkers;
this.enableNetworkCache = enableNetworkCache;
this.disablePathInterpolatorCache = disablePathInterpolatorCache;
this.defaultAsyncUpdates = defaultAsyncUpdates;
this.reducedMotionOption = reducedMotionOption;
}

public static final class Builder {
Expand All @@ -43,6 +47,7 @@ public static final class Builder {
private boolean enableNetworkCache = true;
private boolean disablePathInterpolatorCache = true;
private AsyncUpdates defaultAsyncUpdates = AsyncUpdates.AUTOMATIC;
private ReducedMotionOption reducedMotionOption = new SystemReducedMotionOption();

/**
* Lottie has a default network fetching stack built on {@link java.net.HttpURLConnection}. However, if you would like to hook into your own
Expand Down Expand Up @@ -122,7 +127,7 @@ public Builder setEnableNetworkCache(boolean enable) {
* When parsing animations, Lottie has a path interpolator cache. This cache allows Lottie to reuse PathInterpolators
* across an animation. This is desirable in most cases. However, when shared across screenshot tests, it can cause slight
* deviations in the rendering due to underlying approximations in the PathInterpolator.
*
* <p>
* The cache is enabled by default and should probably only be disabled for screenshot tests.
*/
@NonNull
Expand All @@ -133,6 +138,7 @@ public Builder setDisablePathInterpolatorCache(boolean disable) {

/**
* Sets the default value for async updates.
*
* @see LottieDrawable#setAsyncUpdates(AsyncUpdates)
*/
@NonNull
Expand All @@ -141,10 +147,25 @@ public Builder setDefaultAsyncUpdates(AsyncUpdates asyncUpdates) {
return this;
}

/**
* Provide your own reduce motion option. By default it uses
* {@link SystemReducedMotionOption},
* which returns ReducedMotionMode.REDUCED_MOTION when the system AnimationScale is set to 0.
* <p>
* You can override this behavior by providing your own custom {@link ReducedMotionOption}.
* You can also use {@link IgnoreDisabledSystemAnimationsOption}
* if you want to ignore the system settings and always play the full animation.
*/
@NonNull
public Builder setReducedMotionOption(ReducedMotionOption reducedMotionOption) {
this.reducedMotionOption = reducedMotionOption;
return this;
}

@NonNull
public LottieConfig build() {
return new LottieConfig(networkFetcher, cacheProvider, enableSystraceMarkers, enableNetworkCache, disablePathInterpolatorCache,
defaultAsyncUpdates);
defaultAsyncUpdates, reducedMotionOption);
}
}
}
10 changes: 9 additions & 1 deletion lottie/src/main/java/com/airbnb/lottie/LottieDrawable.java
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import androidx.annotation.RestrictTo;

import com.airbnb.lottie.animation.LPaint;
import com.airbnb.lottie.configurations.reducemotion.ReducedMotionMode;
import com.airbnb.lottie.manager.FontAssetManager;
import com.airbnb.lottie.manager.ImageAssetManager;
import com.airbnb.lottie.model.Font;
Expand Down Expand Up @@ -1244,7 +1245,8 @@ boolean isAnimatingOrWillAnimateOnVisible() {
}

private boolean animationsEnabled() {
return systemAnimationsEnabled || ignoreSystemAnimationsDisabled;
return (systemAnimationsEnabled || ignoreSystemAnimationsDisabled) &&
L.getReducedMotionOption().getCurrentReducedMotionMode(getContext()) == ReducedMotionMode.STANDARD_MOTION;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I'm reading this correctly, if you set ignoreSystemAnimationsDisabled but the system scale is 0 then:

  • You would expect animationsEnabled to return true
  • systemAnimationsEnable: false
  • ignoreSystemAnimationsDisabled: true
  • LottieConfig: REDUCED

The predicate becomes (false || true) && (REDUCED == STANDARD) which would be false.

I think this may also read easier if you treat ignoreSystemAnimationsDisabled == true as higher priority with:

if (ignoreSystemAnimationsDisabled) {
    return true;
}
return L.getReducedMotionOption().getCurrentReducedMotionMode(getContext()) == ReducedMotionMode.STANDARD_MOTION;

Just off the top of my head, I think this is correct and easier to read. Is that right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are right. Ideally getCurrentReducedMotionMode should not even be evaluated when ignoreSystemAnimationsDisabled is set to true. Let me fix it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed it to this

private boolean animationsEnabled() {
    if (ignoreSystemAnimationsDisabled) {
      return true;
    }
    return systemAnimationsEnabled &&
        L.getReducedMotionOption().getCurrentReducedMotionMode(getContext()) == ReducedMotionMode.STANDARD_MOTION;
  }

We still need to consider old systemAnimationsEnabled flag that can be set on drawable directly for backward compatibility

}

/**
Expand All @@ -1256,7 +1258,10 @@ private boolean animationsEnabled() {
* - reducedMotion
* - reduced_motion
* - reduced-motion
*
* @deprecated Use {@link com.airbnb.lottie.configurations.reducemotion.ReducedMotionOption} instead and set them on the {@link LottieConfig}
*/
@Deprecated
public void setSystemAnimationsAreEnabled(Boolean areEnabled) {
systemAnimationsEnabled = areEnabled;
}
Expand All @@ -1269,7 +1274,10 @@ public void setSystemAnimationsAreEnabled(Boolean areEnabled) {
* Defaults to false.
*
* @param ignore if true animations will run even when they are disabled in the system settings.
* @deprecated Use {@link com.airbnb.lottie.configurations.reducemotion.IgnoreDisabledSystemAnimationsOption}
* instead and set them on the {@link LottieConfig}
*/
@Deprecated
public void setIgnoreDisabledSystemAnimations(boolean ignore) {
ignoreSystemAnimationsDisabled = ignore;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package com.airbnb.lottie.configurations.reducemotion;

import android.content.Context;

/**
* Allows ignoring system animations settings, therefore allowing animations to run even if they are disabled.
*/
public class IgnoreDisabledSystemAnimationsOption implements ReducedMotionOption {

@Override
public ReducedMotionMode getCurrentReducedMotionMode(Context context) {
return ReducedMotionMode.STANDARD_MOTION;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package com.airbnb.lottie.configurations.reducemotion;


public enum ReducedMotionMode {
/**
* The default behavior where Lottie animations play normally with no overrides.
* By default this mode is used when {@link com.airbnb.lottie.utils.Utils#getAnimationScale(Context)} is not 0.
*/
STANDARD_MOTION,

/**
* Lottie animations with a "reduced motion" marker will play that marker instead of any other animations.
* By default this mode is used when {@link com.airbnb.lottie.utils.Utils#getAnimationScale(Context)} == 0.
*/
REDUCED_MOTION
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package com.airbnb.lottie.configurations.reducemotion;

import android.content.Context;

public interface ReducedMotionOption {

/**
* Returns the current reduced motion mode.
*/
ReducedMotionMode getCurrentReducedMotionMode(Context context);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package com.airbnb.lottie.configurations.reducemotion;

import android.content.Context;
import com.airbnb.lottie.utils.Utils;

/**
* Lottie animations with a "reduced motion" marker will play that marker instead of any other animations.
* This class uses {@link com.airbnb.lottie.utils.Utils#getAnimationScale(Context)} to determine if animations are disabled
* and if it should play the reduced motion marker.
*
* If the animation is provided a "reduced motion"
* marker name, they will be shown instead of the first or last frame. Supported marker names are case insensitive, and include:
* - reduced motion
* - reducedMotion
* - reduced_motion
* - reduced-motion
*/
public class SystemReducedMotionOption implements ReducedMotionOption {

@Override
public ReducedMotionMode getCurrentReducedMotionMode(Context context) {
if (Utils.getAnimationScale(context) != 0f) {
return ReducedMotionMode.STANDARD_MOTION;
} else {
return ReducedMotionMode.REDUCED_MOTION;
}
}
}
2 changes: 1 addition & 1 deletion lottie/src/main/res/values/attrs.xml
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@
<attr name="lottie_colorFilter" format="color" />
<attr name="lottie_speed" format="float" />
<attr name="lottie_cacheComposition" format="boolean" />
<attr name="lottie_ignoreDisabledSystemAnimations" format="boolean" />
<attr name="lottie_useCompositionFrameRate" format="boolean" />
<attr name="lottie_ignoreDisabledSystemAnimations" format="boolean" />
<attr name="lottie_clipToCompositionBounds" format="boolean" />
<attr name="lottie_clipTextToBoundingBox" format="boolean" />
<!-- The default file extension that Lottie will use when finding fonts in assets/fonts/fontFamily.* -->
Expand Down
32 changes: 32 additions & 0 deletions lottie/src/test/java/com/airbnb/lottie/LottieDrawableTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import android.graphics.Rect;
import androidx.collection.LongSparseArray;
import androidx.collection.SparseArrayCompat;
import com.airbnb.lottie.configurations.reducemotion.ReducedMotionMode;

import org.junit.Before;
import org.junit.Test;
Expand Down Expand Up @@ -77,6 +78,7 @@ public void testMinMaxFrame() {

@Test
public void testPlayWhenSystemAnimationDisabled() {
disableSystemAnimation();
LottieComposition composition = createComposition(31, 391);
LottieDrawable drawable = new LottieDrawable();
drawable.addAnimatorListener(animatorListener);
Expand All @@ -98,4 +100,34 @@ public void testResumeWhenSystemAnimationDisabled() {
assertEquals(391, drawable.getFrame());
verify(animatorListener, atLeastOnce()).onAnimationEnd(any(Animator.class), eq(false));
}

@Test
public void testPlayWhenSystemAnimationDisabledFromLottieConfig() {
disableSystemAnimation();
LottieComposition composition = createComposition(31, 391);
LottieDrawable drawable = new LottieDrawable();
drawable.addAnimatorListener(animatorListener);
drawable.setComposition(composition);
drawable.playAnimation();
assertEquals(391, drawable.getFrame());
verify(animatorListener, atLeastOnce()).onAnimationEnd(any(Animator.class), eq(false));
}

@Test
public void testResumeWhenSystemAnimationDisabledFromLottieConfig() {
disableSystemAnimation();
LottieComposition composition = createComposition(31, 391);
LottieDrawable drawable = new LottieDrawable();
drawable.addAnimatorListener(animatorListener);
drawable.setComposition(composition);
drawable.resumeAnimation();
assertEquals(391, drawable.getFrame());
verify(animatorListener, atLeastOnce()).onAnimationEnd(any(Animator.class), eq(false));
}

private void disableSystemAnimation() {
Lottie.initialize(new LottieConfig.Builder().setReducedMotionOption(
context -> ReducedMotionMode.REDUCED_MOTION
).build());
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
package com.airbnb.lottie.snapshots.tests

import com.airbnb.lottie.Lottie
import com.airbnb.lottie.LottieConfig
import com.airbnb.lottie.configurations.reducemotion.ReducedMotionMode
import com.airbnb.lottie.snapshots.SnapshotTestCase
import com.airbnb.lottie.snapshots.SnapshotTestCaseContext
import com.airbnb.lottie.snapshots.withDrawable
Expand All @@ -21,5 +24,30 @@ class DisabledAnimationsTestCase : SnapshotTestCase {
drawable.playAnimation()
}
}
withDrawable("Tests/ReducedMotion.json", "System Animations", "Lottie config Disabled") { drawable ->
withContext(Dispatchers.Main) {
disableSystemAnimation()
drawable.playAnimation()
}
}

withDrawable("Tests/ReducedMotion.json", "System Animations", "Lottie config enabled") { drawable ->
withContext(Dispatchers.Main) {
disableSystemAnimation(disable = false)
drawable.playAnimation()
}
}
}

private fun disableSystemAnimation(disable: Boolean = true) {
Lottie.initialize(
LottieConfig.Builder().setReducedMotionOption {
if (disable) {
ReducedMotionMode.REDUCED_MOTION
} else {
ReducedMotionMode.STANDARD_MOTION
}
}.build(),
)
}
}
Loading