Add layout and block spacing to details block#53282
Merged
tellthemachines merged 2 commits intotrunkfrom Aug 4, 2023
Merged
Conversation
ramonjd
approved these changes
Aug 3, 2023
Member
ramonjd
left a comment
There was a problem hiding this comment.
LGTM - tested in Chrome, Safari Firefox
Is this still a draft?
Editor
2023-08-04.09.45.19.mp4
Frontend
Contributor
Author
Yeah there's some dead code that will need to be removed. I added the feature first to see how well it worked, and will address the rest of the points in the issue now. |
andrewserong
reviewed
Aug 4, 2023
Contributor
andrewserong
left a comment
There was a problem hiding this comment.
This is testing quite nicely for me, too! No styling issues between trunk and this PR, so it appears to be backwards compatible 👍
One subtle thing, since the hard-coded styling rules haven't been removed, is that the block's CSS overrides the root layout styles:
This isn't an issue at the post level, as block spacing set at the individual block level does override it:
However, it does seem to be an issue for styles set within global styles, though. To reproduce, go to adjust the Details block's block spacing within global styles, then save, and view the block on the site frontend. It looks like the block's styling rules are overriding the block-level-in-global-styles rules:
Other than that, though, this is working very well!
Contributor
Author
|
Updated to remove custom margins from the block CSS. |
|
Size Change: -52 B (0%) Total Size: 1.44 MB
ℹ️ View Unchanged
|
andrewserong
approved these changes
Aug 4, 2023
Contributor
andrewserong
left a comment
There was a problem hiding this comment.
Works nicely with the styles removal 👍
✅ Base layout rules provide the same visual styles as on trunk
✅ Block-level in global styles block spacing rules work as expected
✅ Block-level in post content overrides both of the above as expected
LGTM, thanks for the quick work on this one! ✨
Member
|
Re-tested after CSS removal. 🚀 |
This was referenced Oct 9, 2023
8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What?
Fixes #53252.
Adds flow layout and block spacing to the Details block
Quick draft to check all works as expected; some items in the issue such as hardcoded style removal haven't been addressed yet.
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast