Skip to content
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

Disable Width Settings in Button Block #66254

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from

Conversation

karthick-murugan
Copy link
Contributor

@karthick-murugan karthick-murugan commented Oct 18, 2024

What?

Fixes Part of this issue

This PR introduces the ability to disable the button block's width settings through the theme.json file.

Why?

This functionality is important for themes that are tailored to specific designs, where controlling the button width setting may be unnecessary or undesired. This provides better flexibility for theme developers, allowing them to disable the width controls as needed.

How?

The edit.js file of the button block has been updated to retrieve the width value from the theme’s theme.json file. If the width setting is disabled (i.e., set to false), the width panel is no longer displayed in the editor for the button block.

Testing Instructions

  1. Checkout this branch to your local

  2. Include the following JSON object within the settings section of the theme.json file for the active theme:

"blocks": {
	"core/button": [
		{
			"width": false
		}
	]
},
  1. Now insert button block in the editor and check the width Settings of the button and it will be removed. Just update false to true in theme.json and check the width Settings and it can be seen.

Testing Instructions for Keyboard

None

Screenshots or screencast

For no changes in theme.json or adding width value to true

button_with_settings

For theme.json width value false

button_without_settings

Copy link

github-actions bot commented Oct 18, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: karthick-murugan <karthickmurugan@git.wordpress.org>
Co-authored-by: carolinan <poena@git.wordpress.org>
Co-authored-by: justintadlock <greenshady@git.wordpress.org>
Co-authored-by: ndiego <ndiego@git.wordpress.org>
Co-authored-by: youknowriad <youknowriad@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Copy link

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @karthick-murugan! In case you missed it, we'd love to have you join us in our Slack community.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

@github-actions github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Oct 18, 2024
@carolinan
Copy link
Contributor

Hi @karthick-murugan
When changes are made to theme.json, the theme.json schema must also be updated.
I mean that the schema must reflect that setting the width to false is valid.
https://github.com/WordPress/gutenberg/blob/trunk/schemas/json/theme.json

@justintadlock
Copy link
Contributor

justintadlock commented Oct 24, 2024

Good job! I like the forward movement toward handling this pretty common request (disabling block settings).

Aside from the theme.json schema update, I'd want to be very cautious about introducing methods for disabling block settings. I feel like there's probably an architectural decision that should be made on how to best do this for any block's setting. If we introduce one-off settings for individual blocks like this, does this create legacy code that we have to support? I don't know the answer to that, but I think it's worth asking. I'm not sure what the best path is, but this needs a decision.

@ndiego
Copy link
Member

ndiego commented Oct 24, 2024

Aside from the theme.json schema update, I'd want to be very cautious about introducing methods for disabling block settings. I feel like there's probably an architectural decision that should be made on how to best do this for any block's setting. If we introduce one-off settings for individual blocks like this, does this create legacy code that we have to support? I don't know the answer to that, but I think it's worth asking. I'm not sure what the best path is, but this needs a decision.

+1 to this. If the approach in this PR is correct, I could see this being applied to all block settings. This is a very common feature request. But we need to make sure this is the correct course of action more broadly. Thoughts on this from an architectural standpoint @youknowriad @mtias?

@youknowriad
Copy link
Contributor

theme.json is the right approach for these things but I wouldn't tie this to a block. We should think about "block supports" more globally.

  • What other blocks have a "width" control?
  • Should we add a "width" block support and support it for more blocks? I think the answer is probably yes, I can also see the need for a maxWidth, minWidth block support too. (maybe they're enabled/disabled together)
  • It's ok for blocks to have inconsistent implementation for "width" (history reasons or valid reasons)
  • It also seems like we already have a dimensions.minHeight config in theme.json (I wish this was just dimensions.height kind of but I guess it's ok to have both later.

Based on this, I think I'd be ok having a dimensions.width config in theme.json to enable/disable all width controls regardless of blocks (and obviously you can do it for a given block if needed just like any setting).

I would love if we can do an audit of existing "width" controls in the core blocks to see if it adapts to all of these.

Now for the implementation, the following should probably be enough to retrieve the right config for a given block.

const [ isWidthEnabled ] = useSettings( 'dimensions.width' );

@youknowriad
Copy link
Contributor

width and height block supports are tracked here #28356

@ndiego
Copy link
Member

ndiego commented Oct 24, 2024

The Columns block is another good candidate for this. The width should be part of the Dimensions panel.

image

@carolinan
Copy link
Contributor

The search block has a width option too

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Buttons Affects the Buttons Block First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants