Cursor position

[perunt]

Upstream PR Link

facebook#36979
Expensify/App#16078

Summary

This PR extends the onSelectionChange method on textInput to return the x and y position of the input caret

Changelog

In addition to the start and end indices of the selected text onSelectionChange returns also positionY and positionX position of the input caret. The new object returned by the method looks like this:

  selection: {
    start: number,
    end: number,
    positionY: number,
    positionX: number
  }
}

telegram-cloud-document-2-5337254171093513879.mp4

positionX and positionY represent the coordinates inside the text input area, not including padding or margin. If you add a margin, you should add that extra space when using these coords to draw stuff on the text input.

[CATEGORY] [TYPE] - Message

Test Plan

added console.log statements to the onSelectionChange method to print the positionY and positionX values of the input caret. This allows for easy verification of the correct functioning of the new functionality added by this PR.

[github-actions]

Fails

🚫

📋 Verify Changelog Format - A changelog entry has the following format: [CATEGORY] [TYPE] - Message. CATEGORY may be: General iOS Android JavaScript Internal (for changes that do not need to be called out in the release notes) TYPE may be: Added, for new features. Changed, for changes in existing functionality. Deprecated, for soon-to-be removed features. Removed, for now removed features. Fixed, for any bug fixes. Security, in case of vulnerabilities. MESSAGE may answer "what and why" on a feature level. Use this to briefly tell React Native users about notable changes.

Generated by 🚫 dangerJS against b9c8dfb

[mountiny]

The code looks good to me however, can we test this first? Could you build app against this commit/branch of our fork and test it there directly? Not sure how this testing was done before cc @roryabraham

[roryabraham]

@perunt It looks like this is missing an upstream PR.

For every change we make in our React Native fork, it's very important that we have:

1. An issue in the upstream React Native repo describing the problem we want to solve.
2. An upstream PR in the React Native repo that's essentially a duplicate of the PR we have in our fork.

Furthermore, we should try to get that upstream PR reviewed and merged before we merge changes in our fork, and only fall back on using our fork when that fails because review from Meta is taking too long.

[roryabraham]

Also, new changes need to target the ExpensifyRC1-0.72.0-alpha.0 branch

[perunt]

@mountiny, I can place an example in the example folder, allowing you to test it using RNTester.

[perunt]

@roryabraham Alright, I'll work on creating PR to RN. However, I'm not certain how quickly the RN team will respond. If this PR is held up for 2-6 months, would that also delay the implementation of this feature in our project for the same duration?

[mountiny]

Alright, I'll work on creating PR to RN. However, I'm not certain how quickly the RN team will respond. If this PR is held up for 2-6 months, would that also delay the implementation of this feature in our project for the same duration?

No we would most likely not, but we want the issue up so we can drive towards updating this upstream too so we dont have to maintain the diff in our repo forever.

[perunt]

I observed that in version 0.71.6 of React Native, the mReactEditText variable can be null, so this is a hotfix to address that issue

[mountiny]

I think this is looking great now, thanks for updating the base version.

@roryabraham all yours

[puneetlath]

@roryabraham would you like to review this as well?

[roryabraham]

Just a few very minor points. Otherwise LGTM

[mountiny]

I think we are good to merge now.