-
-
Notifications
You must be signed in to change notification settings - Fork 730
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
fix: add grid w/container query for projects #8344
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Manifest Files |
export const ContentGridContainer = styled('div')({ | ||
containerType: 'inline-size', | ||
}); |
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 😄 )
'&>*': { | ||
backgroundColor: theme.palette.background.paper, | ||
}, |
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.
make sure all direct children have a background color. This can be pushed to the gird items if we want.
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 comment
The 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 comment
The 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 comment
The 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 comment
The 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 comment
The 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 EmptyGridItem = styled('div')(({ theme }) => ({ | ||
display: 'none', | ||
|
||
'@container (min-width: 1000px)': { | ||
display: 'block', | ||
}, | ||
|
||
'@supports not (container-type: inline-size)': { | ||
[theme.breakpoints.up('lg')]: { | ||
display: 'block', | ||
}, | ||
}, | ||
})); |
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.
This is an empty element to take up spaces in the grid when it's multi-column (the special .
area). We hide it to avoid showing an empty div in the list when we collapse the grid.
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.
This file is mostly just indentation caused by the container element that I added. The only relevant change here is removing item
, lg
, and md
props from the grid items and switching to using the new components instead. Also: add grid areas.
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.
Very nice work!
This PR is a work in progress, but the current state aims to suggest a direction. The main goals of this are:
To achieve those goals, this PR:
Container query demo (both with the same screen sizes):
Sidebar closed:
Sidebar open: