Implement intrinsic size based on viewBox (height / width auto)

[janicduplessis]

Summary

This implements intrinsic size for Svg element based on the viewBox size. When width or height is unset or set to auto it will use the viewBox size to calculate the auto dimension value.

This uses yoga measure function to calculate and return the proper size to yoga.

Test Plan

I've compared my implementation with the web version to make sure it behaves the same. I found some edge cases where the behavior is different, probably because of some differences with how yoga implements flexbox, or just some odd web behavior that might be there for legacy purposes.

Before

Expected (Web)

Actual (iOS)

After

Actual (iOS)

What's required for testing (prerequisites)?

N/A

What are the steps to reproduce (after prerequisites)?

Run example app and use new auto width / height examples

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

[WoLewicki]

Would you be able to update the PR so it works on the newest version of the library on both architectures?

[janicduplessis]

Sure, will try to find some time in the next few weeks

[janicduplessis]

@WoLewicki I've updated this to support fabric, turns out it was a bit harder than expected.

To add a measure function the yoga node needs to be marked as a leaf node, this means it should not have any layoutable children. However this is not the case currently since all our views are ConcreteViewShadowNodes which have yoga nodes associated. Fabric actually validates this here https://github.com/facebook/react-native/blob/main/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.cpp#L791-L793.

The solution is to use ConcreteShadowNode as the base class for our shadow nodes, however this means we have to use interfaceOnly for codegen and write them manually. The shadow node must also set FormsView and FormsStackingContext traits to make sure native views are still created and attached properly.

Then comes android :)

Android has a concept of virtual nodes and also has a check to see if a view is layoutable which makes our new shadow nodes type not work at all. See https://github.com/facebook/react-native/blob/main/ReactCommon/react/renderer/mounting/ShadowViewMutation.cpp#L100-L108 and https://github.com/facebook/react-native/blob/main/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/SurfaceMountingManager.java#L837-L847. However for some reason that I haven't fully investigated using ConcreteViewShadowNodes just works on Android so I ended up creating different types of shadow nodes on the 2 platforms. The iOS implementation however makes a lot more sense tho since shadow nodes inside SvgView do not contribute to layout so should not be layoutable.

[janicduplessis]

@WoLewicki Any change you or someone else could have a look at this, just rebased and improved the implementation.

[janicduplessis]

Fabric on Android actually requires shadow nodes to be subclasses of LayoutableShadowNode to be able to create a native view for it so I am now using that instead of having different implementations of iOS / Android.

[janicduplessis]

Also note that currently the example app crashes on fabric Android because of a known issue in reanimated, I updated to a nightly version to test.

[bohdanprog]

Hello @janicduplessis,
Would you be able to update the PR so it works with the latest version of the library?
Thank you.

[janicduplessis]

@bohdanprog Were you able to get it working? I can try to have a look again in the next week.

[bohdanprog]

@bohdanprog Were you able to get it working? I can try to have a look again in the next week.

Yeah, I managed to check and prepare everything.

[bohdanprog]

@janicduplessis, could you take a look at the documentation for SVG sizing? There is a note about using auto for width and height on the 'svg' element, which is considered as 100%.

Additionally, we encountered an edge case when attempting to display the component as follows:

<Svg viewBox="-100 -100 100 100" height={'100%'} width="auto">
    <Circle cx="-50" cy="-50" r="50" fill="black" />
</Svg>

Would you mind taking a look at this situation, please? Thank you.

[janicduplessis]

Sure I’ll try to have a look this week.

[qwertychouskie]

Sure I’ll try to have a look this week.

Any way I could help move this along (testing, etc)? I keep running into this issue, and would love to have this functionality merged.