Turn CXX implementation of TextLayoutManager into interface #39805
Conversation
This file contains 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: D49907594 |
|
This pull request was exported from Phabricator. Differential Revision: D49907594 |
rshest
added a commit
to rshest/react-native
that referenced
this pull request
Oct 4, 2023
…#39805) Summary: Pull Request resolved: facebook#39805 ## Changelog: [Internal] - This makes corresponsing text measure methods inside `TextLayoutManager` overridable, so that it can be substituted with a custom implementation without introducing a new "platform". Rationale: CXX platform is rather general and less specific than Android or iOS, so we may potentially have multiple alternative implementations of text layout there. An alternative could be making `TextLayoutManager` an interface across all the platforms, and actual implementations called e.g. `TextLayoutManagerImpl` in each of them, however this would be quite a bit bigger blast radius without much added benefit for Android/iOS. Differential Revision: D49907594 fbshipit-source-id: c42cf4cd93607ccfd7aed44f15cce300814de233
rshest
force-pushed
the
export-D49907594
branch
from
114629a
to
82c4edc
Compare
|
This pull request was exported from Phabricator. Differential Revision: D49907594 |
rshest
added a commit
to rshest/react-native
that referenced
this pull request
Oct 4, 2023
…#39805) Summary: Pull Request resolved: facebook#39805 ## Changelog: [Internal] - This makes corresponsing text measure methods inside `TextLayoutManager` overridable, so that it can be substituted with a custom implementation without introducing a new "platform". Rationale: CXX platform is rather general and less specific than Android or iOS, so we may potentially have multiple alternative implementations of text layout there. An alternative could be making `TextLayoutManager` an interface across all the platforms, and actual implementations called e.g. `TextLayoutManagerImpl` in each of them, however this would be quite a bit bigger blast radius without much added benefit for Android/iOS. Differential Revision: D49907594 fbshipit-source-id: 565020f83a3f9e6af2f8b04580b519f9ecca26e5
rshest
force-pushed
the
export-D49907594
branch
from
82c4edc
to
1c8776b
Compare
|
This pull request was exported from Phabricator. Differential Revision: D49907594 |
rshest
added a commit
to rshest/react-native
that referenced
this pull request
Oct 4, 2023
…#39805) Summary: Pull Request resolved: facebook#39805 ## Changelog: [Internal] - This makes corresponsing text measure methods inside `TextLayoutManager` overridable, so that it can be substituted with a custom implementation without introducing a new "platform". Rationale: CXX platform is rather general and less specific than Android or iOS, so we may potentially have multiple alternative implementations of text layout there. An alternative could be making `TextLayoutManager` an interface across all the platforms, and actual implementations called e.g. `TextLayoutManagerImpl` in each of them, however this would be quite a bit bigger blast radius without much added benefit for Android/iOS. Differential Revision: D49907594 fbshipit-source-id: b0e5e37a5dcd2312abc3d499fa10a02f88713307
rshest
force-pushed
the
export-D49907594
branch
from
1c8776b
to
867f757
Compare
|
This pull request was exported from Phabricator. Differential Revision: D49907594 |
rshest
added a commit
to rshest/react-native
that referenced
this pull request
Oct 4, 2023
…#39805) Summary: Pull Request resolved: facebook#39805 ## Changelog: [Internal] - This makes corresponsing text measure methods inside `TextLayoutManager` overridable, so that it can be substituted with a custom implementation without introducing a new "platform". Rationale: CXX platform is rather general and less specific than Android or iOS, so we may potentially have multiple alternative implementations of text layout there. An alternative could be making `TextLayoutManager` an interface across all the platforms, and actual implementations called e.g. `TextLayoutManagerImpl` in each of them, however this would be quite a bit bigger blast radius without much added benefit for Android/iOS. Differential Revision: D49907594 fbshipit-source-id: 8ec10f22b3ea78cc2e92069c43c159c07b91fc93
rshest
force-pushed
the
export-D49907594
branch
from
867f757
to
26686e3
Compare
Base commit: 2000acc |
|
This pull request was exported from Phabricator. Differential Revision: D49907594 |
rshest
force-pushed
the
export-D49907594
branch
from
26686e3
to
5aac612
Compare
rshest
added a commit
to rshest/react-native
that referenced
this pull request
Oct 4, 2023
…#39805) Summary: Pull Request resolved: facebook#39805 ## Changelog: [Internal] - This makes corresponsing text measure methods inside `TextLayoutManager` overridable, so that it can be substituted with a custom implementation without introducing a new "platform". Rationale: CXX platform is rather general and less specific than Android or iOS, so we may potentially have multiple alternative implementations of text layout there. An alternative could be making `TextLayoutManager` an interface across all the platforms, and actual implementations called e.g. `TextLayoutManagerImpl` in each of them, however this would be quite a bit bigger blast radius without much added benefit for Android/iOS. Differential Revision: D49907594 fbshipit-source-id: 71b177107db706ad3245c043ddc13828ac0716fd
…#39805) Summary: ## Changelog: [Internal] - This makes corresponsing text measure methods inside `TextLayoutManager` overridable, so that it can be substituted with a custom implementation without introducing a new "platform". Rationale: CXX platform is rather general and less specific than Android or iOS, so we may potentially have multiple alternative implementations of text layout there. An alternative could be making `TextLayoutManager` an interface across all the platforms, and actual implementations called e.g. `TextLayoutManagerImpl` in each of them, however this would be quite a bit bigger blast radius without much added benefit for Android/iOS. Reviewed By: christophpurrer Differential Revision: D49907594
rshest
force-pushed
the
export-D49907594
branch
from
5aac612
to
7baedd0
Compare
|
This pull request was exported from Phabricator. Differential Revision: D49907594 |
|
This pull request has been merged in c969d8c. |
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:
Changelog:
[Internal] -
This makes corresponsing text measure methods inside
TextLayoutManageroverridable, so that it can be substituted with a custom implementation without introducing a new "platform".Rationale: CXX platform is rather general and less specific than Android or iOS, so we may potentially have multiple alternative implementations of text layout there.
An alternative could be making
TextLayoutManageran interface across all the platforms, and actual implementations called e.g.TextLayoutManagerImplin each of them, however this would be quite a bit bigger blast radius without much added benefit for Android/iOS.Differential Revision: D49907594