offline notice: Animate entrance/exit, and placeholder's height to match

For how we avoid being affected by the bad interactions with
react-native-screens on Android, see comments in
OfflineNoticeProvider.

Author: chrisbobbe

src/ZulipMobile.js

@@ -32,6 +32,28 @@ if (Platform.OS === 'android') {
   //
   // In the meantime, we should be on the lookout for any issues with
   // this feature on Android.
+  //
+  // There are some bad interactions with react-native-screens. From
+  // testing, they seem to be Android only, and they go away with
+  // react-native-screens disabled:
+  //  - With react-native-screens enabled, at least at 3.13.1 and 3.17.0, be
+  //    careful when you want a single action to trigger animated layout
+  //    changes on multiple screens at the same time. If a screen isn't
+  //    focused when the layout change happens, the layout change and its
+  //    animation might not happen until you focus the screen. I seemed to
+  //    be able to fix this by calling react-native-screens's
+  //    enableFreeze(true) at the top of this file, but I don't really
+  //    understand why; here's the doc:
+  //      https://github.com/software-mansion/react-freeze
+  //  - With react-native-screens enabled, at least at 3.13.1 and 3.17.0, if
+  //    I'm on a screen where a layout change is happening, and I navigate
+  //    away during the animation, when I come back to the first screen it
+  //    seems stuck at a snapshot of its layout, mid-animation, when I
+  //    navigated away. I haven't found a fix except, at the moment the
+  //    screen regains focus, to unmount and remount *all* affected views on
+  //    the screen, with a changing `key` pseudoprop. This is awkward and
+  //    unfortunately doesn't prevent the frozen layout from showing for a
+  //    split second before the layout resets.
   UIManager.setLayoutAnimationEnabledExperimental(true);
 }
 

src/boot/OfflineNoticeProvider.js

@@ -1,7 +1,7 @@
 // @flow strict-local
-import React, { useCallback, useContext, useEffect, useMemo, useState } from 'react';
+import React, { useCallback, useContext, useEffect, useMemo, useRef, useState } from 'react';
 import type { Node } from 'react';
-import { View } from 'react-native';
+import { View, Animated, LayoutAnimation, Platform, Easing } from 'react-native';
 import NetInfo from '@react-native-community/netinfo';
 import { SafeAreaView } from 'react-native-safe-area-context';
 import type { ViewProps } from 'react-native/Libraries/Components/View/ViewPropTypes';
@@ -10,7 +10,7 @@ import type { DimensionValue } from 'react-native/Libraries/StyleSheet/StyleShee
 import * as logging from '../utils/logging';
 import { useGlobalSelector } from '../react-redux';
 import { getGlobalSession, getGlobalSettings } from '../directSelectors';
-import { useHasStayedTrueForMs } from '../reactUtils';
+import { useHasStayedTrueForMs, usePrevious } from '../reactUtils';
 import type { JSONableDict } from '../utils/jsonable';
 import { createStyleSheet } from '../styles';
 import ZulipTextIntl from '../common/ZulipTextIntl';
@@ -89,7 +89,38 @@ export function OfflineNoticeProvider(props: ProviderProps): Node {
   const theme = useGlobalSelector(state => getGlobalSettings(state).theme);
   const isOnline = useGlobalSelector(state => getGlobalSession(state).isOnline);
   const shouldShowUncertaintyNotice = useShouldShowUncertaintyNotice();
-  const isNoticeVisible = isOnline === false || shouldShowUncertaintyNotice;
+
+  // Use local UI state for isNoticeVisible instead of computing directly as
+  // a `const`, so we can apply LayoutAnimation.configureNext to just the
+  // visibility state change, instead of, e.g., all layout changes
+  // potentially caused by an APP_ONLINE action.
+  const [isNoticeVisible, setIsNoticeVisible] = useState(
+    isOnline === false || shouldShowUncertaintyNotice,
+  );
+
+  useEffect(() => {
+    setIsNoticeVisible(oldValue => {
+      const newValue = isOnline === false || shouldShowUncertaintyNotice;
+      if (oldValue !== newValue) {
+        // Animate the entrance and exit of the offline notice. For how we
+        // animate OfflineNoticePlaceholder, see there.
+        //
+        // For the notice, we shouldn't be affected by the known bad
+        // interactions with react-native-screens, because the notice is
+        // rootward of all the React Navigation screens in the app. For what
+        // those bad interactions are, see the comment in ZulipMobile.js on
+        // `UIManager.setLayoutAnimationEnabledExperimental(true)`.
+        LayoutAnimation.configureNext({
+          ...LayoutAnimation.Presets.easeInEaseOut,
+
+          // Enter slowly to give bad, possibly unexpected news. Leave quickly
+          // to give good, hoped-for news.
+          duration: newValue ? 1000 : 300,
+        });
+      }
+      return newValue;
+    });
+  }, [isOnline, shouldShowUncertaintyNotice]);
 
   const styles = useMemo(
     () =>
@@ -99,6 +130,14 @@ export function OfflineNoticeProvider(props: ProviderProps): Node {
           position: 'absolute',
 
           // Whether the notice is visible or tucked away above the window.
+          //
+          // (Just as we discovered in 3fa7a7f10 with the lightbox, it seems
+          // the Animated API wouldn't let us do a translate-transform
+          // animation with a percentage; that's issue
+          //   https://github.com/facebook/react-native/issues/13107 .
+          // So we use LayoutAnimation, which is probably better anyway
+          // because it lets us animate layout changes at the native layer,
+          // and so won't drop frames when the JavaScript thread is busy.)
           ...(isNoticeVisible ? { top: 0 } : { bottom: '100%' }),
 
           zIndex: 1,
@@ -237,17 +276,60 @@ export function OfflineNoticePlaceholder(props: PlaceholderProps): Node {
   const { style: callerStyle } = props;
 
   const { isNoticeVisible, noticeContentAreaHeight } = useContext(OfflineNoticeContext);
+  const prevIsNoticeVisible = usePrevious(isNoticeVisible, isNoticeVisible);
+
+  const plainHeight = isNoticeVisible ? noticeContentAreaHeight : 0;
+  const animHeight = useRef(new Animated.Value(plainHeight)).current;
+
+  // Part of an Android workaround; see where we set the View's height.
+  useEffect(() => {
+    if (Platform.OS !== 'android' || prevIsNoticeVisible === isNoticeVisible) {
+      return;
+    }
+
+    // Should approximate OfflineNoticeProvider's animation curve.
+    const animation = Animated.timing(animHeight, {
+      toValue: plainHeight,
+      duration: isNoticeVisible ? 1000 : 300,
+      easing: Easing.inOut(t => Easing.ease(t)),
+
+      // With `true`, I get an error:
+      // - On Android: "Animated node with tag […] does not exist"
+      // - On iOS: "Style property 'height' is not supported by native
+      //   animated module".
+      useNativeDriver: false,
+    });
+
+    animation.start();
+  }, [isNoticeVisible, prevIsNoticeVisible, plainHeight, animHeight]);
 
   const style = useMemo(
     () => [
       {
-        height: isNoticeVisible ? noticeContentAreaHeight : 0,
+        height:
+          /* prettier-ignore */
+          Platform.OS === 'android'
+            // Avoid some bad interactions on Android with
+            // react-native-screens; see the comment in ZulipMobile.js on
+            // `UIManager.setLayoutAnimationEnabledExperimental(true)`. To
+            // avoid those, don't pass `plainHeight`, which would cause the
+            // resulting layout change to be animated with
+            // OfflineNoticeProvider's LayoutAnimation.configureNext call.
+            // Instead, use RN's Animated API.
+            ? animHeight
+            // Do pass `plainHeight`, to piggy-back on
+            // OfflineNoticeProvider's LayoutAnimation call. Nothing seems
+            // to break this simple use of LayoutAnimation on iOS, and it's
+            // better than Animated because Animated can drop animation
+            // frames when the CPU is busy (at least without
+            // `useNativeDriver: true`, and that doesn't support `height`).
+            : plainHeight,
         width: '100%',
         backgroundColor: 'transparent',
       },
       callerStyle,
     ],
-    [isNoticeVisible, noticeContentAreaHeight, callerStyle],
+    [plainHeight, animHeight, callerStyle],
   );
 
   return (
@@ -264,7 +346,9 @@ export function OfflineNoticePlaceholder(props: PlaceholderProps): Node {
           />
         )
       }
-      <View style={style} />
+      {/* The `Animated.View` is for the Android workaround; iOS could use a
+          regular View. See comment where we set the height attribute. */}
+      <Animated.View style={style} />
     </>
   );
 }