Skip to content

Commit ed11286

Browse files
authored
refactor: review of #3073 - rename few methods, move update logic to shadow state proxy (#3176)
## Description Retrospective review for #3073. ## Changes * moved all the state related to view size transitioning do dedicated file-private type `ViewSizeTransitionState`, * renamed `isTransitionInProgress -> isViewSizeTransitionInProgress`, * renamed `stopAnimation` -> `cleanupViewSizeTransitionState`, * removed `applyTransitioningShadowState(...)` & `applyStaticShadowStateRelativeToAncestor(...)` methods & moved the appropriate logic to shadow state proxy. ## Test code and steps to reproduce See #3073 ## Checklist - [ ] Ensured that CI passes
1 parent 5e16d17 commit ed11286

File tree

4 files changed

+70
-45
lines changed

4 files changed

+70
-45
lines changed

ios/gamma/split-view/RNSSplitViewScreenComponentView.mm

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ - (void)updateLayoutMetrics:(const facebook::react::LayoutMetrics &)layoutMetric
157157
// 3. `setFrame` is called with the width 'B'
158158
// 4. in the same time, we want to track updates and treat intermediate value A' indicated from the presentation layer
159159
// as our source of truth
160-
if (![_controller isTransitionInProgress]) {
160+
if (![_controller isViewSizeTransitionInProgress]) {
161161
[super updateLayoutMetrics:layoutMetrics oldLayoutMetrics:oldLayoutMetrics];
162162
}
163163
}

ios/gamma/split-view/RNSSplitViewScreenController.swift

Lines changed: 44 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,10 @@ public class RNSSplitViewScreenController: UIViewController {
1818
return splitViewScreenComponentView.reactEventEmitter()
1919
}
2020

21-
private var displayLink: CADisplayLink?
22-
private var lastAnimationFrame: CGRect?
23-
private var transitionInProgress: Bool
21+
private var viewSizeTransitionState: ViewSizeTransitionState? = nil
2422

2523
@objc public required init(splitViewScreenComponentView: RNSSplitViewScreenComponentView) {
2624
self.splitViewScreenComponentView = splitViewScreenComponentView
27-
self.transitionInProgress = false
28-
2925
super.init(nibName: nil, bundle: nil)
3026
}
3127

@@ -74,8 +70,8 @@ public class RNSSplitViewScreenController: UIViewController {
7470
/// @return true if the transition is running, false otherwise.
7571
///
7672
@objc
77-
public func isTransitionInProgress() -> Bool {
78-
return transitionInProgress
73+
public func isViewSizeTransitionInProgress() -> Bool {
74+
return viewSizeTransitionState != nil
7975
}
8076

8177
// MARK: Signals
@@ -97,33 +93,31 @@ public class RNSSplitViewScreenController: UIViewController {
9793
) {
9894
super.viewWillTransition(to: size, with: coordinator)
9995

100-
transitionInProgress = true
96+
viewSizeTransitionState = ViewSizeTransitionState()
10197

10298
coordinator.animate(
10399
alongsideTransition: { [weak self] context in
104100
guard let self = self else { return }
105-
if self.displayLink == nil {
106-
self.displayLink = CADisplayLink(
107-
target: self, selector: #selector(trackTransitionProgress))
108-
self.displayLink?.add(to: .main, forMode: .common)
101+
guard let viewSizeTransitionState = self.viewSizeTransitionState else { return }
102+
103+
if viewSizeTransitionState.displayLink == nil {
104+
viewSizeTransitionState.setupDisplayLink(
105+
forTarget: self, selector: #selector(trackTransitionProgress))
109106
}
110107
},
111108
completion: { [weak self] context in
112109
guard let self = self else { return }
113-
self.stopAnimation()
110+
self.cleanupViewSizeTransitionState()
114111
// After the animation completion, ensure that ShadowTree state
115112
// is calculated relatively to the ancestor's frame by requesting
116113
// the state update.
117114
self.updateShadowTreeState()
118115
})
119116
}
120117

121-
private func stopAnimation() {
122-
lastAnimationFrame = nil
123-
transitionInProgress = false
124-
125-
displayLink?.invalidate()
126-
displayLink = nil
118+
private func cleanupViewSizeTransitionState() {
119+
viewSizeTransitionState?.invalidate()
120+
viewSizeTransitionState = nil
127121
}
128122

129123
///
@@ -133,7 +127,7 @@ public class RNSSplitViewScreenController: UIViewController {
133127
@objc
134128
private func trackTransitionProgress() {
135129
if let currentFrame = view.layer.presentation()?.frame {
136-
lastAnimationFrame = currentFrame
130+
viewSizeTransitionState?.lastViewPresentationFrame = currentFrame
137131
updateShadowTreeState()
138132
}
139133
}
@@ -172,11 +166,12 @@ public class RNSSplitViewScreenController: UIViewController {
172166
// If the resize animation is currently running, we prefer to apply dynamic updates,
173167
// based on the results from the presentation layer
174168
// which is read from `trackTransitionProgress` method.
175-
if let currentSize = lastAnimationFrame?.size {
176-
applyTransitioningShadowState(
177-
size: currentSize,
178-
ancestorView: ancestorView!
179-
)
169+
if let lastViewPresentationFrame = viewSizeTransitionState?.lastViewPresentationFrame,
170+
!lastViewPresentationFrame.isNull
171+
{
172+
shadowStateProxy.updateShadowState(
173+
ofComponent: splitViewScreenComponentView, withFrame: lastViewPresentationFrame,
174+
inContextOfAncestorView: ancestorView!)
180175
return
181176
}
182177

@@ -186,27 +181,12 @@ public class RNSSplitViewScreenController: UIViewController {
186181
// to prevent interrupting with the frames that are less important for us.
187182
// This works fine, because after the animation completion, we're sending the last update
188183
// which is compatible with the frame which would be calculated relatively to the ancestor here.
189-
if !isTransitionInProgress() {
190-
applyStaticShadowStateRelativeToAncestor(ancestorView!)
184+
if !isViewSizeTransitionInProgress() {
185+
shadowStateProxy.updateShadowState(
186+
ofComponent: splitViewScreenComponentView, inContextOfAncestorView: ancestorView)
191187
}
192188
}
193189

194-
private func applyTransitioningShadowState(size: CGSize, ancestorView: UIView) {
195-
let localOrigin = splitViewScreenComponentView.convert(
196-
splitViewScreenComponentView.frame.origin,
197-
to: ancestorView
198-
)
199-
let convertedFrame = CGRect(origin: localOrigin, size: size)
200-
shadowStateProxy.updateShadowState(withFrame: convertedFrame)
201-
}
202-
203-
private func applyStaticShadowStateRelativeToAncestor(_ ancestorView: UIView) {
204-
shadowStateProxy.updateShadowState(
205-
ofComponent: splitViewScreenComponentView,
206-
inContextOfAncestorView: ancestorView
207-
)
208-
}
209-
210190
///
211191
/// @brief Request ShadowNode state update when the SplitView screen frame origin has changed.
212192
///
@@ -220,7 +200,7 @@ public class RNSSplitViewScreenController: UIViewController {
220200
// During the transition, we're listening for the animation
221201
// frame updates on the presentation layer and we're
222202
// treating these updates as the source of truth
223-
if !isTransitionInProgress() {
203+
if !isViewSizeTransitionInProgress() {
224204
shadowStateProxy.updateShadowState(
225205
ofComponent: splitViewScreenComponentView, inContextOfAncestorView: splitViewController.view
226206
)
@@ -245,3 +225,23 @@ public class RNSSplitViewScreenController: UIViewController {
245225
reactEventEmitter.emitOnDidDisappear()
246226
}
247227
}
228+
229+
private class ViewSizeTransitionState {
230+
public var displayLink: CADisplayLink?
231+
public var lastViewPresentationFrame: CGRect = CGRect.null
232+
233+
public func setupDisplayLink(forTarget target: Any, selector sel: Selector) {
234+
if displayLink != nil {
235+
displayLink?.invalidate()
236+
}
237+
238+
displayLink = CADisplayLink(target: target, selector: sel)
239+
displayLink!.add(to: .main, forMode: .common)
240+
}
241+
242+
public func invalidate() {
243+
displayLink?.invalidate()
244+
displayLink = nil
245+
lastViewPresentationFrame = CGRect.null
246+
}
247+
}

ios/gamma/split-view/RNSSplitViewScreenShadowStateProxy.h

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,21 @@ NS_ASSUME_NONNULL_BEGIN
4242
- (void)updateShadowStateOfComponent:(RNSSplitViewScreenComponentView *)screenComponentView
4343
inContextOfAncestorView:(UIView *_Nullable)ancestorView;
4444

45+
/**
46+
* @brief Send an update to ShadowNode state with given frame if needed.
47+
*
48+
* Converts the frame to coordinates of the specified ancestor view,
49+
* before applying the update to the Shadow Tree.
50+
*
51+
* @param screenComponentView an instance of RNSSplitViewScreenComponentView whose state should be updated,
52+
* @param frame frame to update the shadow state with; it must be in coordinate system of `screenComponentView`,
53+
* @param ancestorView coordinate-system provider view, relative to which the frame should be converted before sending
54+
* the update.
55+
*/
56+
- (void)updateShadowStateOfComponent:(RNSSplitViewScreenComponentView *)screenComponentView
57+
withFrame:(CGRect)frame
58+
inContextOfAncestorView:(nonnull UIView *)ancestorView;
59+
4560
/**
4661
* @brief Send an update to ShadowNode state with given layout metrics.
4762
*

ios/gamma/split-view/RNSSplitViewScreenShadowStateProxy.mm

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#import "RNSSplitViewScreenShadowStateProxy.h"
22
#import "RNSSplitViewScreenComponentView.h"
33

4+
#import <React/RCTAssert.h>
45
#import <React/RCTConversions.h>
56
#import <rnscreens/RNSSplitViewScreenShadowNode.h>
67

@@ -48,6 +49,15 @@ - (void)updateShadowStateWithFrame:(CGRect)frame
4849
}
4950
}
5051

52+
- (void)updateShadowStateOfComponent:(RNSSplitViewScreenComponentView *)screenComponentView
53+
withFrame:(CGRect)frame
54+
inContextOfAncestorView:(nonnull UIView *)ancestorView
55+
{
56+
RCTAssert(ancestorView != nil, @"[RNScreens] ancestorView must not be nil");
57+
CGRect convertedFrame = [screenComponentView convertRect:frame toView:ancestorView];
58+
[self updateShadowStateWithFrame:convertedFrame];
59+
}
60+
5161
- (void)updateState:(react::State::Shared const &)state oldState:(react::State::Shared const &)oldState
5262
{
5363
_state = std::static_pointer_cast<const react::RNSSplitViewScreenShadowNode::ConcreteState>(state);

0 commit comments

Comments
 (0)