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

fix(fabric, text): render Text in an NSTextView #2286

Merged
merged 3 commits into from
Nov 28, 2024

Conversation

Saadnajmi
Copy link
Collaborator

@Saadnajmi Saadnajmi commented Nov 20, 2024

Summary:

This is part of a series of PRs where we are cherry-picking fixes from #2117 to update our Fabric implementation on macOS.

Cherry pick the first couple of fixes to Text. Notable changes from the initial commits:

  • Since these commits were made, React Native upstream added facebook@f4609db which heavily refactors TextInput on iOS. Specifically, it moves Text rendering from the Paragraph component view to its' inner content view. As such, I refactor the macOS implementation in b33797f to match iOS.
  • The macOS implementation involves direct access to NSTextStorage through RCTTextLayoutManager. However, since facebook@840fd30 , that method doesn't exist. So we refactored a bit to add it back.

Test Plan:

Launched RNTester and Text seems to render as it did on Paper.

@Saadnajmi Saadnajmi requested a review from a team as a code owner November 20, 2024 00:37
@Saadnajmi Saadnajmi changed the title fix(fabric): Cherry-pick fixes to render Text in an NSTextView fix(fabric, text): Cherry-pick fixes to render Text in an NSTextView Nov 20, 2024
@Saadnajmi Saadnajmi changed the title fix(fabric, text): Cherry-pick fixes to render Text in an NSTextView fix(fabric, text): render Text in an NSTextView Nov 20, 2024
Nick Lefever and others added 3 commits November 28, 2024 15:26
…yout manager

Summary: To render text using an NSTextView, we need to gain access to a fully configured NSTextStorage to configure the text view to render the text with the right configuration.

Test Plan: Tested later in this stack.

Reviewers: shawndempsey, chpurrer, #rn-desktop

Reviewed By: shawndempsey

Differential Revision: https://phabricator.intern.facebook.com/D49465173

Tasks: T163838519
Summary: Use an NSTextView to render the text in RCTParagraphComponentView so that we would get UX interactions specific to desktop for free (e.g. text selection)

Test Plan:
- Run Zeratul with Fabric enabled
- Check that text is being rendered correctly with the right size and positioning.

{F1097505779}

Reviewers: shawndempsey, chpurrer, #rn-desktop

Reviewed By: shawndempsey

Differential Revision: https://phabricator.intern.facebook.com/D49465175

Tasks: T163838519
@Saadnajmi Saadnajmi enabled auto-merge (rebase) November 28, 2024 23:35
@Saadnajmi Saadnajmi merged commit 22b7063 into microsoft:main Nov 28, 2024
14 checks passed
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.

2 participants