feat: [iOS] add caretHeight and caretYoffset to TextInput component by OlimpiaZurek · Pull Request #37147 · react/react-native

OlimpiaZurek · 2023-04-28T11:57:56Z

Summary:

This PR adds support for the caret height and caret position properties in iOS multi-line TextInput
PR for the docs update: react/react-native-website#3709

Changelog:

[IOS] [ADDED] - Add caretHeight and caretYOffset props to TextInput component

Test Plan:

Default caret:

default_caret.mov

Caret with caretHeight and caretYOffsetadjustment:

adjusting_caret.mov

facebook-github-bot · 2023-04-28T11:58:02Z

Hi @OlimpiaZurek!

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!

analysis-bot · 2023-04-28T12:22:36Z

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	18,000,725	+118,595
android	hermes	armeabi-v7a	n/a	--
android	hermes	x86	n/a	--
android	hermes	x86_64	n/a	--
android	jsc	arm64-v8a	21,359,082	+118,511
android	jsc	armeabi-v7a	n/a	--
android	jsc	x86	n/a	--
android	jsc	x86_64	n/a	--

Base commit: 12f2c3c
Branch: main

facebook-github-bot · 2023-04-28T13:08:55Z

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

fabioh8010

Hey @OlimpiaZurek ! I reviewed your code and left some comments, minor things I just noticed. Besides that, I have two other points:

Please include a Test Plan in your PR. For example, you could attach a video showing you how your props works in the text input for iOS.
Also create a PR to include your props in the react-native-website repo. You can link each issue to the other to keep track of the overall progress.

OlimpiaZurek · 2023-05-12T09:36:39Z

All comments have already been resolved. Can someone take another look and review? Thanks!

koko57 · 2023-06-16T12:10:46Z

Conflicts resolved, ready for review

sakluger · 2024-01-16T21:09:19Z

Hey @fabioh8010 - would it be possible to add this PR to your list of priorities? It's one of our oldest outstanding bugs and we'd love to get it resolved. Thanks!

fabioh8010 · 2024-01-17T09:53:08Z

Hi @sakluger , I don't work at Meta, I just reviewed because I'm from the same company of the PR author.

cpojer · 2024-01-30T10:21:52Z

@sammy-SC @yungsters could you merge this?

sammy-SC · 2024-02-04T19:58:06Z

why are there two underscores before member variables?

I used double underscores to avoid name collisions. Changed implementation a bit and removed them.

sammy-SC · 2024-02-04T19:59:32Z

what if only _caretYOffset is set? Shouldn't the below block be executed?

The block should only be executed if both properties are set.

sammy-SC · 2024-02-04T20:01:49Z

why is this override needed? I don't understand what it is trying to achieve on top of handling _caretYOffset and _caretHeight in the method caretRectForPosition above.

The goal is to avoid the cursor touching the the symbols of the previous line in the second and subsequent lines:
This is the default behavior derived from caretRectForPosition. To fix this, I added two new properties _caretYOffset and _caretHeight to calculate the caret position and caret height, thus preventing the cursor from overlapping the previous line of text.
To keep the same effect on selection selectionRectsForRange needs to be override as well.

Can you add a screenshot of what this looks like with a multi-line text selection?

Sure, here is a screenshot for multi-line text selection:

OlimpiaZurek · 2024-03-27T08:43:32Z

@sammy-SC All comments have been resolved. Can you take a look and review ? Thanks!
cc: @NickGerleman

javache · 2024-04-02T19:15:10Z

LGTM, few comments.

javache · 2024-04-02T19:10:40Z

+ return originalRect;
+}
+
+- (NSArray *)selectionRectsForRange:(UITextRange *)range {


Suggested change
- (NSArray *)selectionRectsForRange:(UITextRange *)range {
- (NSArray<UITextSelectionRect *> *)selectionRectsForRange:(UITextRange *)range {

javache · 2024-04-02T19:12:55Z

 #import <React/RCTTextAttributes.h>

+//the UITextSelectionRect subclass needs to be created because the original version is not writable
+@interface CustomTextSelectionRect : UITextSelectionRect


UITextSelectionRect is explicitly documented as being an abstract base class so this makes sense. Maybe mention that instead of it not being writable?
Use RCT as prefix
Suggested change
@interfaceCustomTextSelectionRect : UITextSelectionRect
@interfaceRCTTextSelectionRect : UITextSelectionRect

good point, changed

javache · 2024-04-02T19:12:59Z

Can you add a screenshot of what this looks like with a multi-line text selection?

javache · 2024-04-02T19:13:46Z

+
+- (NSArray *)selectionRectsForRange:(UITextRange *)range {
+ NSArray *superRects = [super selectionRectsForRange:range];
+ if(_caretYOffset != 0 && _caretHeight != 0) {


Should this be ||, so this works if you specify either one of the properties?

The block should only be executed if both properties are set.

javache · 2024-04-02T19:14:00Z

+ for (UITextSelectionRect *rect in superRects) {
+ CustomTextSelectionRect *customTextRect = [[CustomTextSelectionRect alloc] init];
+
+ customTextRect.rect = CGRectMake(rect.rect.origin.x, rect.rect.origin.y + _caretYOffset, rect.rect.size.width, _caretHeight);


This then needs to handle _caretHeight being potentially 0.

The block should only be executed if both properties are set.

react-native-bot · 2024-10-03T05:01:09Z

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.

react-native-bot · 2024-10-10T05:01:16Z

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

OlimpiaZurek mentioned this pull request Apr 28, 2023
[HOLD for payment 2024-06-20] [HOLD for payment 2024-06-18] [HOLD for payment 2024-06-06] [HOLD #37147 RN PR][$2000] iOS - Chat - Cursor in the second and following lines touches the symbols of the previous line Expensify/App#15640
Closed
6 tasks

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Apr 28, 2023

fabioh8010 reviewed Apr 28, 2023
View reviewed changes

OlimpiaZurek mentioned this pull request May 4, 2023
feat: add caretYOffset and caretHeight props to textinput.md react/react-native-website#3709
Closed

OlimpiaZurek changed the title ~~feat: add caretHeight and caretYoffset to TextInput component~~feat: [iOS] add caretHeight and caretYoffset to TextInput componentMay 4, 2023

OlimpiaZurek force-pushed the feature/text-input-ios-support-for-adjusting-caret branch from e265c13 to 2cbd5a3Compare May 4, 2023 09:55

OlimpiaZurek mentioned this pull request May 23, 2023
[iOS] add caretHeight and caretYoffset to TextInput component #37147 Expensify/react-native#58
Open

OlimpiaZurek force-pushed the feature/text-input-ios-support-for-adjusting-caret branch from caefbb8 to 5e48d26Compare May 29, 2023 10:23

sammy-SC requested changes Feb 4, 2024
View reviewed changes

OlimpiaZurek added 4 commits February 23, 2024 13:39

feat: add caretHeight and caretYoffset to TextInput component
e8e577e

minor changes after code review
148d162

feat: add caretHeight and caretYoffset to TextInput component
1f1d08b

minor changes after code review
f55a9ff

OlimpiaZurek force-pushed the feature/text-input-ios-support-for-adjusting-caret branch from bd5c769 to f55a9ffCompare February 23, 2024 12:44

remove double double underscores and fix error
f91c45f

facebook-github-bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label Apr 2, 2024

javache reviewed Apr 2, 2024
View reviewed changes

changes after cr
1f3fd22

OlimpiaZurek requested review from javache and sammy-SC April 5, 2024 11:09

react-native-bot added the Stale There has been a lack of activity on this issue and it may be closed soon. label Oct 3, 2024

react-native-bot closed this Oct 10, 2024

	- (NSArray )selectionRectsForRange:(UITextRange )range {
	- (NSArray<UITextSelectionRect > )selectionRectsForRange:(UITextRange *)range {

	@interfaceCustomTextSelectionRect : UITextSelectionRect
	@interfaceRCTTextSelectionRect : UITextSelectionRect

Uh oh!

Conversation

OlimpiaZurek commented Apr 28, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary:

Changelog:

Test Plan:

Uh oh!

facebook-github-bot commented Apr 28, 2023

Action Required

Process

Uh oh!

analysis-bot commented Apr 28, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

facebook-github-bot commented Apr 28, 2023

Uh oh!

fabioh8010 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

OlimpiaZurek commented May 12, 2023

Uh oh!

koko57 commented Jun 16, 2023

Uh oh!

sakluger commented Jan 16, 2024

Uh oh!

fabioh8010 commented Jan 17, 2024

Uh oh!

cpojer commented Jan 30, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

OlimpiaZurek commented Mar 27, 2024

Uh oh!

javache commented Apr 2, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

react-native-bot commented Oct 3, 2024

Uh oh!

OlimpiaZurek commented Apr 28, 2023•
edited
Loading

analysis-bot commented Apr 28, 2023•
edited
Loading