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

Proposal: Consolidate minimal and outlined props on Button components #7202

Open
ggdouglas opened this issue Jan 28, 2025 · 2 comments
Open

Comments

@ggdouglas
Copy link
Contributor

ggdouglas commented Jan 28, 2025

Problem

The current Button components offer three stylistic options: "default," "minimal," and "outlined." Although these styles are mutually exclusive, their corresponding props, minimal and outlined, can both be applied simultaneously. This results in an overly complex API and can cause confusion and unintended styling conflicts. For instance, setting both minimal={true} and outlined={true} leads to the "outlined" styles overriding the "minimal" styles, which can be unintuitive for developers.

Proposal

We propose consolidating the minimal and outlined boolean props into a single enum prop named variant. The variant prop will accept the following values: "solid" (the default), "minimal", and "outlined".

type ButtonVariant = "solid" | "minimal" | "outlined";

To implement this change:

  • Deprecate the minimal and outlined props, indicating their removal in a future major version.
  • Provide migration instructions and codemods to assist developers in updating their codebases.
  • Internally, the <Button> component will continue to use the same Classes.MINIMAL and Classes.OUTLINED classes, applied through the variant prop.

[Classes.MINIMAL]: minimal,
[Classes.OUTLINED]: outlined,

Examples:

// Before
<Button minimal={true} />

// After
<Button variant="minimal" />
// Before
<Button outlined={true} />

// After
<Button variant="outlined" />

Benefits

By unifying these styling options under the variant prop, we simplify the API and eliminate the possibility of mutually exclusive props being applied simultaneously. Setting "solid" as the default variant minimizes migration efforts, as existing buttons without the variant prop will maintain their current styling.

Affected Components

Addendum

The following components also utilize the minimal prop. We need to determine whether these components should be updated for consistency with the proposed changes to the Button component, or if their minimal prop should remain unchanged:

@ggdouglas
Copy link
Contributor Author

#7197

@ggdouglas ggdouglas changed the title Proposal: Consolidate "minimal" and "outlined" props on Button components Proposal: Consolidate minimal and outlined props on Button components Jan 29, 2025
@nerdstep
Copy link
Contributor

This could be solved partly without a breaking change by using a discriminated union type, i.e.

type ButtonProps =
  | { minimal?: true; outlined?: never }
  | { minimal?: never; outlined?: true }

Of course that would only help TypeScript consumers. 🤷‍♂

To help non-TS consumers, a warning could be output to the console (when in development environment).

@ggdouglas ggdouglas self-assigned this Feb 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants