-
-
Notifications
You must be signed in to change notification settings - Fork 736
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: add grid w/container query for projects #8344
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,75 @@ | ||
import { Box, Grid, styled } from '@mui/material'; | ||
import type { Theme } from '@mui/material/styles/createTheme'; | ||
|
||
export const ContentGridContainer = styled('div')({ | ||
containerType: 'inline-size', | ||
}); | ||
|
||
const ContentGrid2 = styled('article')(({ theme }) => { | ||
return { | ||
backgroundColor: theme.palette.divider, | ||
borderRadius: `${theme.shape.borderRadiusLarge}px`, | ||
overflow: 'hidden', | ||
border: `0.5px solid ${theme.palette.divider}`, | ||
gap: `2px`, | ||
|
||
display: 'grid', | ||
|
||
'&>*': { | ||
backgroundColor: theme.palette.background.paper, | ||
}, | ||
Comment on lines
+18
to
+20
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. make sure all direct children have a background color. This can be pushed to the gird items if we want. |
||
}; | ||
}); | ||
|
||
export const ProjectGrid = styled(ContentGrid2)(({ theme }) => ({ | ||
gridTemplateAreas: ` | ||
"title" | ||
"onboarding" | ||
"projects" | ||
"box1" | ||
"box2" | ||
"owners" | ||
`, | ||
|
||
'@container (min-width: 1000px)': { | ||
gridTemplateColumns: '1fr 1fr 1fr', | ||
gridTemplateAreas: ` | ||
"title onboarding onboarding" | ||
"projects box1 box2" | ||
". owners owners" | ||
`, | ||
}, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. because we use named areas in the items, that messes things up if we suddenly don't have any areas mentioned in the 1-column layout. As such, the workaround is to list them in all possible layouts. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Optionally (just thought of this): we could only use a grid layout when we're wider than X px, and use a flex layout otherwise. That's probably a cleaner option. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I prefer the grid/flex distinction depending on size There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I agree! |
||
|
||
'@supports not (container-type: inline-size)': { | ||
[theme.breakpoints.up('lg')]: { | ||
gridTemplateColumns: '1fr 1fr 1fr', | ||
gridTemplateAreas: ` | ||
"title onboarding onboarding" | ||
"projects box1 box2" | ||
". owners owners" | ||
`, | ||
}, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If your browser doesn't support container queries AND we're on 'lg' screens, here's the fallback. |
||
}, | ||
})); | ||
|
||
export const SpacedGridItem2 = styled('div')(({ theme }) => ({ | ||
padding: theme.spacing(4), | ||
})); | ||
|
||
export const EmptyGridItem = styled('div')(({ theme }) => ({ | ||
display: 'none', | ||
|
||
'@container (min-width: 1000px)': { | ||
display: 'block', | ||
}, | ||
|
||
'@supports not (container-type: inline-size)': { | ||
[theme.breakpoints.up('lg')]: { | ||
display: 'block', | ||
}, | ||
}, | ||
})); | ||
Comment on lines
+52
to
+64
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is an empty element to take up spaces in the grid when it's multi-column (the special |
||
|
||
export const ContentGrid = styled(Grid)(({ theme }) => ({ | ||
backgroundColor: theme.palette.background.paper, | ||
borderRadius: `${theme.shape.borderRadiusLarge}px`, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This file is mostly just indentation caused by the container element that I added. The only relevant change here is removing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need external containers to be able to use container queries. This contains the grid so we can check the size of it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why only inline and not block+inline?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That... I don't know. When playing with containers before, I've tried both, and I think this is usually the one that's worked out well for me. Must be something I don't quite get, but it works here, so 🤷🏼
(side note: if you read up on it and grok it proper, please let me know 😄 )