ActionButtonに他カラーを追加 #302
ActionButtonに他カラーを追加 #302
Conversation
|
Deploy preview for ingred-ui ready! Built with commit dab0056 |
|
|
|
buttonがデフォルトでdisabledを持っているが、 |
modeではなくcolorという命名にして、デフォルトのdisabledを使う。 |
hirokikondo86
requested review from
ryokosuge and
noronaoki
and removed request for
a team and
ryokosuge
|
@hirokikondo86 Disabledにhoverした時のマウスカーソルの形状を、他のコンポーネントと同様にしてください |
| export type ActionButtonColorStyle = { | ||
| normal: { | ||
| background: string; | ||
| color: string; | ||
| }; | ||
| hover: { | ||
| background: string; | ||
| color: string; | ||
| }; | ||
| }; |
There was a problem hiding this comment.
これ、 export する必要あるかな？このComponent以外で使ってなさそう
There was a problem hiding this comment.
使ってないですね…。
今後どこかで使うかもみたいな感覚で書いていたのですがやっぱよくないですよね…。
修正します…！
There was a problem hiding this comment.
あ、ActionButtonColorStyleはstyledで使ってました…！
ただ、上のActionButtonColorは使ってないのでそっちを修正します！
There was a problem hiding this comment.
あーだとしたら、双方向でimportしてるのなんかおかしい気がする
お互いに依存しあっちゃってるな
| const getBackgroundColor = (theme: Theme) => ({ | ||
| primary: theme.palette.background.hint, | ||
| warning: theme.palette.danger.highlight, | ||
| disabled: theme.palette.gray.light, | ||
| }); |
There was a problem hiding this comment.
関数名だけ見るとこいつ背景色全部取ってきそうなので、AtNormalとかの方が範囲狭められてよさそう。
There was a problem hiding this comment.
通常
normal
usual
common:
general
共通してないので、commonとかgeneralは違いそう。
usualもいつものっていう意味合いがありそうなので違いそう。
やっぱnormalなのかな…？
| backgroundColorAtNormal: string; | ||
| backgroundColorAtHover: string; |
There was a problem hiding this comment.
normalBackgroundColor: string;
hoverBackgroundColor: string;
の方が読みやすいかな
| const getColorByDisabled = ( | ||
| color: ColorProp, | ||
| disabled?: boolean, | ||
| ): string => { |
There was a problem hiding this comment.
string じゃなくて ColorProp | 'disabled' みたいに厳格に型詰めていきましょう
| }; | ||
|
|
||
| const colorForStyle = getColorByDisabled(color, disabled); | ||
| const normalBackgroundColor = getBackgroundColorAtNormal(theme)[ |
There was a problem hiding this comment.
getBackgroundColorAtNormal は変えない？
| const normalBackgroundColor = getBackgroundColorAtNormal(theme)[ | ||
| colorForStyle | ||
| ]; | ||
| const hoverBackgroundColor = getBackgroundColorAtHover(theme)[ |
There was a problem hiding this comment.
getBackgroundColorAtHover こっちもかな
There was a problem hiding this comment.
完全に見逃してた…。修正します！🙇♂️
Ref
DST #2715
XD
やったこと
スクショ
Before
After
確認方法
https://deploy-preview-302--ingred-ui.netlify.app/?path=/story/components-inputs-actionbutton--example