-
Notifications
You must be signed in to change notification settings - Fork 509
Adding flex-wrap example #524
Conversation
|
💖 Thanks for opening this pull request! 💖 |
schalkneethling
left a comment
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 @rachelandrew - Two small nits, other than that, this looks great!
| border: 3px solid blue; | ||
| width: 100px; | ||
| margin: 10px; | ||
| } No newline at end of file |
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.
Nit: missing empty line at the end of the file
| "type": "css" | ||
| } | ||
| } | ||
| } No newline at end of file |
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.
Nit: missing empty line at the end of the file
|
This looks great, the only thing I'd say is to make sure it looks the way we want when the output window is at its minimum width. On MDN pages the editor is laid out "side by side": that is with the example choices on the left and the output on the right, as it is in the local server that's started by I'm not sure what the exact threshold is, but it seems to give us a minimum output window width of about 285 pixels. At that width the flex items wrap into a single column, which cuts off the top and bottom of the container: I wasn't sure if this was intentional, but if not perhaps it would be good to make the items narrower? |
|
That's probably doable for this one but I can think of examples (especially in grid layout) that will be difficult to make so narrow and still useful. Also, did I miss documentation about minimum widths? If that doesn't exist perhaps it should be added somewhere. |
Very good point @rachelandrew I was thinking the exact same thing as I was reading the comments. I believe this has not been at the top of our minds as we were focus on the JS examples. I will add an issue to mention this clearly in the docs. |
I think that's fine, we should be pragmatic about it :).
No, I don't think it does, and yes, we should add it to the contributing guide. |
|
ok - I've updated this for both of those issues. |
schalkneethling
left a comment
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.
r+ Thanks @rachelandrew
|
Congrats on merging your first pull request! 🎉🎉🎉 |

An initial example for the set of examples I'm building.