lodev09 / react-native-true-sheet

The true native bottom sheet experience 💩
https://sheet.lodev09.com
MIT License
394 stars 12 forks source link

Dismount contents when dismissed #85

Closed ljukas closed 3 weeks ago

ljukas commented 3 weeks ago

Currently the contents of a sheet are mounted even if the sheet is dismissed, would be nice if we could choose if we want the contents the be mounted or not when sheet is dismissed state

lodev09 commented 3 weeks ago

Native handles this automatically so it shouldn't affect performance.

ljukas commented 3 weeks ago

Does this only apply for builds and not when in development mode ? Because on my simulator it seems to be huge burden.

lodev09 commented 3 weeks ago

However, React will render content first during first load and may affect performance. I would suggest you render TrueSheet when the screen is "shown". If you're using RN navigation, you could use the transition lifecycle to detect when to render.

ljukas commented 3 weeks ago

Will try that, thanks

ljukas commented 3 weeks ago

Id like to raise this again. Even if we disregard the performance issues, putting any kind of logic that follows the following:

  1. Mounts
  2. Execute

Is kind of broken since stuff mounts before the modal is presented.

Say you have a query from ie. react-query inside the truesheet, now it will execute before the sheet is presented, or after the sheet is closed if is polling

ljukas commented 3 weeks ago

Have made a temporary solution meanwhile:

import {
  forwardRef,
  useCallback,
  useImperativeHandle,
  useRef,
  useState,
} from 'react';

import { TrueSheet, TrueSheetProps } from '@lodev09/react-native-true-sheet';

export type TrueSheetModalMethods = {
  present: (index?: number) => Promise<void>;
  dismiss: () => Promise<void>;
  resize: (index: number) => Promise<void>;
};

export const TrueSheetModal = forwardRef<TrueSheetModalMethods, TrueSheetProps>(
  (props, ref) => {
    const internalRef = useRef<TrueSheet>(null);

    const { children, onDismiss, ...rest } = props;

    const [mounted, setMounted] = useState(false);

    useImperativeHandle(ref, () => ({
      present: async (index?: number) => {
        if (mounted) return;

        setMounted(true);
        await internalRef.current?.present(index);
      },
      dismiss: async () => {
        if (!mounted) return;

        await internalRef.current?.dismiss();
      },
      resize: async (index: number) => {
        if (!mounted) return;

        await internalRef.current?.resize(index);
      },
    }));

    const handleDismiss = useCallback(() => {
      onDismiss?.();

      setMounted(false);
    }, [onDismiss]);

    return (
      <TrueSheet ref={internalRef} onDismiss={handleDismiss} {...rest}>
        {mounted && children}
      </TrueSheet>
    );
  },
);
lodev09 commented 3 weeks ago

On simple content, pre-rendering data/content is an advantage too. Also the height calculation is more accurate if you're not using dynamic content. The way native works is kinda complicated and requires the content to be rendered first so it's not straightforward to logic in the native side.

I'm going for the "generic" use case and special conditions require special code.

Thanks for the feedback though!