-
Notifications
You must be signed in to change notification settings - Fork 2
feat: 디자인 시스템 - Radio 컴포넌트 구현 #236
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
base: dev
Are you sure you want to change the base?
Conversation
isChecked -> props의 checked 사용 isDisabled -> props의 disabled 사용
interaction과 border radius에 약간의 틈이 있음.
타입에러: "::after"' 형식의 식을 '{}' 인덱스 형식에 사용할 수 없으므로 요소에 암시적으로 'any' 형식이 있습니다. '{}' 형식에 '::after' 속성이 없습니다. -> interaction() 반환타입명시
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WonJuneKim
left a comment
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.
라디오 구현 고생 많으셨습니다 :) 코멘트 확인 부탁드려요!
| position: 'relative' as const, | ||
| outline: 'none' as const, |
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.
이러한 처리가 들어가게 된 원인이 궁금합니다! 리터럴 타입으로 인식이 되어서 그런건가욥?
| export interface RadioProps extends ComponentPropsWithoutRef<'input'> { | ||
| radioSize?: RadioSize; |
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.
radioContent의 스토리북을 보면 비제어형 컴포넌트, 제어형 컴포넌트로 사용될 때의 경우를 모두 표현해주셨는데, 이미 ComponentPropsWithoutRef<'input'> 에 포함되어 있는 값이지만 checked, onChange, defaultChecked(비제어형까지 고려 시) 정도는 명시적으로 표현을 해줘도 좋을 거 같습니다!
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.
JDS 라디오 설명 란에 사용자는 동시에 하나의 옵션만 선택할 수 있다. 고 되어있는데, 해당 부분을 고려 + 라디오 버튼의 주 케이스를 고려했을 때 Controlled Component로 사용되는 케이스가 더 많을 거 같아요.
이런 경우, Radio의 상태를 관리할 수 있는 Context까지 생성하는 것도 좋을 거 같아요!
++) 요거는 구현하기에 따라서 어느정도 제어형 컴포넌트의 리렌더링 문제도 해소 가능할 거 같습니다.
| <RadioLabel radioSize={radioSize}> | ||
| <RadioInput ref={ref} type='radio' {...props} /> | ||
| <RadioSpan className='visual' aria-hidden='true' radioSize={radioSize} /> | ||
| </RadioLabel> |
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.
설명해 주신대로 태그 구조가 잘 설정되어있네요~
| const theme = useTheme(); | ||
| const labelColor = disabled ? theme.color.object.subtle : theme.color.object.bold; | ||
| const subLabelColor = disabled ? theme.color.object.subtle : theme.color.object.assistive; |
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.
해당 부분도 스타일 자체에서 처리하고 props의 경우 기본값 할당으로 변경할 수 있을 거같아요.
| isStyleOutline={radioStyle === 'outline'} | ||
| > | ||
| {align === 'right' && ( | ||
| <Label size={radioSize} textAlign='left' weight='normal' color={labelColor}> |
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.
(참고) align 값에 따라 배치가 반전되는건 RadioContainerLabel 에서 해당 값의 유무에 따라 flex-direction 설정을 추가해주면 tsx에서 조건문을 걸어주지 않아도 될 거 같습니다!
| subLabelVisible?: boolean; | ||
| subLabel?: ReactNode; |
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.
코드로만 보았을 때에는 subLabelVisible 이라는 boolean 값은 subLabel의 유무로도 충분히 추론이 될 거 같아요! (엣지 케이스가 SubLabel 이 존재하는데, SubLabel이 보이지 않게 설정하는 경우인데, 해당 케이스가 존재하는지에 대한 건 동영님께 한번 여쭤봐야할듯!)
맥락상으론 두개가 같은 상태를 가질 것 같아서 코멘트 드립니다!
| disabled?: boolean; | ||
| subLabelVisible?: boolean; | ||
| subLabel?: ReactNode; | ||
| children: ReactNode; |
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.
오 radio가 <input> 요소 이기 때문에 children을 가지지 않아서 요로케 표현해도 되겠군요 ㅎㅎㅎ
💡 작업 내용
💡 자세한 설명
Radio
피그마에서 Radio/basic에 해당하는 컴포넌트입니다.
input의 type='radio' 태그는 안보이도록 처리하고 label 태그와 span 태그를 이용하여 스타일을 구현했습니다.
피그마에서는
isChecked,isDisabled속성명을 가지고 있지만 라디오 input 태그가 가진 자체 속성을 이용해 기본 동작이 가능하도록 했습니다. label 태그를 사용하면 input 태그와 span(스타일담당) 태그와 연결되는 특징을 활용했습니다. 따라서isChecked->checked,isDisabeld->disabled로 그대로 사용해주시면 될 것 같습니다.RadioContent
피그마에서 Radio/content에 해당하는 컴포넌트입니다.
피그마에서 사용하는 속성명과 다릅니다.
기본 속성명과 겹치는 속성명은 이와 같이 변경되었습니다.
size->radioSize,style->radioStyle,isChecked->checked,isDisabled->disabled레이블이 달려있으며 align(left, right) 속성에 따라 라디오 인풋이 왼쪽, 오른쪽으로 배치할 수 있습니다. 또한 서브 레이블을 작성할 수 있습니다. 메인 레이블은 children을 통해 작성하고 서브레이블이 필요할 경우, props의 subLabel 속성을 통해 작성할 수 있도록 했습니다. 서브레이블에는 하이퍼링크를 달 수 있어야하기 때문에 ReactNode 타입으로 지정했으며 하이퍼링크를 달아둔 스토리도 예시로 작성해두었습니다.
RadioContent에도 Radio 컴포넌트가 사용되는데 Radio 컴포넌트(input)에 focus-visible되었을 경우 RadioContent에만 파란 아웃라인이 보이도록 하기 위해 Radio 컴포넌트에 적용된 아웃라인은 제거해주었습니다.
이 외 사용 예시 등은 스토리북을 참고해주세요!
기본적인 구현 방법
기본적인 내부 레이아웃은 grid를 사용해 구현했습니다.
라디오 버튼과 메인 레이블은 중앙 정렬, 메인레이블과 서브 레이블 사이의 왼쪽 정렬을 해야하는데 flex로 구현하게 되면 복잡해져 grid로 구현했습니다.
RadioContent에서 radioStyle='empty'인 경우, 인터랙션의 범위가 RadioContent의 범위보다 크게 설정되어있습니다. 이는 RadioContent의 너비,높이에 일정 비율에 따라 확장되는 것이 아닌, 고정된 크기만큼 커지는 것이기 때문에
calc()을 사용하여 기본 인터랙션의 가상요소를 오버라이딩하여 구현했습니다. 이때 고정된 크기만큼 커지도록하기 위해 px값으로 사용했습니다.디자인에서 아래 사진과 같이 왼쪽이 1px 더 크게 확장되어 되어있기 때문에 transform으로 1px 왼쪽 이동하도록 했습니다.
문제점
가상요소와 부모요소 사이의 미세한 틈
radioStyle=outline 컴포넌트를 크게 확대했을 경우 아래 사진처럼 인터랙션과 border 사이에 틈이 보입니다. 이는 렌더링 과정에서 소수점 계산 시 오차로 인해 발생하는 문제로 추측됩니다. 키보드 포커스 했을 경우에도 border을 덮어버리면서 아웃라인이 생기는 문제가 있는데 이는 가상요소 자체에도 border을 추가해서 눈속임(?)으로 최대한 비슷하게 구현했습니다..!
transition 모션
hover일 경우, 인터랙션에 fluent 모션이 100ms 간 들어갑니다. 이외 active일 경우에는 모션이 없습니다.
문제는 마우스 클릭(active)했다가 떼는 순간 active -> hover로 감지되어 active의 exit일 때도 모션이 있는 것처럼 표현되고 있습니다. 100ms라서 미관상 큰 차이는 없습니다! (500ms으로 늘렸을 때 살짝 보임)
📗 참고 자료 (선택)
📢 리뷰 요구 사항 (선택)
✅ 셀프 체크리스트
#221