Skip to content

Commit

Permalink
Native Animated - Support multiple events attached to the same prop
Browse files Browse the repository at this point in the history
Summary:
Re-applying the diff that was reverted in D4659669 / facebook@b87f4ab because of some crashes with fixes from D4659708 merged in.

 ---

Fixes a bug that happens when trying to use ScrollView with sticky headers and native `Animated.event` with `onScroll`. Made a few changes to the ListViewPaging UIExplorer example to repro https://gist.github.com/janicduplessis/17e2fcd99c6ea49ced2954d881011b09.

What happens is we need to be able to add multiple events to the same prop + viewTag pair. To do that I simple changed the data structure to `Map<prop+viewTag, List<AnimatedEventDriver>>` and try to optimize for the case where there is only one item in the list since it will be the case 99% of the time.

**Test plan**
Tested by reproducing the bug with the above gist and made sure it was fixed after applying this diff.
Closes facebook#12697

Reviewed By: fkgozali

Differential Revision: D4661105

Pulled By: sahrens

fbshipit-source-id: c719dc85f45c1a142ef5b9ebfe0a82ae8ec66497
  • Loading branch information
janicduplessis authored and facebook-github-bot committed Mar 9, 2017
1 parent bfb2766 commit 921b9ac
Show file tree
Hide file tree
Showing 7 changed files with 98 additions and 38 deletions.
8 changes: 7 additions & 1 deletion Libraries/Animated/src/AnimatedImplementation.js
Original file line number Diff line number Diff line change
Expand Up @@ -2187,7 +2187,13 @@ function attachNativeEvent(viewRef: any, eventName: string, argMapping: Array<?M
return {
detach() {
NativeAnimatedAPI.removeAnimatedEventFromView(viewTag, eventName);
eventMappings.forEach((mapping) => {
NativeAnimatedAPI.removeAnimatedEventFromView(
viewTag,
eventName,
mapping.animatedValueTag,
);
});
},
};
}
Expand Down
4 changes: 2 additions & 2 deletions Libraries/Animated/src/NativeAnimatedHelper.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,9 @@ const API = {
assertNativeAnimatedModule();
NativeAnimatedModule.addAnimatedEventToView(viewTag, eventName, eventMapping);
},
removeAnimatedEventFromView(viewTag: ?number, eventName: string) {
removeAnimatedEventFromView(viewTag: ?number, eventName: string, animatedNodeTag: ?number) {
assertNativeAnimatedModule();
NativeAnimatedModule.removeAnimatedEventFromView(viewTag, eventName);
NativeAnimatedModule.removeAnimatedEventFromView(viewTag, eventName, animatedNodeTag);
}
};

Expand Down
5 changes: 3 additions & 2 deletions Libraries/NativeAnimation/RCTNativeAnimatedModule.m
Original file line number Diff line number Diff line change
Expand Up @@ -168,10 +168,11 @@ - (void)setBridge:(RCTBridge *)bridge
}

RCT_EXPORT_METHOD(removeAnimatedEventFromView:(nonnull NSNumber *)viewTag
eventName:(nonnull NSString *)eventName)
eventName:(nonnull NSString *)eventName
animatedNodeTag:(nonnull NSNumber *)animatedNodeTag)
{
[_operations addObject:^(RCTNativeAnimatedNodesManager *nodesManager) {
[nodesManager removeAnimatedEventFromView:viewTag eventName:eventName];
[nodesManager removeAnimatedEventFromView:viewTag eventName:eventName animatedNodeTag:animatedNodeTag];
}];
}

Expand Down
5 changes: 3 additions & 2 deletions Libraries/NativeAnimation/RCTNativeAnimatedNodesManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@

#import <Foundation/Foundation.h>

#import <React/RCTUIManager.h>
#import <React/RCTBridgeModule.h>
#import <React/RCTUIManager.h>

#import "RCTValueAnimatedNode.h"

Expand Down Expand Up @@ -70,7 +70,8 @@
eventMapping:(NSDictionary<NSString *, id> *__nonnull)eventMapping;

- (void)removeAnimatedEventFromView:(nonnull NSNumber *)viewTag
eventName:(nonnull NSString *)eventName;
eventName:(nonnull NSString *)eventName
animatedNodeTag:(nonnull NSNumber *)animatedNodeTag;

- (void)handleAnimatedEvent:(nonnull id<RCTEvent>)event;

Expand Down
60 changes: 42 additions & 18 deletions Libraries/NativeAnimation/RCTNativeAnimatedNodesManager.m
Original file line number Diff line number Diff line change
Expand Up @@ -11,29 +11,29 @@

#import <React/RCTConvert.h>

#import "RCTAdditionAnimatedNode.h"
#import "RCTAnimatedNode.h"
#import "RCTAnimationDriver.h"
#import "RCTEventAnimation.h"

#import "RCTAdditionAnimatedNode.h"
#import "RCTInterpolationAnimatedNode.h"
#import "RCTDiffClampAnimatedNode.h"
#import "RCTDivisionAnimatedNode.h"
#import "RCTEventAnimation.h"
#import "RCTFrameAnimation.h"
#import "RCTInterpolationAnimatedNode.h"
#import "RCTModuloAnimatedNode.h"
#import "RCTMultiplicationAnimatedNode.h"
#import "RCTModuloAnimatedNode.h"
#import "RCTPropsAnimatedNode.h"
#import "RCTSpringAnimation.h"
#import "RCTStyleAnimatedNode.h"
#import "RCTTransformAnimatedNode.h"
#import "RCTValueAnimatedNode.h"
#import "RCTFrameAnimation.h"
#import "RCTSpringAnimation.h"

@implementation RCTNativeAnimatedNodesManager
{
RCTUIManager *_uiManager;
NSMutableDictionary<NSNumber *, RCTAnimatedNode *> *_animationNodes;
NSMutableDictionary<NSString *, RCTEventAnimation *> *_eventDrivers;
// Mapping of a view tag and an event name to a list of event animation drivers. 99% of the time
// there will be only one driver per mapping so all code code should be optimized around that.
NSMutableDictionary<NSString *, NSMutableArray<RCTEventAnimation *> *> *_eventDrivers;
NSMutableSet<id<RCTAnimationDriver>> *_activeAnimations;
CADisplayLink *_displayLink;
}
Expand Down Expand Up @@ -207,7 +207,7 @@ - (void)startAnimatingNode:(nonnull NSNumber *)animationId
RCTValueAnimatedNode *valueNode = (RCTValueAnimatedNode *)_animationNodes[nodeTag];

NSString *type = config[@"type"];
id<RCTAnimationDriver>animationDriver;
id<RCTAnimationDriver> animationDriver;

if ([type isEqual:@"frames"]) {
animationDriver = [[RCTFrameAnimation alloc] initWithId:animationId
Expand All @@ -233,7 +233,7 @@ - (void)startAnimatingNode:(nonnull NSNumber *)animationId

- (void)stopAnimation:(nonnull NSNumber *)animationId
{
for (id<RCTAnimationDriver>driver in _activeAnimations) {
for (id<RCTAnimationDriver> driver in _activeAnimations) {
if ([driver.animationId isEqual:animationId]) {
[driver removeAnimation];
[_activeAnimations removeObject:driver];
Expand Down Expand Up @@ -264,15 +264,36 @@ - (void)addAnimatedEventToView:(nonnull NSNumber *)viewTag
NSArray<NSString *> *eventPath = [RCTConvert NSStringArray:eventMapping[@"nativeEventPath"]];

RCTEventAnimation *driver =
[[RCTEventAnimation alloc] initWithEventPath:eventPath valueNode:(RCTValueAnimatedNode *)node];
[[RCTEventAnimation alloc] initWithEventPath:eventPath valueNode:(RCTValueAnimatedNode *)node];

_eventDrivers[[NSString stringWithFormat:@"%@%@", viewTag, eventName]] = driver;
NSString *key = [NSString stringWithFormat:@"%@%@", viewTag, eventName];
if (_eventDrivers[key] != nil) {
[_eventDrivers[key] addObject:driver];
} else {
NSMutableArray<RCTEventAnimation *> *drivers = [NSMutableArray new];
[drivers addObject:driver];
_eventDrivers[key] = drivers;
}
}

- (void)removeAnimatedEventFromView:(nonnull NSNumber *)viewTag
eventName:(nonnull NSString *)eventName
animatedNodeTag:(nonnull NSNumber *)animatedNodeTag
{
[_eventDrivers removeObjectForKey:[NSString stringWithFormat:@"%@%@", viewTag, eventName]];
NSString *key = [NSString stringWithFormat:@"%@%@", viewTag, eventName];
if (_eventDrivers[key] != nil) {
if (_eventDrivers[key].count == 1) {
[_eventDrivers removeObjectForKey:key];
} else {
NSMutableArray<RCTEventAnimation *> *driversForKey = _eventDrivers[key];
for (NSUInteger i = 0; i < driversForKey.count; i++) {
if (driversForKey[i].valueNode.nodeTag == animatedNodeTag) {
[driversForKey removeObjectAtIndex:i];
break;
}
}
}
}
}

- (void)handleAnimatedEvent:(id<RCTEvent>)event
Expand All @@ -282,9 +303,12 @@ - (void)handleAnimatedEvent:(id<RCTEvent>)event
}

NSString *key = [NSString stringWithFormat:@"%@%@", event.viewTag, event.eventName];
RCTEventAnimation *driver = _eventDrivers[key];
if (driver) {
[driver updateWithEvent:event];
NSMutableArray<RCTEventAnimation *> *driversForKey = _eventDrivers[key];
if (driversForKey) {
for (RCTEventAnimation *driver in driversForKey) {
[driver updateWithEvent:event];
}

[self updateAnimations];
}
}
Expand Down Expand Up @@ -337,13 +361,13 @@ - (void)stopAnimationLoop

- (void)stepAnimations
{
for (id<RCTAnimationDriver>animationDriver in _activeAnimations) {
for (id<RCTAnimationDriver> animationDriver in _activeAnimations) {
[animationDriver stepAnimation];
}

[self updateAnimations];

for (id<RCTAnimationDriver>animationDriver in [_activeAnimations copy]) {
for (id<RCTAnimationDriver> animationDriver in [_activeAnimations copy]) {
if (animationDriver.animationHasFinished) {
[animationDriver removeAnimation];
[_activeAnimations removeObject:animationDriver];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -353,11 +353,11 @@ public void execute(NativeAnimatedNodesManager animatedNodesManager) {
}

@ReactMethod
public void removeAnimatedEventFromView(final int viewTag, final String eventName) {
public void removeAnimatedEventFromView(final int viewTag, final String eventName, final int animatedValueTag) {
mOperations.add(new UIThreadOperation() {
@Override
public void execute(NativeAnimatedNodesManager animatedNodesManager) {
animatedNodesManager.removeAnimatedEventFromView(viewTag, eventName);
animatedNodesManager.removeAnimatedEventFromView(viewTag, eventName, animatedValueTag);
}
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.ListIterator;
import java.util.Map;
import java.util.Queue;

Expand All @@ -55,12 +57,14 @@
private final SparseArray<AnimatedNode> mAnimatedNodes = new SparseArray<>();
private final SparseArray<AnimationDriver> mActiveAnimations = new SparseArray<>();
private final SparseArray<AnimatedNode> mUpdatedNodes = new SparseArray<>();
private final Map<String, EventAnimationDriver> mEventDrivers = new HashMap<>();
// Mapping of a view tag and an event name to a list of event animation drivers. 99% of the time
// there will be only one driver per mapping so all code code should be optimized around that.
private final Map<String, List<EventAnimationDriver>> mEventDrivers = new HashMap<>();
private final Map<String, Map<String, String>> mCustomEventTypes;
private final UIImplementation mUIImplementation;
private int mAnimatedGraphBFSColor = 0;
// Used to avoid allocating a new array on every frame in runUpdates.
private final List<AnimatedNode> mRunUpdateNodeList = new ArrayList<>();
// Used to avoid allocating a new array on every frame in `runUpdates` and `onEventDispatch`.
private final List<AnimatedNode> mRunUpdateNodeList = new LinkedList<>();

public NativeAnimatedNodesManager(UIManagerModule uiManager) {
mUIImplementation = uiManager.getUIImplementation();
Expand Down Expand Up @@ -312,11 +316,32 @@ public void addAnimatedEventToView(int viewTag, String eventName, ReadableMap ev
}

EventAnimationDriver event = new EventAnimationDriver(pathList, (ValueAnimatedNode) node);
mEventDrivers.put(viewTag + eventName, event);
String key = viewTag + eventName;
if (mEventDrivers.containsKey(key)) {
mEventDrivers.get(key).add(event);
} else {
List<EventAnimationDriver> drivers = new ArrayList<>(1);
drivers.add(event);
mEventDrivers.put(key, drivers);
}
}

public void removeAnimatedEventFromView(int viewTag, String eventName) {
mEventDrivers.remove(viewTag + eventName);
public void removeAnimatedEventFromView(int viewTag, String eventName, int animatedValueTag) {
String key = viewTag + eventName;
if (mEventDrivers.containsKey(key)) {
List<EventAnimationDriver> driversForKey = mEventDrivers.get(key);
if (driversForKey.size() == 1) {
mEventDrivers.remove(viewTag + eventName);
} else {
ListIterator<EventAnimationDriver> it = driversForKey.listIterator();
while (it.hasNext()) {
if (it.next().mValueNode.mTag == animatedValueTag) {
it.remove();
break;
}
}
}
}
}

@Override
Expand All @@ -334,11 +359,14 @@ public void onEventDispatch(Event event) {
eventName = customEventType.get("registrationName");
}

EventAnimationDriver eventDriver = mEventDrivers.get(event.getViewTag() + eventName);
if (eventDriver != null) {
event.dispatch(eventDriver);

updateNodes(Collections.singletonList((AnimatedNode) eventDriver.mValueNode));
List<EventAnimationDriver> driversForKey = mEventDrivers.get(event.getViewTag() + eventName);
if (driversForKey != null) {
for (EventAnimationDriver driver : driversForKey) {
event.dispatch(driver);
mRunUpdateNodeList.add(driver.mValueNode);
}
updateNodes(mRunUpdateNodeList);
mRunUpdateNodeList.clear();
}
}
}
Expand Down

0 comments on commit 921b9ac

Please sign in to comment.