-
Notifications
You must be signed in to change notification settings - Fork 2
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
[FE] feat: 공통 컴포넌트인 체크박스와 체크박스 아이템 제작 #240
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 | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,40 @@ | ||||||
import { ChangeEvent } from 'react'; | ||||||
|
||||||
import CheckedIcon from '@/assets/checked.svg'; | ||||||
import UncheckedIcon from '@/assets/unchecked.svg'; | ||||||
|
||||||
import * as S from './styles'; | ||||||
|
||||||
// NOTE: 공통 컴포넌트에서 이 스타일 속성을 계속 쓰는 것 같은데 이걸 아예 공통 타입으로 빼버릴지 고민 | ||||||
export interface CheckboxStyleProps { | ||||||
$style?: React.CSSProperties; | ||||||
} | ||||||
|
||||||
interface CheckboxProps extends CheckboxStyleProps { | ||||||
id: string; | ||||||
isChecked: boolean; | ||||||
onChange: (event: ChangeEvent<HTMLInputElement>) => void; | ||||||
name?: string; | ||||||
isDisabled?: boolean; | ||||||
} | ||||||
|
||||||
const Checkbox = ({ id, name, isChecked, isDisabled, onChange, $style }: CheckboxProps) => { | ||||||
return ( | ||||||
<S.CheckboxContainer style={$style}> | ||||||
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.
Suggested change
CheckbocContainer의 props타입은 CheckboxStyleProps인데, props를 넣어줄 때는 style에다가 $style를 넣어주고 있어요. 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. 인간 디버거 바다 👍👍 |
||||||
<S.CheckboxLabel> | ||||||
<input | ||||||
id={id} | ||||||
name={name} | ||||||
checked={isChecked} | ||||||
onChange={onChange} | ||||||
disabled={isDisabled} | ||||||
aria-hidden={false} | ||||||
type="checkbox" | ||||||
/> | ||||||
<img src={isChecked ? CheckedIcon : UncheckedIcon} alt="체크박스" /> | ||||||
</S.CheckboxLabel> | ||||||
</S.CheckboxContainer> | ||||||
); | ||||||
}; | ||||||
|
||||||
export default Checkbox; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
import styled from '@emotion/styled'; | ||
|
||
import { CheckboxStyleProps } from './index'; | ||
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. 중요하진 않지만! 경로 index 빼도 된다는 사실! 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. 자꾸 오타난 것 같아서 덧붙이게 되네요...🫠 |
||
|
||
export const CheckboxContainer = styled.div<CheckboxStyleProps>` | ||
width: ${(props) => props.$style?.width || '2.8rem'}; | ||
height: ${(props) => props.$style?.height || '2.8rem'}; | ||
|
||
padding: 0; | ||
|
||
background-color: transparent; | ||
border: none; | ||
|
||
input { | ||
display: hidden; | ||
|
||
position: absolute; | ||
width: 0; | ||
height: 0; | ||
} | ||
`; | ||
|
||
export const CheckboxLabel = styled.label` | ||
cursor: pointer; | ||
object-fit: contain; | ||
|
||
img { | ||
width: 100%; | ||
height: 100%; | ||
} | ||
`; |
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,33 @@ | ||||||||||||||
import { ChangeEvent } from 'react'; | ||||||||||||||
|
||||||||||||||
import Checkbox, { CheckboxStyleProps } from '../Checkbox'; | ||||||||||||||
|
||||||||||||||
import * as S from './styles'; | ||||||||||||||
|
||||||||||||||
interface CheckboxItemProps extends CheckboxStyleProps { | ||||||||||||||
id: string; | ||||||||||||||
label: string; | ||||||||||||||
isChecked: boolean; | ||||||||||||||
onChange: (event: ChangeEvent<HTMLInputElement>) => void; | ||||||||||||||
name?: string; | ||||||||||||||
isDisabled?: boolean; | ||||||||||||||
} | ||||||||||||||
const CheckboxItem = ({ id, name, label, isChecked, onChange, isDisabled = false, $style }: CheckboxItemProps) => { | ||||||||||||||
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.
Suggested change
이렇게 Checkbox에게 넘겨주기 위해 받는 props들은 구조분해할당 이용해서 넘겨주면 제 경험상 Checkbox props가 변경되어도 props넘겨주는 부분은 변경하지 않아도 돼서 좋았어요 만약 나중에 리팩토링을 한다면 이 방법 추천해요 isDisabled의 기본값은 이걸 사용하는 Checkbox에서 설정하면 좋고요 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. 오 js로 rest를 접했을 때는 사용한 속성들을 너무 축약하는 느낌이라 잘 안 쓰게 됐는데 이제 어차피 ts를 쓰니까 인터페이스에서 어떤 속성을 썼는지 찾아볼 수 있겠네요! 꿀팁 감사합니다 |
||||||||||||||
return ( | ||||||||||||||
<S.CheckboxItem> | ||||||||||||||
<S.CheckboxLabel> | ||||||||||||||
<Checkbox | ||||||||||||||
id={id} | ||||||||||||||
name={name} | ||||||||||||||
isChecked={isChecked} | ||||||||||||||
onChange={onChange} | ||||||||||||||
isDisabled={isDisabled} | ||||||||||||||
$style={$style} | ||||||||||||||
/> | ||||||||||||||
{label} | ||||||||||||||
</S.CheckboxLabel> | ||||||||||||||
</S.CheckboxItem> | ||||||||||||||
); | ||||||||||||||
}; | ||||||||||||||
|
||||||||||||||
export default CheckboxItem; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
import styled from '@emotion/styled'; | ||
|
||
export const CheckboxItem = styled.div``; | ||
|
||
export const CheckboxLabel = styled.label` | ||
font-weight: ${({ theme }) => theme.fontWeight.bold}; | ||
display: flex; | ||
align-items: center; | ||
gap: 0.7rem; | ||
`; |
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.
$style
이 공통적으로 쓰이긴 하지만 다른 prop들과 함께 사용되는 경우도 있어요. 예를 들어 작업 중인 textarea에서는isError
와 함께 전달되고 있어요.공통 타입으로의 분리가 오히려 코드의 복잡성을 늘릴 수도 있을 것 같아서, 이 부분은 조금 더 얘기를 해봐도 좋을 것 같아요.
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.
앗 이걸 PR 본문에도 적었어야 했는데 😅 캐치해줘서 고마워요! 항상 분리하는 게 능사는 아니죠 다음에 얘기해봐요~