Reimplement Incubator.TouchableOpacity with reanimatedv2

[ethanshar]

Description

Reimplement Incubator.TouchableOpacity with Reanimated v2
The component is currently exported as Incubator.TouchableOpacity2
eventually, we'll replace it with Incubator.TouchableOpacity (without a migration, since the API is the same)

Changelog

Reimplement Incubator.TouchableOpacity with Reanimated v2 as Incubator.TouchableOpacity2

[lidord-wix]

Hey, few things:

1. onLongPress acts on release instead of after a constant time.
2. The onCancel doesn't work.
3. The tabController doesn't work while using this touchableOpacity.

[ethanshar]

@lidord-wix
I fixed both 1. and 2.
And about 3. The current TabController is not using this TouchableOpacity so there shouldn't be any problem

[lidord-wix]

@ethanshar
Will the TabController not use this TouchableOpacity in the future?

[ethanshar]

@ethanshar
Will the TabController not use this TouchableOpacity in the future?

I'm planning to use it in TabController2

[lidord-wix]

Why does the onPress need all the props as an argument?
This will update the callback for every prop change

[ethanshar]

Well technically you are right, we might consider an optimization for it.
But see first my big comment on the props feature we pass back during press callbacks.

.....That's the reason we pass all props, and that's why we all the props are in the deps arrays.

Anyway, I agree it's not optimal.
Maybe we can make an assumption that the "custom" prop the user passed is a static prop that won't change, then we can ignore all the props in the deps array. It means the user will get back the initial value they passed with a custom prop.
WDYT?

[lidord-wix]

Is that a reasonable assumption?
Maybe the user wants to know about props change, no?

[ethanshar]

I wanna say that yes, it's reasonable. But technically if our assumption is wrong then it might introduce a bug.
Anyway, I have another solution.

Not so long ago, we added a dedicate prop called customValue to the original TouchableOpacity component.
See here -> https://github.com/wix/react-native-ui-lib/blob/master/src/components/touchableOpacity/index.tsx#L45

It's purpose was to be an explicit prop to pass on a custom value as I described in the previous comment.
Also the reason we had to declare an explicit type for it is due to typings.
When people use <TouchableOpacity someValue={...}/> it threw a TS error that someValue is not a valid prop.
So we have customValue and now they can just <TouchableOpacity customValue={...}/>

So back to our concern, see my recent code change.
I only condition the onPress callbacks on props.onPress and on props.customValue - which is the value that the user expect to pass on..

Let me know it all makes sense? (:

[lidord-wix]

Same here :)

[ethanshar]

Same answer (:

[lidord-wix]

Why does the type of props here is any?

[ethanshar]

It's because of a behavior/feature we added in uilib to our touchable components.
We pass back the props the user originally passed to the component.

Why?
It solve a common issue when rendering multiple repeating items (like in the case of lists) and other use-cases..

For instance, You can have the following renderItem method for a FlatList

renderItem = ({item, index}) => {
  return <TouchableOpacity onPress={() => doSomething(index)}  />
}

You see we have to pass an inline callback here (which is bad for performance) cause we need to pass on the specific index of the item, for each item.

With the feature we added, the user can do the following

doSomething = (touchableProps) => {
  const {index} = touchableProps;
  console.log('item index is:`, index)
}

renderItem = ({item, index}) => {
  return <TouchableOpacity onPress={doSomething} index={index}  />
}

Here we didn't have to pass an inline callback, we passed the callback by ref and another custom prop, the index.

That's why props in the onPress callback type is any, cause the user can basically pass/accept anything here.

Let me know if it's not clear, we can sync about It quickly..