Add `variant` prop to Button and ButtonGroup #7197

ggdouglas · 2025-01-24T22:19:59Z

Part of #7202

Proposed changes:

Adds a new variant prop to replace the (now deprecated) minimal and outlined props on Button/AnchorButton and ButtonGroup.

svc-palantir-github · 2025-01-24T22:34:39Z

Add example and docs

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

svc-palantir-github · 2025-01-28T21:18:08Z

Add deprecation warnings in console

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

svc-palantir-github · 2025-01-28T21:49:38Z

Add "variant" prop to ButtonGroup component

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

svc-palantir-github · 2025-01-29T15:33:12Z

Move eslint deprecation lines for multi-line declaration

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

svc-palantir-github · 2025-02-04T16:29:23Z

Move eslint deprecation lines for multi-line declaration

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

ggdouglas · 2025-02-04T16:32:13Z

packages/docs-app/src/examples/core-examples/buttonExamples.tsx

@@ -51,12 +65,12 @@ export const ButtonOutlinedExample: React.FC<ExampleProps> = props => {
    const code = dedent`
        <Button text="Outlined" outlined={true} />
        <Button text="Primary" outlined={true} intent="primary" />
-        <Button text="Disabled" minimal={true} disabled={true} />`;


This was a typo in the current Button outlined example (unrelated to the rest of these changes)

packages/core/src/common/errors.ts

svc-palantir-github · 2025-02-04T16:45:10Z

Extract ButtonVariant type

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

svc-palantir-github · 2025-02-04T17:15:31Z

Organize jsdoc declarations

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

jscheiny · 2025-02-04T18:33:08Z

packages/core/src/components/button/buttons.tsx

+            [Classes.MINIMAL]: minimal || variant === "minimal",
+            [Classes.OUTLINED]: outlined || variant === "outlined",


This bit of code has some unintended side effects. I think that we should make the behavior be that variant supersedes both outlined and minimal. However, if we do the following:

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

We end up with an outlined button instead of a minimal button as would be hoped for. The same issue occurs with ButtonGroups.

Technically, this behavior already exists between the minimal and outlined props where outlined overwrites the styles of minimal if both are applied simultaneously.

<Button text="Button" minimal={true} outlined={true} />

I think it is fair, however, that we wouldn't want to continue this behavior!

a2889bd wraps the logic into a utility function, providing the proper application of classes based on precedence. This technically changes the behavior of the presence of classes (for the case described above), but in practice it doesn't matter much since, as we've established, the outlined classes overwrite minimal anyways.

Edit: fd62b36 preserves the existing behavior

svc-palantir-github · 2025-02-04T21:01:14Z

Add comments heading up constants

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

svc-palantir-github · 2025-02-04T21:48:03Z

Update Button/ButtonGroup interactive examples with new variant select

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

svc-palantir-github · 2025-02-04T23:05:20Z

Refactor variantClass utility

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

svc-palantir-github · 2025-02-05T18:41:09Z

Simplify types passed to variantClass utility

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

jscheiny

LGTM! Excited about this change

packages/core/src/components/button/button-group.md

Invalidated by push of a6e75cf

svc-palantir-github · 2025-02-12T18:25:05Z

Add `variant` prop to blocklist of removeNonHTMLProps

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

svc-palantir-github · 2025-02-13T17:30:52Z

Update ESLint deprecation ignore statements

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

svc-palantir-github · 2025-02-13T22:05:07Z

Remove comment that is no longer relevant

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

Invalidated by push of a618564

svc-palantir-github · 2025-02-14T02:18:41Z

Remove minimal/outlined from ButtonGroup popover example

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

svc-palantir-github · 2025-02-14T02:42:12Z

Ignore errors constant file in test coverage

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

bvandercar-vt · 2025-03-03T17:59:02Z

Tag component has deprecated size but not deprecated variants.

ggdouglas mentioned this pull request Jan 28, 2025

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

Closed

ggdouglas changed the title ~~Add variant prop to Button~~ Add variant prop to Button and ButtonGroup Jan 28, 2025

ggdouglas force-pushed the gdouglas/proposal-button-variant branch from 723e7cc to a71f72a Compare February 4, 2025 16:15

ggdouglas commented Feb 4, 2025

View reviewed changes

packages/core/src/common/errors.ts Outdated Show resolved Hide resolved

ggdouglas marked this pull request as ready for review February 4, 2025 16:34

jscheiny reviewed Feb 4, 2025

View reviewed changes

ggdouglas and others added 13 commits February 4, 2025 17:05

Add variant prop to Button/AnchorButton components

b7d898d

Add example and docs

bb2658d

Add deprecation warnings in console

e0d45fe

Add "variant" prop to ButtonGroup component

234759c

Move eslint deprecation lines for multi-line declaration

86b626d

Move warning constants in errors.ts

33de37a

Extract ButtonVariant type

27d40b4

Organize jsdoc declarations

25fb9d0

Consolidate Button variant class logic into utility function

a2889bd

Include gaps in Button variant example

11d3105

Add comments heading up constants

3b50b01

Migrate all internal components from minimal to variant

297d62a

Update Button/ButtonGroup interactive examples with new variant select

cd08b19

ggdouglas force-pushed the gdouglas/proposal-button-variant branch from e96ff83 to cd08b19 Compare February 4, 2025 22:08

ggdouglas added 2 commits February 5, 2025 13:06

Preserve existing class resolution behavior for minimal/outlined props

fd62b36

Simplify types passed to variantClass utility

86746b6

ggdouglas force-pushed the gdouglas/proposal-button-variant branch from abf4155 to 86746b6 Compare February 5, 2025 18:26

jscheiny previously approved these changes Feb 5, 2025

View reviewed changes

jscheiny reviewed Feb 5, 2025

View reviewed changes

packages/core/src/components/button/button-group.md Outdated Show resolved Hide resolved

ggdouglas added the no changelog label Feb 12, 2025

ggdouglas added 2 commits February 12, 2025 13:09

Update ButtonGroup docs

d1befe6

Add variant prop to blocklist of removeNonHTMLProps

a6e75cf

ggdouglas added 2 commits February 13, 2025 12:10

Merge branch 'develop' into gdouglas/proposal-button-variant

66d4138

Update ESLint deprecation ignore statements

3879293

ggdouglas added 4 commits February 13, 2025 16:28

Merge branch 'develop' into gdouglas/proposal-button-variant

2e7caad

Refactor variantClass utility

6b782b6

Move deprecation warning constants

bcad96a

Remove comment that is no longer relevant

36f1672

jscheiny previously approved these changes Feb 13, 2025

View reviewed changes

ggdouglas added 2 commits February 13, 2025 21:03

Ignore deprecation lint rule in button/button group examples

bd58fd8

Remove minimal/outlined from ButtonGroup popover example

a618564

Ignore errors constant file in test coverage

c0c0e4c

ggdouglas merged commit d315706 into develop Feb 14, 2025
12 of 13 checks passed

ggdouglas deleted the gdouglas/proposal-button-variant branch February 14, 2025 03:36

		[Classes.MINIMAL]: minimal \|\| variant === "minimal",
		[Classes.OUTLINED]: outlined \|\| variant === "outlined",

Add variant prop to Button and ButtonGroup #7197

Add variant prop to Button and ButtonGroup #7197

Uh oh!

Conversation

ggdouglas commented Jan 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Part of #7202

Proposed changes:

Uh oh!

svc-palantir-github commented Jan 24, 2025

Add example and docs

Uh oh!

svc-palantir-github commented Jan 28, 2025

Add deprecation warnings in console

Uh oh!

svc-palantir-github commented Jan 28, 2025

Add "variant" prop to ButtonGroup component

Uh oh!

svc-palantir-github commented Jan 29, 2025

Move eslint deprecation lines for multi-line declaration

Uh oh!

svc-palantir-github commented Feb 4, 2025

Move eslint deprecation lines for multi-line declaration

Uh oh!

ggdouglas Feb 4, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

svc-palantir-github commented Feb 4, 2025

Extract ButtonVariant type

Uh oh!

svc-palantir-github commented Feb 4, 2025

Organize jsdoc declarations

Uh oh!

jscheiny Feb 4, 2025

Choose a reason for hiding this comment

Uh oh!

ggdouglas Feb 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

svc-palantir-github commented Feb 4, 2025

Add comments heading up constants

Uh oh!

svc-palantir-github commented Feb 4, 2025

Update Button/ButtonGroup interactive examples with new variant select

Uh oh!

svc-palantir-github commented Feb 4, 2025

Refactor variantClass utility

Uh oh!

svc-palantir-github commented Feb 5, 2025

Simplify types passed to variantClass utility

Uh oh!

jscheiny left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

svc-palantir-github commented Feb 12, 2025

Add `variant` prop to blocklist of removeNonHTMLProps

Uh oh!

svc-palantir-github commented Feb 13, 2025

Update ESLint deprecation ignore statements

Uh oh!

svc-palantir-github commented Feb 13, 2025

Remove comment that is no longer relevant

Uh oh!

svc-palantir-github commented Feb 14, 2025

Remove minimal/outlined from ButtonGroup popover example

Uh oh!

svc-palantir-github commented Feb 14, 2025

Ignore errors constant file in test coverage

Uh oh!

Uh oh!

bvandercar-vt commented Mar 3, 2025

Uh oh!

Uh oh!

Add `variant` prop to Button and ButtonGroup #7197

Add `variant` prop to Button and ButtonGroup #7197

ggdouglas commented Jan 24, 2025 •

edited

Loading

ggdouglas Feb 4, 2025 •

edited

Loading