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

fix: initialize dimensions before first paint #526

Merged
merged 1 commit into from Feb 2, 2023
Merged

fix: initialize dimensions before first paint #526

merged 1 commit into from Feb 2, 2023

Conversation

Gelio
Copy link
Contributor

@Gelio Gelio commented Jan 11, 2023

The container dimensions are set in the ResizeObserver callback. It fires only after the first paint. This means that the dimensions are not initialized during the first paint, which causes a layout shift.

The dimensions are known before the first paint. We can retrieve the container width and height by using getBoundingClientRect inside useLayoutEffect. This way we can initialize the dimensions correctly so the panes are already sized the right way during the first render.

This started being a problem after #404 since the panes are visible during the first paint now.

Videos

Here are videos from the initial render before and after the fix. I suspect this bug is only noticeable when there are many useLayoutEffect/useEffect/React elements to render, as the delay between the first and second paint is greater in these cases.

Before the fix:

2023-01-11.15-55-53.mp4

After the fix:

2023-01-11.15-57-25.mp4

The container dimensions are in the `ResizeObserver` callback. It fires
only after the first paint. This means that the dimensions are not
initialized during the first paint, which causes a layout shift.

The dimensions are known before the first paint. We can retrieve the
container width and height by using `getBoundingClientRect` inside
`useLayoutEffect`. This way we can initialize the dimensions correctly
so the panes are already sized the right way during the first render.
@netlify
Copy link

netlify bot commented Jan 11, 2023

Deploy Preview for allotment-website canceled.

Name Link
🔨 Latest commit 3df3fcb
🔍 Latest deploy log https://app.netlify.com/sites/allotment-website/deploys/63bed4310975700009988e31

@netlify
Copy link

netlify bot commented Jan 11, 2023

Deploy Preview for allotment-storybook ready!

Name Link
🔨 Latest commit 3df3fcb
🔍 Latest deploy log https://app.netlify.com/sites/allotment-storybook/deploys/63bed43145df8f000a582249
😎 Deploy Preview https://deploy-preview-526--allotment-storybook.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@johnwalley
Copy link
Owner

Hi @Gelio. Sorry, I missed this! Looks really good and I appreciate the time you've taken to put together the PR and videos. I'll get around to getting this merged and released in the next few days.

@Gelio
Copy link
Contributor Author

Gelio commented Jan 31, 2023

No worries, thanks for getting back to this PR. I'm looking forward to seeing the fix published

@johnwalley johnwalley added the bug Something isn't working label Feb 2, 2023
@johnwalley johnwalley merged commit 2706777 into johnwalley:main Feb 2, 2023
@Gelio Gelio deleted the fix-initial-layout-shift branch February 2, 2023 20:07
@Gelio
Copy link
Contributor Author

Gelio commented Feb 2, 2023

Thanks for publishing the fix 🚀

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

Successfully merging this pull request may close these issues.

None yet

2 participants