Skip to content

Commit 831f35e

Browse files
rozelerozele
committed
fix(NativeAnimatedModule): Restore default values when removing props
Replicating fix from Android, see facebook/react-native@2b4ff6e for more details.
1 parent 4e2f21f commit 831f35e

8 files changed

+236
-102
lines changed

‎ReactWindows/ReactNative.Shared/Animated/NativeAnimatedModule.cs

+105-54
Original file line numberDiff line numberDiff line change
@@ -59,15 +59,18 @@ namespace ReactNative.Animated
5959
/// may be caused by concurrent updates of animated graph while UI thread
6060
/// is "executing" the animation loop.
6161
/// </remarks>
62-
public class NativeAnimatedModule : ReactContextNativeModuleBase, IOnBatchCompleteListener, ILifecycleEventListener
62+
public class NativeAnimatedModule : ReactContextNativeModuleBase, ILifecycleEventListener
6363
{
6464
private readonly object _operationsGate = new object();
6565

6666
private EventHandler<FrameEventArgs> _animatedFrameCallback;
6767

6868
private List<Action<NativeAnimatedNodesManager>> _operations =
6969
new List<Action<NativeAnimatedNodesManager>>();
70-
private List<Action<NativeAnimatedNodesManager>> _readyOperations;
70+
private List<Action<NativeAnimatedNodesManager>> _preOperations =
71+
new List<Action<NativeAnimatedNodesManager>>();
72+
73+
private NativeAnimatedNodesManager _nodesManager;
7174

7275
/// <summary>
7376
/// Instantiates the <see cref="NativeAnimatedModule"/>.
@@ -89,6 +92,20 @@ public override string Name
8992
}
9093
}
9194

95+
private NativeAnimatedNodesManager NodesManager
96+
{
97+
get
98+
{
99+
if (_nodesManager == null)
100+
{
101+
var uiManager = Context.GetNativeModule<UIManagerModule>();
102+
_nodesManager = new NativeAnimatedNodesManager(uiManager);
103+
}
104+
105+
return _nodesManager;
106+
}
107+
}
108+
92109
/// <summary>
93110
/// Called after the creation of a <see cref="IReactInstance"/>, in
94111
/// order to initialize native modules that require the React or
@@ -98,26 +115,12 @@ public override void Initialize()
98115
{
99116
var ctx = Context;
100117
var uiManager = ctx.GetNativeModule<UIManagerModule>();
101-
var nodesManager = new NativeAnimatedNodesManager(uiManager);
118+
uiManager.DispatchingViewUpdates += OnDispatchingViewUpdates;
102119
_animatedFrameCallback = (sender, args) =>
103120
{
104121
try
105122
{
106-
var operations = default(List<Action<NativeAnimatedNodesManager>>);
107-
lock (_operationsGate)
108-
{
109-
operations = _readyOperations;
110-
_readyOperations = null;
111-
}
112-
113-
if (operations != null)
114-
{
115-
foreach (var operation in operations)
116-
{
117-
operation(nodesManager);
118-
}
119-
}
120-
123+
var nodesManager = NodesManager;
121124
if (nodesManager.HasActiveAnimations)
122125
{
123126
nodesManager.RunUpdates(args.RenderingTime);
@@ -136,29 +139,41 @@ public override void Initialize()
136139
ctx.AddLifecycleEventListener(this);
137140
}
138141

139-
/// <summary>
140-
/// Invoked when a batch of JavaScript to native calls has finished.
141-
/// </summary>
142-
public void OnBatchComplete()
142+
private void OnDispatchingViewUpdates(object sender, EventArgs e)
143143
{
144-
var operations = _operations.Count == 0 ? null : _operations;
145-
_operations = new List<Action<NativeAnimatedNodesManager>>();
146-
if (operations != null)
144+
if (_operations.Count == 0 && _preOperations.Count == 0)
145+
{
146+
return;
147+
}
148+
149+
var uiManager = (UIManagerModule)sender;
150+
List<Action<NativeAnimatedNodesManager>> preOperations;
151+
List<Action<NativeAnimatedNodesManager>> operations;
152+
lock (_operationsGate)
153+
{
154+
preOperations = _preOperations;
155+
operations = _operations;
156+
_preOperations = new List<Action<NativeAnimatedNodesManager>>();
157+
_operations = new List<Action<NativeAnimatedNodesManager>>();
158+
}
159+
160+
uiManager.PrependUIBlock(new UIBlock(() =>
147161
{
148-
lock (_operationsGate)
162+
var nodesManager = NodesManager;
163+
foreach (var operation in preOperations)
149164
{
150-
if (_readyOperations == null)
151-
{
152-
_readyOperations = operations;
153-
}
154-
else
155-
{
156-
_readyOperations.AddRange(operations);
157-
}
165+
operation(nodesManager);
158166
}
167+
}));
159168

160-
ReactChoreographer.Instance.ActivateCallback(nameof(NativeAnimatedModule));
161-
}
169+
uiManager.AddUIBlock(new UIBlock(() =>
170+
{
171+
var nodesManager = NodesManager;
172+
foreach (var operation in operations)
173+
{
174+
operation(nodesManager);
175+
}
176+
}));
162177
}
163178

164179
/// <summary>
@@ -193,7 +208,7 @@ public void OnSuspend()
193208
[ReactMethod]
194209
public void createAnimatedNode(int tag, JObject config)
195210
{
196-
_operations.Add(manager =>
211+
AddOperation(manager =>
197212
manager.CreateAnimatedNode(tag, config));
198213
}
199214

@@ -204,7 +219,7 @@ public void createAnimatedNode(int tag, JObject config)
204219
[ReactMethod]
205220
public void startListeningToAnimatedNodeValue(int tag)
206221
{
207-
_operations.Add(manager =>
222+
AddOperation(manager =>
208223
manager.StartListeningToAnimatedNodeValue(tag, value =>
209224
Context.GetJavaScriptModule<RCTDeviceEventEmitter>()
210225
.emit("onAnimatedValueUpdate", new JObject
@@ -221,7 +236,7 @@ public void startListeningToAnimatedNodeValue(int tag)
221236
[ReactMethod]
222237
public void stopListeningToAnimatedNodeValue(int tag)
223238
{
224-
_operations.Add(manager =>
239+
AddOperation(manager =>
225240
manager.StopListeningToAnimatedNodeValue(tag));
226241
}
227242

@@ -232,7 +247,7 @@ public void stopListeningToAnimatedNodeValue(int tag)
232247
[ReactMethod]
233248
public void dropAnimatedNode(int tag)
234249
{
235-
_operations.Add(manager =>
250+
AddOperation(manager =>
236251
manager.DropAnimatedNode(tag));
237252
}
238253

@@ -244,7 +259,7 @@ public void dropAnimatedNode(int tag)
244259
[ReactMethod]
245260
public void setAnimatedNodeValue(int tag, double value)
246261
{
247-
_operations.Add(manager =>
262+
AddOperation(manager =>
248263
manager.SetAnimatedNodeValue(tag, value));
249264
}
250265

@@ -256,7 +271,7 @@ public void setAnimatedNodeValue(int tag, double value)
256271
[ReactMethod]
257272
public void setAnimatedNodeOffset(int tag, double value)
258273
{
259-
_operations.Add(manager =>
274+
AddOperation(manager =>
260275
manager.SetAnimatedNodeOffset(tag, value));
261276
}
262277

@@ -267,7 +282,7 @@ public void setAnimatedNodeOffset(int tag, double value)
267282
[ReactMethod]
268283
public void flattenAnimatedNodeOffset(int tag)
269284
{
270-
_operations.Add(manager =>
285+
AddOperation(manager =>
271286
manager.FlattenAnimatedNodeOffset(tag));
272287
}
273288

@@ -278,7 +293,7 @@ public void flattenAnimatedNodeOffset(int tag)
278293
[ReactMethod]
279294
public void extractAnimatedNodeOffset(int tag)
280295
{
281-
_operations.Add(manager =>
296+
AddOperation(manager =>
282297
manager.ExtractAnimatedNodeOffset(tag));
283298
}
284299

@@ -296,7 +311,7 @@ public void startAnimatingNode(
296311
JObject animationConfig,
297312
ICallback endCallback)
298313
{
299-
_operations.Add(manager =>
314+
AddOperation(manager =>
300315
manager.StartAnimatingNode(
301316
animationId,
302317
animatedNodeTag,
@@ -311,7 +326,7 @@ public void startAnimatingNode(
311326
[ReactMethod]
312327
public void stopAnimation(int animationId)
313328
{
314-
_operations.Add(manager =>
329+
AddOperation(manager =>
315330
manager.StopAnimation(animationId));
316331
}
317332

@@ -323,7 +338,7 @@ public void stopAnimation(int animationId)
323338
[ReactMethod]
324339
public void connectAnimatedNodes(int parentNodeTag, int childNodeTag)
325340
{
326-
_operations.Add(manager =>
341+
AddOperation(manager =>
327342
manager.ConnectAnimatedNodes(parentNodeTag, childNodeTag));
328343
}
329344

@@ -335,7 +350,7 @@ public void connectAnimatedNodes(int parentNodeTag, int childNodeTag)
335350
[ReactMethod]
336351
public void disconnectAnimatedNodes(int parentNodeTag, int childNodeTag)
337352
{
338-
_operations.Add(manager =>
353+
AddOperation(manager =>
339354
manager.DisconnectAnimatedNodes(parentNodeTag, childNodeTag));
340355
}
341356

@@ -347,20 +362,25 @@ public void disconnectAnimatedNodes(int parentNodeTag, int childNodeTag)
347362
[ReactMethod]
348363
public void connectAnimatedNodeToView(int animatedNodeTag, int viewTag)
349364
{
350-
_operations.Add(manager =>
365+
AddOperation(manager =>
351366
manager.ConnectAnimatedNodeToView(animatedNodeTag, viewTag));
352367
}
353368

354369
/// <summary>
355370
/// Disconnects animated node from view.
356371
/// </summary>
357372
/// <param name="animatedNodeTag">Animated node tag.</param>
358-
/// <param name="viewTag">React view tag.s</param>
373+
/// <param name="viewTag">React view tag.</param>
359374
[ReactMethod]
360375
public void disconnectAnimatedNodeFromView(int animatedNodeTag, int viewTag)
361376
{
362-
_operations.Add(manager =>
363-
manager.DisconnectAnimatedNodeFromView(animatedNodeTag, viewTag));
377+
lock (_operationsGate)
378+
{
379+
_preOperations.Add(manager =>
380+
manager.RestoreDefaultValues(animatedNodeTag, viewTag));
381+
_operations.Add(manager =>
382+
manager.DisconnectAnimatedNodeFromView(animatedNodeTag, viewTag));
383+
}
364384
}
365385

366386
/// <summary>
@@ -372,7 +392,7 @@ public void disconnectAnimatedNodeFromView(int animatedNodeTag, int viewTag)
372392
[ReactMethod]
373393
public void addAnimatedEventToView(int viewTag, string eventName, JObject eventMapping)
374394
{
375-
_operations.Add(manager =>
395+
AddOperation(manager =>
376396
manager.AddAnimatedEventToView(viewTag, eventName, eventMapping));
377397
}
378398

@@ -385,8 +405,39 @@ public void addAnimatedEventToView(int viewTag, string eventName, JObject eventM
385405
[ReactMethod]
386406
public void removeAnimatedEventFromView(int viewTag, string eventName, int animatedValueTag)
387407
{
388-
_operations.Add(manager =>
408+
AddOperation(manager =>
389409
manager.RemoveAnimatedEventFromView(viewTag, eventName, animatedValueTag));
390410
}
411+
412+
private void AddOperation(Action<NativeAnimatedNodesManager> action)
413+
{
414+
lock (_operationsGate)
415+
{
416+
_operations.Add(action);
417+
}
418+
}
419+
420+
private void AddPreOperation(Action<NativeAnimatedNodesManager> action)
421+
{
422+
lock (_operationsGate)
423+
{
424+
_preOperations.Add(action);
425+
}
426+
}
427+
428+
class UIBlock : IUIBlock
429+
{
430+
private readonly Action _action;
431+
432+
public UIBlock(Action action)
433+
{
434+
_action = action;
435+
}
436+
437+
public void Execute(NativeViewHierarchyManager nativeViewHierarchyManager)
438+
{
439+
_action();
440+
}
441+
}
391442
}
392443
}

‎ReactWindows/ReactNative.Shared/Animated/NativeAnimatedNodesManager.cs

+23-13
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ public void CreateAnimatedNode(int tag, JObject config)
7878
node = new ValueAnimatedNode(tag, config);
7979
break;
8080
case "props":
81-
node = new PropsAnimatedNode(tag, config, this);
81+
node = new PropsAnimatedNode(tag, config, this, _uiImplementation);
8282
break;
8383
case "interpolation":
8484
node = new InterpolationAnimatedNode(tag, config);
@@ -281,13 +281,7 @@ public void ConnectAnimatedNodeToView(int animatedNodeTag, int viewTag)
281281
throw new InvalidOperationException("Animated node connected to view should be props node.");
282282
}
283283

284-
if (propsAnimatedNode.ConnectedViewTag != -1)
285-
{
286-
throw new InvalidOperationException(
287-
Invariant($"Animated node '{animatedNodeTag}' is already attached to a view."));
288-
}
289-
290-
propsAnimatedNode.ConnectedViewTag = viewTag;
284+
propsAnimatedNode.ConnectToView(viewTag);
291285
_updatedNodes[animatedNodeTag] = node;
292286
}
293287

@@ -301,13 +295,29 @@ public void DisconnectAnimatedNodeFromView(int animatedNodeTag, int viewTag)
301295
throw new InvalidOperationException("Animated node connected to view should be props node.");
302296
}
303297

304-
if (propsAnimatedNode.ConnectedViewTag != viewTag)
298+
propsAnimatedNode.DisconnectFromView(viewTag);
299+
}
300+
301+
public void RestoreDefaultValues(int animatedNodeTag, int viewTag)
302+
{
303+
var node = default(AnimatedNode);
304+
if (!_animatedNodes.TryGetValue(animatedNodeTag, out node))
305305
{
306-
throw new InvalidOperationException(
307-
"Attempting to disconnect view that has not been connected with the given animated node.");
306+
// Restoring default values needs to happen before UIManager
307+
// operations so it is possible the node hasn't been created yet if
308+
// it is being connected and disconnected in the same batch. In
309+
// that case we don't need to restore default values since it will
310+
// never actually update the view.
311+
return;
312+
}
313+
314+
var propsAnimatedNode = node as PropsAnimatedNode;
315+
if (propsAnimatedNode == null)
316+
{
317+
throw new InvalidOperationException("Animated node connected to view should be props node.");
308318
}
309319

310-
propsAnimatedNode.ConnectedViewTag = -1;
320+
propsAnimatedNode.RestoreDefaultValues();
311321
}
312322

313323
public void AddAnimatedEventToView(int viewTag, string eventName, JObject eventMapping)
@@ -576,7 +586,7 @@ private void UpdateNodes(IList<AnimatedNode> nodes)
576586
var valueNode = default(ValueAnimatedNode);
577587
if (propsNode != null)
578588
{
579-
propsNode.UpdateView(_uiImplementation);
589+
propsNode.UpdateView();
580590
}
581591
else if ((valueNode = nextNode as ValueAnimatedNode) != null)
582592
{

0 commit comments

Comments
 (0)