bvaughn / react-resizable-panels

https://react-resizable-panels.vercel.app/
MIT License
3.87k stars 135 forks source link

Assertion failed error #233

Closed rortan134 closed 9 months ago

rortan134 commented 9 months ago

Version 1.0.1

image image

<PanelGroup direction="horizontal" onLayout={onLayout}>
      <Panel order={1}>{/* ... */}</Panel>
      <PanelResizeHandle />
      <Panel order={2}>{/* ... */}</Panel>
      <PanelResizeHandle />
      <Panel order={3}>{/* ... */}</Panel>
    </PanelGroup>
bvaughn commented 9 months ago

@rortan134 I'm going to need more of a repro than this. Can you share a Code Sandbox that reproduces the bug you're seeing? I have no idea what's in your app-index.js file on line 32.

rortan134 commented 9 months ago

app-index is dev (nextjs) compiled code image

Reverting back to 0.64 works fine (with the props changed)

reproduction

bvaughn commented 9 months ago

The sandbox you linked to me does not log or throw any errors. Works fine.

bvaughn commented 9 months ago

Looking at your code, it is worth pointing out that you have invalid constraints:

        <ResizablePanel order={0} minSize={200} maxSize={300}></ResizablePanel>
        <ResizablePanelHandle />
        <ResizablePanel order={1} minSize={200} maxSize={200}></ResizablePanel>
        <ResizablePanelHandle />
        <ResizablePanel order={2} minSize={200} maxSize={300}></ResizablePanel>

Min and max sizes should be percentages (0-100%) so 200% and 300% aren't valid numbers.

rortan134 commented 9 months ago

Looking at your code, it is worth pointing out that you have invalid constraints:

I abstracted it so I can pass in pixel values (look at the my-drawer.tsx components)

The sandbox you linked to me does not log or throw any errors. Works fine.

I just tried it in two other browsers and it seems to happen consistently (Opera above and chrome below) image

(I forgot switching the error tab on chrome to show the error but its there)

Doesn't seem to happen on firefox though...

bvaughn commented 9 months ago

I'm testing on Chrome and it doesn't happen for me. My guess is that there's a problem in your abstraction somewhere.

bvaughn commented 9 months ago

You might be able to record a Replay of the bug happening which would be helpful for me to look at (but Replay is also Gecko based so maybe not)

rortan134 commented 9 months ago

Oh wow, turns out I still had the panels persisted layout stored in localStorage on every browser I tested from previous updates and mistakenly reused the same panels IDs on every test I did... Sorry for the misunderstanding. 🤦‍♂️

bvaughn commented 9 months ago

No problem. That’s an easy mistake to make.

I’m not sure how to detect and warn about this case, but it does seem like some thing that I could maybe do a better job of surfacing in development mode

any ideas?

rortan134 commented 9 months ago

No problem. That’s an easy mistake to make.

I’m not sure how to detect and warn about this case, but it does seem like some thing that I could maybe do a better job of surfacing in development mode

any ideas?

I guess you could attach a version identifier to the stored localstorage key and compare it with the current package version on first render. The only reason this happened though is that we went from defaultSize to defaultSizePixels back to defaultSize, so I don't think it'll be happening again now that it's at 1.0 right?

bvaughn commented 9 months ago

The only reason this happened though is that we went from defaultSize to defaultSizePixels back to defaultSize, so I don't think it'll be happening again now that it's at 1.0 right?

I think that's true.

bvaughn commented 9 months ago

Updated the storage key to avoid this scenario in 1.0.2