iOS - TextInput - cursor jumps to the end of input when secureTextEntry is toggled #38678

pac-guerreiro · 2023-07-28T22:04:33Z

Summary:

In iOS, when toggling secureTextEntry, the cursor position of TextInput should remain in the same position like it happens in Android instead of jumping to the end of the input.

This issue resolves #38676

Changelog:

[IOS] [FIXED] - Cursor jumps to the end of TextInput when secureTextEntry is toggled

Test Plan:

Screen.Recording.2023-07-28.at.22.01.02.mov

facebook-github-bot · 2023-07-28T22:04:37Z

Hi @pac-guerreiro!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks!

…Entry is toggled

analysis-bot · 2023-07-28T22:36:09Z

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	8,885,496	-8,805
android	hermes	armeabi-v7a	7,935,863	-7,009
android	hermes	x86	9,282,121	-10,081
android	hermes	x86_64	9,185,606	-8,130
android	jsc	arm64-v8a	9,473,478	-7,478
android	jsc	armeabi-v7a	8,416,827	-5,694
android	jsc	x86	9,456,228	-8,757
android	jsc	x86_64	9,772,437	-6,804

Base commit: 1f24750
Branch: main

facebook-github-bot · 2023-07-28T23:09:06Z

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

yungsters · 2023-08-01T18:27:45Z

packages/react-native/Libraries/Components/TextInput/TextInput.js

@@ -1206,6 +1206,24 @@ function InternalTextInput(props: Props): React.Node {
    }
  }, [inputRef]);

+  useLayoutEffect(() => {
+    if (inputRef.current != null) {
+      viewCommands.setTextAndSelection(


With this effect, viewCommands.setTextAndSelection is going to be invoked whenever any of the dependencies (e.g. lastNativeSelection, etc.) change — not only when props.secureTextEntry changes.

I think you need a ref that tracks the value of props.secureTextEntry the last time that this effect was invoked, and only invoke this if that prop has changed.

Good point @yungsters !

I'm going to apply your suggestion 😉

@yungsters

It is done 😄

facebook-github-bot · 2023-08-02T15:42:31Z

@yungsters has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

yungsters · 2023-08-02T21:14:40Z

packages/react-native/Libraries/Components/TextInput/TextInput.js

+      viewCommands.setTextAndSelection(
+        inputRef.current,
+        mostRecentEventCount,
+        lastNativeText,


Hmm… Flow is unhappy with this because lastNativeText is ?string (but the argument must be string).

How did you decide between using lastNativeText (and lastNativeSelection) instead of text (and selection)?

Given that this layout effect is invoked after the one online 1152, it seems like we should actually maybe using the latter. Thoughts?

Perhaps you're right, my idea was just to preserve the actual state based on the lastNativeText and lastNativeSelection, which are internal while text and selection are external and are optional.

The more I think about it, the more I wonder whether this logic should actually be incorporated into the layout effect on line 1151. If props.secureTextEntry and text both change in the same update, we would end up invoking viewCommands.setTextAndSelection twice (and as currently written, with different values — once with text and once with lastNativeText).

What do you think about merging this into that layout effect by doing the following?

Include props.secureTextEntry into the earlier layout effect's dependencies array.

Adding the prevSecureTextEntry.current !== props.secureTextEntry check to line 1169.

This proposed change would also eliminate the question of whether we should be calling with text or lastNativeText.

@yungsters

Sorry for the delay, I got caught in some work 😅

I've been doing some thinking and I'm not sure if what you're suggesting will do it because what I want to achieve is to enforce the previous component state into the native component.

I'm not sure how TextInput should behave in these situations, but what makes sense to me is:

Native component gets updated

React component receives native state update and checks if it differs from props (controlled component)
2.1. If there are no text and/or selection props, use react component state instead
2.2. If it differs, update react component state and enforce props into native state
2.3. If it's equal, then don't do anything

This way, in controlled and uncontrolled situations, the native component should always reflect either the props or the react component state.

Let me know what you think 😄

@yungsters bump

pac-guerreiro · 2023-11-20T11:03:02Z

Bump

pac-guerreiro · 2023-11-27T16:49:19Z

Bump

pac-guerreiro · 2023-12-07T14:18:22Z

Bump

github-actions · 2024-06-05T05:01:00Z

This PR is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days.

github-actions · 2024-06-12T05:13:37Z

This PR was closed because it has been stalled for 7 days with no activity.

pac-guerreiro mentioned this pull request Jul 28, 2023

[$1000] Chat - Tapping eye icon for protected pdf file moves cursor to the start of the field Expensify/App#21727

Closed

6 tasks

fix(iOS)[TextInput]: cursor jumps to the end of input when secureText…

a6434ef

…Entry is toggled

pac-guerreiro force-pushed the fix/pac-guerreiro/ios-textinput-cursor-jumps-to-end-on-securetextentry-toggle branch from cd873e0 to a6434ef Compare July 28, 2023 22:12

facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Jul 28, 2023

style: apply prettier

f8fbb5c

yungsters requested changes Aug 1, 2023

View reviewed changes

perf(ios)[TextInput]: prevent unnecessary re-renders

12ee9ae

pac-guerreiro force-pushed the fix/pac-guerreiro/ios-textinput-cursor-jumps-to-end-on-securetextentry-toggle branch from a12fed6 to 12ee9ae Compare August 2, 2023 13:58

yungsters reviewed Aug 2, 2023

View reviewed changes

github-actions bot added the Stale There has been a lack of activity on this issue and it may be closed soon. label Jun 5, 2024

github-actions bot closed this Jun 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

iOS - TextInput - cursor jumps to the end of input when secureTextEntry is toggled #38678

iOS - TextInput - cursor jumps to the end of input when secureTextEntry is toggled #38678

pac-guerreiro commented Jul 28, 2023

facebook-github-bot commented Jul 28, 2023

analysis-bot commented Jul 28, 2023 •

edited

Loading

facebook-github-bot commented Jul 28, 2023

yungsters Aug 1, 2023

pac-guerreiro Aug 2, 2023

pac-guerreiro Aug 2, 2023

facebook-github-bot commented Aug 2, 2023

yungsters Aug 2, 2023

pac-guerreiro Aug 3, 2023

yungsters Aug 3, 2023

pac-guerreiro Aug 9, 2023

pac-guerreiro Aug 29, 2023

pac-guerreiro commented Nov 20, 2023

pac-guerreiro commented Nov 27, 2023

pac-guerreiro commented Dec 7, 2023

github-actions bot commented Jun 5, 2024

github-actions bot commented Jun 12, 2024

iOS - TextInput - cursor jumps to the end of input when secureTextEntry is toggled #38678

iOS - TextInput - cursor jumps to the end of input when secureTextEntry is toggled #38678

Conversation

pac-guerreiro commented Jul 28, 2023

Summary:

Changelog:

Test Plan:

facebook-github-bot commented Jul 28, 2023

Action Required

Process

analysis-bot commented Jul 28, 2023 • edited Loading

facebook-github-bot commented Jul 28, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

facebook-github-bot commented Aug 2, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pac-guerreiro commented Nov 20, 2023

pac-guerreiro commented Nov 27, 2023

pac-guerreiro commented Dec 7, 2023

github-actions bot commented Jun 5, 2024

github-actions bot commented Jun 12, 2024

analysis-bot commented Jul 28, 2023 •

edited

Loading