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

feat: Add support for boxShadow #6749

Open
wants to merge 31 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
6f85a87
add support for boxShadow
patrycjakalinska Nov 22, 2024
2954f56
PR review changes
patrycjakalinska Nov 22, 2024
c8d4615
add named import, ups
patrycjakalinska Nov 22, 2024
481c3ff
Merge branch 'main' into @patrycjakalinska/support-box-shadow
patrycjakalinska Nov 25, 2024
fd9d5da
fixes
patrycjakalinska Nov 25, 2024
a68aade
more fixes
patrycjakalinska Nov 25, 2024
96296d8
fix animation not animating bug
patrycjakalinska Dec 12, 2024
3deb7bc
add unit tests for boxShadow
patrycjakalinska Dec 12, 2024
e3b5f76
change processBoxShadow to work in place
patrycjakalinska Dec 12, 2024
dec4ba2
fix colors flickering when using withSpring
patrycjakalinska Dec 12, 2024
1e6d049
add type for object
patrycjakalinska Dec 12, 2024
244e2c0
lint fixes
patrycjakalinska Dec 12, 2024
6f61bb9
add init runtime tests
patrycjakalinska Dec 12, 2024
cba386f
add static runtime test
patrycjakalinska Dec 12, 2024
64a9e7a
add todo to boxShadow runtime test - as it is a newArch prop, the box…
patrycjakalinska Dec 12, 2024
6d05428
small fix for runtime test
patrycjakalinska Dec 12, 2024
5fa603a
Add ViewStyle type
patrycjakalinska Dec 12, 2024
18259dc
smol fix
patrycjakalinska Dec 12, 2024
af7a519
remove misleading comment
patrycjakalinska Dec 13, 2024
9bace78
Add comment explaining behaviour of spreading an animation object
patrycjakalinska Dec 13, 2024
e43a655
move clampRGBA to separate PR
patrycjakalinska Dec 16, 2024
af634fc
Merge branch 'main' into @patrycjakalinska/support-box-shadow
patrycjakalinska Dec 16, 2024
deedd17
replace comment with skip directive
patrycjakalinska Dec 19, 2024
346a461
replace array with prop
patrycjakalinska Dec 19, 2024
6bc6fec
Add TODO: to enable test when implemented on Fabric
patrycjakalinska Dec 19, 2024
456a2e3
typescript adjustments
patrycjakalinska Dec 20, 2024
3f7c69e
Merge branch 'main' into @patrycjakalinska/support-box-shadow
patrycjakalinska Dec 20, 2024
9f8bd41
Add comment with explaination
patrycjakalinska Dec 20, 2024
b34a22d
add boxShadow to test comparators
patrycjakalinska Dec 20, 2024
2ab6687
Merge branch 'main' into @patrycjakalinska/support-box-shadow
patrycjakalinska Dec 20, 2024
6b3cfd3
Change approach of color process in NestedProps
patrycjakalinska Jan 7, 2025
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
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ export function getComparisonModeForProp(prop: ValidPropNames): ComparisonMode {
top: ComparisonMode.PIXEL,
left: ComparisonMode.PIXEL,
backgroundColor: ComparisonMode.COLOR,
boxShadow: ComparisonMode.ARRAY,
};
return propToComparisonModeDict[prop];
}
12 changes: 10 additions & 2 deletions apps/common-app/src/examples/RuntimeTests/ReJest/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,19 @@ export type TestSuite = {
decorator?: DescribeDecorator | null;
};

export type ValidPropNames = 'zIndex' | 'opacity' | 'width' | 'height' | 'top' | 'left' | 'backgroundColor';
export type ValidPropNames =
| 'zIndex'
| 'opacity'
| 'width'
| 'height'
| 'top'
| 'left'
| 'backgroundColor'
| 'boxShadow';

export function isValidPropName(propName: string): propName is ValidPropNames {
'worklet';
return ['zIndex', 'opacity', 'width', 'height', 'top', 'left', 'backgroundColor'].includes(propName);
return ['zIndex', 'opacity', 'width', 'height', 'top', 'left', 'backgroundColor', 'boxShadow'].includes(propName);
}

export enum ComparisonMode {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,12 @@ export default function RuntimeTestsExample() {
require('./tests/core/useSharedValue/animationsCompilerApi.test');
},
},
{
testSuiteName: 'props',
importTest: () => {
require('./tests/props/boxShadow.test');
},
},
{
testSuiteName: 'utilities',
importTest: () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
// TODO: Enable this test after RuntimeTests are implemented on Fabric

import { useEffect } from 'react';
import type { BoxShadowValue } from 'react-native';
import type { AnimatableValue } from 'react-native-reanimated';
import type { DefaultStyle } from 'react-native-reanimated/lib/typescript/hook/commonTypes';
import { ComparisonMode } from '../../ReJest/types';
import { View, StyleSheet } from 'react-native';
import Animated, { useSharedValue, withSpring, useAnimatedStyle } from 'react-native-reanimated';
import { describe, test, expect, render, useTestRef, getTestComponent, wait } from '../../ReJest/RuntimeTestsApi';

describe.skip('animation of BoxShadow', () => {
enum Component {
ACTIVE = 'ACTIVE',
PASSIVE = 'PASSIVE',
}
function BoxShadowComponent({
startBoxShadow,
finalBoxShadow,
}: {
startBoxShadow: BoxShadowValue;
finalBoxShadow: BoxShadowValue;
}) {
const boxShadowActiveSV = useSharedValue(startBoxShadow);
const boxShadowPassiveSV = useSharedValue(startBoxShadow);

const refActive = useTestRef('ACTIVE');
const refPassive = useTestRef('PASSIVE');

const styleActive = useAnimatedStyle(() => {
return {
boxShadow: [withSpring(boxShadowActiveSV.value as unknown as AnimatableValue, { duration: 700 })],
} as DefaultStyle;
});

const stylePassive = useAnimatedStyle(() => {
return {
boxShadow: [boxShadowPassiveSV.value],
} as DefaultStyle;
});

useEffect(() => {
const timeout = setTimeout(() => {
boxShadowActiveSV.value = finalBoxShadow;
boxShadowPassiveSV.value = finalBoxShadow;
}, 1000);

return () => clearTimeout(timeout);
}, [finalBoxShadow, boxShadowActiveSV, boxShadowPassiveSV]);

return (
<View style={styles.container}>
<Animated.View ref={refActive} style={[styles.animatedBox, styleActive]} />
<Animated.View ref={refPassive} style={[styles.animatedBox, stylePassive]} />
</View>
);
}

test.each([
{
startBoxShadow: {
offsetX: -10,
offsetY: 6,
blurRadius: 7,
spreadDistance: 10,
color: 'rgba(245, 40, 145, 0.8)',
},

finalBoxShadow: {
offsetX: -20,
offsetY: 4,
blurRadius: 10,
spreadDistance: 20,
color: 'rgba(39, 185, 245, 0.8)',
},

description: 'one boxShadow',
},
])(
'${description}, from ${startBoxShadow} to ${finalBoxShadow}',
async ({ startBoxShadow, finalBoxShadow }: { startBoxShadow: BoxShadowValue; finalBoxShadow: BoxShadowValue }) => {
await render(<BoxShadowComponent startBoxShadow={startBoxShadow} finalBoxShadow={finalBoxShadow} />);

const activeComponent = getTestComponent(Component.ACTIVE);
const passiveComponent = getTestComponent(Component.PASSIVE);

await wait(200);

expect(await activeComponent.getAnimatedStyle('boxShadow')).toBe([finalBoxShadow], ComparisonMode.ARRAY);
expect(await passiveComponent.getAnimatedStyle('boxShadow')).toBe([finalBoxShadow], ComparisonMode.ARRAY);
},
);
});

const styles = StyleSheet.create({
container: {
flex: 1,
justifyContent: 'center',
alignItems: 'center',
},
animatedBox: {
backgroundColor: 'palevioletred',
width: 100,
height: 100,
margin: 30,
},
});
158 changes: 158 additions & 0 deletions packages/react-native-reanimated/__tests__/props.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
import { View, Pressable, Text } from 'react-native';
import type { ViewStyle } from 'react-native';
import { render, fireEvent } from '@testing-library/react-native';
import Animated, {
interpolate,
interpolateColor,
useSharedValue,
useAnimatedStyle,
} from '../src';
import { getAnimatedStyle } from '../src/jestUtils';
import { processBoxShadow } from '../src/processBoxShadow';

const AnimatedPressable = Animated.createAnimatedComponent(Pressable);

const AnimatedComponent = () => {
const pressed = useSharedValue(0);

const animatedBoxShadow = useAnimatedStyle(() => {
const blurRadius = interpolate(pressed.value, [0, 1], [10, 0]);
const color = interpolateColor(
pressed.value,
[0, 1],
['rgba(255, 0, 0, 1)', 'rgba(0, 0, 255, 1)']
);

const boxShadow = `0px 4px ${blurRadius}px 0px ${color}`;

return {
boxShadow,
};
});

const handlePress = () => {
pressed.value = pressed.value === 0 ? 1 : 0;
};

return (
<View
style={{
padding: 24,
}}>
<AnimatedPressable
testID={'pressable'}
style={[
animatedBoxShadow,
{
backgroundColor: 'red',
padding: 16,
boxShadow: '0px 4px 10px 0px rgba(255, 0, 0, 1)',
},
]}
onPress={handlePress}>
<Text>Button</Text>
</AnimatedPressable>
</View>
);
};

const getDefaultStyle = () => ({
padding: 16,
backgroundColor: 'red',
boxShadow: '0px 4px 10px 0px rgba(255, 0, 0, 1)',
});

const getMultipleBoxShadowStyle = () => ({
padding: 16,
backgroundColor: 'red',
boxShadow:
'-10px 6px 8px 10px rgba(255, 0, 0, 1), 10px 0px 15px 6px rgba(0, 0, 255, 1)',
});

describe('Test of boxShadow prop', () => {
beforeEach(() => {
jest.useFakeTimers();
});

afterEach(() => {
jest.runOnlyPendingTimers();
jest.useRealTimers();
});

test('boxShadow prop animation', () => {
const style = getDefaultStyle();

const { getByTestId } = render(<AnimatedComponent />);
const pressable = getByTestId('pressable');

expect(pressable.props.style.boxShadow).toBe(
'0px 4px 10px 0px rgba(255, 0, 0, 1)'
);
expect(pressable).toHaveAnimatedStyle(style);
fireEvent.press(pressable);
jest.advanceTimersByTime(600);
style.boxShadow = '0px 4px 0px 0px rgba(0, 0, 255, 1)';
expect(pressable).toHaveAnimatedStyle(style);
});

test('boxShadow prop animation, get animated style', () => {
const { getByTestId } = render(<AnimatedComponent />);
const pressable = getByTestId('pressable');

fireEvent.press(pressable);
jest.advanceTimersByTime(600);

const style = getAnimatedStyle(pressable);
expect((style as ViewStyle).boxShadow).toBe(
'0px 4px 0px 0px rgba(0, 0, 255, 1)'
);
});
test('one boxShadow string parsing', () => {
const { getByTestId } = render(<AnimatedComponent />);
const pressable = getByTestId('pressable');

expect(pressable.props.style.boxShadow).toBe(
'0px 4px 10px 0px rgba(255, 0, 0, 1)'
);

processBoxShadow(pressable.props.style);

expect(pressable.props.style.boxShadow).toEqual([
{
offsetX: 0,
offsetY: 4,
blurRadius: 10,
spreadDistance: 0,
color: 'rgba(255, 0, 0, 1)',
},
]);

const style = getAnimatedStyle(pressable);
expect((style as ViewStyle).boxShadow).toBe(
'0px 4px 10px 0px rgba(255, 0, 0, 1)'
);
});

test('two boxShadows string parsing', () => {
const multipleBoxShadowStyle = getMultipleBoxShadowStyle();

processBoxShadow(multipleBoxShadowStyle);

expect(multipleBoxShadowStyle.boxShadow).toEqual([
{
offsetX: -10,
offsetY: 6,
blurRadius: 8,
spreadDistance: 10,
color: 'rgba(255, 0, 0, 1)',
},
{
offsetX: 10,
offsetY: 0,
blurRadius: 15,
spreadDistance: 6,
color: 'rgba(0, 0, 255, 1)',
},
]);
});
});
17 changes: 17 additions & 0 deletions packages/react-native-reanimated/src/Colors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,10 @@ export const ColorProperties = makeShareable([
'stroke',
]);

const NestedColorProperties = makeShareable({
boxShadow: 'color',
});

// // ts-prune-ignore-next Exported for the purpose of tests only
export function normalizeColor(color: unknown): number | null {
'worklet';
Expand Down Expand Up @@ -675,6 +679,19 @@ export function processColorsInProps(props: StyleProps) {
for (const key in props) {
if (ColorProperties.includes(key)) {
props[key] = processColor(props[key]);
} else if (
NestedColorProperties[key as keyof typeof NestedColorProperties]
) {
const propGroupList = props[key] as StyleProps[];
for (const propGroup of propGroupList) {
const nestedPropertyName =
NestedColorProperties[key as keyof typeof NestedColorProperties];
if (propGroup[nestedPropertyName] !== undefined) {
propGroup[nestedPropertyName] = processColor(
propGroup[nestedPropertyName]
);
}
}
}
}
}
Expand Down
31 changes: 29 additions & 2 deletions packages/react-native-reanimated/src/hook/useAnimatedStyle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import type {
AnimatedStyle,
} from '../commonTypes';
import { isWorkletFunction } from '../commonTypes';
import { processBoxShadow } from '../processBoxShadow';
import { ReanimatedError } from '../errors';

const SHOULD_BE_USE_WEB = shouldBeUseWeb();
Expand Down Expand Up @@ -150,7 +151,16 @@ function runAnimations(
animation.callback && animation.callback(true /* finished */);
}
}
result[key] = animation.current;
/*
* If `animation.current` is an object, spread its properties into a new object
* to avoid modifying the original reference. This ensures when `newValues` has a nested color prop, it stays unparsed
* in rgba format, allowing the animation to run correctly.
*/
if (typeof animation.current === 'object') {
result[key] = { ...animation.current };
Copy link
Member

Choose a reason for hiding this comment

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

Do we also need to copy other objects from the style, such as transforms? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this approach we also copy transform yes, but it doesn't affect it - it only makes sure any nested the color property will work

Copy link
Member

Choose a reason for hiding this comment

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

Maybe to avoid potential performance degradation, let's check if this property is specifically a box-shadow. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this line the original key value is 0 rather that boxShadow, so we cannot use it. It is because we are doing:

    animation.forEach((entry, index) => {
      if (
        !runAnimations(
          entry,
          timestamp,
          index, // key
          result[key],
          animationsActive
        )
      ) {

I would suggest saving initial key to currentKey prop, and passing it. It supports your idea of avoiding performance degradation as well as it's not invasive.

function runAnimations(
  animation: AnimatedStyle<any>,
  timestamp: Timestamp,
  key: number | string,
  result: AnimatedStyle<any>,
  animationsActive: SharedValue<boolean>,
  originalKey?: string
): boolean {
 // ...
  const currentKey = originalKey || (key as string);
  if (Array.isArray(animation)) {
 // ...
    animation.forEach((entry, index) => {
      if (
        !runAnimations(
          entry,
          timestamp,
          index,
          result[key],
          animationsActive,
          currentKey
        )
      ) {
      // ...
      }
  } else if (typeof animation === 'object' && animation.onFrame) {
   // ....
    /*
     * If `animation.current` is an object, spread its properties into a new object
     * to avoid modifying the original reference. This ensures when `newValues` has a nested color prop, it stays unparsed
     * in rgba format, allowing the animation to run correctly.
     */
    if (currentKey === 'boxShadow') {
      result[key] = { ...animation.current };
    } else {
      result[key] = animation.current;
    }
    return finished;

by default, on the first run, runAnimations function doesn't have originalKey so currentKey becomes key. But when we go deeper i.ex. run animation for each item of the array (like we do in boxShadow case), the currentKey stays as boxShadow (or any other propName) and lets us determine if we want to spread the animation.current

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 can push this changes so you can see first hand

} else {
result[key] = animation.current;
}
tjzel marked this conversation as resolved.
Show resolved Hide resolved
return finished;
} else if (typeof animation === 'object') {
result[key] = {};
Expand Down Expand Up @@ -191,6 +201,9 @@ function styleUpdater(
let hasAnimations = false;
let frameTimestamp: number | undefined;
let hasNonAnimatedValues = false;
if (typeof newValues.boxShadow === 'string') {
processBoxShadow(newValues);
}
for (const key in newValues) {
const value = newValues[key];
if (isAnimated(value)) {
Expand Down Expand Up @@ -226,7 +239,21 @@ function styleUpdater(
animationsActive
);
if (finished) {
last[propName] = updates[propName];
/**
* If the animated prop is an array, we need to directly set each
* property (manually spread it). This prevents issues where the color
* prop might be incorrectly linked with its `toValue` and `current`
* states, causing abrupt transitions or 'jumps' in animation states.
*/
if (Array.isArray(updates[propName])) {
updates[propName].forEach((obj: StyleProps) => {
for (const prop in obj) {
last[propName][prop] = obj[prop];
}
});
} else {
Comment on lines +248 to +254
Copy link
Member

Choose a reason for hiding this comment

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

  1. When updates[propName] is an array?
  2. What about spreed operator?
  3. We also need a proper commentary, as this change is not obvious :/
  4. Is any change to avoid that copying here? - just wondering 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. transform and boxShadow
  2. spread operator removed problem of reference, but only copying prop to prop assured the independence of toValue and current (in other words it made sure the animation wouldn't jump)
  3. on it!
  4. in a meaning to avoid copying prop to prop? I tried some other approaches but only this guaranteed smooth animation

last[propName] = updates[propName];
}
tjzel marked this conversation as resolved.
Show resolved Hide resolved
delete animations[propName];
} else {
allFinished = false;
Expand Down
Loading
Loading