Skip to content

Commit

Permalink
Fix Animated.Color callbacks when switching between native colors
Browse files Browse the repository at this point in the history 
Summary:
Animated.Color was never calling `flush`, and thus didn't trigger any animated views to update when the Animated.Color had (a native-only) color change when using the JS driver.

Changelog: [General][Fixed] Improved handling of native colors in Animated.Colors

Reviewed By: mdvacca

Differential Revision: D41519965

fbshipit-source-id: 52f78460cf67ab9260d3868b0d27692b64fc3c58
  • Loading branch information
@javache @facebook-github-bot
javache authored and facebook-github-bot committed Nov 29, 2022
1 parent 1b99473 commit dccb57f
Show file tree
Hide file tree
Showing 5 changed files with 92 additions and 99 deletions.
39 changes: 34 additions & 5 deletions Libraries/Animated/__tests__/Animated-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,10 @@

import * as React from 'react';

const {PlatformColor} = require('../../StyleSheet/PlatformColorValueTypes');
let Animated = require('../Animated').default;
let AnimatedProps = require('../nodes/AnimatedProps').default;
let TestRenderer = require('react-test-renderer');
const AnimatedProps = require('../nodes/AnimatedProps').default;
const TestRenderer = require('react-test-renderer');

jest.mock('../../BatchedBridge/NativeModules', () => ({
NativeAnimatedModule: {},
Expand Down Expand Up @@ -1039,10 +1040,10 @@ describe('Animated tests', () => {
});

node.__attach();
expect(callback.mock.calls.length).toBe(0);
expect(callback).not.toHaveBeenCalled();

color.setValue({r: 11, g: 22, b: 33, a: 0.5});
expect(callback.mock.calls.length).toBe(4);
expect(callback).toHaveBeenCalledTimes(5);
expect(node.__getValue()).toEqual({
style: {
backgroundColor: 'rgba(11, 22, 33, 0.5)',
Expand All @@ -1052,7 +1053,7 @@ describe('Animated tests', () => {

node.__detach();
color.setValue({r: 255, g: 0, b: 0, a: 1.0});
expect(callback.mock.calls.length).toBe(4);
expect(callback).toHaveBeenCalledTimes(5);
});

it('should track colors', () => {
Expand Down Expand Up @@ -1088,5 +1089,33 @@ describe('Animated tests', () => {
jest.runAllTimers();
expect(color2.__getValue()).toEqual('rgba(44, 55, 66, 0)');
});

it('should provide updates for native colors', () => {
const color = new Animated.Color('red');

const listener = jest.fn();
color.addListener(listener);

const callback = jest.fn(() => {
console.log('callback', color.__getValue());
});
const node = new AnimatedProps({style: {color}}, callback);
node.__attach();

color.setValue('blue');
expect(callback).toHaveBeenCalledTimes(5);
expect(listener).toHaveBeenCalledTimes(1);
expect(listener).toHaveBeenCalledWith({value: 'rgba(0, 0, 255, 1)'});
expect(color.__getValue()).toEqual('rgba(0, 0, 255, 1)');

callback.mockClear();
listener.mockClear();

color.setValue(PlatformColor('bar'));
expect(callback).toHaveBeenCalledTimes(1);
expect(listener).toHaveBeenCalledTimes(1);
expect(listener).toHaveBeenCalledWith({value: PlatformColor('bar')});
expect(color.__getValue()).toEqual(PlatformColor('bar'));
});
});
});
127 changes: 47 additions & 80 deletions Libraries/Animated/nodes/AnimatedColor.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import type {PlatformConfig} from '../AnimatedPlatformConfig';
import normalizeColor from '../../StyleSheet/normalizeColor';
import {processColorObject} from '../../StyleSheet/PlatformColorValueTypes';
import NativeAnimatedHelper from '../NativeAnimatedHelper';
import AnimatedValue from './AnimatedValue';
import AnimatedValue, {flushValue} from './AnimatedValue';
import AnimatedWithChildren from './AnimatedWithChildren';

export type AnimatedColorConfig = $ReadOnly<{
Expand All @@ -43,10 +43,11 @@ type RgbaAnimatedValue = {
...
};

export type InputValue = ?(RgbaValue | RgbaAnimatedValue | ColorValue);

const NativeAnimatedAPI = NativeAnimatedHelper.API;

const defaultColor: RgbaValue = {r: 0, g: 0, b: 0, a: 1.0};
let _uniqueId = 1;

/* eslint no-bitwise: 0 */
function processColor(
Expand Down Expand Up @@ -113,22 +114,12 @@ export default class AnimatedColor extends AnimatedWithChildren {
b: AnimatedValue;
a: AnimatedValue;
nativeColor: ?NativeColorValue;
_listeners: {
[key: string]: {
r: string,
g: string,
b: string,
a: string,
...
},
...
} = {};

constructor(
valueIn?: ?(RgbaValue | RgbaAnimatedValue | ColorValue),
config?: ?AnimatedColorConfig,
) {

_suspendCallbacks: number = 0;

constructor(valueIn?: InputValue, config?: ?AnimatedColorConfig) {
super();

let value: RgbaValue | RgbaAnimatedValue | ColorValue =
valueIn ?? defaultColor;
if (isRgbaAnimatedValue(value)) {
Expand Down Expand Up @@ -156,7 +147,8 @@ export default class AnimatedColor extends AnimatedWithChildren {
this.b = new AnimatedValue(initColor.b);
this.a = new AnimatedValue(initColor.a);
}
if (this.nativeColor || (config && config.useNativeDriver)) {

if (config?.useNativeDriver) {
this.__makeNative();
}
}
Expand All @@ -174,25 +166,27 @@ export default class AnimatedColor extends AnimatedWithChildren {

const processedColor: RgbaValue | NativeColorValue =
processColor(value) ?? defaultColor;
if (isRgbaValue(processedColor)) {
// $FlowIgnore[incompatible-type] - Type is verified above
const rgbaValue: RgbaValue = processedColor;
this.r.setValue(rgbaValue.r);
this.g.setValue(rgbaValue.g);
this.b.setValue(rgbaValue.b);
this.a.setValue(rgbaValue.a);
if (this.nativeColor != null) {
this.nativeColor = null;
shouldUpdateNodeConfig = true;
}
} else {
// $FlowIgnore[incompatible-type] - Type is verified above
const nativeColor: NativeColorValue = processedColor;
if (this.nativeColor !== nativeColor) {
this.nativeColor = nativeColor;
shouldUpdateNodeConfig = true;
this._withSuspendedCallbacks(() => {
if (isRgbaValue(processedColor)) {
// $FlowIgnore[incompatible-type] - Type is verified above
const rgbaValue: RgbaValue = processedColor;
this.r.setValue(rgbaValue.r);
this.g.setValue(rgbaValue.g);
this.b.setValue(rgbaValue.b);
this.a.setValue(rgbaValue.a);
if (this.nativeColor != null) {
this.nativeColor = null;
shouldUpdateNodeConfig = true;
}
} else {
// $FlowIgnore[incompatible-type] - Type is verified above
const nativeColor: NativeColorValue = processedColor;
if (this.nativeColor !== nativeColor) {
this.nativeColor = nativeColor;
shouldUpdateNodeConfig = true;
}
}
}
});

if (this.__isNative) {
const nativeTag = this.__getNativeTag();
Expand All @@ -203,7 +197,12 @@ export default class AnimatedColor extends AnimatedWithChildren {
);
}
NativeAnimatedAPI.unsetWaitingForIdentifier(nativeTag.toString());
} else {
flushValue(this);
}

// $FlowFixMe[incompatible-call]
this.__callListeners(this.__getValue());
}

/**
Expand Down Expand Up @@ -240,50 +239,6 @@ export default class AnimatedColor extends AnimatedWithChildren {
this.a.extractOffset();
}

/**
* Adds an asynchronous listener to the value so you can observe updates from
* animations. This is useful because there is no way to synchronously read
* the value because it might be driven natively.
*
* Returns a string that serves as an identifier for the listener.
*/
addListener(callback: ColorListenerCallback): string {
const id = String(_uniqueId++);
const jointCallback = ({value: number}: any) => {
callback(this.__getValue());
};
this._listeners[id] = {
r: this.r.addListener(jointCallback),
g: this.g.addListener(jointCallback),
b: this.b.addListener(jointCallback),
a: this.a.addListener(jointCallback),
};
return id;
}

/**
* Unregister a listener. The `id` param shall match the identifier
* previously returned by `addListener()`.
*/
removeListener(id: string): void {
this.r.removeListener(this._listeners[id].r);
this.g.removeListener(this._listeners[id].g);
this.b.removeListener(this._listeners[id].b);
this.a.removeListener(this._listeners[id].a);
delete this._listeners[id];
}

/**
* Remove all registered listeners.
*/
removeAllListeners(): void {
this.r.removeAllListeners();
this.g.removeAllListeners();
this.b.removeAllListeners();
this.a.removeAllListeners();
this._listeners = {};
}

/**
* Stops any running animation or tracking. `callback` is invoked with the
* final value after stopping the animation, which is useful for updating
Expand Down Expand Up @@ -332,6 +287,18 @@ export default class AnimatedColor extends AnimatedWithChildren {
super.__detach();
}

_withSuspendedCallbacks(callback: () => void) {
this._suspendCallbacks++;
callback();
this._suspendCallbacks--;
}

__callListeners(value: number): void {
if (this._suspendCallbacks === 0) {
super.__callListeners(value);
}
}

__makeNative(platformConfig: ?PlatformConfig) {
this.r.__makeNative(platformConfig);
this.g.__makeNative(platformConfig);
Expand Down
3 changes: 2 additions & 1 deletion Libraries/Animated/nodes/AnimatedNode.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export default class AnimatedNode {
}
__addChild(child: AnimatedNode) {}
__removeChild(child: AnimatedNode) {}
__getChildren(): Array<AnimatedNode> {
__getChildren(): $ReadOnlyArray<AnimatedNode> {
return [];
}

Expand Down Expand Up @@ -184,6 +184,7 @@ export default class AnimatedNode {
'This JS animated node type cannot be used as native animated node',
);
}

toJSON(): any {
return this.__getValue();
}
Expand Down
20 changes: 8 additions & 12 deletions Libraries/Animated/nodes/AnimatedValue.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,21 +48,18 @@ const NativeAnimatedAPI = NativeAnimatedHelper.API;
* this two-phases process is to deal with composite props such as
* transform which can receive values from multiple parents.
*/
function _flush(rootNode: AnimatedValue): void {
const animatedStyles = new Set<AnimatedValue | AnimatedNode>();
function findAnimatedStyles(node: AnimatedValue | AnimatedNode) {
/* $FlowFixMe[prop-missing] (>=0.68.0 site=react_native_fb) This comment
* suppresses an error found when Flow v0.68 was deployed. To see the error
* delete this comment and run Flow. */
export function flushValue(rootNode: AnimatedNode): void {
const leaves = new Set<{update: () => void, ...}>();
function findAnimatedStyles(node: AnimatedNode) {
// $FlowFixMe[prop-missing]
if (typeof node.update === 'function') {
animatedStyles.add(node);
leaves.add((node: any));
} else {
node.__getChildren().forEach(findAnimatedStyles);
}
}
findAnimatedStyles(rootNode);
// $FlowFixMe[prop-missing]
animatedStyles.forEach(animatedStyle => animatedStyle.update());
leaves.forEach(leaf => leaf.update());
}

/**
Expand Down Expand Up @@ -91,7 +88,6 @@ export default class AnimatedValue extends AnimatedWithChildren {
_animation: ?Animation;
_tracking: ?AnimatedTracking;

// $FlowFixMe[missing-local-annot]
constructor(value: number, config?: ?AnimatedValueConfig) {
super();
if (typeof value !== 'number') {
Expand Down Expand Up @@ -291,9 +287,9 @@ export default class AnimatedValue extends AnimatedWithChildren {

this._value = value;
if (flush) {
_flush(this);
flushValue(this);
}
super.__callListeners(this.__getValue());
this.__callListeners(this.__getValue());
}

__getNativeConfig(): Object {
Expand Down
2 changes: 1 addition & 1 deletion Libraries/Animated/nodes/AnimatedWithChildren.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ export default class AnimatedWithChildren extends AnimatedNode {
}
}

__getChildren(): Array<AnimatedNode> {
__getChildren(): $ReadOnlyArray<AnimatedNode> {
return this._children;
}

Expand Down

0 comments on commit dccb57f

Please sign in to comment.