react-native
react-native copied to clipboard
Initial implementations of spans
Summary:
This is the initial implementation of "spans", the feature tracked in https://github.com/facebook/react-native/issues/42602.
Changelog:
[INTERNAL] [ADDED] - Initial implementation of spans
Test Plan:
@NickGerleman
This PR includes bits based on your feedback from:
- https://github.com/facebook/react-native/pull/42704
- https://github.com/facebook/react-native/pull/42701
- Would you take a look at these commits...
- https://github.com/facebook/react-native/pull/43150/commits/1762362e2b6b948d5d474d213a6a584c32d7134f (Introduce
SpanFragmentandSpanAttributes) - https://github.com/facebook/react-native/pull/43150/commits/08ddbff8fd07159ebe2b5f905044c2250a2ab9a5 (Support
display: inlinestyle) - https://github.com/facebook/react-native/pull/43150/commits/7f2467e14cc969f3fc2d85e6ace65fe588e1806e (Draw spans on Android)
...and give me some high-level feedback? I'm especially interested if there is any chance at all that this direction could lead to the feature being merged.
-
I know that this PR is big. Do you have any ideas of how this could be split into chunks in a way that any of them would be considered acceptable on its own?
-
This is a Fabric-only implementation. Would it be acceptable for a new feature to be Fabric-only, or do all features have to support the legacy Paper renderer?
| Platform | Engine | Arch | Size (bytes) | Diff |
|---|---|---|---|---|
| android | hermes | arm64-v8a | 17,988,480 | +53,419 |
| android | hermes | armeabi-v7a | n/a | -- |
| android | hermes | x86 | n/a | -- |
| android | hermes | x86_64 | n/a | -- |
| android | jsc | arm64-v8a | 21,346,691 | +53,407 |
| android | jsc | armeabi-v7a | n/a | -- |
| android | jsc | x86 | n/a | -- |
| android | jsc | x86_64 | n/a | -- |
Base commit: 767330f21885e668bc0d9b5b3063113d0446bcbc Branch: main
@cipolleschi
We had an opportunity to discuss the public API of AttributedString in this PR:
- https://github.com/facebook/react-native/pull/42596
Would you find a moment to take a look at this commit...
- https://github.com/facebook/react-native/pull/43150/commits/1762362e2b6b948d5d474d213a6a584c32d7134f (Introduce
SpanFragmentandSpanAttributes)
...? It significantly changes the API and implementation of AttributedString and its befriended classes. Would such change be considered acceptable when its purpose is implementing a new valuable feature?
Hi @cubuspl42, thanks for the effort! Unfortunately, this PR is too big to be reviewed properly in one go, and it requires different layers of knowledge on the various technologies of the frameworks.
If we want to increase the chances of landing this, I suggest to split it in smaller pieces. Ideally, we can have:
- a PR for the shared code
- a PR for iOS
- a PR for Android
- a PR for JS.
Also, due to how our systems works, we might have to land the native code some days before the JS code.