Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix cursor not showing when tabbing into multiline TextInput #1342

Merged

Conversation

christophpurrer
Copy link

Please select one of the following

  • I am removing an existing difference between facebook/react-native and microsoft/react-native-macos 👍
  • I am cherry-picking a change from Facebook's react-native into microsoft/react-native-macos 👍
  • I am making a fix / change for the macOS implementation of react-native
  • I am making a change required for Microsoft usage of react-native

Summary

This bug happens because RCTUITextView (which multiline TextInput uses) was overriding NSTextView's becomeFirstResponder method and didn't call [super becomeFirstResponder].
This seems to mess with AppKit's logic of drawing the cursor initially.

This is alluded to in the docs for [NSTextView becomeFirstResponder]:

If the previous first responder was not a text view on the same layout manager as the receiving text view, this method draws the selection and updates the insertion point if necessary.

Simply switching to call [super becomeFirstResponder] led to a cryptic exception within AppKit. This was likely because in the reactFocus (and reactFocusIfNeeded) we were calling becomeFirstResponder directly.
The docs for [NSResponder becomeFirstResponder] say:

Use the NSWindow makeFirstResponder: method, not this method, to make an object the first responder. Never invoke this method directly.

There necessary fixes are already part of 84c0863#diff-8a3ec110f2ee884d2ef078efddc4ebba9413c7e8cb90b6f5cebff19b2c0a1951 and hence not included in this diff

#if TARGET_OS_OSX // [TODO(macOS GH#774). 
	if (![[self window] makeFirstResponder:self]) {
#else
// ...
}

- (void)reactFocusIfNeeded
{
	if (self.reactIsFocusNeeded) {
#if TARGET_OS_OSX // [TODO(macOS GH#774)
		if ([[self window] makeFirstResponder:self]) {
#else
// ...
}

Changelog

[macOS] [Fixed] - Fix cursor not showing when tabbing into multiline TextInput

Test Plan

Using this example

            <>
              <View
                focusable={true}
                style={styles.row}
                validKeysDown={['g', 'Escape', 'Enter', 'ArrowLeft']}
                onKeyDown={this.onKeyDownEvent}
                validKeysUp={['c', 'd']}
                onKeyUp={this.onKeyUpEvent}></View>
              <TextInput
                blurOnSubmit={false}
                placeholder={'Singleline textInput'}
                multiline={false}
                focusable={true}
                style={styles.row}
                validKeysDown={['ArrowRight', 'ArrowDown']}
                onKeyDown={this.onKeyDownEvent}
                validKeysUp={['Escape', 'Enter']}
                onKeyUp={this.onKeyUpEvent}
              />
              <TextInput
                placeholder={'Multiline textInput'}
                multiline={true}
                focusable={true}
                style={styles.row}
                validKeysDown={['ArrowLeft', 'ArrowUp']}
                onKeyDown={this.onKeyDownEvent}
                validKeysUp={['Escape', 'Enter']}
                onKeyUp={this.onKeyUpEvent}
              />
            </>

Before

before.mov

After

after.mov

@christophpurrer christophpurrer requested a review from a team as a code owner August 9, 2022 01:05
This bug happens because `RCTUITextView` (which multiline TextInput uses) was overriding NSTextView's `becomeFirstResponder` method and didn't call `[super becomeFirstResponder]`.
This seems to mess with AppKit's logic of drawing the cursor initially.

This is alluded to in the [docs](https://developer.apple.com/documentation/appkit/nstextview/1807130-becomefirstresponder?language=objc#) for `[NSTextView becomeFirstResponder]`:

> If the previous first responder was not a text view on the same layout manager as the receiving text view, this method draws the selection and updates the insertion point if necessary.

Simply switching to call `[super becomeFirstResponder]` led to a cryptic exception within AppKit. This was likely because in the `reactFocus` (and `reactFocusIfNeeded`) we were calling `becomeFirstResponder` directly.
The [docs](https://developer.apple.com/documentation/appkit/nsresponder/1526750-becomefirstresponder?language=objc#) for `[NSResponder becomeFirstResponder]` say:

> Use the NSWindow makeFirstResponder: method, not this method, to make an object the first responder. Never invoke this method directly.

This fixed the issue
@christophpurrer christophpurrer force-pushed the fixCursorTabbingIntoMultilineText branch from ac0ec7a to 87afcb7 Compare August 9, 2022 01:09
@Saadnajmi
Copy link
Collaborator

Thanks! Glad my changes are kind of "followed up" here

@Saadnajmi Saadnajmi merged commit 8c5d0ec into microsoft:main Aug 9, 2022
@christophpurrer christophpurrer deleted the fixCursorTabbingIntoMultilineText branch August 9, 2022 23:30
shwanton pushed a commit to shwanton/react-native-macos that referenced this pull request Feb 13, 2023
…ft#1342)

This bug happens because `RCTUITextView` (which multiline TextInput uses) was overriding NSTextView's `becomeFirstResponder` method and didn't call `[super becomeFirstResponder]`.
This seems to mess with AppKit's logic of drawing the cursor initially.

This is alluded to in the [docs](https://developer.apple.com/documentation/appkit/nstextview/1807130-becomefirstresponder?language=objc#) for `[NSTextView becomeFirstResponder]`:

> If the previous first responder was not a text view on the same layout manager as the receiving text view, this method draws the selection and updates the insertion point if necessary.

Simply switching to call `[super becomeFirstResponder]` led to a cryptic exception within AppKit. This was likely because in the `reactFocus` (and `reactFocusIfNeeded`) we were calling `becomeFirstResponder` directly.
The [docs](https://developer.apple.com/documentation/appkit/nsresponder/1526750-becomefirstresponder?language=objc#) for `[NSResponder becomeFirstResponder]` say:

> Use the NSWindow makeFirstResponder: method, not this method, to make an object the first responder. Never invoke this method directly.

This fixed the issue

Co-authored-by: Liron Yahdav <lyahdav@fb.com>
# Conflicts:
#	React/Views/UIView+React.m
shwanton pushed a commit to shwanton/react-native-macos that referenced this pull request Mar 10, 2023
…ft#1342)

This bug happens because `RCTUITextView` (which multiline TextInput uses) was overriding NSTextView's `becomeFirstResponder` method and didn't call `[super becomeFirstResponder]`.
This seems to mess with AppKit's logic of drawing the cursor initially.

This is alluded to in the [docs](https://developer.apple.com/documentation/appkit/nstextview/1807130-becomefirstresponder?language=objc#) for `[NSTextView becomeFirstResponder]`:

> If the previous first responder was not a text view on the same layout manager as the receiving text view, this method draws the selection and updates the insertion point if necessary.

Simply switching to call `[super becomeFirstResponder]` led to a cryptic exception within AppKit. This was likely because in the `reactFocus` (and `reactFocusIfNeeded`) we were calling `becomeFirstResponder` directly.
The [docs](https://developer.apple.com/documentation/appkit/nsresponder/1526750-becomefirstresponder?language=objc#) for `[NSResponder becomeFirstResponder]` say:

> Use the NSWindow makeFirstResponder: method, not this method, to make an object the first responder. Never invoke this method directly.

This fixed the issue

Co-authored-by: Liron Yahdav <lyahdav@fb.com>
# Conflicts:
#	React/Views/UIView+React.m
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants