Skip to content

Commit

Permalink
refactor: onChangeText from delegate (#435)
Browse files Browse the repository at this point in the history
## 📜 Description

Consume `onChangeText` from delegate level.

## 💡 Motivation and Context

To make some of functionality properly working in reach RN ecosystem you
had to apply patch to `react-native-text-input-mask`.

However it makes it complicated in terms of setup so I always thought on
how to improve it. In
#426
we started to inject our delegate to intercept some `TextInput` events.
So now we can intercept necessary events without event-bus subscription
(we can take them directly from the delegate).

In this PR I re-worked this part (removed `TextInputObserver` class) and
started to use delegate for these purposes. Also I've discovered again
that we may substitute back delegate and then substitute our own again
(when user typed first letter) - it's not critical but it doesn't comply
with keyboard-controller delegate paradigm - we should set delegate only
once (on focus) and unset it on un-focus. So I also re-worked this part
and now, when we're trying to delete our observers - we check, that we
are actually move focused to another input.

## 📢 Changelog

### Docs

- removed **Known issues** from `useFocusedInputHandler` hook;

### E2E

- compare text between `worklet` and `formatted`;

### iOS

- substitute delegate only one time on focus;
- handle text changes on delegate level;

## 🤔 How Has This Been Tested?

Tested on CI and iPhone 15 Pro manually.

## 📸 Screenshots (if appropriate):

No visual changes.

## 📝 Checklist

- [x] CI successfully passed
- [x] I added new mocks and corresponding unit-tests if library API was
changed
  • Loading branch information
kirillzyusko committed May 10, 2024
1 parent f2ea4fe commit 427c26c
Show file tree
Hide file tree
Showing 7 changed files with 59 additions and 113 deletions.
3 changes: 2 additions & 1 deletion cspell.json
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,8 @@
"xcworkspace",
"ccache",
"ivar",
"objc"
"objc",
"QWERT"
],
"ignorePaths": [
"node_modules",
Expand Down
4 changes: 0 additions & 4 deletions docs/docs/api/hooks/input/use-focused-input-handler.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,3 @@ type FocusedInputSelectionChangedEvent = {
```

This handler can be handy when you need to have an access to input on a global level (i. e. when you don't have a direct access to your `TextInput`) or if you need to have an access to coordinates of text selection (for example to draw a custom element that follows caret position).

## Known issues

- [react-native-text-input-mask#305](https://github.com/react-native-text-input-mask/react-native-text-input-mask/pull/305): `onChangeText` handler ignores an input from `react-native-text-input-mask` on `iOS`
24 changes: 16 additions & 8 deletions e2e/kit/003-input-handlers.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ describe("input handlers functionality", () => {
await waitAndTap("focused_input_handlers");
await waitForElementById("masked_input");
await waitAndTap("masked_input");
await delay(500); // for keyboard to appear
// make sure keyboard is shown
await delay(500);
await waitAndType("masked_input", "1234567890");
await expect(element(by.id("formatted_text"))).toHaveText(
"Formatted: +1 (123) 456 78 90",
Expand All @@ -31,16 +32,15 @@ describe("input handlers functionality", () => {
});

it("should fire `onSelectionChange` with expected values", async () => {
await expect(element(by.id("selection_text_start_end"))).toHaveText(
"start: 18, end: 18",
);
await expect(
element(by.id("original_selection_text_start_end")),
).toHaveText("start: 18, end: 18");

await waitAndTap("multiline_input");
await waitAndType("multiline_input", "QWERTY\nqwerty");

await expect(element(by.id("formatted_text"))).toHaveText(
"Formatted: QWERTY\nqwerty",
);
await expect(element(by.id("worklet_text"))).toHaveText(
"Worklet: QWERTY\nqwerty",
);
await expect(element(by.id("selection_text_start_end"))).toHaveText(
"start: 13, end: 13",
);
Expand All @@ -51,6 +51,10 @@ describe("input handlers functionality", () => {
await element(by.id("multiline_input")).clearText();
await waitAndType("multiline_input", "QWERTY");

await expect(element(by.id("formatted_text"))).toHaveText(
"Formatted: QWERTY",
);
await expect(element(by.id("worklet_text"))).toHaveText("Worklet: QWERTY");
await expect(element(by.id("selection_text_start_end"))).toHaveText(
"start: 6, end: 6",
);
Expand All @@ -60,6 +64,10 @@ describe("input handlers functionality", () => {

await element(by.id("multiline_input")).tapBackspaceKey();

await expect(element(by.id("formatted_text"))).toHaveText(
"Formatted: QWERT",
);
await expect(element(by.id("worklet_text"))).toHaveText("Worklet: QWERT");
await expect(element(by.id("selection_text_start_end"))).toHaveText(
"start: 5, end: 5",
);
Expand Down
45 changes: 0 additions & 45 deletions example/patches/react-native-text-input-mask+3.1.5.patch

This file was deleted.

27 changes: 26 additions & 1 deletion ios/delegates/KCTextInputCompositeDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,17 @@ func textSelection(in textInput: UITextInput) -> Selection? {
class KCTextInputCompositeDelegate: NSObject, UITextViewDelegate, UITextFieldDelegate {
// constructor members
var onSelectionChange: (_ event: NSDictionary) -> Void
var onTextChange: (_ text: String?) -> Void
// delegates
weak var textViewDelegate: UITextViewDelegate?
weak var textFieldDelegate: UITextFieldDelegate?

public init(onSelectionChange: @escaping (_ event: NSDictionary) -> Void) {
public init(
onSelectionChange: @escaping (_ event: NSDictionary) -> Void,
onTextChange: @escaping (_ text: String?) -> Void
) {
self.onSelectionChange = onSelectionChange
self.onTextChange = onTextChange
}

// MARK: setters/getters
Expand All @@ -77,13 +82,33 @@ class KCTextInputCompositeDelegate: NSObject, UITextViewDelegate, UITextFieldDel
updateSelectionPosition(textInput: textView)
}

func textViewDidChange(_ textView: UITextView) {
defer {
self.onTextChange(textView.text)
}

textViewDelegate?.textViewDidChange?(textView)
}

// MARK: UITextFieldDelegate

func textFieldDidChangeSelection(_ textField: UITextField) {
textFieldDelegate?.textFieldDidChangeSelection?(textField)
updateSelectionPosition(textInput: textField)
}

func textField(
_ textField: UITextField,
shouldChangeCharactersIn range: NSRange,
replacementString string: String
) -> Bool {
defer {
self.onTextChange(textField.text)
}

return textFieldDelegate?.textField?(textField, shouldChangeCharactersIn: range, replacementString: string) ?? true
}

// MARK: call forwarding

override func responds(to aSelector: Selector!) -> Bool {
Expand Down
28 changes: 15 additions & 13 deletions ios/observers/FocusedInputObserver.swift
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,6 @@ public class FocusedInputObserver: NSObject {
private var currentResponder: UIView?
private var hasObservers = false
private var lastEventDispatched: [AnyHashable: Any] = noFocusedInputEvent
// observers
private let textChangeObserver = TextChangeObserver()

@objc public init(
onLayoutChangedHandler: @escaping (NSDictionary) -> Void,
Expand All @@ -54,14 +52,19 @@ public class FocusedInputObserver: NSObject {
self.onFocusDidSet = onFocusDidSet

// Temporary initialization of the delegate with an empty closure
delegate = KCTextInputCompositeDelegate(onSelectionChange: { _ in })
delegate = KCTextInputCompositeDelegate(onSelectionChange: { _ in }, onTextChange: { _ in })

super.init()

// Initialize the delegate
delegate = KCTextInputCompositeDelegate(onSelectionChange: { [weak self] event in
self?.onSelectionChange(event)
})
delegate = KCTextInputCompositeDelegate(
onSelectionChange: { [weak self] event in
self?.onSelectionChange(event)
},
onTextChange: { [weak self] text in
self?.onTextChanged(text: text)
}
)
}

@objc public func mount() {
Expand Down Expand Up @@ -92,8 +95,9 @@ public class FocusedInputObserver: NSObject {
}

@objc func keyboardWillShow(_: Notification) {
removeObservers()
currentResponder = UIResponder.current as? UIView
let responder = UIResponder.current as? UIView
removeObservers(newResponder: responder)
currentResponder = responder
currentInput = currentResponder?.superview as UIView?

setupObservers()
Expand All @@ -111,7 +115,7 @@ public class FocusedInputObserver: NSObject {
}

@objc func keyboardWillHide(_: Notification) {
removeObservers()
removeObservers(newResponder: nil)
currentInput = nil
currentResponder = nil
dispatchEventToJS(data: noFocusedInputEvent)
Expand Down Expand Up @@ -163,20 +167,18 @@ public class FocusedInputObserver: NSObject {
if currentInput != nil {
hasObservers = true
currentInput?.addObserver(self, forKeyPath: "center", options: .new, context: nil)
textChangeObserver.observeTextChanges(for: currentResponder, handler: onTextChanged)

substituteDelegate(currentResponder)
}
}

private func removeObservers() {
if !hasObservers {
private func removeObservers(newResponder: UIResponder?) {
if newResponder == currentResponder || !hasObservers {
return
}

hasObservers = false
currentInput?.removeObserver(self, forKeyPath: "center", context: nil)
textChangeObserver.removeObserver()

substituteDelegateBack(currentResponder)
}
Expand Down
41 changes: 0 additions & 41 deletions ios/observers/TextChangeObserver.swift

This file was deleted.

0 comments on commit 427c26c

Please sign in to comment.