-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feat/button #34
Feat/button #34
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,27 @@ | ||
import React, { HTMLAttributes } from 'react'; | ||
import React, { Ref, forwardRef } from 'react'; | ||
import classnames from 'classnames'; | ||
import { CombineElementProps } from 'src/types/utils'; | ||
import { Text } from '..'; | ||
|
||
export default function Button(props: HTMLAttributes<HTMLButtonElement>): JSX.Element { | ||
return <button className="button" {...props} />; | ||
interface ButtonBaseProps { | ||
size?: 'small' | 'medium' | 'large'; | ||
} | ||
type ButtonProps = Omit<CombineElementProps<'button', ButtonBaseProps>, 'type'>; | ||
|
||
const Button = ( | ||
{ size = 'small', disabled, style, ...props }: ButtonProps, | ||
ref: Ref<HTMLButtonElement> | ||
) => { | ||
return ( | ||
<button | ||
className={classnames('lubycon-button', `${size}`)} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 사이즈도 |
||
disabled={disabled} | ||
style={style} | ||
ref={ref} | ||
> | ||
<Text typography={size === 'large' ? 'p1' : 'p2'} fontWeight="bold" {...props}></Text> | ||
</button> | ||
); | ||
}; | ||
|
||
export default forwardRef(Button) as typeof Button; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,37 @@ | ||
.button { | ||
outline: 0; | ||
button { | ||
font: inherit; | ||
outline: none; | ||
border: 0; | ||
margin: 0; | ||
padding: 0; | ||
text-align: inherit; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 요거는 저희 라이브러리의 스코프를 벗어나는 선택자인데...괜찮을까유? 사용자가 예상하지 못하는 동작을 일으킬 수도 있을 것 같습니다 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 아 그렇네요. 이거 버튼 안에다 넣겠습니다 |
||
|
||
.lubycon-button { | ||
background-color: get-color('blue50'); | ||
border-radius: 4px; | ||
color: #ffffff; | ||
cursor: pointer; | ||
|
||
&.small { | ||
height: 32px; | ||
padding: 4px 16px; | ||
} | ||
&.medium { | ||
height: 40px; | ||
padding: 8px 16px; | ||
} | ||
&.large { | ||
height: 56px; | ||
padding: 12px 32px; | ||
} | ||
|
||
&:hover, &:active { | ||
background-color: get-color('blue60'); | ||
} | ||
&:disabled { | ||
color: get-color('gray60'); | ||
background-color: get-color('gray40'); | ||
cursor: not-allowed; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,47 @@ | ||
import React from 'react'; | ||
import Button from 'components/Button'; | ||
import Text from 'components/Text'; | ||
import { Meta } from '@storybook/react/types-6-0'; | ||
|
||
export default { | ||
title: 'Lubycon UI Kit/Button', | ||
component: Button, | ||
} as Meta; | ||
|
||
export const Default = () => <Button>기본 버튼</Button>; | ||
const sizeList = ['small', 'medium', 'large'] as const; | ||
const btnText = '버튼 텍스트'; | ||
|
||
export const Default = () => { | ||
return ( | ||
<div> | ||
<Text as="div" typography="h5" style={{ marginBottom: '40px' }}> | ||
Rounded Button | ||
</Text> | ||
<ul style={{ listStyle: 'none' }}> | ||
{sizeList.map((size, index) => ( | ||
<li | ||
key={index} | ||
style={{ | ||
display: 'grid', | ||
gridTemplateColumns: '100px 150px 150px', | ||
gridGap: '50px', | ||
marginBottom: '30px', | ||
alignItems: 'center', | ||
}} | ||
> | ||
<Text style={{ width: '100px' }}>{size.charAt(0).toUpperCase() + size.slice(1)}</Text> | ||
<div> | ||
<Button size={size} key={index}> | ||
{btnText} | ||
</Button> | ||
</div> | ||
<div> | ||
<Button size={size} key={index + 'disabled'} disabled> | ||
{btnText} | ||
</Button> | ||
</div> | ||
</li> | ||
))} | ||
</ul> | ||
</div> | ||
); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
요거 타입 넣어줘도 되지 않을까유? 타입이 있는게 본래 버튼 엘리먼트의 사용성을 해치지 않는 방법이기도 하고, 어차피 타입 종류도
submit
이랑reset
밖에 없어서 큰 문제가 있을 것 같지는 않습니당.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이거 이해못한채로 그냥 가져다써서 그렇습니다 😅
동욱님 쓰신글 보고와서 수정했어요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@evan-moon
동욱님 근데 이거 타입에서 ref 쓰는거와 관련해서 궁금한게 있는데요.
현재 대부분 컴포넌트에서
export default forwardRef(Checkbox) as typeof Checkbox;
이런식으로 컴포넌트를 추출하는데,
이러면 상위 컴포넌트에서 ref 사용했을때 타입을 못읽어와서요.
타입 지정해준
as typeof Checkbox
를 지우면 잘 동작하구요.ref를 forwardRef 콜백의 첫번째 인자의 타입 가지고만 판단하는 것 같은데,
이거 혹시 이유를 아시나요 😶
검색해서 예제만 봤을때는 쉽게 이해가 가지가 않네요 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
그건 버그인 것 같네유ㅜㅜ사실 기존에도 as로 강제적인 타이핑을 한 거라 트리키한 방법이기는 했슴다. 요건 조만간 한번 고치고 가는게 좋겠네요 😢