Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: migrate SvgViewManager from ReactViewManager to typed ViewGroupManager #2304

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

CDFN
Copy link
Collaborator

@CDFN CDFN commented Jun 17, 2024

Summary

Explain the motivation for making this change: here are some points to help you:

  • What is the feature? (if applicable)
    • This feature migrates SvgView from non-typed ReactViewManager to typed ViewGroupManager. Initially it was meant to help with Kotlin migration as Kotlin has more strict type checking, however it appears that this PR will also solve issue of need to implement tons of properties that are not applicable to SVG or were just delegating calls to super class.
  • How did you implement the solution?
    • Changed type SvgView extends from ReactViewManager to ViewGroupManager<SvgView>
  • What areas of the library does it impact?
    • It impacts SvgView component on Android platform (large portion of unsupported anyway properties was removed)
    • It might impact unsupported as of now (pre 70.0 RN) versions, as hitSlop implementation now falls back to default implementation instead having our own that was there due to different signature between RN versions. This is no longer issue after 0.70 version.

Test Plan

No changes applicable to end user

What's required for testing (prerequisites)?

Android simulator with example app

What are the steps to reproduce (after prerequisites)?

Check if there are no behavior changes between previous implementation and current

Compatibility

OS Implemented
iOS
Android

Checklist

  • I have tested this on a device and a simulator
  • I added documentation in README.md
  • I updated the typed files (typescript)
  • I added a test for the API in the __tests__ folder

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant