Split layout props in GenericTouchable instead of using containerStyle

[janicduplessis]

Description

Touchable components do not layout as expected since they are wrapped in an extra view. To workaround this they have an extra containerStyle prop, but this is confusing and different from the ones in react-native.

This removes the containerStyle props and instead split the style prop passed between styles that should apply to the inner and outer view. This makes it seemlessly behave as if it was a single view.

Test plan

Using something like:

<View style={{ flexDirection: "row" }}>
  <TouchableOpacity
    style={{
      flex: 1,
      width: 50,
      height: 50,
      backgroundColor: "red",
      marginRight: 16,
      borderColor: "blue",
      borderWidth: 4,
      borderRadius: 6,
    }}
  />
  <TouchableOpacity
    feedback="opacity"
    sx={{
      flex: 1,
      width: 50,
      height: 50,
      backgroundColor: "red",
      marginRight: 16,
    }}
  />
  <TouchableOpacity
    feedback="opacity"
    sx={{
      flex: 1,
      width: 50,
      height: 50,
      backgroundColor: "red",
      marginRight: 16,
    }}
  />
</View>

Before:

After:

[yfunk]

Does AnimatedBaseButton actually support all the accessibility props from coreProps? As far as I know this is the reason for using nested Views inside *Button components (docs).

[janicduplessis]

Yea looks like you’re right, I think it would be possible to keep the extra view, but split the style between the 2 views to make sure it lays out as expected if it was only a single view.

For example the flex style needs to be on the outer view while flexDirection should be on the inner one.

[yfunk]

I thought about doing the same thing when making my own touchable component, but turns out View has quite a few style props so I didn't bother in the end (here's the list).

This would definitely be a nice feature though if you can find a neat way to split the styles.

[janicduplessis]

@yfunk I was also doing something like this in my Touchable component. I've taken the code and revised it to make sure all styles are included. Let me know what you think about this approach.

Will see if I can add an example too to test complex layout cases.

[pontusab]

The reason for containerStyle is because of the extra <Animated.View />, right?

I solve this by removing the containerStyle altogether and instead of a View wrap the BaseButton with:

const AnimatedBaseButton = Animated.createAnimatedComponent(BaseButton);

Then we can remove the View and therefore have the same behavior as in RN.

If you want I can create a PR with this so you can have a look, I think it's better to skip the containerStyle and just keep style as RN.

wdyt?

[yfunk]

@janicduplessis Sorry for my late answer on this. I left a few comments, but overall the approach looks good to me 👍

[yfunk]

I'm not sure if these are really all the style props that need to be applied to the outer view or if there are more (e.g. backfaceVisibility, elevation, transform and all of ShadowStyleIOS).

It would probably be best to add complex layout examples to test this, as you mentioned.

[yfunk]

The style prop will be split on every render (and also causes bridge traffic since the object changes). Not sure if this has any noticeable performance implications but that's something to keep in mind since some screens may contain a lot of touchables.

[yfunk]

I think this should be splitStyleProp<T extends ViewStyle> for better clarity.

[yfunk]

The reason for containerStyle is because of the extra <Animated.View />, right?

I solve this by removing the containerStyle altogether and instead of a View wrap the BaseButton with:

const AnimatedBaseButton = Animated.createAnimatedComponent(BaseButton);

Then we can remove the View and therefore have the same behavior as in RN.

This was also the original approach of this PR, but unfortunately the extra Animated.View is needed to support accessibility props (see discussion above).

[jakub-gonet]

@janicduplessis, what's the state on this?

[janicduplessis]

I ended up hitting some edge cases where splitting props broke certain layouts, I don't remember exactly what, but will have to have another look to see if I can figure out something.