From c642afd97d2f0ce7657405d72538cecaefadc036 Mon Sep 17 00:00:00 2001 From: Valentin Shergin Date: Mon, 19 Oct 2020 19:18:51 -0700 Subject: [PATCH] Fabric: Making `_propKeysManagedByAnimated` to preserve previous prop keys Summary: The previous implementation always replaced the prop set on every props update coming from Animated. Now, we merge the new set of props with the previous one. Changelog: [Internal] Fabric-specific internal change. Reviewed By: sammy-SC Differential Revision: D24370751 fbshipit-source-id: 779272d47c776cc42071d3c93d06443b6c96a877 --- .../View/RCTViewComponentView.mm | 29 ++++++++++++------- .../Mounting/RCTComponentViewProtocol.h | 8 +++-- React/Fabric/Mounting/RCTMountingManager.mm | 7 +++-- .../Mounting/UIView+ComponentViewProtocol.h | 3 +- .../Mounting/UIView+ComponentViewProtocol.mm | 7 ++++- 5 files changed, 37 insertions(+), 17 deletions(-) diff --git a/React/Fabric/Mounting/ComponentViews/View/RCTViewComponentView.mm b/React/Fabric/Mounting/ComponentViews/View/RCTViewComponentView.mm index f15be9a11b5..0a4d903be62 100644 --- a/React/Fabric/Mounting/ComponentViews/View/RCTViewComponentView.mm +++ b/React/Fabric/Mounting/ComponentViews/View/RCTViewComponentView.mm @@ -23,7 +23,7 @@ using namespace facebook::react; UIColor *_backgroundColor; CALayer *_borderLayer; BOOL _needsInvalidateLayer; - NSSet *_propKeysManagedByAnimated; + NSSet *_Nullable _propKeysManagedByAnimated_DO_NOT_USE_THIS_IS_BROKEN; } - (instancetype)initWithFrame:(CGRect)frame @@ -85,11 +85,6 @@ using namespace facebook::react; return concreteComponentDescriptorProvider(); } -- (void)setPropKeysManagedByAnimated:(nullable NSSet *)propKeys -{ - _propKeysManagedByAnimated = propKeys; -} - - (void)updateProps:(Props::Shared const &)props oldProps:(Props::Shared const &)oldProps { #ifndef NS_BLOCK_ASSERTIONS @@ -109,7 +104,8 @@ using namespace facebook::react; BOOL needsInvalidateLayer = NO; // `opacity` - if (oldViewProps.opacity != newViewProps.opacity && ![_propKeysManagedByAnimated containsObject:@"opacity"]) { + if (oldViewProps.opacity != newViewProps.opacity && + ![_propKeysManagedByAnimated_DO_NOT_USE_THIS_IS_BROKEN containsObject:@"opacity"]) { self.layer.opacity = (CGFloat)newViewProps.opacity; needsInvalidateLayer = YES; } @@ -168,7 +164,8 @@ using namespace facebook::react; } // `transform` - if (oldViewProps.transform != newViewProps.transform && ![_propKeysManagedByAnimated containsObject:@"transform"]) { + if (oldViewProps.transform != newViewProps.transform && + ![_propKeysManagedByAnimated_DO_NOT_USE_THIS_IS_BROKEN containsObject:@"transform"]) { self.layer.transform = RCTCATransform3DFromTransformMatrix(newViewProps.transform); self.layer.allowsEdgeAntialiasing = newViewProps.transform != Transform::Identity(); } @@ -296,17 +293,27 @@ using namespace facebook::react; // If view was managed by animated, its props need to align with UIView's properties. auto const &props = *std::static_pointer_cast(_props); - if ([_propKeysManagedByAnimated containsObject:@"transform"]) { + if ([_propKeysManagedByAnimated_DO_NOT_USE_THIS_IS_BROKEN containsObject:@"transform"]) { self.layer.transform = RCTCATransform3DFromTransformMatrix(props.transform); } - if ([_propKeysManagedByAnimated containsObject:@"opacity"]) { + if ([_propKeysManagedByAnimated_DO_NOT_USE_THIS_IS_BROKEN containsObject:@"opacity"]) { self.layer.opacity = (CGFloat)props.opacity; } - _propKeysManagedByAnimated = nil; + _propKeysManagedByAnimated_DO_NOT_USE_THIS_IS_BROKEN = nil; _eventEmitter.reset(); } +- (void)setPropKeysManagedByAnimated_DO_NOT_USE_THIS_IS_BROKEN:(NSSet *_Nullable)props +{ + _propKeysManagedByAnimated_DO_NOT_USE_THIS_IS_BROKEN = props; +} + +- (NSSet *_Nullable)propKeysManagedByAnimated_DO_NOT_USE_THIS_IS_BROKEN +{ + return _propKeysManagedByAnimated_DO_NOT_USE_THIS_IS_BROKEN; +} + - (UIView *)betterHitTest:(CGPoint)point withEvent:(UIEvent *)event { // This is a classic textbook implementation of `hitTest:` with a couple of improvements: diff --git a/React/Fabric/Mounting/RCTComponentViewProtocol.h b/React/Fabric/Mounting/RCTComponentViewProtocol.h index 2595f3b09f1..8183e419202 100644 --- a/React/Fabric/Mounting/RCTComponentViewProtocol.h +++ b/React/Fabric/Mounting/RCTComponentViewProtocol.h @@ -111,12 +111,16 @@ typedef NS_OPTIONS(NSInteger, RNComponentViewUpdateMask) { */ - (void)prepareForRecycle; -/** +/* * Read the last props used to update the view. */ - (facebook::react::SharedProps)props; -- (void)setPropKeysManagedByAnimated:(nullable NSSet *)propKeys; +/* + * This is broken. Do not use. + */ +- (void)setPropKeysManagedByAnimated_DO_NOT_USE_THIS_IS_BROKEN:(nullable NSSet *)props; +- (nullable NSSet *)propKeysManagedByAnimated_DO_NOT_USE_THIS_IS_BROKEN; @end diff --git a/React/Fabric/Mounting/RCTMountingManager.mm b/React/Fabric/Mounting/RCTMountingManager.mm index b593f122c74..2f7363c6da9 100644 --- a/React/Fabric/Mounting/RCTMountingManager.mm +++ b/React/Fabric/Mounting/RCTMountingManager.mm @@ -222,9 +222,12 @@ static void RCTPerformMountInstructions( UIView *componentView = [_componentViewRegistry findComponentViewWithTag:reactTag]; SharedProps oldProps = [componentView props]; SharedProps newProps = componentDescriptor.cloneProps(oldProps, RawProps(convertIdToFollyDynamic(props))); - [componentView setPropKeysManagedByAnimated:nil]; + + NSSet *propKeys = componentView.propKeysManagedByAnimated_DO_NOT_USE_THIS_IS_BROKEN ?: [NSSet new]; + propKeys = [propKeys setByAddingObjectsFromArray:props.allKeys]; + componentView.propKeysManagedByAnimated_DO_NOT_USE_THIS_IS_BROKEN = nil; [componentView updateProps:newProps oldProps:oldProps]; - [componentView setPropKeysManagedByAnimated:[NSSet setWithArray:props.allKeys]]; + componentView.propKeysManagedByAnimated_DO_NOT_USE_THIS_IS_BROKEN = propKeys; } - (void)synchronouslyDispatchCommandOnUIThread:(ReactTag)reactTag diff --git a/React/Fabric/Mounting/UIView+ComponentViewProtocol.h b/React/Fabric/Mounting/UIView+ComponentViewProtocol.h index 09d35535469..3cb45b8f7b7 100644 --- a/React/Fabric/Mounting/UIView+ComponentViewProtocol.h +++ b/React/Fabric/Mounting/UIView+ComponentViewProtocol.h @@ -39,7 +39,8 @@ NS_ASSUME_NONNULL_BEGIN - (facebook::react::SharedProps)props; -- (void)setPropKeysManagedByAnimated:(nullable NSSet *)propKeys; +- (void)setPropKeysManagedByAnimated_DO_NOT_USE_THIS_IS_BROKEN:(nullable NSSet *)props; +- (nullable NSSet *)propKeysManagedByAnimated_DO_NOT_USE_THIS_IS_BROKEN; @end diff --git a/React/Fabric/Mounting/UIView+ComponentViewProtocol.mm b/React/Fabric/Mounting/UIView+ComponentViewProtocol.mm index 39d9af5cc41..c10dfbc8654 100644 --- a/React/Fabric/Mounting/UIView+ComponentViewProtocol.mm +++ b/React/Fabric/Mounting/UIView+ComponentViewProtocol.mm @@ -132,9 +132,14 @@ using namespace facebook::react; return nullptr; } -- (void)setPropKeysManagedByAnimated:(nullable NSSet *)propKeys +- (void)setPropKeysManagedByAnimated_DO_NOT_USE_THIS_IS_BROKEN:(nullable NSSet *)propKeys { // Default implementation does nothing. } +- (nullable NSSet *)propKeysManagedByAnimated_DO_NOT_USE_THIS_IS_BROKEN +{ + return nil; +} + @end