UI: Improved Button and IconButton #24266

cdedreuille · 2023-09-22T07:40:22Z

What I did

As part of all our efforts to update our core UI components, we are improving both the Button and IconButton as part of this PR. The main concept here is to make it more of an atomic version that can be used anywhere. We worked on creating props that would make it easy to customise and extend the way you want without breaking the design. It also work with the existing icons library as well as our new icon library.

No breaking changes for 8.0.
One button to rule them all. We are removing all the need for special button instances.
We are keeping the children prop as it is.
new asChild prop to allow for a custom wrapper (for <Link> or <a />).
IconButton is now just a wrapper around Button with special props.
Match design of old props to the closest version we have in the new design.

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

stories
unit tests
integration tests
end-to-end tests

Manual testing

This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!

Open Storybook locally with yarn storybook:ui
Go to the IconButton story to test it out.

Documentation

Add or update documentation reflecting your changes
If you are deprecating/removing a feature, make sure to update
MIGRATION.MD

Checklist for Maintainers

When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli/src/sandbox-templates.ts
Make sure this PR contains one of the labels below:
Available labels
- bug: Internal changes that fixes incorrect behavior.
- maintenance: User-facing maintenance tasks.
- dependencies: Upgrading (sometimes downgrading) dependencies.
- build: Internal-facing build tooling & test updates. Will not show up in release changelog.
- cleanup: Minor cleanup style change. Will not show up in release changelog.
- documentation: Documentation only changes. Will not show up in release changelog.
- feature request: Introducing a new feature.
- BREAKING CHANGE: Changes that break compatibility in some way with current major version.
- other: Changes that don't fit in the above categories.

🦋 Canary release

This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the @storybookjs/core team here.

core team members can create a canary release here or locally with gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=<PR_NUMBER>

This reverts commit c9a9f06.

cdedreuille · 2023-10-03T22:35:43Z

Thanks a lot for reviewing this PR @JReinhold. Here's some answers:

Using the deprecated props will not make the button look at all like it used to. Here are the new deprecated stories, compare them with the current stories for the button and there is a clear visual difference. I'm not sure if we should consider this a breaking change or not. The API is not broken, but how the button looks in the UI has definitely changed.

This was a conscious decision. From the audit that I made so far I could find a single instance where any of the main button components were use in both internal and external addons. The idea is to make sure that automatically default to our new default design with the closest match.

There are some thing missing here, mainly migration notes and runtime deprecation warnings. do you plan on adding this later, or as part of this PR? what other follow-up work are you planning to do, if any?

I'll work on that tomorrow on migration notes. I'm not sure what need to be done regarding runtime depreciation warnings though. Can you tell me more?

I can't get the animations to work in the stories, am I doing something wrong? I'm just opening the base button story and setting the onClickAnimation prop but that doesn't seem to do anything.

This is now fixed. I added more stories to test animations.

I think we should use this opportunity to make the stories here great, with proper descriptions for the components and the props, so the Button/Docs page is something actually useful. setting a trend for our future work on the UI.

Yes you're right. I can work on it tomorrow morning.

JReinhold · 2023-10-04T07:25:33Z

I'm consistently seeing some weird pixel-rendering issues (in Chromium) around the corners, is it just me, or are we using an unusual combination of shadows/borders that does this?

JReinhold · 2023-10-04T07:29:56Z

This was a conscious decision. From the audit that I made so far I could find a single instance where any of the main button components were use in both internal and external addons. The idea is to make sure that automatically default to our new default design with the closest match.

That's fair, we should make sure to mention it in the migration notes.

I'll work on that tomorrow on migration notes. I'm not sure what need to be done regarding runtime depreciation warnings though. Can you tell me more?

we should show a warning in the console when we detect that someone is using any of the deprecated props. See how we do it for Description here: https://github.com/storybookjs/storybook/blob/next/code/ui/blocks/src/blocks/Description.tsx#L143-L157

JReinhold

Looks good! I have a few minor comments but I'm sure you'll figure it out. Make sure to review all the UI Tests in Chromatic.

JReinhold · 2023-10-05T09:09:39Z

MIGRATION.md

@@ -320,6 +321,10 @@ In Storybook 7 it was possible to use `addons.setConfig({...});` to configure St
 - `showPanel: boolean` is now split into `bottomPanelHeight: number` and `rightPanelWidth: number`, where the numbers represents the size of the panel in pixels.
 - `isFullscreen: boolean` is no longer supported, but can be achieved by setting a combination of the above.

+#### New UI and props for Button and IconButton components
+
+We used to have a lot of different buttons in `@storybook/components` that were not used anywhere. We have removed them and added a new `Button` component that can be used in all places. The `IconButton` component has also been updated to use the new `Button` component. Going forward addon creators and Storybook maintainers should use the new `Button` component. `IconButton` is still available for use, but we might remove it in the future as this is just a wrapper around the `Button` component. `Form.Button` is depreciated and will be removed in the future.


We need to:

be specific about which exact components have been deprecated (currently it just says "a lot of different buttons")

which exact props have been deprecated

which new props replace which old props

give an example of migrating from the old props to the new props

We don't need to specify stuff that we "might remove" in the future, it doesn't help the user and just creates uncertainty.

In reality this is mostly an accessible version of what's written in the source code and the types, but we can't expect users to read those at all. Consider that users will see the warning in the console with a link to this section, they'll need a way forward here to resolve the warning.

Here's a proposal:

The X component from @storybook/components have been deprecated.
The following props to the Button component have been deprecated:

X use Y instead

X use Y instead

...
(same for IconButton if it has any deprecations)
Here's an example of migrating from the old props to the new props:

// deprecated usage <Button ... /> // new usage <Button ... />

I just pushed an update with some new notes and clear descriptions of what props are deprecated and what is the alternative.

code/ui/components/src/components/Button/Button.tsx

JReinhold · 2023-10-05T09:19:03Z

code/ui/components/src/components/Button/Button.tsx

-      path: {
-        fill: 'currentColor',
-      },
+    background: `${(() => {


it's not actually fixed here 😁

JReinhold · 2023-10-05T09:22:25Z

code/ui/components/src/components/Button/Button.tsx

+  verticalAlign: 'top',
+  whiteSpace: 'nowrap',
+  userSelect: 'none',
+  opacity: disabled ? 0.5 : 1,


this way of setting something to be disabled is a bit unfortunate. if the button is on top of something that has multiple colors, it will actually be semi-transparent, where I think what we really want is the colors to be slighlty muted?

I agree with you on this one but I think this is something we might change after the theming v2.0 project. For now there is pretty low risk that the button will be below something else.

Add the new IconButton

e760142

cdedreuille added maintenance User-facing maintenance tasks ci:normal labels Sep 22, 2023

cdedreuille self-assigned this Sep 22, 2023

Replace IconButton

1f2fe6b

cdedreuille changed the base branch from next to v8 September 22, 2023 07:48

cdedreuille added 5 commits September 22, 2023 08:49

Merge branch 'v8' into icon-button

e3c3e59

Merge branch 'v8' into icon-button

8b6b89b

Make IconButton works with text for backward compatibility

9e5ee81

Remove the old IconButton

f6f7e26

Update separator

930d1e2

cdedreuille changed the base branch from v8 to release-8-0 September 22, 2023 14:27

cdedreuille added 8 commits September 22, 2023 16:28

Merge branch 'release-8-0' into icon-button

9b12740

Fix icons in panel not showing correctly

926da91

Merge branch 'release-8-0' into icon-button

c207df0

Update all props on IconButton

4cd425c

Merge branch 'release-8-0' into icon-button

2c49acc

Fix missing IconButton

c9a9f06

Make icon prop optional

4b1e76e

Update IconButton.tsx

ae268a5

cdedreuille marked this pull request as ready for review October 2, 2023 14:52

cdedreuille requested review from ndelangen, JReinhold, tmeasday, yannbf, valentinpalkovic, kylegach and jonniebigodes as code owners October 2, 2023 14:52

cdedreuille added 2 commits October 2, 2023 16:09

Revert "Fix missing IconButton"

4ac1af7

This reverts commit c9a9f06.

Revert

26383b5

cdedreuille added 6 commits October 3, 2023 23:02

Fix name

2c820ac

Fixed. I used Stack and Row

cf89b4f

Removed string

3f8ec42

Update Button.tsx

1dd8cc2

Update Button.tsx

b24e5c4

use 'a' if we are using isLink

213faad

cdedreuille added 2 commits October 3, 2023 23:39

Fixes

dd8d9e2

Merge branch 'release-8-0' into icon-button

1da9c7c

cdedreuille added 5 commits October 4, 2023 09:51

Add migration + deprecated warnings

76c5601

Add isLink story

83147eb

Improve deprecated warning

18e6465

Improve Button docs

2003a4b

Improve docs

602ecd7

cdedreuille requested a review from JReinhold October 4, 2023 10:26

Merge branch 'release-8-0' into icon-button

fe09c25

JReinhold approved these changes Oct 5, 2023

View reviewed changes

JReinhold added the ui label Oct 5, 2023

valentinpalkovic removed their request for review October 5, 2023 09:55

cdedreuille added 6 commits October 6, 2023 11:32

Improved migration notes

3de2756

Fixed spelling mistake

acbbd3b

Update Button.tsx

64ba705

Cleaning

aecc130

Merge branch 'release-8-0' into icon-button

0d56d6e

Fix button shrinking

b30db0e

cdedreuille merged commit b444604 into release-8-0 Oct 6, 2023

cdedreuille deleted the icon-button branch October 6, 2023 11:26

trfv mentioned this pull request Jun 6, 2024

bump up @storybook/components and @storybook/manager-api kemuridama/storybook-addon-github#8

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

UI: Improved Button and IconButton #24266

UI: Improved Button and IconButton #24266

cdedreuille commented Sep 22, 2023 •

edited

Loading

cdedreuille commented Oct 3, 2023

JReinhold commented Oct 4, 2023

JReinhold commented Oct 4, 2023

JReinhold left a comment

JReinhold Oct 5, 2023

cdedreuille Oct 6, 2023

JReinhold Oct 5, 2023

JReinhold Oct 5, 2023

cdedreuille Oct 6, 2023

UI: Improved Button and IconButton #24266

UI: Improved Button and IconButton #24266

Conversation

cdedreuille commented Sep 22, 2023 • edited Loading

What I did

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

Manual testing

Documentation

Checklist for Maintainers

🦋 Canary release

cdedreuille commented Oct 3, 2023

JReinhold commented Oct 4, 2023

JReinhold commented Oct 4, 2023

JReinhold left a comment

Choose a reason for hiding this comment

JReinhold Oct 5, 2023

Choose a reason for hiding this comment

cdedreuille Oct 6, 2023

Choose a reason for hiding this comment

JReinhold Oct 5, 2023

Choose a reason for hiding this comment

JReinhold Oct 5, 2023

Choose a reason for hiding this comment

cdedreuille Oct 6, 2023

Choose a reason for hiding this comment

cdedreuille commented Sep 22, 2023 •

edited

Loading