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.

Sign up for GitHub

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

WIP: Replace react-native Animated API with react-native-reanimated #52956

Closed
wants to merge 7 commits into from
Closed

WIP: Replace react-native Animated API with react-native-reanimated #52956

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
74361e3
Migrate Switch to Reanimated
blazejkustra Nov 22, 2024
a7dfc67
Tooltip migration
sumo-slonik Nov 22, 2024
334a0de
code review fixes for tooltip
sumo-slonik Nov 26, 2024
1bc7257
migrate AttachmentModal.tsx
sumo-slonik Nov 27, 2024
4385dcf
migrate GrowlNotification
sumo-slonik Nov 27, 2024
e20c704
migrate TextInput
sumo-slonik Nov 28, 2024
46d8efb
add worklet signatures to functions
sumo-slonik Nov 29, 2024
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
45 changes: 23 additions & 22 deletions src/components/Switch.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import React, {useEffect, useRef} from 'react';
import {Animated, InteractionManager} from 'react-native';
import React from 'react';
import {InteractionManager} from 'react-native';
import Animated, {interpolateColor, useAnimatedStyle, useSharedValue, withTiming} from 'react-native-reanimated';
import useTheme from '@hooks/useTheme';
import useThemeStyles from '@hooks/useThemeStyles';
import useNativeDriver from '@libs/useNativeDriver';
import CONST from '@src/CONST';
import Icon from './Icon';
import * as Expensicons from './Icon/Expensicons';
Expand Down Expand Up @@ -35,7 +35,7 @@ const OFFSET_X = {

function Switch({isOn, onToggle, accessibilityLabel, disabled, showLockIcon, disabledAction}: SwitchProps) {
const styles = useThemeStyles();
const offsetX = useRef(new Animated.Value(isOn ? OFFSET_X.ON : OFFSET_X.OFF));
const offsetX = useSharedValue(isOn ? OFFSET_X.ON : OFFSET_X.OFF);
const theme = useTheme();

const handleSwitchPress = () => {
Expand All @@ -44,22 +44,22 @@ function Switch({isOn, onToggle, accessibilityLabel, disabled, showLockIcon, dis
disabledAction?.();
return;
}
offsetX.value = withTiming(isOn ? OFFSET_X.OFF : OFFSET_X.ON, {duration: 300});
onToggle(!isOn);
});
};

useEffect(() => {
Animated.timing(offsetX.current, {
toValue: isOn ? OFFSET_X.ON : OFFSET_X.OFF,
duration: 300,
useNativeDriver,
}).start();
}, [isOn]);
const animatedThumbStyle = useAnimatedStyle(() => ({
transform: [{translateX: offsetX.value}],
}));

const animatedSwitchTrackStyle = useAnimatedStyle(() => ({
backgroundColor: interpolateColor(offsetX.value, [OFFSET_X.OFF, OFFSET_X.ON], [theme.icon, theme.success]),
}));

return (
<PressableWithFeedback
disabled={!disabledAction && disabled}
style={[styles.switchTrack, !isOn && styles.switchInactive]}
onPress={handleSwitchPress}
onLongPress={handleSwitchPress}
role={CONST.ROLE.SWITCH}
Expand All @@ -69,16 +69,17 @@ function Switch({isOn, onToggle, accessibilityLabel, disabled, showLockIcon, dis
hoverDimmingValue={1}
pressDimmingValue={0.8}
>
{/* eslint-disable-next-line react-compiler/react-compiler */}
<Animated.View style={[styles.switchThumb, styles.switchThumbTransformation(offsetX.current)]}>
{(!!disabled || !!showLockIcon) && (
<Icon
src={Expensicons.Lock}
fill={isOn ? theme.text : theme.icon}
width={styles.toggleSwitchLockIcon.width}
height={styles.toggleSwitchLockIcon.height}
/>
)}
<Animated.View style={[styles.switchTrack, animatedSwitchTrackStyle]}>
<Animated.View style={[styles.switchThumb, animatedThumbStyle]}>
{(!!disabled || !!showLockIcon) && (
<Icon
src={Expensicons.Lock}
fill={isOn ? theme.text : theme.icon}
width={styles.toggleSwitchLockIcon.width}
height={styles.toggleSwitchLockIcon.height}
/>
)}
</Animated.View>
</Animated.View>
</PressableWithFeedback>
);
Expand Down
12 changes: 8 additions & 4 deletions src/components/Tooltip/BaseGenericTooltip/index.native.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import {Portal} from '@gorhom/portal';
import React, {useMemo, useRef, useState} from 'react';
import {Animated, InteractionManager, View} from 'react-native';
import {InteractionManager, View} from 'react-native';
// eslint-disable-next-line no-restricted-imports
import type {View as RNView} from 'react-native';
import Animated, {useAnimatedStyle} from 'react-native-reanimated';
import TransparentOverlay from '@components/AutoCompleteSuggestions/AutoCompleteSuggestionsPortal/TransparentOverlay/TransparentOverlay';
import Text from '@components/Text';
import useStyleUtils from '@hooks/useStyleUtils';
Expand Down Expand Up @@ -46,13 +47,12 @@ function BaseGenericTooltip({
const rootWrapper = useRef<RNView>(null);

const StyleUtils = useStyleUtils();

const {animationStyle, rootWrapperStyle, textStyle, pointerWrapperStyle, pointerStyle} = useMemo(
const {rootWrapperStyle, textStyle, pointerWrapperStyle, pointerStyle} = useMemo(
() =>
StyleUtils.getTooltipStyles({
// eslint-disable-next-line react-compiler/react-compiler
tooltip: rootWrapper.current,
currentSize: animation,
currentSize: animation.value,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This line should be removed, right? And the logic inside getTooltipStyles adjusted so that animationStyles aren't returned.

Copy link
Contributor

Choose a reason for hiding this comment

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

I will correct it, I didn't want to change utils that are already written, but I can safely remove it is not used anywhere

windowWidth,
xOffset,
yOffset,
Expand Down Expand Up @@ -87,6 +87,10 @@ function BaseGenericTooltip({
],
);

const animationStyle = useAnimatedStyle(() => {
return StyleUtils.getTooltipAnimatedStyles({tooltipContentWidth: contentMeasuredWidth, tooltipWrapperHeight: wrapperMeasuredHeight, currentSize: animation});
});

let content;
if (renderTooltipContent) {
content = <View>{renderTooltipContent()}</View>;
Expand Down
12 changes: 9 additions & 3 deletions src/components/Tooltip/BaseGenericTooltip/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
/* eslint-disable react-compiler/react-compiler */
import React, {useLayoutEffect, useMemo, useRef, useState} from 'react';
import ReactDOM from 'react-dom';
import {Animated, View} from 'react-native';
import {View} from 'react-native';
import Animated, {useAnimatedStyle} from 'react-native-reanimated';
import TransparentOverlay from '@components/AutoCompleteSuggestions/AutoCompleteSuggestionsPortal/TransparentOverlay/TransparentOverlay';
import Text from '@components/Text';
import useStyleUtils from '@hooks/useStyleUtils';
Expand All @@ -15,6 +16,7 @@ import type {BaseGenericTooltipProps} from './types';
// We also update the state on layout changes which will be triggered often.
// There will be n number of tooltip components in the page.
// It's good to memoize this one.

function BaseGenericTooltip({
animation,
windowWidth,
Expand Down Expand Up @@ -64,11 +66,11 @@ function BaseGenericTooltip({
}
}, []);

const {animationStyle, rootWrapperStyle, textStyle, pointerWrapperStyle, pointerStyle} = useMemo(
const {rootWrapperStyle, textStyle, pointerWrapperStyle, pointerStyle} = useMemo(
() =>
StyleUtils.getTooltipStyles({
tooltip: rootWrapper.current,
currentSize: animation,
currentSize: animation.value,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same here

windowWidth,
xOffset,
yOffset,
Expand Down Expand Up @@ -102,6 +104,10 @@ function BaseGenericTooltip({
],
);

const animationStyle = useAnimatedStyle(() => {
return StyleUtils.getTooltipAnimatedStyles({tooltipContentWidth: contentMeasuredWidth, tooltipWrapperHeight: wrapperMeasuredHeight, currentSize: animation});
});

let content;
if (renderTooltipContent) {
content = <View ref={viewRef(contentRef)}>{renderTooltipContent()}</View>;
Expand Down
3 changes: 2 additions & 1 deletion src/components/Tooltip/BaseGenericTooltip/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import type {Animated} from 'react-native';

Check failure on line 1 in src/components/Tooltip/BaseGenericTooltip/types.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

'Animated' is defined but never used

Check failure on line 1 in src/components/Tooltip/BaseGenericTooltip/types.ts

View workflow job for this annotation

GitHub Actions / ESLint check 

'Animated' is defined but never used
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Eslint failing @sumo-slonik

import type {SharedTooltipProps} from '@components/Tooltip/types';

Check failure on line 2 in src/components/Tooltip/BaseGenericTooltip/types.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

Replace `type·{SharedTooltipProps}·from·'@components/Tooltip/types';⏎import·{·SharedValue·}·from·"react-native-reanimated"` with `{SharedValue}·from·'react-native-reanimated';⏎import·type·{SharedTooltipProps}·from·'@components/Tooltip/types'`

Check failure on line 2 in src/components/Tooltip/BaseGenericTooltip/types.ts

View workflow job for this annotation

GitHub Actions / ESLint check 

Replace `type·{SharedTooltipProps}·from·'@components/Tooltip/types';⏎import·{·SharedValue·}·from·"react-native-reanimated"` with `{SharedValue}·from·'react-native-reanimated';⏎import·type·{SharedTooltipProps}·from·'@components/Tooltip/types'`
import { SharedValue } from "react-native-reanimated";

Check failure on line 3 in src/components/Tooltip/BaseGenericTooltip/types.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

All imports in the declaration are only used as types. Use `import type`

Check failure on line 3 in src/components/Tooltip/BaseGenericTooltip/types.ts

View workflow job for this annotation

GitHub Actions / ESLint check 

All imports in the declaration are only used as types. Use `import type`

type BaseGenericTooltipProps = {
/** Window width */
windowWidth: number;

/** Tooltip Animation value */
animation: Animated.Value;
animation: SharedValue<number>;

/** The distance between the left side of the wrapper view and the left side of the window */
xOffset: number;
Expand Down
64 changes: 31 additions & 33 deletions src/components/Tooltip/GenericTooltip.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
import React, {memo, useCallback, useEffect, useImperativeHandle, useRef, useState} from 'react';
import React, {memo, useCallback, useEffect, useState} from 'react';
import type {LayoutRectangle} from 'react-native';
import {Animated} from 'react-native';
import {cancelAnimation, runOnJS, useSharedValue, withDelay, withTiming} from 'react-native-reanimated';
import useLocalize from '@hooks/useLocalize';
import usePrevious from '@hooks/usePrevious';
import useWindowDimensions from '@hooks/useWindowDimensions';
import Log from '@libs/Log';
import StringUtils from '@libs/StringUtils';
import TooltipRefManager from '@libs/TooltipRefManager';
import variables from '@styles/variables';
import CONST from '@src/CONST';
import callOrReturn from '@src/types/utils/callOrReturn';
Expand Down Expand Up @@ -60,9 +59,9 @@
const [shouldUseOverlay, setShouldUseOverlay] = useState(shouldUseOverlayProp);

// Whether the tooltip is first tooltip to activate the TooltipSense
const isTooltipSenseInitiator = useRef(false);
const animation = useRef(new Animated.Value(0));
const isAnimationCanceled = useRef(false);
const animationSharedValue = useSharedValue<number>(0);
const isTooltipSenseInitiatorShared = useSharedValue<boolean>(true);
const isAnimationCanceledShared = useSharedValue<boolean>(false);
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 think we don't need to add SharedValue and Shared suffixes to these variables, what do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can do without it

const prevText = usePrevious(text);

useEffect(() => {
Expand All @@ -79,34 +78,40 @@
setIsRendered(true);
setIsVisible(true);

animation.current.stopAnimation();
cancelAnimation(animationSharedValue);

// When TooltipSense is active, immediately show the tooltip
if (TooltipSense.isActive() && !shouldForceAnimate) {
animation.current.setValue(1);
animationSharedValue.value = 1;
} else {
isTooltipSenseInitiator.current = true;
Animated.timing(animation.current, {
toValue: 1,
duration: 140,
delay: 500,
useNativeDriver: false,
}).start(({finished}) => {
isAnimationCanceled.current = !finished;
});
isTooltipSenseInitiatorShared.value = true;
animationSharedValue.value = withDelay(
500,
withTiming(
1,
{
duration: 140,
},
(finished) => {
runOnJS(() => {
isAnimationCanceledShared.value = !finished;
})();
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'm wondering why do we need to run it on JS thread?

Copy link
Contributor

Choose a reason for hiding this comment

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

I will correct it in that case if it is not needed :)

},
),
);
}
TooltipSense.activate();
}, [shouldForceAnimate]);

Check warning on line 104 in src/components/Tooltip/GenericTooltip.tsx

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

React Hook useCallback has missing dependencies: 'animationSharedValue', 'isAnimationCanceledShared', and 'isTooltipSenseInitiatorShared'. Either include them or remove the dependency array

Check warning on line 104 in src/components/Tooltip/GenericTooltip.tsx

View workflow job for this annotation

GitHub Actions / ESLint check 

React Hook useCallback has missing dependencies: 'animationSharedValue', 'isAnimationCanceledShared', and 'isTooltipSenseInitiatorShared'. Either include them or remove the dependency array
Copy link
Contributor Author

Choose a reason for hiding this comment

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

More ESLint warining, you need to add these shared values as dependencies (they won't cause reruns as these are stable references)


// eslint-disable-next-line rulesdir/prefer-early-return
useEffect(() => {
// if the tooltip text changed before the initial animation was finished, then the tooltip won't be shown
// we need to show the tooltip again
if (isVisible && isAnimationCanceled.current && text && prevText !== text) {
isAnimationCanceled.current = false;
if (isVisible && isAnimationCanceledShared.value && text && prevText !== text) {
isAnimationCanceledShared.value = false;
showTooltip();
}
}, [isVisible, text, prevText, showTooltip]);

Check warning on line 114 in src/components/Tooltip/GenericTooltip.tsx

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

React Hook useEffect has a missing dependency: 'isAnimationCanceledShared'. Either include it or remove the dependency array

Check warning on line 114 in src/components/Tooltip/GenericTooltip.tsx

View workflow job for this annotation

GitHub Actions / ESLint check 

React Hook useEffect has a missing dependency: 'isAnimationCanceledShared'. Either include it or remove the dependency array

/**
* Update the tooltip's target bounding rectangle
Expand All @@ -125,24 +130,19 @@
* Hide the tooltip in an animation.
*/
const hideTooltip = useCallback(() => {
animation.current.stopAnimation();
cancelAnimation(animationSharedValue);

if (TooltipSense.isActive() && !isTooltipSenseInitiator.current) {
animation.current.setValue(0);
if (TooltipSense.isActive() && !isTooltipSenseInitiatorShared.value) {
// eslint-disable-next-line react-compiler/react-compiler
animationSharedValue.value = 0;
} else {
// Hide the first tooltip which initiated the TooltipSense with animation
isTooltipSenseInitiator.current = false;
Animated.timing(animation.current, {
toValue: 0,
duration: 140,
useNativeDriver: false,
}).start();
isTooltipSenseInitiatorShared.value = false;
animationSharedValue.value = 0;
}

TooltipSense.deactivate();

setIsVisible(false);
}, []);

Check warning on line 145 in src/components/Tooltip/GenericTooltip.tsx

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

React Hook useCallback has missing dependencies: 'animationSharedValue' and 'isTooltipSenseInitiatorShared'. Either include them or remove the dependency array

Check warning on line 145 in src/components/Tooltip/GenericTooltip.tsx

View workflow job for this annotation

GitHub Actions / ESLint check 

React Hook useCallback has missing dependencies: 'animationSharedValue' and 'isTooltipSenseInitiatorShared'. Either include them or remove the dependency array

const onPressOverlay = useCallback(() => {
if (!shouldUseOverlay) {
Expand All @@ -153,8 +153,6 @@
onHideTooltip();
}, [shouldUseOverlay, onHideTooltip, hideTooltip]);

useImperativeHandle(TooltipRefManager.ref, () => ({hideTooltip}), [hideTooltip]);

// Skip the tooltip and return the children if the text is empty, we don't have a render function.
if (StringUtils.isEmptyString(text) && renderTooltipContent == null) {
// eslint-disable-next-line react-compiler/react-compiler
Expand All @@ -166,7 +164,7 @@
{isRendered && (
<BaseGenericTooltip
// eslint-disable-next-line react-compiler/react-compiler
animation={animation.current}
animation={animationSharedValue}
windowWidth={windowWidth}
xOffset={xOffset}
yOffset={yOffset}
Expand Down
6 changes: 0 additions & 6 deletions src/styles/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3100,7 +3100,6 @@ const styles = (theme: ThemeColors) =>
justifyContent: 'center',
borderRadius: 20,
padding: 15,
backgroundColor: theme.success,
},

switchInactive: {
Expand All @@ -3118,11 +3117,6 @@ const styles = (theme: ThemeColors) =>
backgroundColor: theme.appBG,
},

switchThumbTransformation: (translateX: AnimatableNumericValue) =>
({
transform: [{translateX}],
} satisfies ViewStyle),

radioButtonContainer: {
backgroundColor: theme.componentBG,
borderRadius: 14,
Expand Down
30 changes: 26 additions & 4 deletions src/styles/utils/generators/TooltipStyleUtils/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type {StyleProp, TextStyle, View, ViewStyle} from 'react-native';
import {Animated, StyleSheet} from 'react-native';
import {StyleSheet} from 'react-native';
import type {SharedValue} from 'react-native-reanimated';
import FontUtils from '@styles/utils/FontUtils';
// eslint-disable-next-line no-restricted-imports
import type StyleUtilGenerator from '@styles/utils/generators/types';
Expand Down Expand Up @@ -30,7 +31,7 @@ type TooltipStyles = {

type TooltipParams = {
tooltip: View | HTMLDivElement | null;
currentSize: Animated.Value;
currentSize: number;
windowWidth: number;
xOffset: number;
yOffset: number;
Expand All @@ -47,7 +48,13 @@ type TooltipParams = {
shouldAddHorizontalPadding?: boolean;
};

type GetTooltipStylesStyleUtil = {getTooltipStyles: (props: TooltipParams) => TooltipStyles};
type TooltipAnimationProps = {
tooltipContentWidth?: number;
tooltipWrapperHeight?: number;
currentSize: SharedValue<number>;
};

type GetTooltipStylesStyleUtil = {getTooltipStyles: (props: TooltipParams) => TooltipStyles; getTooltipAnimatedStyles: (props: TooltipAnimationProps) => {transform: [{scale: number}]}};

/**
* Generate styles for the tooltip component.
Expand Down Expand Up @@ -108,7 +115,7 @@ const createTooltipStyleUtils: StyleUtilGenerator<GetTooltipStylesStyleUtil> = (
const isTooltipSizeReady = tooltipWidth !== undefined && tooltipHeight !== undefined;

// Set the scale to 1 to be able to measure the tooltip size correctly when it's not ready yet.
let scale = new Animated.Value(1);
let scale = 1;
let shouldShowBelow = false;
let horizontalShift = 0;
let horizontalShiftPointer = 0;
Expand Down Expand Up @@ -269,6 +276,21 @@ const createTooltipStyleUtils: StyleUtilGenerator<GetTooltipStylesStyleUtil> = (
},
};
},

// Utility function to create and manage scale animations with React Native Reanimated
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's use JSDoc syntax, this way when you hover over functions you get intellisense 😄

Suggested change
// Utility function to create and manage scale animations with React Native Reanimated
/** Utility function to create and manage scale animations with React Native Reanimated */


Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change

getTooltipAnimatedStyles: (props: TooltipAnimationProps) => {
const tooltipHorizontalPadding = spacing.ph2.paddingHorizontal * 2;
const tooltipWidth = props.tooltipContentWidth && props.tooltipContentWidth + tooltipHorizontalPadding + 1;
const isTooltipSizeReady = tooltipWidth !== undefined && props.tooltipWrapperHeight !== undefined;
let scale = 1;
if (isTooltipSizeReady) {
scale = props.currentSize.value;
}
return {
transform: [{scale}],
};
},
});

export default createTooltipStyleUtils;
Loading