-
Notifications
You must be signed in to change notification settings - Fork 22.6k
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
CSS module: box alignment #37566
CSS module: box alignment #37566
Conversation
Preview URLs (9 pages)
Flaws (258)Note! 3 documents with no flaws that don't need to be listed. 🎉 URL:
URL:
URL:
URL:
URL:
URL:
External URLs (1)URL:
(comment last updated: 2025-01-15 18:19:05) |
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.
@estelle stellar work! I've left some comments for you to consider.
@@ -0,0 +1,292 @@ | |||
--- | |||
title: CSS box alignment | |||
slug: Web/CSS/CSS_box_alignment/box_alignment |
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 page makes sense, and it is structurally fine. However, calling the page the same as its parent page and having a slug of CSS_box_alignment/box_alignment
seems redundant and a bit weird.
Calling the page "CSS box alignment overview" and having its slug as CSS_box_alignment/Overview
(or similar) would make more sense, IMO.
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.
The slug is still a bit odd, so flagging this again. If you feel strongly that it is not a problem, I'll leave it alone.
files/en-us/web/css/css_box_alignment/box_alignment_in_multi-column_layout/index.md
Outdated
Show resolved
Hide resolved
Co-authored-by: Chris Mills <[email protected]>
Co-authored-by: Chris Mills <[email protected]>
Co-authored-by: Chris Mills <[email protected]>
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.
Looking good @estelle. There are a couple of changes that I think would still be good to make, and I've commented those bits in my previous review, but I think it is up to you whether you want to tackle them now or leave them to a different PR.
I've also made a commit directly to your branch to fix a snafu I made in the text with one of my previous suggestions. Sorry about that!
Approving. Feel free to merge when ready.
Updating the CSS box alignment module landing page.
To maek the PR review process manageable:
alignment-baseline
is in a separate PR - New CSS page: alignment-baseline #37564fixes openwebdocs/project#215