KenanYusuf / react-resplit

Resizable split pane layouts for React applications 🖖
MIT License
27 stars 5 forks source link

Bug with setPaneSizes from useResplitContext #28

Closed TopVV closed 5 months ago

TopVV commented 7 months ago

Hello,

I faced a bug when I was trying to specify setPaneSizes from the useResplitContext hook in useEffect. It throws an error that states "Cannot read properties of undefined (reading 'type')" You can see an example on the code sandbox here https://codesandbox.io/p/sandbox/react-resplit-example-r7w6v2?file=%2Fsrc%2FPanes.js%3A13%2C50

I'm assuming it's coming from this line https://github.com/KenanYusuf/react-resplit/blob/c06b1caf90160e489d13958cc2c01e1c741d271f/lib/Root.tsx#L128C1-L129C1

Could you please take a look and help me with this one?

goliney commented 7 months ago

I see this issue too. @KenanYusuf could you please take a look?

KenanYusuf commented 7 months ago

Hey @TopVV @goliney

Thanks for providing a reproduction of your issue here. One issue I have spotted with the example is that you are trying to set the value to a number e.g. 0.1 instead of a string "0.1fr".

There is still an issue, however, where setPaneSizes is being called before the panes have been initiated properly.

I would try to avoid calling setPaneSizes in a useEffect, and set the initial value on the Resplit.Pane itself.

I have added a working example here: https://github.com/KenanYusuf/react-resplit/blob/main/src/examples/Context.tsx#L16-L18

TopVV commented 7 months ago

Hey @TopVV @goliney

Thanks for providing a reproduction of your issue here. One issue I have spotted with the example is that you are trying to set the value to a number e.g. 0.1 instead of a string "0.1fr".

There is still an issue, however, where setPaneSizes is being called before the panes have been initiated properly.

I would try to avoid calling setPaneSizes in a useEffect, and set the initial value on the Resplit.Pane itself.

I have added a working example here: https://github.com/KenanYusuf/react-resplit/blob/main/src/examples/Context.tsx#L16-L18

Hey @KenanYusuf , Thank you for the reply.

I might add some context here so it will be easier to understand what we are trying to achieve. We want to imperatively set panel sizes at some point for the component, e.g. they could be changed and received from the BE at the component mount or some point in its existence. Do you reckon we can do that somehow?

KenanYusuf commented 7 months ago

@TopVV at the moment, I don't think it is possible in a useEffect that runs on mount because the panes are not registered in time

A couple of options I can think of right now:

  1. Wait for your API response before rendering your resplit layout - this way the initial values can be set declaratively as props.
  2. Continue with your current approach of setting the pane size in a useEffect, but ensure it doesn't run before the resplit panes have been registerd. Although, I can imagine this could be tricky.

I appreciate the options are not ideal. Unfortunately, I do not have time to explore an alternative solutions to registering panes at the moment, but let me know if you have any ideas and I can work at on it at some point in the future, or feel free to submit a PR.