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

Grid wrapper example for CSS Layout Cookbook #12

Merged
merged 1 commit into from Nov 5, 2018

Conversation

Projects
None yet
2 participants
@mbarker84
Copy link
Contributor

mbarker84 commented Oct 20, 2018

This is a pattern I use a lot with CSS Grid, consisting of a number of central columns with a maximum width, and a flexible column either side. It enables you to align grid items to the central wrapper or break them out to edge of the grid. I find it really useful and use it on practically every project, so I’d love to contribute it to the Layout Cookbook.

@chrisdavidmills

This comment has been minimized.

Copy link
Contributor

chrisdavidmills commented Oct 22, 2018

Hi Michelle, thanks for the new recipe! @rachelandrew , do you want to give this a quick review?

We'll also need a page adding to https://developer.mozilla.org/en-US/docs/Web/CSS/Layout_cookbook, to cover it, using the sample template Rachel used for the others. Are you OK to write one?

@mbarker84

This comment has been minimized.

Copy link
Contributor

mbarker84 commented Oct 22, 2018

Hi @chrisdavidmills, yes I’m happy to create a page – I‘ll just need page-creation permission for my account :) Thanks!

@chrisdavidmills

This comment has been minimized.

Copy link
Contributor

chrisdavidmills commented Oct 22, 2018

Sure thing. What's your MDN username?

@mbarker84

This comment has been minimized.

Copy link
Contributor

mbarker84 commented Oct 23, 2018

It’s mbarker84, same as my Github. Thanks!

@chrisdavidmills

This comment has been minimized.

Copy link
Contributor

chrisdavidmills commented Oct 23, 2018

OK, you've now got the necessary permissions. Cheers for the contribution! And feel free to ping me if you have questions.

@mbarker84

This comment has been minimized.

Copy link
Contributor

mbarker84 commented Nov 4, 2018

Thanks @chrisdavidmills, I’ve got a few things written up, will wait to see if this PR is merged before submitting a page

@chrisdavidmills

This comment has been minimized.

Copy link
Contributor

chrisdavidmills commented Nov 5, 2018

I'm gonna merge this; it looks useful and works nicely. Thanks @mbarker84 !

@chrisdavidmills chrisdavidmills merged commit cee5ce6 into mdn:master Nov 5, 2018

@mbarker84

This comment has been minimized.

Copy link
Contributor

mbarker84 commented Nov 6, 2018

Thanks @chrisdavidmills, the page is now live, in case you or anyone else want to review :)

@chrisdavidmills

This comment has been minimized.

Copy link
Contributor

chrisdavidmills commented Nov 7, 2018

Great work Michelle, thanks!

I have given it a review, and made a few edits. You might want to give it another read through to make sure you are happy with my edits.

Looks fine, apart from one minor comment. In the first code block in the "Useful fallbacks or alternative methods" section, shouldn't you set display: grid inside the @supports block? It seems a bit odd to be setting it inside the code for browsers that don't support grid.

@mbarker84

This comment has been minimized.

Copy link
Contributor

mbarker84 commented Nov 7, 2018

Thanks so much for reviewing @chrisdavidmills, and yes, that totally makes sense. I usually only put the rules that I need to override inside @supports personally, so if I’m just leaving the display property as default for the fallback then I tend to leave it outside the block, knowing that the browser will ignore that rule it if it doesn’t support it. But admittedly that’s in Sass, and is a bit clearer when nested IMO. So it’s probably more sensible to have display: grid inside @supports for the example. I’ve now updated that code block :)

@chrisdavidmills

This comment has been minimized.

Copy link
Contributor

chrisdavidmills commented Nov 8, 2018

Lovely, we are all sorted then. Thanks again for the awesome contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment