refactor(Tab): Tab 컴포넌트 storybook 및 접근성 개선 #286
Conversation
|
|
ljh0608
left a comment
There was a problem hiding this comment.
PR이 상세해서 많이 배우기도 했고 이해하기 좋았어요 감사합니다~
| {translator ? translator[item] : item} | ||
| </button> | ||
| <div className={`${S.tabItemUnderline} ${isSelected}`} /> | ||
| <div className={`${S.tabItemUnderline} ${isSelected ? 'selected' : ''}`} /> |
There was a problem hiding this comment.
selected className이 어떤 용도로 활용되는건가요??
이부분도 중보되고 있어서 상단에 변수로 선언해두면 좋을 것 같아요
const selectedClassName= isSelected ? 'selected' : ''
There was a problem hiding this comment.
수정하신 isSelected 문제 없어보이네요.
그리고 클래스네임은 빈 문자열보다는 null (없음)은 어떠신지
위 재훈님이 말씀하신 것도 동의합니다
| selectedInitial: { control: 'select', options: ['Tab1', 'Tab2', 'Tab3'] }, | ||
| tabItems: { | ||
| description: '여러 Tab 항목을 전달합니다.', | ||
| table: { type: { summary: 'T[]' } }, |
There was a problem hiding this comment.
story는 구체적인 타입이나 예시를 넣는것이 문서를 읽는데 더 좋은 방향인 것 같아요 제네릭 타입보단 string[] 을 사용해보는건 어떤가요?
| table: { type: { summary: 'T (optional)' } }, | ||
| }, | ||
| translator: { | ||
| description: 'Tab 항목의 텍스트를 원하는 컴포넌트로 변경합니다.', |
There was a problem hiding this comment.
Tab 항목의 텍스트를 ReactNode로 매핑합니다. (아이콘, 강조 텍스트 등 가능)
와 같이 ReactNode 사용을 명시해줘도 좋을 것 같아요!
| size: { | ||
| control: 'inline-radio', | ||
| options: ['sm', 'md', 'lg'], | ||
| description: 'Tab의 크기를 설정합니다.', | ||
| table: { type: { summary: 'sm | md | lg' } }, | ||
| }, |
There was a problem hiding this comment.
해당 문서에 defaultValue가 빠진 것 같아요! 이부분 추가해주세요!
| {translator ? translator[item] : item} | ||
| </button> | ||
| <div className={`${S.tabItemUnderline} ${isSelected}`} /> | ||
| <div className={`${S.tabItemUnderline} ${isSelected ? 'selected' : ''}`} /> |
There was a problem hiding this comment.
수정하신 isSelected 문제 없어보이네요.
그리고 클래스네임은 빈 문자열보다는 null (없음)은 어떠신지
위 재훈님이 말씀하신 것도 동의합니다
변경사항
링크
시급한 정도
🏃♂️ 보통 : 최대한 빠르게 리뷰 부탁드립니다.
기타 사항
1️⃣ 내부 isSelected 일부 변경
기존 코드를 살펴보니
isSelected가 아래와 같이 설정된 것을 볼 수 있었습니다.물론 반복되는 코드를 줄이기 위해 사용한 것은 좋지만,
is-prefix의 의미가 대부분boolean값을 가진다는 점과aria-selected에서 선택 여부에 해당하는boolean값을 필요로 한다는 것을 근거로 삼아 코드를 아래와 같이 수정하였습니다.Tab의 interface를 바꾸지 않아 크게 문제가 될 것이 없다고 생각하지만, 이 부분에 대해서 잘못된 방향이 있다면 코멘트 부탁드립니다!
2️⃣ tabIndex 사용 접근성 개선
모든 탭이 tab key로 이동되어도 되지만, 접근성을 고려할 때 활성화 되는 탭을 이동시켜 주는 것이 좋다고 판단했습니다.
또한 material design 코드에도
tabIndex가 tab bar에 적용된 것을 근거로 사용하는 것이 접근성에 더 좋다고 판단하여 추가하였습니다.material design - tab bar 링크
2025-09-30.7.02.11.mov
3️⃣ storybook show code 컴포넌트 이름 이슈
storybook에서 show code를 누르면 컴포넌트명이 이상하게 뜨는 이슈가 있었습니다. 이 부분은 구현 컴포넌트의 이름을 제대로 인식하지 못하는 이유라고 판단해서 처음에는 import/export문을 수정해보기도 했는데 결국 해결한 방법은 아래와 같습니다.
Tab 컴포넌트 파일 최하단에
displayName을 지정해줘서 해결했습니다. 이 부분에 대해서 storybook github issue등에서 찾아보니 뭔가 해결됐다고 코멘트를 남겼지만, 제대로 해결이 안된 사람들이 많은 것 같았습니다. 그래서 issue안에서 제일 많이 해결하는 방법 중 하나가 displayName 지정이라고 판단해서 추가했는데, 더 좋은 의견 있으시다면 알려주시면 감사하겠습니다.storybookjs/storybook#20920
storybookjs/storybook#18972
Note
Tab 컴포넌트 스토리북에서
WithComponent에 대한 상황은 지금 기준으로 어드민에서만 사용하는 것 같습니다.이 부분이 아직 mds 피그마에는 반영이 안된 것 같아서 PD분들께 말씀드려 이번주 내로 회의 후 작업 진행하겠다는 답변을 받았습니다.
이후 추가로 mds에 제가 작업할 부분이 있다면 답변 받는대로 작업하겠습니다!