-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Latest Comments Block: Remove list padding and margin #29402
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,8 @@ | ||
.wp-block-latest-comments { | ||
padding-left: 0; | ||
margin-left: 0; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tested with TT1 blocks and this change is not reflected in the So maybe patch the editor and There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right, I should not have tested with Twenty Twenty-One 🤦 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you for the PR! The padding change is great, but the margin change, I believe, will cause headaches. Many themes, to Nik's point, center blocks using margin-left: auto, and by setting it to zero, you might make the latest comments block bump againt the left edge. We have the same challenge with the Image block, which is wrapped in a For that reason, I'd leave the margin change out from this PR, and let the theme handle that. Then, longer term, something like #29335 could potentially help address the fundamental issue. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The margin rule has been removed from the front but not the editor. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't the front and backend ideally be the same though? Or does the ol inherit from wp admin which diverges it anyway? 🙈 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes 🙊 , the list styles we are overriding are: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🙈 I think it might still be best to remove that margin change, as it "paints over" an issue that we need to fix at the root, for example through bringing the iframe to the post editor, or #28878. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤷♀️ It will look different in the editor and front even without the wp admin styles. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This is no different from what's currently in trunk, though, is it? I definitely agree it needs to be addressed, just want to make sure we do it right. |
||
} | ||
|
||
.wp-block-latest-comments__comment { | ||
line-height: 1.1; | ||
list-style: none; | ||
|
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.
can't we just use
ol.wp-block-latest-comments
here and avoid any specific editor style?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 did not understand. Did you mean to add this comment to the editor.scss file?
That might be possible now after #30034.
I will test 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.
no I mean replace the selector above with
ol.wp-block-latest-comments
and avoid any style in "editor.scss". It worked in my testing.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
ol.wp-block-latest-comments
is still overridden bythat is why
ol.wp-block-latest-comments.wp-block-latest-comments
was used in the editor style.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.
It is only overridden if the block is part of the post when it gets loaded in the editor. this is an issue with the lazy loading behavior of block assets in the editor. It affects more blocks than just the comments block I believe we should fix the root issue (stylesheet loading order) not its consequences. I already talked with @aristath and @gziolo about it.