-
Notifications
You must be signed in to change notification settings - Fork 0
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 : checkboxGroup 추가 #115
base: main
Are you sure you want to change the base?
Changes from 4 commits
e87547b
2d42820
d239c3f
b1e403f
2238020
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,15 +1,16 @@ | ||
import { ChangeEvent } from 'react'; | ||
import { ChangeEvent, useContext } from 'react'; | ||
import cx from 'classnames'; | ||
import { twMerge } from 'tailwind-merge'; | ||
import CheckIcon from '@material-design-icons/svg/outlined/check.svg'; | ||
import { CheckboxContext } from './CheckboxGroup'; | ||
|
||
import { Text } from '../Text'; | ||
|
||
interface Props { | ||
/** | ||
* 체크박스의 라벨에 들어가는 텍스트입니다. | ||
*/ | ||
labelText: string; | ||
children: string; | ||
/** | ||
* 체크박스의 체크 여부입니다. | ||
*/ | ||
|
@@ -18,16 +19,27 @@ interface Props { | |
* 체크박스 활성화 여부입니다. | ||
*/ | ||
disabled?: boolean; | ||
/** | ||
* 체크 정보가 있는 context(문자열 배열)과 대조하기 위한 키 값입니다. | ||
*/ | ||
value: string; | ||
/** | ||
* 체크 여부 변경 시 호출되는 함수입니다. | ||
*/ | ||
onChange?: (e: ChangeEvent<HTMLInputElement>) => void; | ||
} | ||
|
||
export function Checkbox({ labelText, checked = false, disabled = false, onChange }: Props) { | ||
export function Checkbox({ children, checked = false, disabled = false, value, onChange }: Props) { | ||
const context = useContext(CheckboxContext); | ||
|
||
if (context) { | ||
checked = context.selectedValue.includes(value); | ||
onChange = context.onChange; | ||
} | ||
Comment on lines
+35
to
+38
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. onChangeFromProps와 onChangeFromContext 모두 event를 전달하여 동작하게끔 하는 건 어떨까요? 요게 이유가 Checkbox에 핸들러를 전달했는데, context에 핸들러가 있을 경우 무시하게 되는데, 개발자 입장에서 interface엔 onChange가 열려있는데 왜 동작을 안 하지?,, 라고 오해할 수도 있을 것 같아요! 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. 네네! 두 핸들러 모두 동작해요!! 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. 오호 유틸함수 추가해주신걸로 주말에 샴푸님이랑 리팩토링해볼게용 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. 와 이게 뭐에요 |
||
|
||
return ( | ||
<label className={cx('flex flex-row items-center gap-2', { 'opacity-[0.3]': disabled })}> | ||
<input type="checkbox" className="hidden" checked={checked} onChange={onChange} /> | ||
<input type="checkbox" className="hidden" checked={checked} value={value} onChange={onChange} /> | ||
<div | ||
className={cx( | ||
' flex h-[20px] w-[20px] items-center justify-center rounded-md border-2 border-border-primary transition-colors duration-200 hover:border-color-system_200 dark:border-border-primary_dark dark:hover:border-color-system_200', | ||
|
@@ -43,7 +55,7 @@ export function Checkbox({ labelText, checked = false, disabled = false, onChang | |
)} | ||
/> | ||
</div> | ||
<Text text={labelText} size="body2" weight="regular" /> | ||
<Text text={children} size="body2" weight="regular" /> | ||
</label> | ||
); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
import type { Meta, StoryObj } from '@storybook/react'; | ||
import { CheckboxGroup } from './CheckboxGroup'; | ||
import { Checkbox } from './Checkbox'; | ||
|
||
type Component = typeof CheckboxGroup; | ||
const meta: Meta<Component> = { | ||
title: 'rookies/CheckboxGroup', | ||
component: CheckboxGroup, | ||
}; | ||
|
||
type Story = StoryObj<Component>; | ||
|
||
export const Primary: Story = { | ||
args: { | ||
selectedValue: ['label1', 'label3'], | ||
children: ( | ||
<> | ||
<Checkbox value="label1">children</Checkbox> | ||
<Checkbox value="label2">children</Checkbox> | ||
<Checkbox value="label3">children</Checkbox> | ||
</> | ||
), | ||
}, | ||
}; | ||
|
||
export const WithDisabled: Story = { | ||
args: { | ||
selectedValue: ['label1', 'label3'], | ||
children: ( | ||
<> | ||
<Checkbox value="label1">children</Checkbox> | ||
<Checkbox value="label2">children</Checkbox> | ||
<Checkbox value="label3" disabled> | ||
children | ||
</Checkbox> | ||
</> | ||
), | ||
}, | ||
}; | ||
|
||
export default meta; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
import { ReactNode, ChangeEventHandler, createContext } from 'react'; | ||
|
||
interface Props { | ||
children: ReactNode; | ||
selectedValue: string[]; | ||
onChange?: ChangeEventHandler<HTMLInputElement>; | ||
} | ||
|
||
type CheckboxContextType = Omit<Props, 'children'>; | ||
|
||
export const CheckboxContext = createContext<CheckboxContextType | null>(null); | ||
Comment on lines
+9
to
+11
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. @asdf99245, @GulSam00 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. 좋습니다 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. 요거는 utils의 createContext 이용해서 분리하게 되는거죠? 일단은 냅둘게요...! |
||
|
||
export function CheckboxGroup({ children, ...restProps }: Props) { | ||
return ( | ||
<CheckboxContext.Provider value={{ ...restProps }}> | ||
<fieldset className="flex flex-col gap-2">{children}</fieldset> | ||
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. 나중에 row, col 선택할 수 있게 추가하는건 어떨까요? 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. 저도 동의,,! |
||
</CheckboxContext.Provider> | ||
); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,2 @@ | ||
export { Checkbox } from './Checkbox'; | ||
export { CheckboxGroup } from './CheckboxGroup'; |
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.
헉걱걱