microsoft / react-native-windows

A framework for building native Windows apps with React.
https://microsoft.github.io/react-native-windows/
Other
16.34k stars 1.14k forks source link

Have RNW respect the Windows OS setting, 'AnimationsEnabled' #3508

Closed KAnder425 closed 5 years ago

KAnder425 commented 5 years ago

Proposal:

Have RNW respect the Windows OS setting, 'AnimationsEnabled'.

Summary

RNW doesn't currently respect the OS setting to enable/disable XAML animations . (eg. Windows::UI::ViewManagement::UISettings::AnimationsEnabled)

Motivation

In Stellar, we are struggling with getting our animations to look 'stellar', so we want to enable and disable animation support in RNW at run-time based on the OS setting.

acoates-ms commented 5 years ago

XAML never did either. Its impossible for the OS / platform to know if the animation is just cosmetic, or if disabling it would have bad consequences. (Ex: You wouldn't want spinning dots for progress to not animate).

I'm guessing the best we could do it expose the setting as part of a native module, so you can decide to use an animation or not. -- But that should also probably ship as a separate package.

kmelmon commented 5 years ago

I agree we should just surface an API.

After doing a quick search of XAML code, I see some built-in XAML controls do respect the setting. Thus, any such control that gets wrapped will respect the setting. If folks build controls out of primitives, we should give them a way to make their controls respect the setting as well. I don't know if other platforms have a similar setting. If so it would make sense to have this be a xplat module.

RobMeyer commented 5 years ago

The downside of putting the burden on apps to respect the setting is that most folks aren't even aware that this preference exists, yet alone have the time/desire to go add an if clause around every animation across their codebase. Worse yet, you're sharing open source controls (perhaps written for Android and iOS, but that work fine on Windows) which don't have the check or expose an interface by which you may disable animations. The user is the one who suffers due to inconsistent experience across apps.

I agree in principal that the OS/platform can't guarantee no bad consequences -- hypothetically one could construct a component with implicit assumptions around timing or even go so far as to have native code that interacts with the XAML storyboard corresponding to the animation they prepared in JS. But, in practice I think we'd be able to come up with a simple solution that addresses the 99.99% case "for free" and makes it easy for a developer to opt-out of automatic AnimationsEnabled support if they do encounter an issue and would rather blanket their codebase with AnimationsEnabled checks.

My recommendation would be (1) by default we should match what iOS and Android do. Do either of those platforms have an OS setting? Does RN respect the OS setting by default? (2) We could expose an opt-in or opt-out way for an app to say, "please always-respect/always-ignore the OS 'AnimationsEnabled' setting".

For (2), in order to mitigate issues where there are bad consequences because the client has implicit assumptions around observable side-effects such as the animation completed callback, I'd recommend when respecting the OS setting we should strive to keeping the JS-observable side effects the same for both the enabled and disabled cases so it is seamless to the client app. The animation completed event would still fire after the same duration (so if someone is using an Animated.Value as a timer, it still functions as such). To achive the user's desired visuals, the final value would be applied as both the start and final value whenever AnimationsEnabled=false and the client app is configured to respect the OS setting.

jonthysell commented 5 years ago

Possibly relevant API in the AccessibilityInfo native module: https://facebook.github.io/react-native/docs/accessibilityinfo#isreducemotionenabled

chrisglein commented 5 years ago

Recommendation: Let's implement isReduceMotionEnabled (doesn't do anything today #3110) and app authors should check this before firing animations. A global disable on the app's behalf has proven problematic (we can't tell which animations are essential and which are not).