bvaughn / react-resizable-panels

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

Layout shift with latest version while pre-rendered on the server side #240

Closed xeladotbe closed 9 months ago

xeladotbe commented 9 months ago

In the latest version (1.0.4) you can see a layout shift when the panes are pre-rendered on the server side. With version 0.0.54 (which is also used in the example repository) there is no layout shift.

This example uses the latest version, here you can see the layout shift: https://codesandbox.io/p/devbox/infallible-bird-forked-y8w6sl?file=%2Fpackage.json%3A21%2C1-22%2C1

This examples uses 0.0.55, there is no layout shift: https://codesandbox.io/p/devbox/infallible-bird-q9cmdq?file=%2Fpackage.json%3A21%2C1-22%2C1

senadir commented 9 months ago

Seeing the same issue as well.

mikdanjey commented 9 months ago

I'm also facing same issue in Nextjs. Please fix this

bvaughn commented 9 months ago

Please fix this

This is a free library. If you’re offering to pay for a fix, then we can talk about a timeline/urgency. Otherwise, I’ll get to it when I get to it. :)

senadir commented 9 months ago

It goes without saying, but still needs saying, thank you for the effort @bvaughn

bvaughn commented 9 months ago

Thanks senadir.

This regression is likely due to a change in the format this component serializes persisted layout data to, which was done in order to support the request in #234. (Even small changes often come with a cost.) I'll think about the best way to fix this. I believe I was mistaken. I think the issue is the way the PanelGroup now defers calculation of style until mount, even if a defaultSize prop has been provided.

bvaughn commented 9 months ago

This issue has been fixed in 1.0.5.

Happy holidays.


❤️ → ☕ givebrian.coffee

xeladotbe commented 9 months ago

Hey @bvaughn,

thanks for the fix, unfortunately I still have the layout shifts with the latest version 1.0.5. I have updated the codesandbox link to reproduce the behavior.

Merry christmas!

bvaughn commented 9 months ago

There was a layout shift for me the first time I loaded that page, but not after that. Resizing and reloading after the first load works fine. I assume maybe you didn't initialize things to the same default on the server and client?

xeladotbe commented 9 months ago

I have, as you can see in my codesandbox link the layout is stored in a cookie which is read and passed to the client component.

I just looked at your PR, shouldn't it actually be if (size == undefined) { or if (!size) { because size is initalized with const size = layout[panelIndex]; and that doesn't return null, I wrote a test locally for this case and so my static markup is initalized with flex-grow: <value>

/**
 * @jest-environment node
 */

import { renderToString } from "react-dom/server";
import { act } from "react-dom/test-utils";
import { Panel, PanelGroup, PanelResizeHandle } from ".";

describe("PanelGroup", () => {
  it("should render default sizes to html markup", () => {
    act(() => {
      expect(
        renderToString(
          <PanelGroup direction="horizontal" id="group-without-handle">
            <Panel defaultSize={30} />
            <PanelResizeHandle />
            <Panel defaultSize={70} />
          </PanelGroup>
        )
      ).toMatchSnapshot();
    });
  });
});
bvaughn commented 9 months ago

I just looked at your PR, shouldn't it actually be if (size == undefined) { or if (!size) { because size is initalized with const size = layout[panelIndex]; and that doesn't return null

No, == null is shorthand for === undefined || === null

xeladotbe commented 9 months ago

here is a video that shows the behavior I see with the latest version

https://github.com/bvaughn/react-resizable-panels/assets/11685228/099fed97-7f0d-44b0-bee9-fe759af3750f

I have a separate, local test Harness using Next.js that confirms the same behavior.

bvaughn commented 9 months ago

So you're reporting a different behavior than originally. Slight layout shift?

bvaughn commented 9 months ago

Not sure what to say. Using this Sandbox here is what I'm seeing:

https://github.com/bvaughn/react-resizable-panels/assets/29597/1d644995-2d7b-4fd1-b229-40bf9582c4db

xeladotbe commented 9 months ago

okay, it must be my chrome, I can't reproduce the behavior in edge and firefox either. I also tried it in chrome in incognito mode, there I have the same problem. (I apologize for the inconvenience!)

I would like to buy you 2 coffees, but I don't have a credit card. are you on another platform where you can pay with paypal?

bvaughn commented 9 months ago

That’s pretty interesting. I wonder if it has something to do with an extension you have installed or something?

Thats very kind of you to offer. <3 uh…I’m on Venmo and Messenger (Pay) I guess 😅 if you really wanted you could email me and I’ll link you, but dont feel like to have to.

senadir commented 9 months ago

I can confirm the issue was fixed on my side, FWIW I saw this issue when using the component from shadcn/ui lib.

EvanBarnesAZ commented 2 months ago

I can confirm the issue was fixed on my side, FWIW I saw this issue when using the component from shadcn/ui lib.

I am having an issue with the component from the shadcn/ui lib.. Did you find a fix by chance?