Skip to content

Conversation

@akwasniewski
Copy link
Contributor

Description

Implements V3 State Manager, to manually manage gestures.

Test plan

Tested on the following example, the long press should activate instantely, while the minDuration is extremely big.

Details
import React from 'react';
import { View, Text, StyleSheet } from 'react-native';
import { GestureHandlerRootView, GestureDetector, useLongPressGesture, GestureStateManager } from 'react-native-gesture-handler';

export default function TwoPressables() {
  const longPress = useLongPressGesture({
    onTouchesDown: (e) => {
      'worklet';
      console.log("touches down")
      GestureStateManager.begin(e.handlerTag)
      GestureStateManager.activate(e.handlerTag)
    },
    onActivate: () => {
      'worklet';
      console.log("long pressed")

    },
    minDuration: 100000000,
    disableReanimated: true
  })
  return (
    <GestureHandlerRootView>
      <View style={styles.root}>
        <GestureDetector gesture={longPress}>
          <View style={styles.outer}>
            <Text style={styles.label}>Outer</Text>
          </View>
        </GestureDetector>
      </View>
    </GestureHandlerRootView>
  );
}

const styles = StyleSheet.create({
  root: {
    flex: 1,
    justifyContent: 'center',
    alignItems: 'center',
    backgroundColor: '#f7f7f7',
  },
  outer: {
    padding: 20,
    backgroundColor: '#ddd',
    borderRadius: 12,
  },
  label: {
    fontSize: 18,
    marginBottom: 10,
  },
});

Comment on lines 4 to 7
declare const globalThis: {
_setGestureStateSync?: (handlerTag: number, state: State) => void;
_setGestureStateAsync?: (handlerTag: number, state: State) => void;
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be added as global.d.ts?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't that leak those definitions outside of the library?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually you might be right. As far as I remember we did it with @tjzel to overcome some TS problems, but this may be a better solution.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we maybe get rid of the file you linked with this approach?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't exist on next 😅

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We made the global.d.ts file with @m-bert so it's read only when developing the package and it's not exposed to the library consumer. If you leave this declare cost here then it will leak to the user.

You should move these types to global.d.ts.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, done in 30c8c82

Copy link
Member

@j-piasecki j-piasecki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but please wait for the comments to be resolved.

j-piasecki added a commit that referenced this pull request Dec 16, 2025
## Description

Restores `handlerTag` to the update and state change events. This
information will be necessary to use the new state manager added in
#3880.

## Test plan

Verify that `handlerTag` exists on events
Copy link
Contributor

@m-bert m-bert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if I'm a fan of .begin(tag), something along .for(tag).begin() in my opinion fits better, but given that it is relatively new feature maybe simpler API will be better.

Copy link
Contributor

@m-bert m-bert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@j-piasecki
Copy link
Member

j-piasecki commented Dec 16, 2025

Hmm, StateManager.forTag(tag).begin() sounds nice, but we'd need to create a new state manager for each forTag(tag) invocation. Though it's possible that the impact of that is negligible.

Edit: Maybe let's stick to the current approach, and we will see what feedback we get on that. WDYT?

@j-piasecki j-piasecki mentioned this pull request Dec 17, 2025
@akwasniewski
Copy link
Contributor Author

akwasniewski commented Dec 18, 2025

Hmm, StateManager.forTag(tag).begin() sounds nice, but we'd need to create a new state manager for each forTag(tag) invocation. Though it's possible that the impact of that is negligible.

Edit: Maybe let's stick to the current approach, and we will see what feedback we get on that. WDYT?

To be honest I prefer the simpler approach, but if you both want it to be forTag(tag).begin() then by the virtues of democracy I will change it. Let's see what feedback we get, and then decide.

Copy link
Contributor

@m-bert m-bert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know it might be an overkill, but since we have two different implementations, maybe we could introduce an interface to make sure that they both meet the same criteria? This probably won't change, but it won't hurt us to have one. Let me know what you think.

setGestureState(handlerTag, State.BEGAN);
};
export const GestureStateManager = {
begin(handlerTag: number): void {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
begin(handlerTag: number): void {
begin(handlerTag: number) {

I don't think we need those type declarations here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@akwasniewski
Copy link
Contributor Author

akwasniewski commented Dec 23, 2025

I know it might be an overkill, but since we have two different implementations, maybe we could introduce an interface to make sure that they both meet the same criteria? This probably won't change, but it won't hurt us to have one. Let me know what you think.

They are different by design, one is gesture specific, and the other global, accepting handlerTag

EDIT: as we discussed in the office, I misunderstood. Good idea, added interface in 7fa4cfa

Copy link
Contributor

@m-bert m-bert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merry Christmas 🎄🎅

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.

5 participants