Feat/android controller functions #3
Conversation
|
…react-native into feat/android-controller-functions
| private const val COMMAND_SET_LOOP = "setLoop" | ||
| private const val COMMAND_SET_LOOP_ID = 7 | ||
|
|
||
| private const val COMMAND_SET_PROGRESS = "setProgress" |
There was a problem hiding this comment.
what is this command is used for ?
There was a problem hiding this comment.
Will remove this. was trying to use it for segment
| setFrame: setFrameWithUIManager, | ||
| freeze: freezeWithUIManager, | ||
| unfreeze: unfreezeWithUIManager, | ||
| })); |
There was a problem hiding this comment.
I think useImperativeHandle should accept a dependency array, which would include all the methods that port the commands to the native side. https://react.dev/reference/react/useImperativeHandle#parameters.
There was a problem hiding this comment.
We don't need to add them as they are not states
| loop={false} | ||
| autoplay={false} |
There was a problem hiding this comment.
Is there a reason for hardcoding these props to false? Aren't they false by default?
There was a problem hiding this comment.
this stands as the default value.
but the props spread overrides it
| }; | ||
|
|
||
| export const DotLottie = forwardRef( | ||
| ({ source, ...props }: DotlottieReactNativeProps, ref) => { |
There was a problem hiding this comment.
If any of the props, like source or loop, are updated, will those changes be reflected in the native environment? I'm assuming they won't, since we need to call setLoop, setSource on the dotLottie instance, right ?
There was a problem hiding this comment.
yes it does immediately
src/DotLottie.tsx
Outdated
| setStartStateMachine: () => void; | ||
| setStopStateMachine: () => void; | ||
| setLoadStateMachine: (stateMachineId: string) => void; | ||
| setPostEvent: (event: string) => void; | ||
| setAddStateMachineEventListener: () => void; | ||
| setRemoveStateMachineEventListener: () => void; | ||
| setResize: (width: number, height: number) => void; |
There was a problem hiding this comment.
Is this set prefix is something required on react-native side? I think without set prefix for these functions makes more sense to me.
startStateMachine: () => void;
stopStateMachine: () => void;
loadStateMachine: (stateMachineId: string) => void;
postEvent: (event: string) => void;
addStateMachineEventListener: () => void;
removeStateMachineEventListener: () => void;
resize: (width: number, height: number) => void;
There was a problem hiding this comment.
I can remove that. just naming conventions
src/DotLottie.tsx
Outdated
| setStopStateMachine: () => void; | ||
| setLoadStateMachine: (stateMachineId: string) => void; | ||
| setPostEvent: (event: string) => void; | ||
| setAddStateMachineEventListener: () => void; |
There was a problem hiding this comment.
Both of these methods accepts parameters on the native code. Is it something we could do on react-native side?
fun addStateMachineEventListener(listener: StateMachineEventListener)
fun removeStateMachineEventListener(listener: StateMachineEventListener)
There was a problem hiding this comment.
we do everything on the native side and listen to it on a method prop in react native. so the listener is on the native side
| object : DotLottieEventListener { | ||
| override fun onLoad() { | ||
| onReceiveNativeEvent("onLoad", null) | ||
| } | ||
| override fun onComplete() { | ||
|
|
||
| onReceiveNativeEvent("onComplete", null) | ||
| } | ||
| override fun onLoadError() { | ||
|
|
||
| onReceiveNativeEvent("onLoadError", null) | ||
| } | ||
| override fun onPlay() { | ||
| onReceiveNativeEvent("onPlay", null) | ||
| } | ||
| override fun onStop() { | ||
| onReceiveNativeEvent("onRender", null) | ||
| } | ||
| override fun onPause() { | ||
| onReceiveNativeEvent("onPause", null) | ||
| } | ||
| override fun onFreeze() { | ||
| onReceiveNativeEvent("onFreeze", null) | ||
| } | ||
| override fun onUnFreeze() { | ||
| onReceiveNativeEvent("onUnFreeze", null) | ||
| } | ||
| override fun onDestroy() { | ||
| onReceiveNativeEvent("onDestroy", null) | ||
| } | ||
| } |
There was a problem hiding this comment.
Lets create eventListeners same as dotLottieController with remember. I think current approach will create new DotLottieEventListener on every composition.
There was a problem hiding this comment.
no it won't because it's on the class so it is created only once
| setMarker: (marker: string) => void; | ||
| loadTheme: (themeId: string) => void; | ||
| loadAnimation: (animationId: string) => void; | ||
| setManifest: () => void; |
There was a problem hiding this comment.
@shamsudeeen-yusuf I don't think that there is a native method called setManifest
No description provided.