Skip to content

Conversation

shwanton
Copy link

@shwanton shwanton commented Nov 18, 2022

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

TextInput clearTextOnFocus and selectTextOnFocus were never supported on MacOS
https://reactnative.dev/docs/textinput#selecttextonfocus
https://reactnative.dev/docs/textinput#cleartextonfocus-ios

https://github.com/microsoft/react-native-macos/blame/e0e598e19c39bd7cb80b2aefd0ac5f19778721aa/Libraries/Text/TextInput/RCTBaseTextInputViewManager.m#LL62

On iOS, selectTextOnFocus has a custom implementation of selectAll that dispatched a selection.
https://github.com/facebook/react-native/blob/main/Libraries/Text/TextInput/Multiline/RCTUITextView.m#L181-L190
For NSTextInput, selectAll works without needing the custom dispatch.

For NSTextField, the default behavior is to select all when focused. This change added support for removing the selection if selectTextOnFocus is false.

NOTE: Text selection disappears once a MouseDown is received. This is native text input behavior.

This also fixes a bug w/ MultiLineTextInput where the selection box was retained even after losing focus.

CleanShot.2022-11-18.at.12.25.45.mp4

Changelog

[macOS] [Fixed] - TextInput support for clearTextOnFocus and selectTextOnFocus

Test Plan

CleanShot.2022-11-18.at.12.15.30.mp4

@shwanton shwanton marked this pull request as ready for review November 18, 2022 20:26
@shwanton shwanton requested a review from a team as a code owner November 18, 2022 20:26
@shwanton
Copy link
Author

Need to fix build error.

CleanShot 2022-11-22 at 15 16 28

@shwanton
Copy link
Author

Oops, forgot that this still needs to support iOS as well. Will add back the older path and test.

@shwanton
Copy link
Author

Fixed on iOS

CleanShot.2022-11-22.at.16.52.07.mp4

@amgleitman
Copy link
Member

I don't see anything regarding clearTextOnFocus except enabling it for macOS in RCTBaseTextInputViewManager.m. If this prop just worked as is, can you explicitly mention that?

@chiuam
Copy link

chiuam commented Dec 13, 2022

Pulled your branch and tested the props, selectTextOnFocus worked fine on iOS, but not on macOS :/

@shwanton
Copy link
Author

shwanton commented Dec 16, 2022

Pulled your branch and tested the props, selectTextOnFocus worked fine on iOS, but not on macOS :/

There is some nuance to how this works on desktop. Since the text input can get focus when clicked, the selection will also disappear if you click into it.

Slowed down example:
https://user-images.githubusercontent.com/96719/207995918-f9217241-6cee-4ace-96be-8725e3e8b691.mp4

If you tab into either single or multiline text input, it will select the text. Note, you have to reverse-tab to select the multiline text field since the previous text input will capture additional tabs

CleanShot.2022-12-15.at.16.31.33.mp4

Clicking on the view, but not focusing the TextInput will also select the text.

CleanShot.2022-12-15.at.16.32.45.mp4

@ghost ghost removed the Needs: Author Feedback label Dec 16, 2022
@analysis-bot
Copy link

analysis-bot commented Dec 16, 2022

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: f16348c
Branch: main

@Saadnajmi
Copy link
Collaborator

@shwanton by the way, you can use Ctrl+Tab to tab in / out of text fields that capture tab

Copy link
Collaborator

@Saadnajmi Saadnajmi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apart from tags, LGTM!

@Saadnajmi Saadnajmi merged commit 71e7ccf into microsoft:main Dec 28, 2022
@shwanton shwanton deleted the textview-select-and-clear-all branch December 28, 2022 19:53
shwanton added a commit to shwanton/react-native-macos that referenced this pull request Feb 13, 2023
…rosoft#1517)

* Enable `selectTextOnFocus` and `clearTextOnFocus` for TextInput

* Fix select all for multiline textview

* Only reset selected range for macOS

* Don't run else branch for iOS

* Fix tags

Co-authored-by: Shawn Dempsey <shawndempsey@fb.com>
Co-authored-by: chiuam <67026167+chiuam@users.noreply.github.com>
Co-authored-by: Saad Najmi <sanajmi@microsoft.com>
shwanton pushed a commit to shwanton/react-native-macos that referenced this pull request May 1, 2024
Summary:
microsoft#1517 aligned the behavior of text selection for TextInput on macOS with iOS by clearing the selection by default on blur/focus.

On native desktop applications, the default behavior for text fields is to maintain the text selection state between blur/focus events so that they shouldn't be programmatically saved and restored.

This diff removes the text selection clearing to implement the default desktop behavior that diverges from iOS.

Test Plan:
Selection range:
- Enter text in a text field
- Select part of the text
- Make another text field key
- Make the previous text field key again
- The selection should be maintained while blurred and after focusing the field back

Cursor position:
- Place the cursor at a specific location in the text
- Make another text field key
- Make the previous text field key again
- The cursor should be at the same location as set before

Without the fix, the selection gets cleared:
https://pxl.cl/34Z4X

With the fix, the selection is maintained:
https://pxl.cl/34Z56

Reviewers: shawndempsey, ericroz, chpurrer, vzaidman, #rn-desktop, #zeratul, #messenger_desktop_experience

Reviewed By: ericroz

Differential Revision: https://phabricator.intern.facebook.com/D47922029

Tasks: T159618521, T159224566
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.

5 participants