Buttons block: overhaul alignment/justification controls #23168
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
6 tasks
|
Size Change: +718 B (0%) Total Size: 1.21 MB
ℹ️ View Unchanged
|
ZebulanStanphill
force-pushed
the
update/buttons-block-align-options
branch
from
e861e17
to
a1d12c5
Compare
ZebulanStanphill
changed the title
ZebulanStanphill
force-pushed
the
update/buttons-block-align-options
branch
2 times, most recently
from
51cec7c
to
e9cdc50
Compare
|
Remaining things that need to be addressed for this PR to be ready:
|
ZebulanStanphill
added
the
Needs Technical Feedback
ZebulanStanphill
force-pushed
the
update/buttons-block-align-options
branch
6 times, most recently
from
fffa5f9
to
5749cc1
Compare
ZebulanStanphill
added
the
[Status] Blocked
6 tasks
ZebulanStanphill
force-pushed
the
update/buttons-block-align-options
branch
from
5749cc1
to
9637656
Compare
6 tasks
ZebulanStanphill
force-pushed
the
update/buttons-block-align-options
branch
from
9637656
to
aba0d5a
Compare
ZebulanStanphill
changed the base branch from
master
to
update/buttons-block-even-lighter-editor-dom
ZebulanStanphill
force-pushed
the
update/buttons-block-align-options
branch
2 times, most recently
from
0318405
to
63d897a
Compare
Base automatically changed from
update/buttons-block-even-lighter-editor-dom
to
master
September 25, 2020 14:44
ZebulanStanphill
force-pushed
the
update/buttons-block-align-options
branch
3 times, most recently
from
c9e899f
to
e17916b
Compare
ZebulanStanphill
removed
the
[Status] Blocked
|
Thanks for having a look @geriux ! @ZebulanStanphill Do you mind delaying this PR a bit? We need to find some time for someone to help on that task if we want to avoid this (non-fatal) regression on mobile. |
|
@Tug I'm fine with delaying it. I'd prefer to get it in for WP 5.6, but I understand that it would be best to have someone get a mobile implementation ready first. |
6 tasks
|
Hey, @ZebulanStanphill 👋 just drafted a PR from your branch adding support for mobile. |
ZebulanStanphill
force-pushed
the
update/buttons-block-align-options
branch
from
67463b4
to
225ff95
Compare
ZebulanStanphill
added
Needs Dev Note
ZebulanStanphill
force-pushed
the
update/buttons-block-align-options
branch
from
7b3c4fe
to
9e5a534
Compare
talldan
approved these changes
Oct 27, 2020
There was a problem hiding this comment.
Approving from a web point of view. This is working really nicely in my testing! Thanks for persevering, @ZebulanStanphill, and for the mobile implementation @geriux.
packages/block-library/src/buttons/content-justification-dropdown.js
Outdated
Show resolved
Hide resolved
6 tasks
…tificationDropdown (#26166)
ZebulanStanphill
force-pushed
the
update/buttons-block-align-options
branch
from
9e5a534
to
4fb29cb
Compare
|
Rebased after the changes in #26380. Thanks for all your help getting this PR polished and completed, everyone! The end-to-end test failures are unrelated, so I'm going to go ahead and merge now. |
stacimc
pushed a commit
to stacimc/gutenberg
that referenced
this pull request
Oct 29, 2020
PR WordPress#23168 overhauls alignment controls on the Buttons container and allows for full width. For a Button inside a very large container, in order to be set correctly to 100% it must be able to exceed any configured max-width. This commit does not remove the max-width for any button that does NOT have a custom width percentage selected. This is so that default buttons continue to be sized as they normally would.
apeatling
pushed a commit
that referenced
this pull request
Nov 3, 2020
* Add sidebar width selector for Button block Initial commit adding a width option for the Button block with options at 25, 50, 75, 100% widths. By default none are applied and the button is sized by its content. * Persist width changes * Apply styling through class name rather than inline * Clean up CSS * Allow toggling a custom width off A width selection can now be toggled off by clicking the already selected option. * Update InspectorControls to use sentence case Co-authored-by: Miguel Fonseca <[email protected]> * Remove max width from buttons with custom width set PR #23168 overhauls alignment controls on the Buttons container and allows for full width. For a Button inside a very large container, in order to be set correctly to 100% it must be able to exceed any configured max-width. This commit does not remove the max-width for any button that does NOT have a custom width percentage selected. This is so that default buttons continue to be sized as they normally would. * Apply `has-custom-width` class on frontend Fixes a bug where the inner button was not being set correctly to 100% width on the frontend, due to the missing classname.` Co-authored-by: Staci Cooper <[email protected]> Co-authored-by: Miguel Fonseca <[email protected]>
7 tasks
youknowriad
removed
the
Needs Dev Note
9 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.
Purpose
Replaces #23133.
This PR does two things:
The left and right options (which, as a reminder, are float options) were not really useful at all for the Buttons block, and the center alignment option is something that probably should've been handled by content justification options from the start.
The left/right content justification options actually give the effect that some may have expected the "block alignment" left and right options to do. I suspect that adding the "block alignment" controls to the Buttons block in the first place was a mistake caused by someone thinking that they were content alignment controls, hence the existing
text-align: centerstyle being applied to Buttons blocks with thealigncenterclass in order to make the center-block-alignment option act like a center-content-alignment option.The Navigation block does have support for the wide and full "block alignment" options, and I think there are actually use-cases for those options, so this PR adds support for those two to the Buttons block. If desired, I could remove those options, and thus remove the
alignattribute from the block entirely.This PR was motivated by this discussion. I initially tried to do all this in multiple PRs, but I realized that doing so would require multiple deprecations if I didn't get certain changes in soon enough. For that reason, I have opted for a single PR to make all the necessary changes at once so that the proper controls are added at the same time that the confusing weird ones are removed.
Deprecation handling
Buttons block with a center-alignment will also be converted to use content alignment, and in this case the end result will look identical.
Existing Buttons blocks with a left/right float alignment will have that float alignment removed and replaced with left or right content justification, since that is what most people thought the options would do anyway. In fact, the editor styles were (confusingly) already applying some content alignment styles, though they weren't noticeable since the
alignxxxclasses were also applying float behavior.The existing front-end styles for block alignment are retained for compatibility with already-published posts that have yet to be reopened in the editor.
How has this been tested?
I tested to make sure existing Buttons blocks using the deprecated align options are converted to the new content justification system as described earlier.
Checklist: