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

Button: Support "unstyled" usage #67320

Open
mirka opened this issue Nov 26, 2024 · 4 comments
Open

Button: Support "unstyled" usage #67320

mirka opened this issue Nov 26, 2024 · 4 comments
Labels
[Package] Components /packages/components [Type] Enhancement A suggestion for improvement.

Comments

@mirka
Copy link
Member

mirka commented Nov 26, 2024

What problem does this address?

There have been cases where the Button component is used with heavy style overrides. This can lead to subtle bugs and breakages down the line.

Although we should always first reconsider if one of the standard Button style variants can be used instead, in some cases heavy style customization is warranted. While styling a simple button element from scratch may seem like a valid alternative, that may also lead to developers overlooking subtle considerations (focus styles, accessibility, etc) that are already handled in Button.

What is your proposed solution?

We should audit the repo for the kinds of valid style overrides we have, and figure out how Button can support those use cases cleanly. Some possibilities include:

  • an unstyled variant
  • providing modular styles (focus styles, hover styles, etc) through something like a useButtonStyles hook
@ciampo
Copy link
Contributor

ciampo commented Dec 4, 2024

A useful exercise could be to run through Button's code and, for each "feature", assign it to either "core behavior" or a separate "add on" (ie. styles).

Also, we may want to "merge" the unstyled variant work with the potential refactor from default / primary / secondary / tertiary / link to solid / outline / text proposed here and also related to #63856

@yogeshbhutkar
Copy link
Contributor

I’d like to implement this enhancement but wanted to discuss the approach first.

To keep the code DRY, I propose extracting base button styles into reusable mixins (focus, active, disabled, etc.) and applying them under .components-button.

The useButtonStyles hook would return specific class names (e.g., .with-focus-styles) based on options, with each class implementing a corresponding mixin.

For example:

&:focus:not(:disabled) {
box-shadow: 0 0 0 var(--wp-admin-border-width-focus) $components-color-accent;
// Windows High Contrast mode will show this outline, but not the box-shadow.
outline: 3px solid transparent;
}

Extracting the above styles into a mixin (default-button-focus), and also implementing it within .with-focus-styles class which can be returned by the hook. These classes can then directly be passed to the Button.

What do you think of this approach? I’d appreciate any guidance or suggestions for improvement.

@ciampo
Copy link
Contributor

ciampo commented Feb 17, 2025

Hey @yogeshbhutkar, thank you for the comment!

I'm not exactly sure what @mirka had in mind when proposing a useButtonStyles hook. For example, would useButtonStyles take in the variant as an argument? And would it output separate style "snippets", eg restingStateStyles, hoverStyles, focusStyles, activeStyles, visitedStyles...?

Probably better to wait for @mirka 's feedback before moving forward, also considering that an alternative to the approach was to provide an unstyled variant.

@yogeshbhutkar
Copy link
Contributor

yogeshbhutkar commented Feb 17, 2025

Thanks for the reply, @ciampo.

an alternative to the approach was to provide an unstyled variant.

The issue with an unstyled variant is that a developer may only need to override layout styles like height while keeping the existing focus, hover, and other interactive styles. Exposing these styles via a hook allows them to be easily retrieved and applied as class names. Otherwise, important accessibility considerations might need to be made consciously.

I’m thinking useButtonStyles should return the default class names like .with-default-focus, .with-default-hover, .with-default-active, .with-default-disabled, etc.

const { ...defaults } = useButtonStyles();

// To apply only the focus styles.
const { focusStyle } = useButtonStyles(); 

Essentially, useButtonStyles() would provide the default button styles to be applied to an unstyled button component. This should ensure that a11y is properly followed even on an unstyled variant.

Edit:
An unstyled button might by default not have focus, active and disabled styles, if we just need to override the height, it makes sense to inherit the same focus, active and disabled styles as that of the base button component.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Components /packages/components [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

No branches or pull requests

3 participants