Skip to content
New issue

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

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

Already on GitHub? # to your account

fix: proper transform prop handling #1895

Merged
merged 9 commits into from
Oct 25, 2022
Merged

Conversation

WoLewicki
Copy link
Member

@WoLewicki WoLewicki commented Oct 18, 2022

Summary

PR aligning handling of transform prop between web and native and adding proper handling of transform prop on web.

  • origin prop is now treated as transform-origin since it seems like the current behavior of this prop on native platforms.
  • transform prop cannot be an object with transform properties since it does not provide order of transformations which would lead to undefined behavior so this option has been removed.
  • RN style of transform prop (array of transformation objects) can now be used
  • font-size on web seems to be 16px by default and it is 12 in library - maybe we should align it somehow
  • removed maskTransform prop since it does not exist in SVG standard and did nothing on native side
  • fixed typo in Fabric Pattern updateProps method
  • added Transforms examples

For some reason iOS implementation of Pattern differs from web and Android one, but it will be handled in another PR.

@WoLewicki WoLewicki merged commit 6a5242f into main Oct 25, 2022
@WoLewicki WoLewicki deleted the @wolewicki/fix-transform-parsing branch October 25, 2022 13:55
WoLewicki added a commit that referenced this pull request Nov 3, 2022
Changed `requireNativeComponent` to `codegenNativeComponent` so that upcoming changes (Static View Configs, Bridgeless Mode and idk what more) in `react-native` are available in the library. Also, types and native components are now taken directly from `fabric` folder to make sure the values passed to the native components are the ones defined in props. It should work on all supported versions since `codegenNativeComponent` function exists from RN v. 0.61.0. Suggested by @RSNara and @cipolleschi

Reason for [`5394bbb` (#1847)](5394bbb): 
- on `Paper`, `Animated` uses `setNativeProps` method when we set `useNativeDriver`  to `false`, and does not rerender the component. Therefore, new transform lands only in `SvgView` and is parsed in `RCTViewManager.m` .
- on `Fabric`, the same code makes the components rerender. Due to this, information about new transform is passed to the `SvgView` child: `G` , making it apply translations from the transform in its `updateProps` method.
- other than `Animated` use-case, on both archs, if we just passed `transform` prop to `Svg` component, it would end up in double transformations now as well. All of those changes are due to #1895, which added proper parsing of RN style `transform` prop (array of transformations objects) therefore making `G` properly handle `transform` prop passed from `Svg`.

Reason for [`19bcb24` (#1847)](19bcb24): Same as software-mansion/react-native-screens#1624
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant