Migrate ReactTextViewManager to Kotlin
#51721
Conversation
…ctor/migrate-react-text-view-manager-to-kotlin
facebook-github-bot
added
the
CLA Signed
facebook-github-bot
added
the
Shared with Meta
Could From the history, these two can really just be merged together. There used to be a hierarchy there, but now Not necessarily needed for this change though. |
| "Paragraph Attributes: %s", | ||
| // if (paragraphAttributes != null) paragraphAttributes.toString() else "<empty>" |
There was a problem hiding this comment.
| "Paragraph Attributes: %s", | |
| // if (paragraphAttributes != null) paragraphAttributes.toString() else "<empty>" | |
| "Paragraph Attributes: %s", |
There was a problem hiding this comment.
This crash actually went away at some point, so we could also remove the try/catch entirely
There was a problem hiding this comment.
I've removed the try/catch
|
|
||
| @ReactProp(name = "accessible") | ||
| public fun setAccessible(view: ReactTextView, accessible: Boolean) { | ||
| internal fun setAccessible(view: ReactTextView, accessible: Boolean) { |
There was a problem hiding this comment.
Are prop setters allowed to be internal?
There was a problem hiding this comment.
There doesn't seem to be any build/runtime issue, so it looks like it is allowed
It is unfortunately not possible while keeping it internal as the consumer class is public, we could make the consumer class internal as well to avoid this but it has some OSS usages
Interesting, thanks for the info! I think just to keep this PR within the scope of migration, maybe that can be done separately – I'm happy to follow up later on. I've annotated the class with |
|
@NickGerleman has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@NickGerleman merged this pull request in d631ec9. |
|
This pull request was successfully merged by @mateoguzmana in d631ec9 When will my fix make it into a release? | How to file a pick request? |
4 participants
Summary:
Migrate com.facebook.react.views.text.ReactTextViewManager to Kotlin.
In this diff,
ReactTextAnchorViewManageris made public again as it's extended byReactTextViewManager.Changelog:
[INTERNAL] - Migrate com.facebook.react.views.text.ReactTextViewManager to Kotlin
Test Plan: