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

[docs] Fix useMediaQuery ssr implementation example #18325

Merged
merged 1 commit into from
Nov 12, 2019
Merged

[docs] Fix useMediaQuery ssr implementation example #18325

merged 1 commit into from
Nov 12, 2019

Conversation

carloscuesta
Copy link
Contributor

@carloscuesta carloscuesta commented Nov 12, 2019

Hello! 👋

A colleague (@marlonbs) and me we've been struggling our heads a few days on the useMediaQuery SSR implementation for a Next.js project that we're building on.

After reading css-mediaquery docs and examples we figured it out that our SSR useMediaQuery implementation was not working because the px string was not included into the match. 🙈

I'm making a PR to update the docs just in case it could help anyone ❤️

Related issues: #15187 and #16132

Demo

When you use breakpoint helpers with useMediaQuery

import { useTheme } from '@material-ui/core/styles';
import useMediaQuery from '@material-ui/core/useMediaQuery';

function MyComponent() {
  const theme = useTheme();
  const matches = useMediaQuery(theme.breakpoints.up('sm'));

  return <span>{`theme.breakpoints.up('sm') matches: ${matches}`}</span>;
}

This statement theme.breakpoints.down('xs') returns something like this:

'(max-width:767.95px)'

Then the css-mediaquery won't match the value if the px string is not present:

Screenshot 2019-11-12 at 12 55 28

Screenshot 2019-11-12 at 12 55 47

@mui-pr-bot
Copy link

No bundle size changes comparing 4bfc2e4...0a4d4a6

Generated by 🚫 dangerJS against 0a4d4a6

@oliviertassinari oliviertassinari added the docs Improvements or additions to the documentation label Nov 12, 2019
@@ -121,7 +121,7 @@ function handleRender(req, res) {
const ssrMatchMedia = query => ({
matches: mediaQuery.match(query, {
// The estimated CSS width of the browser.
width: deviceType === 'mobile' ? 0 : 1024,
width: deviceType === 'mobile' ? '0px' : '1024px',
Copy link
Member

Choose a reason for hiding this comment

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

Wasn't there a meme about JS devs preaching strict equality === but using if (!value)? That applies here: https://github.com/ericf/css-mediaquery/blob/34ba6343938e72b270205447b297611c40140a4f/index.js#L40

So 1024 would work but the numeric literal 0 doesn't since it's falsy.

@eps1lon eps1lon merged commit 9a8ce0f into mui:master Nov 12, 2019
@eps1lon
Copy link
Member

eps1lon commented Nov 12, 2019

That must've been a frustrating debugging session. The fix is much appreciated!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants