pavelbabenko / react-native-awesome-gallery

Performant, native-like, and well-customizable gallery for React Native.
MIT License
494 stars 54 forks source link

Remove unnecessary checks #58

Closed mykyta-rusyn closed 11 months ago

mykyta-rusyn commented 11 months ago

Both Tap and LongPress gestures have unnecessary checks. For example, tapGesture is disabled when onTap is not provided, so we can add a non-null assertion without checking its value. Similar for rest gestures.

Changes in the tapGesture:

.onEnd(() => {
  'worklet';
  if (!isActive.value) return;
  if (onTap && !interruptedScroll.value) {
    runOnJS(onTap)();
  }
  interruptedScroll.value = false;
});

.onEnd(() => {
  'worklet';
  if (!isActive.value) return;
  if (!interruptedScroll.value) {
    runOnJS(onTap!)();
  }
  interruptedScroll.value = false;
});

In the doubleTapGesture:

.onEnd(({ x, y, numberOfPointers }) => {
  'worklet';
  if (!isActive.value) return;
  if (numberOfPointers !== 1) return;
  if (onTap && interruptedScroll.value) {
    interruptedScroll.value = false;
    if (onTap) {
      runOnJS(onTap)();
    }
    return;
  }
  if (onDoubleTap) {
    runOnJS(onDoubleTap)();
  }

.onEnd(({ x, y, numberOfPointers }) => {
  'worklet';
  if (!isActive.value) return;
  if (numberOfPointers !== 1) return;
  if (onTap && interruptedScroll.value) {
    interruptedScroll.value = false;
    runOnJS(onTap)();
    return;
  }
  if (onDoubleTap) {
    runOnJS(onDoubleTap)();
  }

And in the longPress:

const longPressGesture = Gesture.LongPress()
  .enabled(!!onLongPress)
  .maxDistance(10)
  .onStart(() => {
    'worklet';
    if (interruptedScroll.value) {
      interruptedScroll.value = false;
      return;
    }
    if (onLongPress) {
      runOnJS(onLongPress)();
    }
  });

const longPressGesture = Gesture.LongPress()
  .enabled(!!onLongPress)
  .maxDistance(10)
  .onStart(() => {
    'worklet';
    if (interruptedScroll.value) {
      interruptedScroll.value = false;
      return;
    }
    runOnJS(onLongPress)();
  });
pavelbabenko commented 11 months ago

Hi @mykyta-rusyn The check are required for TypeScript

mykyta-rusyn commented 11 months ago

Hello. You can use the none-null assertion to hide this TypeScript error. And JavaScript will not need to check for the presence of functions every time, because this checks is already in the handlers options

pavelbabenko commented 11 months ago

@mykyta-rusyn Tbh, I don't like none-null assertion and I don't think it's a good approach. Callbacks are not running in JS, but in UI thread, so no need to do that.

mykyta-rusyn commented 11 months ago

Okay, thanks for your answer!