Skip to content

Conversation

@j-piasecki
Copy link
Member

@j-piasecki j-piasecki commented Jul 24, 2025

Description

Changes the button implementation of the native GestureHandlerButton component, not to re-export the button component. Instead, it provides a "sandwich" implementation based on display: contents.

The issues we had with the styling of the buttons in the past stemmed from it extending ViewGroup instead of ReactViewGroup. This cannot be changed due to the difference in handling of onTouchEvent. We experimented with wrapping the button with a View in the past, but properly handling layout turned out to be quite a challenge. Hopefully, with the addition of display: contents, this will be easier now.

This PR changes the structure of the button to be:

  • RNGestureHandlerButtonWrapperNativeComponent
    • View
      • ButtonComponent

The styles passed from the user are split into two categories:

  • visual only
  • affecting layout

Those affecting the layout need to be set on the button itself, while visual changes need to be set on the View wrapping it. Exceptions are: zIndex, transform, and transformOrigin, which need to be on the View. zIndex because the View is being rendered in the hierarchy at the level the button would be expected, and transforms to properly calculate touch coordinate transforms.

Why not just ButtonComponent?

We need a View component as its parent to handle all styles that React Native supports properly.

Why not just ButtonComponent wrapped with View?

In order for the view to always match the layout of the button, we need to set it at the shadow node level. Shadow nodes only have references to their children, not their parents. To have access to the layout of the button and the ability to copy it to the view wrapping it, there needs to be one more node as the parent of that subtree. That node is responsible for making sure that the size and positioning of the View match the Button.

The native components for the wrapper serve no purpose, since it's being rendered with display: contents, but RN doesn't know that - there needs to be something it can link to.

Test plan

See the ContentsButton file

@j-piasecki j-piasecki force-pushed the @jpiasecki/display-contents-button branch from c5661d1 to fbf37bb Compare September 12, 2025 08:54
@j-piasecki j-piasecki force-pushed the @jpiasecki/display-contents-button branch from 612fb8e to 773da37 Compare October 15, 2025 13:31
@j-piasecki j-piasecki force-pushed the @jpiasecki/display-contents-button branch from 773da37 to d735767 Compare October 24, 2025 13:08
@j-piasecki j-piasecki marked this pull request as ready for review October 24, 2025 13:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants