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/media query default breakpoint unused #5638

Conversation

ishowta
Copy link
Contributor

@ishowta ishowta commented Feb 21, 2022

📝 Description

Since matchMedia always returns false during SSR, the code to check defaultBreakpoint was not reached by this refactor
Also, the test was not able to set up the same situation as SSR, so it was not failed.

const [currentBreakpoint, setCurrentBreakpoint] = React.useState(() => {
if (env.window.matchMedia) {
// set correct breakpoint on first render
const matchingBreakpointDetail = queries.find(
({ query }) => env.window.matchMedia(query).matches,
)
return matchingBreakpointDetail?.breakpoint
}
if (defaultBreakpoint) {
// use fallback if available
const fallbackBreakpointDetail = queries.find(
({ breakpoint }) => breakpoint === defaultBreakpoint,
)
return fallbackBreakpointDetail?.breakpoint
}
return undefined
})

⛳️ Current behavior (updates)

Always return undefined during SSR

🚀 New behavior

return defaultValue when SSR

💣 Is this a breaking change (Yes/No):

No

📝 Additional Information

@changeset-bot
Copy link

changeset-bot bot commented Feb 21, 2022

🦋 Changeset detected

Latest commit: 84ca354

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

This PR includes changesets to release 5 packages
Name Type
@chakra-ui/media-query Patch
@chakra-ui/react Patch
@chakra-ui/skeleton Patch
@chakra-ui/props-docs Patch
@chakra-ui/test-utils 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

@vercel
Copy link

vercel bot commented Feb 21, 2022

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/chakra-ui/chakra-ui-storybook/4nY5VVPxxpqx2mk8EHYhGCLVDPUt
✅ Preview: https://chakra-ui-storybook-git-fork-aidealab-fix-medi-e4b164-chakra-ui.vercel.app

@codesandbox-ci
Copy link

codesandbox-ci bot commented Feb 21, 2022

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 84ca354:

Sandbox Source
create-react-app-ts Configuration

Copy link
Contributor

@TimKolberger TimKolberger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this PR @ishowta 💖
Especially for the updated test suite!

@TimKolberger
Copy link
Contributor

Thank you again for you work @ishowta

I used the changes to fix another SSR / CSR issue with the useBreakpointValue hook.

Your commits + history are still intact and all credits are going to you for fixing this bug!
I'll close this PR in favor of #5651.

@all-contributors please add @ishowta for code.

@allcontributors
Copy link
Contributor

@TimKolberger

I've put up a pull request to add @ishowta! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants