-
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
Try using the column-gap property to improve spacing between custom-sized buttons #29165
Conversation
Size Change: +238 B (0%) Total Size: 1.4 MB
ℹ️ View Unchanged
|
Wow this is wild! And I can see it working, the code is solid. Before: After: The flex "gap" property would've been ideal, but I can see from the discussion that support is lacklustre. But from what I can see, column-gap isn't much better? https://caniuse.com/?search=column-gap The thing is, I consider Social Links, Navigation, and Buttons to all be 3 configurations of the same general block UI, and 3 blocks that could/should support some of the same things. For example, the justification toolbar is being added to all three (#29160) and made generic. And right now, those horizontal margins for all three are big headaches. If we could replace that with a gap value, one that you might even configure in the sidebar, it would be so ideal. So I support the end goal, but I'd love if we could find a solution that not only worked better with compatibility, but let us retire BOTH the margins, and the baked-in $block-margin SASS variable & calc usage. I know CSS grid can do this, but that seems like a thing to avoid if we can. I may be dreaming? |
Yeah, it's basically the same.
CSS grid generally isn't a great solution for cases when we don't know the total number of rows/columns we want. I think |
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 approach looks sound to me. Not sure if you want a review from someone else.
Thanks for the review. If @jasmussen's cool with it, I suggest we merge this in and iterate later if need be. |
We've been going back and forth. But it's we're ready to look at follow-ups, let's try it. |
Sounds good, let's give it a try! I'll keep my eye on it and see if anything weird happens. Depending on how this sorts out, we may be able to remove the fallback code relatively soon. |
Possible fix for #28468.
This PR tries setting a
column-gap
rule for the parentwp-block-buttons
block, and then adjusts the width of child buttons to account for that gap. This lets combinations of custom-sized buttons take up the full width of the container, (hopefully) without any extra right margin. I left the existing implementation in place for legacy browsers — this new one only kicks in if your browser supports thecolumn-gap
property.I think all my math works, but it's late in the afternoon on a Friday, so it's possible I've missed something. 😅