yyllff / split-pane-react

Resizable split panes for React.js.
https://yyllff.github.io/split-pane-react
MIT License
95 stars 25 forks source link

Pane is always expected to be the direct child #8

Open ilyasmez opened 2 years ago

ilyasmez commented 2 years ago

Nice work on the component, I like the flexibility it offers.

I facing one issue though. Because of this piece of code, SplitPane is always expecting <Pane> as a direct child, meaning that I can't do something like this:

const MyPane = ({children}) => (<Pane minSize={200} maxSize={500}>{children}</Pane>);

const MyPage = () => (
  <SplitPane>
    <MyPane>Foo</MyPane>
    <MyPane>Bar</MyPane>
  </SplitPane>
);

This is just a simplified example, <MyPane> can be very complex with a lot of customization.

ilyasmez commented 2 years ago

I'd like to add that even the example below won't work because of how Emotion wraps components:

const MyPage = () => (
  <SplitPane>
    <Pane css={myStyles}>Foo</Pane>
    <Pane css={myStyles}>Bar</Pane>
  </SplitPane>
);

As a quick solution I'd suggest to remove if (childNode.type === Pane) here: SplitPane.tsx#L64.

And as a long term solution it would make more sense to use Context/Provider and make the SplitPane provide some functions that Pane can use to register itself, e.g.

const Pane = (props) => (
  const { registerPane } = useSplitPane();

  useEffect(() => {
    registerPane(props);
  }, []);

  [...]
);

const SplitPane = ({children}) => (
  const registerPane = ({minSize, maxSize}) => {}; // logic to calculate the limits

  return <SplitPaneProvider value={registerPane}>{children}</SplitPaneProvider>
);
yyllff commented 2 years ago
const MyPane = ({children}) => (<Pane minSize={200} maxSize={500}>{children}</Pane>);

const MyPage = () => (
  <SplitPane>
    <MyPane>Foo</MyPane>
    <MyPane>Bar</MyPane>
  </SplitPane>
);
const MyPage = () => (
 <SplitPane>
   <Pane css={myStyles}>Foo</Pane>
   <Pane css={myStyles}>Bar</Pane>
 </SplitPane>
);

Thank you for your suggestions. Next, I will expand the use of this method.

yyllff commented 2 years ago

And as a long term solution it would make more sense to use Context/Provider and make the SplitPane provide some functions that Pane can use to register itself, e.g.

const Pane = (props) => (
  const { registerPane } = useSplitPane();

  useEffect(() => {
    registerPane(props);
  }, []);

  [...]
);

const SplitPane = ({children}) => (
  const registerPane = ({minSize, maxSize}) => {}; // logic to calculate the limits

  return <SplitPaneProvider value={registerPane}>{children}</SplitPaneProvider>
);

It's also a good idea to provide SplitPaneProvider. Can you provide a more complete case? I haven't fully understood your meaning.

ILoveQier commented 2 years ago

Nice work on the component, I'm liking the flexibility it offers.

I facing one issue though. Because of this piece of code, SplitPane is always expecting <Pane> as a direct child, meaning that I can't do something like this:

const MyPane = ({children}) => (<Pane minSize={200} maxSize={500}>{children}</Pane>);

const MyPage = () => (
  <SplitPane>
    <MyPane>Foo</MyPane>
    <MyPane>Bar</MyPane>
  </SplitPane>
);

This is just a simplified example, <MyPane> can be very complex with a lot of customization.

what you have suggested really makes sense , i will considered it and add it as a feature