Try: Fix buttons faux space-between. #28485
Merged
Conversation
This file contains 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
jasmussen
added
[Block] Buttons
|
Size Change: +37 B (0%) Total Size: 1.37 MB
ℹ️ View Unchanged
|
jasmussen
force-pushed
the
try/buttons-flex-fix
branch
from
f8e0692 to
853eea3
Compare
youknowriad
added
Backport to WP 6.7 Beta/RC
|
I can't replicate this in master. |
Sorry, I should've clarified in the instructions. It depends a little on the theme. Some people style those Button blocks, and inadvertently fix this, others do less. I think if a theme doesn't style those buttons, it's more likely to happen. You can try TT1 Blocks, I can reproduce it there. Theoretically you could also reproduce it by toggling off editor styles:
|
|
This looks good in the editor but not so good in TT1 Blocks on the frontend: |
To me this seems to be a function of the way we deal wide/full alignments. We try to keep the site content 100% wide so that we can implement wide and full alignments, but this causes other issues. IMO we should just constrain the site to whatever the content width is and then let blocks break out of it when they are wide/full width... |
|
Thank you for reviewing and chiming in, I appreciate that a lot.
So negative margins or something like that? This is tricky to accomplish in the editor without the iframe, because vw units don't work. It's also a little bit less flexible with regards to various layouts you might want to use. Technically a theme could do this already now in the editor style (aside from the vw unit). Perhaps the real solution is the refactor of wide and fullwide to be more of a prop on the container than a default for post content, which Riad has been talking about for a while. Looking at that frontend issue now, thank you, good catch! |
jasmussen
force-pushed
the
try/buttons-flex-fix
branch
from
853eea3 to
86edbeb
Compare
|
Bring on the iframe! |
|
Thanks for testing, everyone. It appears there's an issue with TT1 Blocks on the frontend, and I will look at what that is as soon as I can. I'm currently having some trouble getting TT1 Blocks to work with the plugin, and I believe things will work once WordPress/theme-experiments#182 lands. When that happens, I'll test again, and revisit! |
jasmussen
force-pushed
the
try/buttons-flex-fix
branch
from
86edbeb to
175ca14
Compare
|
I managed to push a fix for TT1 blocks frontend. Before:
After:
It's this TT1 blocks CSS that does it:
Blanket targetting like that is arguably not best practice, but nevertheless it's fair enough to do, so I felt it okay to push a fix here. Pleas test this one again, and test:
|
|
Should this fix be in Gutenberg, or in TT1-blocks? 🤔 |
|
Thanks for working on this Joen!
I also feel like this should be in TT1-blocks, since everything works well when testing with Also in this PR the buttons don't work well with |
|
That was my question to myself as well. However two reasons made me choose to include the fix here:
|
|
Thank you Nik, I'll take a look at the vertical orientation. |
|
@ntsekouras what issues are you seeing with the vertical orientation? It seems to be working for me:
|
Sorry I didn't clarify. It doesn't seem to work on the front-end. Buttons seem to always be |
|
I'm not seeing that in TT1 blocks, or my own theme, in this branch:
|
ntsekouras
approved these changes
Feb 5, 2021
|
Let's land this as it improves the flex situation of the Buttons block a great deal, and I'm happy to follow up on any issues. Thanks all. |
noisysocks
pushed a commit
that referenced
this pull request
Feb 7, 2021
* Try: Fix buttons faux space-between. * TT1 fix.
noisysocks
pushed a commit
that referenced
this pull request
Feb 8, 2021
* Try: Fix buttons faux space-between. * TT1 fix.
noisysocks
removed
the
Backport to WP 6.7 Beta/RC
6 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Buttons in the Buttons block appear to be have "space-between" applied, even when they don't:
This is because inside a flex container, if a margin is set to auto, that's the net result.
This PR fixes that. After:
The frontend already worked.
One larger issue is this rule from #21971:
It's going to continue to cause headaches for flex containers and themes. I wonder if there's a way we can refactor that rule away, without regressing what 21971 meant to fix. CC: @kjellr @scruffian.
Another issue was the use of the
_variables.scssstylesheet to store a margin property that was used across the Button and Buttons blocks. It was added in #25999, but no block values should be added there, that stylesheet is purely for UI. And in adding a variable there, it also goes against the componentization / isolation of styles to each block container.For that reason, I'd very much welcome input on how we can refactor this:
$blocks-block__marginis used primarily in the Button block stylesheet. This is fine.