Adding cursor coordinates to TextInput

[perunt]

Summary:

This pull request aims to address the lack of information about the cursor (caret) position in the TextInput component. Currently, only the number of symbols where the cursor is located can be obtained(start and end cursor position), but this PR introduces a new property to track the cursor position in the x-y system coordinates.

Changelog:

[GENERAL] [ADDED] - To achieve this, a new property has been added to track the cursor position, and the initializer has been updated to accept it. The conversion method has also been modified to parse this property from JSON and pass it to the updated initializer. This allows the code to handle cursor position alongside text selection information.

With this PR, the onSelectionChange method will now return the cursor position alongside the start and end positions, as follows:

selection: {
    start: number,
    end: number,
    cursorPositionY: number,
    cursorPositionX: number
}

https://user-images.githubusercontent.com/48593211/233366575-c98a3486-8bd9-46a4-8915-a66b007c817e.mp4

Test Plan:

[facebook-github-bot]

Hi @perunt!

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 [email protected]. Thanks!

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	8,757,981	-256,628
android	hermes	armeabi-v7a	8,070,554	-205,283
android	hermes	x86	9,250,582	-277,742
android	hermes	x86_64	9,099,727	-270,992
android	jsc	arm64-v8a	9,319,506	-256,451
android	jsc	armeabi-v7a	8,509,417	-205,083
android	jsc	x86	9,383,000	-277,535
android	jsc	x86_64	9,636,240	-270,796

Base commit: 28c26dc30533180c6c0ac10836ea60d889a6114d Branch: main

[facebook-github-bot]

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

[perunt]

Now it's compiling successfully, and all tests are passing well. Please let me know if you notice any other issues.

[acoates-ms]

Can you add a RNTester page or add to an exisiting one, a usage of the new API? I dont see the new properties added to the type information in the JS? I'd expect to see an update to TextInput.js's Selection type, and a matching change in TextInput.d.ts.

[acoates-ms]

I suspect this also needs some changes in fabric to support the extra properties?

[NickGerleman]

Yes, I think previously the RN team had decided we will not be taking new API surface with only a Paper implementation, since these will stop working the moment we switch to Fabric as the default.

Though Android does reuse view managers on Fabric, and it looks like on iOS this might be some shared view between them, so this might just work. But we should be explicitly testing that.

[perunt]

Got it, let me address all this stuff

[PPatBoyd]

Would it make sense for the caret position to be a pair of points / rect? What point does the current XY represent?

[perunt]

The XY is just pointing to the top-left start of the selection right now. I get what you're saying - it might be a good move to add the end point too.

[acoates-ms]

The XY is just pointing to the top-left start of the selection right now. I get what you're saying - it might be a good move to add the end point too.

I'm not sure what you are doing with these coords, but many of the use cases I can think of would require having both topleft and bottom right coords. Want to show a menu at the cursor? - depending on if the menu is going to show above or below the cursor, you'd need both coords.

[perunt]

I'm not sure what you are doing with these coords, but many of the use cases I can think of would require having both topleft and bottom right coords. Want to show a menu at the cursor? - depending on if the menu is going to show above or below the cursor, you'd need both coords.

Yes, my goal is to use this for menu positioning. I've attempted using both coordinates as you've suggested, but encountered a bit of a hiccup when it came to selection. I've been considering whether it should just pinpoint to a single location instead. I can do the two-coordinates thing. Do you reckon I should do the same with the selection? I mean, if there were a few lines of selection, it would show the top left and bottom right corner coordinates,

[perunt]

I've made some updates. Now, we receive two points that represent the cursor position or the selection position. In the case of a selection, the start point represents the top left coordinate, and the end point signifies the bottom right coordinate of the selection. For representing the cursor, we use the same concept. The difference between this point is cursor width and height Also i tested it with Fabric and it works as well @NickGerleman will appreciate any thoughts on it, thanks!

[perunt]

I have not looked closely at the implementation for this yet, but the other questions I had were around the new API to allow explicitly controlling cursor position. I don't see this mentioned in your description, but I am curious how explicit control of cursor position interacts with character index based selection. Are these independent?

Thanks for the feedback! So, about that cursor position - It updates when the onSelectionChanged event fires and it doesn't refresh when we explicitly set the selection. Now, about the Fabric support, yep, I dropped the ball there. I was testing on a fork from the main and completely spaced on checking if changing the flag really turned Fabric. Good catch! I'm on it - will set up new PRs for Android and iOS Fabric support asap.

[perunt]

iOS - In this pr (https://github.com/facebook/react-native/pull/38110), I've added Fabric support and fixed the coordinates update when controlling cursor position explicitly.

Android - In this pr (https://github.com/facebook/react-native/pull/38123), It works with Fabric support and has the same updates for coordinates.

I've separated these for iOS and Android for better code review. However, I'm unsure how to handle the JavaScript portion as it's part of the example. @NickGerleman should I create separate pull requests for iOS fabric and paper?

[perunt]

@NickGerleman all necessary updates made in that PRs

iOS - In this pr (https://github.com/facebook/react-native/pull/38110), I've added Fabric support and fixed the coordinates update when controlling cursor position explicitly. Android - In this pr (https://github.com/facebook/react-native/pull/38123), It works with Fabric support and has the same updates for coordinates.

Current PR I'll leave for grouping this issue

[github-actions[bot]]

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[bot]]

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