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 example to v5 theme API #30454

Merged
merged 2 commits into from
Dec 31, 2021

Conversation

ValentinH
Copy link
Contributor

@ValentinH ValentinH commented Dec 31, 2021

I tried following the example and the ssrMatchMedia was never used. I think the current example is still using v4 implementation. I updated it to the new Theme structure of v5.

https://deploy-preview-30454--material-ui.netlify.app/components/use-media-query/#server-side-rendering

@mui-pr-bot
Copy link

mui-pr-bot commented Dec 31, 2021

No bundle size changes

Generated by 🚫 dangerJS against d604502

@oliviertassinari oliviertassinari changed the title fix(docs): update useMediaQuery SSR example to v5 theme API [docs] FIx useMediaQuery SSR example to v5 theme API Dec 31, 2021
@oliviertassinari oliviertassinari added bug 🐛 Something doesn't work docs Improvements or additions to the documentation labels Dec 31, 2021
@hbjORbj hbjORbj changed the title [docs] FIx useMediaQuery SSR example to v5 theme API [docs] Fix useMediaQuery SSR example to v5 theme API Dec 31, 2021
@ValentinH
Copy link
Contributor Author

ValentinH commented Dec 31, 2021

While experimenting with the SSR example, I'm wondering if using defaultMatches could also be a valid and simpler solution?

Here's what I have in mind:

const deviceType = parser(req.headers['user-agent']).device.type || 'desktop';

const theme = createTheme({
    components: {
      // Change the default options of useMediaQuery
      MuiUseMediaQuery: {
        defaultProps: {
          defaultMatches: deviceType === 'mobile',
        },
      },
    },
  });

  const html = ReactDOMServer.renderToString(
    <ThemeProvider theme={theme}>
      <App />
    </ThemeProvider>,
  );

@oliviertassinari
Copy link
Member

oliviertassinari commented Dec 31, 2021

I'm wondering if using defaultMatches could also be a valid and simpler solution?

From my perspective, defaultMatches has one problem: it doesn't account for what the media query is, is asking for below sm? above md? Results won't be consistent in the app.

@oliviertassinari oliviertassinari merged commit dfea493 into mui:master Dec 31, 2021
@oliviertassinari
Copy link
Member

@ValentinH Thanks!

@ValentinH
Copy link
Contributor Author

I'm wondering if using defaultMatches could also be a valid and simpler solution?

From my perspective, defaultMatches has one problem: it doesn't account for what the media query is, is asking for below sm? above md? Results won't be consistent in the app.

Yes this totally makes sense. I was stuck in the mindset of only having mobile or desktop variants.

@ValentinH ValentinH deleted the patch-2 branch December 31, 2021 18:45
wladimirguerra pushed a commit to wladimirguerra/material-ui that referenced this pull request Feb 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work docs Improvements or additions to the documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants