Extract TextLayoutManager::baseline() #50888
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
facebook-github-bot
added
CLA Signed
|
This pull request was exported from Phabricator. Differential Revision: D73555465 |
Summary: We construct this from JNI, which doesn't care about visibility, and then only want to expose `StateWrapper` as the public interface. Changelog: [Android][Removed] - Make StateWrapperImpl Internal Differential Revision: D73161592
Differential Revision: D73159146
Summary: This forms the basis for a replacement of `TextView`. This started off with Litho's [`RCTextView`](https://github.com/facebook/litho/blob/master/litho-rendercore-text/src/main/java/com/facebook/rendercore/text/RCTextView.java), which is a simple view, for rendering a text layout, and providing some built-in keyboard navigation and a11y support. Many changes were made to it, including: 1. Removing many parts not relevant to RN, or which will be replaced by other RN infra. E.g. we will reuse existing a11y delegates, have existing ways of creating Spannables and text layouts, inline views, etc 2. Converting to Kotlin 3. Adding back in some changes required for RN's drawing, and expected view manager APIs (e.g. overflow/clipping customization) 4. Making it target a ViewGroup instead of a View, for correct inline view support down the line Because we rely on drawing text layout, with the same Spannable as before, most things "just work", because they are part of the layout we are drawing, generated by TextLayoutManager on the Fabric side. We don't offer much customization to what can be drawn, forcing it to have happened in the layout we are showing already. There are quite a few bits not implemented yet. Some of these are cases, like `textAlignVertical`, were previously incorrectly implemented just at the ReactTextView layer, so Fabric layout was unaware of them. Another similar class to this is any non-default fonts which we must load. `adjustsFontSizeToFit` (stubbed out in later diff) will also need some tweaking with the new assumption we don’t want to mutate Spans/layouts set in State. Fine grained selection support is the largest tbd. Changelog: [Internal] Differential Revision: D73282649 Reviewed By: Abbondanzo
Summary: This implements the view manager for `PreparedLayoutTextView`, originating by taking the view managers composing `ReactTextView`, converting to Kotlin, and removing everything no longer needed. In Facsimile, anything influencing text appearance is applied earlier, when creating the Fabric layout, so there are many less setters here. Most visual attributes are instead present in the state we are presenting. We have tasks for some of these, that need to be reimplemented, as they do not currently influence the Spannable being measured. That includes e.g. `ReactTextViewManagerCallback`, used for injection, and `dataDetectorType` for linkifying Spannable. Changelog: [Internal] Differential Revision: D73287706 Reviewed By: rshest
|
This pull request was exported from Phabricator. Differential Revision: D73555465 |
NickGerleman
force-pushed
the
export-D73555465
branch
from
d7ee4cc to
165cafe
Compare
NickGerleman
added a commit
to NickGerleman/react-native
that referenced
this pull request
Apr 24, 2025
Summary: Pull Request resolved: facebook#50888 This is shared between platforms using a very strange pattern. Let's just extract this into its own function. Not considering breaking, since TextLayoutManager is internal interface. Changelog: [internal] Reviewed By: rshest Differential Revision: D73555465
|
This pull request was exported from Phabricator. Differential Revision: D73555465 |
NickGerleman
added a commit
to NickGerleman/react-native
that referenced
this pull request
Apr 24, 2025
Summary: Pull Request resolved: facebook#50888 This is shared between platforms using a very strange pattern. Let's just extract this into its own function. Not considering breaking, since TextLayoutManager is internal interface. Changelog: [internal] Reviewed By: rshest Differential Revision: D73555465
NickGerleman
force-pushed
the
export-D73555465
branch
from
165cafe to
78ef3e6
Compare
|
This pull request was exported from Phabricator. Differential Revision: D73555465 |
NickGerleman
force-pushed
the
export-D73555465
branch
from
78ef3e6 to
1cc7186
Compare
NickGerleman
added a commit
to NickGerleman/react-native
that referenced
this pull request
Apr 24, 2025
Summary: Pull Request resolved: facebook#50888 This is shared between platforms using a very strange pattern. Let's just extract this into its own function. Not considering breaking, since TextLayoutManager is internal interface. Changelog: [internal] Reviewed By: rshest Differential Revision: D73555465
Summary: With Facsimile, we are introducing some new concept of `PreparedText`, where platform TextLayoutManager which implement, can lead to additional optimizations. `#ifdef ANDROID` is not a workable pattern for this. Apart from react-native-cxx getting hooked into it, and all of the existing bugs there, it is bad for editor environment, and hard to reason about. This splits up `ParagraphState`, so that we can control platform specific bits more easily. We do not split `ParagraphShadowNode`, which will use concepts (e.g. `TextLayoutManagerWithPreparedText`) to control which paths it takes, based on platform capaibilities. Changelog: [internal] Differential Revision: D73555441
Summary: Pull Request resolved: facebook#50888 This is shared between platforms using a very strange pattern. Let's just extract this into its own function. Not considering breaking, since TextLayoutManager is internal interface. Changelog: [internal] Reviewed By: rshest Differential Revision: D73555465
|
This pull request was exported from Phabricator. Differential Revision: D73555465 |
NickGerleman
force-pushed
the
export-D73555465
branch
from
1cc7186 to
8004a4b
Compare
|
This pull request has been merged in 01cc16b. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
This is shared between platforms using a very strange pattern. Let's just extract this into its own function. Not considering breaking, since TextLayoutManager is internal interface.
Changelog: [internal]
Differential Revision: D73555465