Add ButtonIcon #194
Add ButtonIcon #194
Conversation
| @@ -0,0 +1,35 @@ | |||
| import { useStyletron } from "baseui"; | |||
| @@ -0,0 +1,130 @@ | |||
| import { StyleObject } from "styletron-standard"; | |||
There was a problem hiding this comment.
Is there possibility to reuse Button component with overrides and not to copy all of that? Probably there are reasons for that, but imho looks weird
There was a problem hiding this comment.
I don't see reason for me to reuse Button. Because it's kinda non-trivial will be support and lead to worse maintainability of component because of some changes in button will affect this component. I want to make it simple as possible because we already took a path form base web so simplicity is the main point now for me.
There was a problem hiding this comment.
Ok, i got your point. Probably then can we create a table of common styles for buttons? Because there is a lot of common styles, and - i guess IconButton will always have the same kinds as Button. Additional styles will be written in particular components, but this we can place inside shared and reuse in both of them.
Something like getButtonKindStyles(), and it is supposed that we will reuse BUTTON_KIND as well.
| onClick, | ||
| disabled = false, | ||
| tabIndex, | ||
| href, |
There was a problem hiding this comment.
Better to restrict href prop if role is button
There was a problem hiding this comment.
and if role is link - at least target and rel would be helpful as props
| danger = "danger", | ||
| } | ||
|
|
||
| export enum BUTTON_ICON_SIZE { |
There was a problem hiding this comment.
You can omit mini size of usual button, and use BUTTON_SIZE in order not to increase amount of enums
| role?: "button" | "link"; | ||
| href?: string; | ||
| icon: ReactElement; | ||
| onClick?: (e: MouseEvent) => void; |
There was a problem hiding this comment.
Maybe its better to use here MouseEvent from react itself (it is generic)
| icon: ReactElement; | ||
| onClick?: (e: MouseEvent) => void; | ||
| disabled?: boolean; | ||
| kind?: BUTTON_ICON_KIND; |
There was a problem hiding this comment.
The same, i suggest to use BUTTON_KIND
|
In general not all requested changes are mandatory. But forward ref, some props changes and reuse code from Button i guess would be helpful. |
|
Don't be actual |
According figma files.
But it's kinda not super at focus but work