Use new button styles #541
Conversation
| onClick?: () => void | ||
| id?: string | ||
| disabled?: boolean | ||
| icon?: {image: React.ReactNode, position: 'before' | 'after'} | ||
| className?: string | ||
| } | ||
|
|
||
| export type Level = 'primary' | 'secondary' | 'tertiary' | ||
| export type Type = 'default' | 'accent' | 'warn' | 'subtle' | ||
| export type Level = 'default' | 'primary' | 'secondary' | 'tertiary' |
There was a problem hiding this comment.
I'm not sure it's clear to me that we should have a default value here - it's not understandable how this semantically fits in between the hierarchy of primary / secondary / tertiary
There was a problem hiding this comment.
I just took that directly from abstract, cc @karenkliu
There was a problem hiding this comment.
@karenkliu @antonok-edm we should probably sync-up as it seems that whilst this has removed some variations, it seems more complicated to use. Right now we have properties that easily allow a developer or designer to know when to use which button:
- 'level' which indicates the importance of the button (most important in the UI vs one of many equal actions) and affects the style of the button,
- 'type' which indicates the type of action (positive, negative, primary brand action) and just really affects the color
These two properties are independent and could allow for all combinations.
In this new version we have a single property called 'Levels' in which I would expect the values to be hierarchical and has 4 variations - primary, secondary, tertiary and default. I wouldn't know when to use 'default' vs the hierarchical values. I would expect most buttons which aren't Primary actions to be Secondary actions, but now I'm guessing they should be Default actions and it's unclear when to use Secondary vs Tertiary.
IMO we shouldn't mix metaphors here - either keep the semantic definitions (and ideally backwards-compatible) or choose new ones.
As another aside, these new button styles are slightly different to the other new button styles that Ross implemented in brave-core's WebUI at brave/brave-core#2778. Those ones have a slight hover effect on 'pressed' and also a nicer focus outline style (that last one we should definitely adopt).
|
Please add a storybook link via running |
Changes
Fixes #536.
Addresses concerns from #486 as well.
With this PR, buttons are all themed using a common
Buttoncomponent, and have a visual update as described in #536.This will require changes in https://github.com/brave/brave-core wherever buttons are used as well.
Test plan
Storybook stories and snapshots have been updated to use the new buttons. I've done a scan through all of the usages of buttons to verify that everything looks acceptable, but another pair of eyes would be appreciated.
Link / storybook path to visual changes
Integration
Does this contain changes to src/components or src/
Does this contain changes to src/features for brave-core?