-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Add width selector for button block #25999
Add width selector for button block #25999
Conversation
b3853a1
to
9ffdfd5
Compare
This tested well for me. As mentioned in discussions when using right align things can end up slightly odd, but I think this is because of the use of |
Code looks good ✨ Apart from the align left/right issues, this tests well for me as well. I do wonder though if this is another opportunity for adding block support for custom widths so other blocks can opt-in and leverage this? This would also allow width options to be configured via themes. Regarding the alignment issues, I'd vote for them to be addressed in a separate PR. There might be a few things we could do to smooth some of the more common rough edges created by the floated block losing the parent width to calculate the percentage widths from. |
This is something I had a lot of difficulty with -- I have a version which I can push up for testing where right-aligning the Buttons block causes the individual Buttons to line up flush to the right side of the container when the post is saved: This seems better to me because it is more accurate to what you see in the editor and it's what I would assume should happen when you right-align the parent. The CSS for that removes the
I think that's accurate. The reason I had trouble with this is because I couldn't figure out how to best emulate the right-align behavior in production, which does not necessarily keep the buttons flush to the right side of the container. For example, this happens on production when I right-align a group of buttons within a column (column highlighted in blue, sorry it's hard to see): |
1fd11f6
to
5e57245
Compare
@mcsf Pinging you to see if this PR looks better since it's adding width specific to the button block. 👍 |
There's another PR that changes how alignment works on the buttons block, so it'd probably worth testing against that as well - #23168 |
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.
Looks good overall (I left some questions on the attribute schema to make sure we've given this a bit of thought), but I'd have someone else test and review the style changes.
@@ -52,6 +52,9 @@ | |||
}, | |||
"gradient": { | |||
"type": "string" | |||
}, | |||
"width": { | |||
"type": "number" |
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.
Thinking about possible futures for this block, is this the best data representation? How would we express custom width values without introducing compatibility issues with the current schema? That is, this new width
attribute is not any number
but actually a percentage represented as 0 <= n <= 100
. Would it be better if it were a real number x 0 <= x <= 1
to distinguish from full pixel values like width: 200
? Would it be better if termed widthPercentage
?
All of these are open questions — I don't have a formed opinion on this.
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.
Another possibility here would be to add an additional widthUnit
attribute if we decide to introduce custom width as pixel values. The search
block takes that approach.
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 is the approach the search and cover blocks take, so this should be futureproof for adding a unit in the future.
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.
A width selection can now be toggled off by clicking the already selected option.
Co-authored-by: Miguel Fonseca <[email protected]>
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.
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 looks good and tests well. I think this is a great start to width support that could be extended in the future to support custom widths and different units.
Fixes a bug where the inner button was not being set correctly to 100% width on the frontend, due to the missing classname.`
e7100b2
to
eb894de
Compare
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 tested well for me.
@mcsf Would you be able to give this one a sign off? |
Going to move ahead with this once since it has approval already. If we need to make further changes the we can open follow up PRs. |
Congratulations on your first merged pull request, @stacimc! We'd like to credit you for your contribution in the post announcing the next WordPress release, but we can't find a WordPress.org profile associated with your GitHub account. When you have a moment, visit the following URL and click "link your GitHub account" under "GitHub Username" to link your accounts: https://profiles.wordpress.org/me/profile/edit/ And if you don't have a WordPress.org account, you can create one on this page: https://login.wordpress.org/register Kudos! |
Is there a way to disable this feature? |
Description
Issue #24705
Adds a width selector to the sidebar for the Button block, allowing the user to set their button to 25%, 50%, 75%, or 100% of the parent container.
By default, no percentage width is selected and the button width is determined as normal by the size of its content.
Notes
This is an alternative approach to #26045, which implements a width selector on the Button block using a new block support._ This is related to issue #26368.
How has this been tested?
Manually with the button block, using left, right, and center alignments.
Buttons
block to a post with one or moreButton
sButton
Screenshots
Types of changes
New feature (non-breaking change which adds functionality)
Checklist: