공통 button 컴포넌트 추가한다 #33
Conversation
| height?: number; | ||
| isDisabled?: boolean; | ||
| children: string; | ||
| onClick?(): void; |
There was a problem hiding this comment.
저는 보통 요런 Prop에 EventHandler 타입을 주는 편입니당!
There was a problem hiding this comment.
onClick?(): React.MouseEvent<HTMLButtonElement>;
이렇게를 의미하는 것이 맞을까요?
There was a problem hiding this comment.
onClick?: React.MouseEvent<HTMLButtonElement>;
저는 요런 식으로 줍니다! 취향인 것도 같으니 참고만 하셔요!
There was a problem hiding this comment.
onClick?: React.MouseEventHandler<HTMLButtonElement>;
이 타입으로 변경했습니다.
onClick?(): React.MouseEvent<HTMLButtonElement>; <- 이 타입은 반환타입이 React.MouseEvent<HTMLButtonElement>; 이것을 의미해서 올바르지 못한 타입이였네요.
| interface TextButtonProps extends React.ButtonHTMLAttributes<HTMLButtonElement> { | ||
| variant: 'contained' | 'outlined' | 'text'; | ||
| fontSize?: number; | ||
| padding?: string; | ||
| isDisabled?: boolean; | ||
| children: string; | ||
| onClick?(): void; | ||
| onSubmit?(): void; | ||
| } |
There was a problem hiding this comment.
variant가 contained | outlined | text니까
컴포넌트 이름을 그냥 Button으로 하고 contained를 default로 주면 어떨까요?
| ${({ padding, buttonStyles, isDisabled, theme }) => ` | ||
| padding: ${padding}; | ||
| background: ${theme.colors[buttonStyles.back[isDisabled]]}; | ||
| border: ${ | ||
| buttonStyles.line[isDisabled] === 'none' | ||
| ? 'none' | ||
| : `1px solid ${theme.colors[buttonStyles.line[isDisabled]]}` | ||
| }; | ||
| cursor: ${isDisabled === 'disabled' ? ' not-allowed' : 'pointer'}; | ||
| `} | ||
| `; |
There was a problem hiding this comment.
사용자가 주요 컬러를 커스텀할 수 있도록 prop을 제공해야하려나?! 필요 없으려나?! 싶은 생각이 듭니다
There was a problem hiding this comment.
그 고민을 안 한건 아닌데...color를 받을지 말지는 payment 컴포넌트 모두 통일하는 것이 좋을 것 같습니다. 그래서 회의 때 결정하시죠!
| variant: 'contained' | 'outlined' | 'text'; | ||
| fontSize?: number; | ||
| padding?: string; | ||
| isDisabled?: boolean; | ||
| children: string; | ||
| onClick?(): React.MouseEvent<HTMLButtonElement>; | ||
| onSubmit?(): React.FormEvent<HTMLButtonElement>; | ||
| } | ||
|
|
||
| type ButtonVariant = 'contained' | 'outlined' | 'text'; |
There was a problem hiding this comment.
variant 타입에 만들어 두신 ButtonVariant 적용하면 좋을 것 같습니다 !
| }; | ||
| }; | ||
|
|
||
| export const buttonColorSet: ButtonStylesConfig = { |
There was a problem hiding this comment.
아 제 리뷰에서 이렇게 하라는 말이였나요ㅋㅋㅋㅋㅋㅋㅋㅋㅋ
| background: ${theme.colors[buttonStyles.back[isDisabled]]}; | ||
| border: ${ | ||
| buttonStyles.line[isDisabled] === 'none' | ||
| ? 'none' | ||
| : `1px solid ${theme.colors[buttonStyles.line[isDisabled]]}` | ||
| }; |
There was a problem hiding this comment.
요기 color custom가능하게 되면 바뀌는 코드이나요?
There was a problem hiding this comment.
아뇹 아직 color를 사용측에서 받는 코드는 없는 상태라서
| }, | ||
| outlined: { | ||
| text: { | ||
| default: 'PRIMARY', | ||
| disabled: 'GRAY_200' |
There was a problem hiding this comment.
관련 컬러 설정한거 바뀌는 건가요? 아니면 이대로 가시는 건가요? 회의 때 말한게 가물가물 하네요
There was a problem hiding this comment.
노션에 적혀있는 것을 보면 primary color 정도만 받기로 했습니당
작업내용
IconButton, TextButton 구현했습니다.
IconButton에서 disabled 처리가 필요한가? 필요하다면 opacity 조절을 통해 해도 괜찮은가? 이 두 의문이 들긴합니다..!
close #4