Rename apply-animated-style wrapper

[Parveshdhull]

Summary

Rename this wrapper to something like use-... so that dev know this wrapper is calling hook

more details: https://github.com/status-im/status-mobile/pull/20024#discussion_r1602097046 https://github.com/status-im/status-mobile/pull/19945#discussion_r1606571797,

[J-Son89]

@Parveshdhull - We also have many instances of this being used in the style.cljs file which is bad because it means there is hooks hidden from the view.

For this reason these guidelines need adjustment - https://github.com/status-im/status-mobile/blob/develop/doc/new-guidelines.md#animated-styles-in-the-style-file

We should come up with some good examples of how we want to pass these styled values. 👍

[J-Son89]

cc @OmarBasem - related to your comment about this guideline: https://github.com/status-im/status-mobile/blob/develop/doc/new-guidelines.md#pass-a-vector-of-styles-to-reanimated-view perhaps we can just remove a lot of uses of this method instead now? 🤔

[OmarBasem]

@Parveshdhull why don't we remove apply-animations-to-style completely since we can simply pass an array of styles now? https://github.com/status-im/status-mobile/blob/develop/doc/new-guidelines.md#pass-a-vector-of-styles-to-reanimated-view

[Parveshdhull]

hi @OmarBasem, probably @ulisesmac will have more info about that.

But from, https://github.com/status-im/status-mobile/pull/18381

Is now use-animated-style useless? No, in some complex components we will still need it, Reanimated presents inline styles here, and the the use of use-animated-style here, so please read them. If you are unsure, I'd suggest implementing the component as before and then trying to use the new approach.

Also, from https://github.com/status-im/status-mobile/pull/18621#pullrequestreview-1844361358

The problem is bottom-tabs-blur-overlay (the style fn) is being used in the reanimated/blur-view, and currently, inline styles using a vector is only supported in reanimated/view

[ulisesmac]

@Parveshdhull why don't we remove apply-animations-to-style completely since we can simply pass an array of styles now? https://github.com/status-im/status-mobile/blob/develop/doc/new-guidelines.md#pass-a-vector-of-styles-to-reanimated-view

@OmarBasem Well, the vector syntax isn't always as powerful enough as useAnimatedStyle (hook being used by apply-animations-to-style).

The problem is bottom-tabs-blur-overlay (the style fn) is being used in the reanimated/blur-view, and currently, inline styles using a vector is only supported in reanimated/view

Yes, I just modified the reanimated/view component to support the vector syntax, but it brings an interesting topic, since the vector syntax for styles is supported in all React Native components.

Look at these styles:

(https://reactnative.dev/docs/0.73/style)

It'd be great if we support them, we could get rid of the merges we do to override styles :smile:

@ilmotta WDYT? do you think we can modify reagent to support this syntax?

This is the place of the code that needs to be changed: ns: reagent.impl.template

The problem is that coll? check, instead of clj->js we should transform the data recursively, as for maps and that should be enough to add support.

I think this is a dirty solution, so I'd like to explore if there's another way, maybe by using a custom compiler :thinking:

[ilmotta]

@ilmotta WDYT? do you think we can modify reagent to support this syntax?

I would also need to dig deeper to be sure @ulisesmac. My impression by skimming Reagent's source code is that the Reagent compiler is flexible, but maybe not as much as we need.

For instance, props are converted in certain parts of impl.template that might be outside our control, which means only a fork or upstream PR would work. More specifically, the function reagent.impl.template/convert-props is being called inside native-element directly as any other function, thus not via the Compiler protocol. So even if we define our own compiler, it won't be used there.

Source: https://github.com/reagent-project/reagent/blob/a14faba55e373000f8f93edfcfce0d1222f7e71a/src/reagent/impl/template.cljs#L206

[OmarBasem]

Thanks @ulisesmac for the clarification.

I am wondering, maybe we can edit our definition of blur-view in reanimated namespace and apply useAnimatedStyle to the received style prop there

[ulisesmac]

@ilmotta WDYT? do you think we can modify reagent to support this syntax?

I would also need to dig deeper to be sure @ulisesmac. My impression by skimming Reagent's source code is that the Reagent compiler is flexible, but maybe not as much as we need.

For instance, props are converted in certain parts of impl.template that might be outside our control, which means only a fork or upstream PR would work. More specifically, the function reagent.impl.template/convert-props is being called inside native-element directly as any other function, thus not via the Compiler protocol. So even if we define our own compiler, it won't be used there.

Source: https://github.com/reagent-project/reagent/blob/a14faba55e373000f8f93edfcfce0d1222f7e71a/src/reagent/impl/template.cljs#L206

Thank you for exploring the idea @ilmotta ! Yes, seems only a PR would work, but I'd prefer to not creating a fork. I'll try asking in the Clojurians slack first :eyes:

[ulisesmac]

Thanks @ulisesmac for the clarification.

I am wondering, maybe we can edit our definition of blur-view in reanimated namespace and apply useAnimatedStyle to the received style prop there

@OmarBasem We could do it, but sadly, we shouldn't. useAnimatedStyle is a hook, and we should declare it at the beginning of the component and keep it outside conditions, if we hide it, it's very likely that devs will do things like this:

(defn view []
  (let [,,,]
    [rn/view
     [,,,]
     [,,,]
     (when something?
       [reanimated/blur-view {,,,}])]
    ,,,))

Which is bad, because the hook is not declared at the beginning and also is wrapped in a condition. Seems very easy to make this mistake.

[OmarBasem]

Which is bad, because the hook is not declared at the beginning and also is wrapped in a condition. Seems very easy to make this mistake.

That's right, makes sense. Thanks!