Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
vassilvk opened this issue Feb 2, 2022 · 5 comments
Labels
bug Something isn't working

Comments

@vassilvk
Copy link

vassilvk commented Feb 2, 2022

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
Copy link
Contributor

mlejva commented Feb 3, 2022

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>

@mlejva mlejva added the bug Something isn't working label Feb 3, 2022
@vassilvk
Copy link
Author

vassilvk commented Feb 3, 2022

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
Copy link
Contributor

mlejva commented Feb 3, 2022

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

@vassilvk
Copy link
Author

vassilvk commented Feb 3, 2022

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
Copy link
Contributor

mlejva commented Feb 3, 2022

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants