Skip to content

Commit

Permalink
fix: conflict when using fullScreenModal presentation in native-sta…
Browse files Browse the repository at this point in the history
…ck (#272)

## 📜 Description

Fixed a conflict when using `fullScreenModal` presentation in
`native-stack`.

## 💡 Motivation and Context

`willMove(toWindow)` (with `newWindow == nil`) will be called in two
cases:
- view is unmounted;
- a navigation to modal window has occurred.

And we don't need to cleanup a listener if navigation has occurred. So
in this PR I'm switching to `willMove(toSuperview)` method that called
only when view is destroyed (when navigation occurs nothing happens).

Also I decided to unify initialization mechanism between architectures.
The view creation steps are following:
- init
- prop setters
- willMove(toSuperview)

On paper I did an initialization and mounting in `willMove` method
(setter was called on non initialized object). On fabric I initialized
in init method and mounted in prop setters (I didn't use lifecycle
methods in Fabric). However I think that ignoring lifecycle method and
keeping subscriptions is not a good idea (though events are not
propagated and there is no crashes) - potentially we may have bad memory
and other unpleasant exceptions.

So in this PR I decided to unify approach and now we're using next
schema:
- initialize observers in `init`;
- mount/unmount in prop setter;
- mount/unmount in lifecycle.

With such approach potentially on initial mount we can initialize
observer 2 times. To avoid that I've added `isMounted` property to
observers. So now they can be initialized only once (even if you call
`.mount` several times).

Last but not least - I've added private methods mount/unmount on view
level to reduce code duplication (now we have two observers, so one
method initialize both of them).

Closes
#271

## 📢 Changelog

### iOS
- switched from `willMove(toWindow)` to `willMove(toSuperview)`;

## 🤔 How Has This Been Tested?

Tested on iPhone 15 Pro (iOS 17.0.1).

## 📝 Checklist

- [x] CI successfully passed
  • Loading branch information
kirillzyusko committed Nov 14, 2023
1 parent ecd237f commit a8bd87b
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 22 deletions.
9 changes: 9 additions & 0 deletions ios/observers/FocusedInputLayoutObserver.swift
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ let noFocusedInputEvent: [String: Any] = [
public class FocusedInputLayoutObserver: NSObject {
// class members
var onEvent: (NSDictionary) -> Void
// state variables
private var isMounted = false
// input tracking
private var currentInput: UIView?
private var hasKVObserver = false
Expand All @@ -37,6 +39,12 @@ public class FocusedInputLayoutObserver: NSObject {
}

@objc public func mount() {
if isMounted {
return
}

isMounted = true

NotificationCenter.default.addObserver(
self,
selector: #selector(keyboardWillShow),
Expand All @@ -52,6 +60,7 @@ public class FocusedInputLayoutObserver: NSObject {
}

@objc public func unmount() {
isMounted = false
// swiftlint:disable:next notification_center_detachment
NotificationCenter.default.removeObserver(self)
}
Expand Down
8 changes: 8 additions & 0 deletions ios/observers/KeyboardMovementObserver.swift
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ public class KeyboardMovementObserver: NSObject {
private var prevKeyboardPosition = 0.0
private var displayLink: CADisplayLink?
private var hasKVObserver = false
private var isMounted = false
// state variables
private var keyboardHeight: CGFloat = 0.0
private var duration = 0
Expand All @@ -45,6 +46,12 @@ public class KeyboardMovementObserver: NSObject {
}

@objc public func mount() {
if isMounted {
return
}

isMounted = true

NotificationCenter.default.addObserver(
self,
selector: #selector(keyboardWillDisappear),
Expand Down Expand Up @@ -138,6 +145,7 @@ public class KeyboardMovementObserver: NSObject {
}

@objc public func unmount() {
isMounted = false
// swiftlint:disable:next notification_center_detachment
NotificationCenter.default.removeObserver(self)
}
Expand Down
27 changes: 23 additions & 4 deletions ios/views/KeyboardControllerView.mm
Original file line number Diff line number Diff line change
Expand Up @@ -156,17 +156,36 @@ - (void)updateProps:(Props::Shared const &)props oldProps:(Props::Shared const &

if (newViewProps.enabled != oldViewProps.enabled) {
if (newViewProps.enabled) {
[inputObserver mount];
[keyboardObserver mount];
[self mount];
} else {
[inputObserver unmount];
[keyboardObserver unmount];
[self unmount];
}
}

[super updateProps:props oldProps:oldProps];
}

- (void)willMoveToSuperview:(UIView *)newSuperview
{
if (newSuperview == nil) {
[self unmount];
} else {
[self mount];
}
}

- (void)mount
{
[inputObserver mount];
[keyboardObserver mount];
}

- (void)unmount
{
[inputObserver unmount];
[keyboardObserver unmount];
}

Class<RCTComponentViewProtocol> KeyboardControllerViewCls(void)
{
return KeyboardControllerView.class;
Expand Down
42 changes: 24 additions & 18 deletions ios/views/KeyboardControllerViewManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,9 @@ class KeyboardControllerView: UIView {
@objc var enabled: ObjCBool = true {
didSet {
if enabled.boolValue {
inputObserver?.mount()
keyboardObserver?.mount()
mount()
} else {
inputObserver?.unmount()
keyboardObserver?.unmount()
unmount()
}
}
}
Expand All @@ -37,28 +35,26 @@ class KeyboardControllerView: UIView {
self.bridge = bridge
eventDispatcher = bridge.eventDispatcher()
super.init(frame: frame)
inputObserver = FocusedInputLayoutObserver(handler: onInput)
keyboardObserver = KeyboardMovementObserver(handler: onEvent, onNotify: onNotify)
}

@available(*, unavailable)
required init?(coder _: NSCoder) {
fatalError("init(coder:) has not been implemented")
}

override func willMove(toWindow newWindow: UIWindow?) {
if newWindow == nil {
// Will be removed from a window
inputObserver?.unmount()
keyboardObserver?.unmount()
}
}
// for mounting/unmounting observers for lifecycle events we're using willMove(toSuperview) method
// not willMove(toWindow)
// see https://github.com/kirillzyusko/react-native-keyboard-controller/issues/271
override func willMove(toSuperview newSuperview: UIView?) {
super.willMove(toSuperview: newSuperview)

override func didMoveToWindow() {
if window != nil {
// Added to a window
inputObserver = FocusedInputLayoutObserver(handler: onInput)
inputObserver?.mount()
keyboardObserver = KeyboardMovementObserver(handler: onEvent, onNotify: onNotify)
keyboardObserver?.mount()
if newSuperview == nil {
// The view is about to be removed from its superview (destroyed)
unmount()
} else {
mount()
}
}

Expand Down Expand Up @@ -90,4 +86,14 @@ class KeyboardControllerView: UIView {
func onNotify(event: String, data: Any) {
KeyboardController.shared()?.sendEvent(event, body: data)
}

private func mount() {
inputObserver?.mount()
keyboardObserver?.mount()
}

private func unmount() {
inputObserver?.unmount()
keyboardObserver?.unmount()
}
}

0 comments on commit a8bd87b

Please sign in to comment.