Android: add the zoomCoverScale no-op setter that codegen requires - #7
Merged
Conversation
Adding zoomCoverScale to the Fabric specs broke every Android build:
e: ModalScreenViewManager.kt: Class 'ModalScreenViewManager' is not abstract
and does not implement abstract member: fun setZoomCoverScale(view: T!, value: Float)
e: ScreenViewManager.kt: Class 'ScreenViewManager' is not abstract and does
not implement abstract member: fun setZoomCoverScale(view: T!, value: Float)
The prop is iOS-only, but the Fabric spec is not. Codegen reads
ScreenNativeComponent.ts on Android too and puts setZoomCoverScale on the
generated ScreenManagerInterface, so the Kotlin view manager has to implement
it whether or not Android does anything with it. The @platform ios annotation
in the docs is not a codegen instruction.
Every other iOS-only zoom prop already carries a no-op override in
ScreenViewManager for exactly this reason, next to setZoomCloseOvershoot and
setZoomShowDebugBorders. This adds the missing one.
ModalScreenViewManager extends ScreenViewManager, so it inherits the override
and both errors clear.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Fixes the Android build break introduced by #6. Production build 0.135.69 failed on this.
What broke
Why
zoomCoverScaleis iOS-only, but the Fabric spec is not. Codegen readsScreenNativeComponent.tson Android too and putssetZoomCoverScaleon the generatedScreenManagerInterface, so the Kotlin view manager must implement it whether or not Android does anything with it. The@platform iosannotation in the JSDoc is documentation, not a codegen instruction.Every other iOS-only zoom prop already carries a no-op override in
ScreenViewManagerfor exactly this reason —setZoomCloseOvershootandsetZoomShowDebugBorderssit immediately around where this one goes. #6 added the spec entry and missed the Kotlin side.ModalScreenViewManagerextendsScreenViewManager, so it inherits the override and both errors clear with the single addition.Scope
Five lines, Android only, no behaviour change on either platform — the setter does nothing, matching its neighbours.