-
Notifications
You must be signed in to change notification settings - Fork 47
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
Rewording post to fit code. #3
Conversation
I believe this was messed up? I might be missing something about the spec.
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.
Unsure how you want to handle white space with your text wrapping but I took a stab.
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.
Thanks for the edits! Only one minor request, and then we can merge.
src/pages/02-first-grid.js
Outdated
@@ -17,9 +17,9 @@ const Tutorial = () => ( | |||
|
|||
<h4>Define rows and columns</h4> | |||
<p> | |||
There are several ways to define rows and columns. For our first grid, we will use properties | |||
There are several ways to define rows and columns. For our first grid, we will use properties |
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.
only minor request. Can you represent the space as {' '} ?
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.
Sure!
Updated! Thanks for the feedback. |
Merged. Thanks! |
I believe this was messed up? I might be missing something about the spec.
Here it says "To create three fixed-height rows" but we are actually creating two.
https://github.com/MozillaDevelopers/playground/blob/master/src/pages/02-first-grid.js#L22
It seems that if you have:
grid-template-columns: 150px 150px 70px;
then it would only have three columns. To meet the text in the post of 'fourth' it would need to have an additional150px
, or say 'third'.https://github.com/MozillaDevelopers/playground/blob/master/src/pages/02-first-grid.js#L33
https://github.com/MozillaDevelopers/playground/blob/master/src/pages/02-first-grid.js#L37
There was a missing space due to indenting for character limits on each line I'm assuming. Added one in.
Thanks for creating this site! It's very helpful.
If there is an issue I am happy to make the language consistent across this page in the direction the maintainer wants to go in so this can get merged in. :)