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

Issue with using Responsive Grid with SSR #206

Closed
dukesx opened this issue Aug 14, 2021 · 7 comments
Closed

Issue with using Responsive Grid with SSR #206

dukesx opened this issue Aug 14, 2021 · 7 comments
Assignees
Labels
help wanted Contributions from community are welcome

Comments

@dukesx
Copy link

dukesx commented Aug 14, 2021

Issue:

Using UseMediaQuery to support responsive layouts is a bad idea. Its just not worth the pain and effort. Media Query api doesn't works on server , which means if you are a Next JS guy, you are stuck with manually adding responsive classes in the CSS or inline or wait for someone to manually change viewport width.

Now thats alot of work, fortunately, someone has thought of this already and Ant design, has a good way of handling this. They Provide Breakpoints such as xs, sm, md,lg,xl,xxl on the grid's property to handle this on server and the client side. They do it on css side too but then its also handled on js side shall the viewport width change. Check this out:

https://ant.design/components/grid/

Thanks for reaching out!

Please provide following information:

  • What package has an issue (@mantine/core, mantine/hooks or other)?
    Core
  • What package version are you using?
    Latest
  • In which browser and operating system issue occurred?
    All Major
  • If possible please attach link to codesandbox with reproduced issue
    No need, issue is clear
  • Do you have an idea how to fix the issue?
    One work around is that you can check ant design grid system, supports SSR, allows you to specify gutter and grid size for each breakpoint
  • Are you willing to participate in fixing this issue (it's totally fine if not) and create a pull request with the fix?
    -Of course
@rtivital
Copy link
Member

rtivital commented Aug 14, 2021

Hi @dukesx, thanks for creating such detailed issue, I was planning to implement something like this, but did not have enough time, if you would like to work on this ticket that is great, feel free to reach out here or on Discord if you need any help.
Here is a link on how to get started with Mantine locally – https://trello.com/c/8j4I7zfg/3-getting-started-with-mantine-locally
Please note that base branch for this feature will be next-minor, not master

@rtivital rtivital added the help wanted Contributions from community are welcome label Aug 15, 2021
@leorotzler
Copy link
Contributor

leorotzler commented Aug 20, 2021

I started with implementing something similar to the Ant Design approach, but there are a couple issues I stumbled upon:

If I use a function to pass props to the media queries, only the first media query styles will be applied. This seems to be a bug in JSS: cssinjs/jss#1343

My current approach:

export default createMemoStyles({
  root: ({ columns, spacing, grow, span }: ColStyles) => ({
    ...mediaQueryStyles(columns, spacing, grow, span),
  }),
  [`@media (min-width: ${xs}px)`]: {
    root: ({ breakpoints, columns, spacing, grow }: ColStyles) => ({
      ...mediaQueryStyles(columns, spacing, grow, breakpoints.xs),
    }),
  },
  [`@media (min-width: ${sm}px)`]: {
    root: ({ breakpoints, columns, spacing, grow }: ColStyles) => ({
      ...mediaQueryStyles(columns, spacing, grow, breakpoints.sm),
    }),
  },
  [`@media (min-width: ${md}px)`]: {
    root: ({ breakpoints, columns, spacing, grow }: ColStyles) => ({
      ...mediaQueryStyles(columns, spacing, grow, breakpoints.md),
    }),
  },
});

A lot of hardcoding, but sadly the only option I see right now. The solution seems to work though, but you get a lot of stylings / empty classes with this approach, which looks ugly.

@rtivital: I extended the default theme with a breakpoints object. The problem is, that I can't use the theme prop inside the media query to get the breakpoints, since as stated above, media queries are bugged inside functions.

Something like this does not work:

export default createMemoStyles({
  root: ({ theme, breakpoints, columns, spacing, grow, span }: ColStyles) => ({
    ...mediaQueryStyles(columns, spacing, grow, span),
    [`@media (min-width: ${theme.breakpoints.xs}px)`]: {
      ...mediaQueryStyles(columns, spacing, grow, breakpoints.xs),
     #},
  }),
});

@rtivital
Copy link
Member

Hi @Everek, thanks for trying things out, I would not count on jss team resolving this issue soon, so I would propose working on a workaround. Since the media queries are bugged in jss we can use <style> tag inside the component. It would be something like this:

export function Grid({
  themeOverride,
  gutter = 'md',
  children,
  grow = false,
  justify = 'flex-start',
  align = 'stretch',
  style,
  columns = 12,
  className,
  breakpoints,
  ...others
}: GridProps) {
  const theme = useMantineTheme(themeOverride);
  const spacing = getSizeValue({ size: gutter, sizes: theme.spacing });

  const cols = (Children.toArray(children) as React.ReactElement[]).map((col, index) =>
    React.cloneElement(col, { gutter, grow, columns, key: index })
  );

  return (
    <>
      <style>
        {`
        @media (max-width: ${breakpoints.sm}px) {
          .mantine-grid {
            // grid styles here
          }

          .mantine-col {
            // col styles here
          }
        }
      `}
      </style>
      <div
        style={{
          margin: -spacing / 2,
          display: 'flex',
          flexWrap: 'wrap',
          justifyContent: justify,
          alignItems: align,
          ...style,
        }}
        className={cx('mantine-grid', className)}
        {...others}
      >
        {cols}
      </div>
    </>
  );
}

It's nasty but should do the trick

@leorotzler
Copy link
Contributor

Good idea! I will try to work with that.

You can assign the ticket to me if you want.

@rtivital
Copy link
Member

Okay, thanks!

@leorotzler
Copy link
Contributor

Pull request has been opened: #230

@rtivital
Copy link
Member

rtivital commented Sep 1, 2021

Released in 2.4.0

@rtivital rtivital closed this as completed Sep 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Contributions from community are welcome
Projects
None yet
Development

No branches or pull requests

3 participants