Regression in 1.3.2: splitter snaps back to initial size on child change

vassilvk commented 2 years ago

Consider the following example:

import Splitter from "@devbookhq/splitter";
import { useState, useCallback } from "react";

function MyComponent() {
  const [count, setCount] = useState(0);
  const handleClick = useCallback(() => setCount(count + 1), [count]);

  return (
    <Splitter>
      <div onClick={handleClick}>Click me</div>
      <div>{count}</div>
    </Splitter>
  );
}

Background

A component which renders a splitter with two children. The component has a state - a counter. When the user clicks the first child of the splitter, the counter is incremented. The second child of the splitter renders the counter.

Reproduction steps

After the component is rendered, grab the splitter and move it to a new location
Click on the "Click me" child of the splitter

Observed behavior with splitter 1.3.2

The splitter snaps back to its initial location and the counter rendered in the second child goes up.

Observed behavior with 1.2.4

The splitter maintains its current position and the counter rendered in the second child goes up.

I believe the issue is caused by this: https://github.com/DevbookHQ/splitter/blob/master/src/index.tsx#L358

The useEffect hook is called whenever the children of the splitter change, which leads to the splitter snapping back to its initial location.

mlejva commented 2 years ago

Thank you for bringing up this issue. You might be indeed correct that it's caused by that useEffect hook. This feels important, so I'll try to make time for this and look into it.

The reason that that hook gets triggered every time Splitter's children change is so Splitter can support dynamically added or removed children. That's useful for conditional rendering, for example

<Splitter>
   {ifTimeHasPassed 
    ? (
         <div>child 1</div>
         <div>child 2</div>
       )
    : <div> child 1</div>
   }
<Splitter/>

Splitter has to know when its children change to recalculate the "splits" and re-render.

To prevent this behavior right now you could do two things. 1) Prevent the component that contains Splitter to re-render. That usually means moving state down or/and memoization. 2) Set the initialSizes prop and the onResizeFinished prop so on the next re-rerender, Splitter's children will have sizes that they had on the last finished resize action.

     const [initialSizes, setInitialSizes] = useState([50, 50])

     const handleResizeFinished = useCallback((_, newSizes) => {
       setInitialSizes(newSizes)
     })

    <Splitter
      initialSizes={initialSizes}
      onResizeFinished={handleResizeFinished}
    >
      <div>Tile 1</div>
      <div>Tile 2</div>
    </Splitter>

vassilvk commented 2 years ago

Thank you @mlejva. I had implemented a workaround by wrapping the devboookhq/splitter in my own splitter component which captures size changes and stores them in a state which is then pushed to devboookhq/splitter through initialSizes (essentially implementing your second suggestion above, but pushing the size state management into the splitter component).

May I suggest that this kind of persistent size state management is implemented directly in devboookhq/splitter?

mlejva commented 2 years ago

This might solve the issue. Can you share some code please? Or go ahead and open a PR if you feel like it!

vassilvk commented 2 years ago

import DevbookSplit, {
  SplitDirection as DevbookSplitDirection,
  GutterTheme as DevbookGutterTheme,
} from "@devbookhq/splitter";
import { FunctionComponent, useState, useCallback } from "react";

interface SplitterProps {
  direction?: keyof typeof DevbookSplitDirection;
  minWidths?: number[];
  minHeights?: number[];
  initialSizes?: number[];
  gutterTheme?: keyof typeof DevbookGutterTheme;
  gutterClassName?: string;
  draggerClassName?: string;
  onResizeStarted?: (pairIdx: number) => void;
  onResizeFinished?: (pairIdx: number, newSizes: number[]) => void;
  classes?: string[];
}

export const SplitDirection = DevbookSplitDirection;
export const GutterTheme = DevbookGutterTheme;

export const Splitter: FunctionComponent<SplitterProps> = ({
  direction = "Horizontal",
  gutterTheme = "Light",
  children,
  initialSizes,
  ...props
}) => {
  // Capture the splitter sizes and store them in a state to avoid https://github.com/DevbookHQ/splitter/issues/11
  const [persistentSizes, setPersistentSizes] = useState<number[] | undefined>(
    initialSizes
  );

  const handleResizeFinished = useCallback((_, newSizes) => {
    setPersistentSizes(newSizes);
  }, []);

  return (
    <DevbookSplit
      direction={DevbookSplitDirection[direction]}
      gutterTheme={DevbookGutterTheme[gutterTheme]}
      onResizeFinished={handleResizeFinished}
      initialSizes={persistentSizes}
      {...props}
    >
      {children}
    </DevbookSplit>
  );
};

mlejva commented 2 years ago

Thank you. I'll try to implement this into the next release.

devbookhq / splitter