Skip to content
This repository has been archived by the owner on Nov 18, 2024. It is now read-only.

Discussion: Remove the outline button style variation #382

Closed
carolinan opened this issue Sep 22, 2024 · 15 comments
Closed

Discussion: Remove the outline button style variation #382

carolinan opened this issue Sep 22, 2024 · 15 comments
Labels
[Type] Discussion For issues that are high-level and not yet ready to implement.

Comments

@carolinan
Copy link
Contributor

carolinan commented Sep 22, 2024

Description
The outline button style variation is difficult to style, both the hover styles in general, and when the button is placed inside a section style. It is causing more problems than it brings value.

A user who wants this style, can style the button to look similar by applying a border using the block options.

What do you think? @juanfra @jasmussen @WordPress/block-themers


The styles can not be unregistered with the PHP function, only JavaScript.
Full code example for how to remove the style with JavaScript:
https://developer.wordpress.org/block-editor/reference-guides/block-api/block-styles/

@carolinan carolinan added the [Type] Discussion For issues that are high-level and not yet ready to implement. label Sep 22, 2024
@t-hamano
Copy link
Contributor

I think the Outline style can be removed from the theme.

The only advantage of the Outline style is that it doesn't have a background color, so we can make the background transparent when we place it over a color or image. However, we can achieve the same behaviour with the default style by setting the transparency to zero in the color palette:

button

In the future, we may be able to deprecate the Outline style on the Button block itself, just as we deprecated the Squared style.

Incidentally, I think the outline style can also be removed via block_type_metadata_settings hook:

Code
function remove_button_outline_style( $metadata ) {

	// Only apply the filter to Button block.
	if ( ! isset( $metadata['name'] ) || 'core/button' !== $metadata['name'] ) {
			return $metadata;
	}

	// // Check if 'styles' key exists.
	if ( ! isset( $metadata['styles'] ) ) {
			return $metadata;
	}

	// Remove "Outline" style.
	$metadata['styles'] = array_filter(
		$metadata['styles'],
		function ( $style ) {
			return 'outline' !== $style['name'];
		}
	);

	return $metadata;
}
add_filter( 'block_type_metadata_settings', 'remove_button_outline_style' );

We could also achieve the same thing with the block_type_metadata or register_block_type_args hook.

@carolinan
Copy link
Contributor Author

Not having to add the JavaScript file would be easier.

@juanfra
Copy link
Member

juanfra commented Sep 23, 2024

I see value in the outline variation, and every piece of the design has an intention. But in practical terms and thinking of implementation, it gets complicated, specifically with the new section style variations and the different states (:hover more than anything).

If we remove it, and from what I could check, the only place needing a bit of work should be this pattern. And by removing it, we'll probably have fewer complications with variations (so far, it's been the main blocker).

A user who wants this style, can style the button to look similar by applying a border using the block options.

While this is true, the user won't be able to set the styles for states like :hover via UI. I believe until we don't have that, it'll get complicated with variations.

@jasmussen
Copy link
Contributor

If we remove it, and from what I could check, the only place needing a bit of work should be #91. And by removing it, we'll probably have fewer complications with variations (so far, it's been the main blocker).

We could still add inline pill-shape styles for buttons in that pattern, and keep it, no?

@juanfra
Copy link
Member

juanfra commented Sep 23, 2024

We could still add inline pill-shape styles for buttons in that pattern, and keep it, no?

Yes, totally.

@richtabor
Copy link
Member

We shouldnt remove a core block style variation from the theme.

The outline button style variation is difficult to style, both the hover styles in general, and when the button is placed inside a section style. It is causing more problems than it brings value.

Why not just use currentColor from the parent theme.json?

@carolinan
Copy link
Contributor Author

carolinan commented Sep 24, 2024

Because you can't add hover on a variation of the buttons block.

@jasmussen
Copy link
Contributor

I'd tend to agree with Rich here, that since it's available in core, we shouldn't remove it here. currentColor as defined in theme.json seems like it would allow us to address the height issue, and any color issues. The expense would be that you can't edit hover, active, focus on buttons block, but you can't do that on navigation yet either. In both of those cases, we should seek to progress this in the block editor software itself. There's a new design for surfacing this, which I think has a chance of making it in 6.8.

@carolinan
Copy link
Contributor Author

It is not at the expense of editing, since editing is not possible today :)
What needs to be ensured is the accessibility.

@jasmussen
Copy link
Contributor

What's the accessibility issue of not being able to edit the hover style? If it's all set to currentColor in theme.json, then the hover style will be the same color as the default state.

@carolinan
Copy link
Contributor Author

carolinan commented Sep 24, 2024

I am not saying that the editing is the issue.
But if the hover is the same color as the default state, there MUST be another indicator that the item is hovered.

@jasmussen
Copy link
Contributor

That's not my reading of the docs. The change from default to pointer cursor is a primary and default indicator when hovering an interactive element, and additional indication—in my read—is only necessary if the hover state triggers additional content to become visible.

Moreover, if this is truly an accessibility issue, then it should be fixed in the block editor. By providing a bandaid in a theme, in fact it reduces useful momentum in actually getting the issues fixed at the source.

@richtabor
Copy link
Member

But if the hover is the same color as the default state, there MUST be another indicator that the item is hovered.

Can we do a color-mix function of a portion of currentColor, so it's always consistent regardless of the section styles applied?

@jasmussen
Copy link
Contributor

Example of a color-mix function: https://codepen.io/joen/pen/ZEdwZdo— we can mix any spot or accent color with either transparent or currentColor.

@carolinan
Copy link
Contributor Author

carolinan commented Sep 25, 2024

I believe we have agreed to rule out removing the variation?
But I do not want to close this issue while the discussion about the outline hover style continues.
I opened a new issue for the problems that are (on first sight) simpler to solve.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
[Type] Discussion For issues that are high-level and not yet ready to implement.
Projects
None yet
Development

No branches or pull requests

5 participants