Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Various post-0.63-merge TODOs #631

Merged
merged 9 commits into from
Oct 29, 2020
Merged

Various post-0.63-merge TODOs #631

merged 9 commits into from
Oct 29, 2020

Conversation

alloy
Copy link
Member

@alloy alloy commented Oct 5, 2020

Related to #621

⚠️ This should not be squashed when merged.

Microsoft Reviewers: Open in CodeFlow

@alloy alloy self-assigned this Oct 5, 2020
@alloy alloy mentioned this pull request Oct 5, 2020
25 tasks
@alloy alloy force-pushed the post-0.63-merge-todos branch 2 times, most recently from e030009 to e14757f Compare October 8, 2020 13:51
#else // [TODO(macOS ISS#2323203)
NSInteger startPosition = MIN(start, end);
NSInteger endPosition = MAX(start, end);
[self.backedTextInputView setSelectedTextRange:NSMakeRange(startPosition, endPosition - startPosition) notifyDelegate:NO];
Copy link
Member Author

Choose a reason for hiding this comment

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

There's no bounds checking here, so this would raise a objc exception.

@alloy alloy requested a review from tom-un October 27, 2020 09:58
@alloy alloy marked this pull request as ready for review October 27, 2020 09:58
@alloy
Copy link
Member Author

alloy commented Oct 27, 2020

@tom-un @HeyImChris This is ready for review. For the remaining TODO (LogBox window support) I’ll create a separate PR.

@alloy alloy requested a review from HeyImChris October 27, 2020 17:25
Libraries/Text/RCTTextAttributes.m Show resolved Hide resolved
RNTester/js/RNTesterApp.ios.js Show resolved Hide resolved
@alloy alloy merged commit 46b4eb6 into master Oct 29, 2020
@alloy alloy deleted the post-0.63-merge-todos branch October 29, 2020 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants