feat: add responsive layouts #273
Conversation
| @@ -104,6 +106,8 @@ | |||
| ED297162215061F000B7C4FE /* JavaScriptCore.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; name = JavaScriptCore.framework; path = System/Library/Frameworks/JavaScriptCore.framework; sourceTree = SDKROOT; }; | |||
| ED2971642150620600B7C4FE /* JavaScriptCore.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; name = JavaScriptCore.framework; path = Platforms/AppleTVOS.platform/Developer/SDKs/AppleTVOS12.0.sdk/System/Library/Frameworks/JavaScriptCore.framework; sourceTree = DEVELOPER_DIR; }; | |||
| F8AA4CA32C0F3FDB00C1A5FF /* runtime_nested_inputs.riv */ = {isa = PBXFileReference; lastKnownFileType = file; path = runtime_nested_inputs.riv; sourceTree = "<group>"; }; | |||
| F8E2789D2CDCD6A200FAA8EF /* layouts_demo.riv */ = {isa = PBXFileReference; lastKnownFileType = file; path = layouts_demo.riv; sourceTree = "<group>"; }; | |||
| F8E278A12CDCDAFA00FAA8EF /* layout_test.riv */ = {isa = PBXFileReference; lastKnownFileType = file; name = layout_test.riv; path = ../../../../../../rive/rive/packages/runtime_wasm/js/examples/_frameworks/layout_example/assets/layout_test.riv; sourceTree = "<group>"; }; | |||
There was a problem hiding this comment.
This is causing an error. It looks like it's pointing to a file on your local machine.
|
This isn't a bug, but it might look like a bug to a user. On the example if you select
This is because this is an old file that doesn't actually have a layout. Maybe we should add one? If you have the .rev, I can do that. |
Nice catch. I don't have the .rev, but we can use any example! If you have one to commit to this PR then let's do, otherwise, let's get this feature out and update the sample app later. |
| PRODUCT_BUNDLE_IDENTIFIER = com.example.rivereactnative; | ||
| PRODUCT_BUNDLE_IDENTIFIER = gordon.example.rivereactnative; |
There was a problem hiding this comment.
Probably fine, just calling this out as a thing that's been committed.
There was a problem hiding this comment.
Ah I forgot this, will remove
| @@ -41,6 +41,8 @@ class RiveReactNativeView: RCTView, RivePlayerDelegate, RiveStateMachineDelegate | |||
|
|
|||
| @objc var fit: String? | |||
|
|
|||
| @objc var layoutScaleFactor: NSNumber = -1.0 // -1.0 will inform the iOS runtime to determine the correct scale factor automatically | |||
There was a problem hiding this comment.
This type is a Double in Swift (and likely ObjC by interop). What's the decision behind using NSNumber?
There was a problem hiding this comment.
Double doesn't work for nullable(optional) values in these bindings: https://reactnative.dev/docs/legacy/native-modules-ios#argument-types
| @@ -99,6 +101,10 @@ class RiveReactNativeView: RCTView, RivePlayerDelegate, RiveStateMachineDelegate | |||
| if (changedProps.contains("alignment")) { | |||
| viewModel?.alignment = convertAlignment(alignment) | |||
| } | |||
|
|
|||
| if (changedProps.contains("layoutScaleFactor")) { | |||
| viewModel?.layoutScaleFactor = layoutScaleFactor.doubleValue | |||
There was a problem hiding this comment.
Ah, usage is here as a double value. But, still curious!
| @@ -6,6 +6,7 @@ @interface RCT_EXTERN_MODULE(RiveReactNativeViewManager, RCTViewManager) | |||
| RCT_EXPORT_VIEW_PROPERTY(resourceName, NSString) | |||
| RCT_EXPORT_VIEW_PROPERTY(url, NSString) | |||
| RCT_EXPORT_VIEW_PROPERTY(fit, NSString) | |||
| RCT_EXPORT_VIEW_PROPERTY(layoutScaleFactor, NSNumber) | |||
There was a problem hiding this comment.
I'm assuming this has something to do with using NSNumber over Double?
There was a problem hiding this comment.
Yup! In one of the other comments I linked to the reference docs
This exposes
Fit.LayoutandlayoutScaleFactorfor Rive React Native.iOS handles
layoutScaleFactorwith a value of "-1" to be not set (null). In which case iOS handles the scale factor automatically.Android does the same, but when
layoutScaleFactoris null. Android is more limited in it's pixel density calculations, as we cannot automatically subscribe to changes.That means in React Native land, users will need to manage the density themselves if they want it to respond to device density changes consistently across Android and iOS - because iOS manages it better and we can't do more for Android. This will need to be a documentation effort
The current implementation attempts to handle both situations, where the value can either be null or -1.
EDIT: Resolved the below!
However, on React Native we have the opposite problem. Where Android can't take a null but iOS can through the bindings 🫠
I'm leaning towards updating the JS to default send -1 if the prop is null. But interested to hear your thoughts, or maybe I missed some way to handle this.