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

UnderlineNav2: Use useIsomorphicLayoutEffect to make it SSR friendly #2625

Merged
merged 3 commits into from
Dec 1, 2022

Conversation

broccolinisoup
Copy link
Member

@broccolinisoup broccolinisoup commented Nov 29, 2022

Closes #2617

This PR updates UnderlineNav using useIsomorphicLayoutEffect instead of React's default useLayoutEffect to make it SSR friendly.

Note: UnderlineNav's initial layout rendering is very much depending on viewport size and JS being ran on the browser to calculate how many items that can fit into the current viewport. Because of that, this update causes an undesirable behaviour in SSR. Nothing blocking the functionality though it is just something we have not worked thoroughly. I'll work with the design team to think about a "loading state" (Thanks @joshblack for this terminology!) until we get JS running on the browser.

In the meantime, it believe this should solve the issue.

Take a look at the What we look for in reviews section of the contributing guidelines for more information on how we review PRs.

@changeset-bot
Copy link

changeset-bot bot commented Nov 29, 2022

🦋 Changeset detected

Latest commit: 08f09b9

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@primer/react Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@broccolinisoup broccolinisoup added the skip changeset This change does not need a changelog label Nov 29, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Nov 29, 2022

size-limit report 📦

Path Size
dist/browser.esm.js 78.95 KB (0%)
dist/browser.umd.js 79.58 KB (0%)

@broccolinisoup broccolinisoup temporarily deployed to github-pages November 29, 2022 05:59 Inactive
@github-actions github-actions bot temporarily deployed to storybook-preview-2625 November 29, 2022 05:59 Inactive
@broccolinisoup broccolinisoup marked this pull request as ready for review December 1, 2022 01:54
@broccolinisoup broccolinisoup requested a review from a team December 1, 2022 01:54
@broccolinisoup broccolinisoup force-pushed the underlineNav-isomorphicLayoutEffect branch from a390edc to 2bd997d Compare December 1, 2022 21:04
@broccolinisoup broccolinisoup removed the skip changeset This change does not need a changelog label Dec 1, 2022
@broccolinisoup broccolinisoup enabled auto-merge (squash) December 1, 2022 21:07
@github-actions github-actions bot temporarily deployed to storybook-preview-2625 December 1, 2022 21:11 Inactive
@broccolinisoup broccolinisoup temporarily deployed to github-pages December 1, 2022 21:12 Inactive
@broccolinisoup broccolinisoup merged commit 404e2b1 into main Dec 1, 2022
@broccolinisoup broccolinisoup deleted the underlineNav-isomorphicLayoutEffect branch December 1, 2022 21:12
@primer-css primer-css mentioned this pull request Dec 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

When SSR, useLayoutEffect warning in UnderlineNav v2
2 participants