refactor(Toast): Toast 컴포넌트 storybook 및 접근성 개선 #281
Conversation
|
|
namdaeun
changed the title
constantly-dev
left a comment
constantly-dev
left a comment
There was a problem hiding this comment.
수고하셨습니다 간단한 코멘트만 확인해주세요!
추가로 useToast나 toast component에서 리팩터링 할 부분은 없을까요?
| const isDefaultIcon = typeof icon === 'string'; | ||
| const DefaultIcon = isDefaultIcon ? convertToIcon[icon] : undefined; |
There was a problem hiding this comment.
단순 궁금증인데 토스트 아이콘에 기본 아이콘 제외하하고 다른 element가 들어갈 일이 있나요?
interface에 React.ReactElement도 열어둬서 해당 조건도 확인해야하고, 아래 조건도 복잡해지는 것 같아서 질문 드립니다!
| style: undefined, | ||
| }, | ||
| render: (args) => { | ||
| const { open } = useToast(); |
There was a problem hiding this comment.
여기 eslint error 떠서 아래 CloseToast처럼 // eslint-disable-next-line react-hooks/rules-of-hooks 이 주석 추가해주거나, 따로 이 부분도 컴포넌트 분리해서 useToast 써야할 것 같아요
| const COPY_ICON = <IconCopy />; | ||
|
|
||
| interface ToastStoryArgs { | ||
| icon?: 'success' | 'alert' | 'error' | 'custom' | undefined; |
There was a problem hiding this comment.
icon props 자체가 optional이라 undefined는 없어도 될 것 같아요
| const option: ToastOptionType = { | ||
| icon: "success", | ||
| content: "프로젝트가 등록되었어요.", | ||
| icon: 'success', | ||
| content: args.content, | ||
| action: args.actionName | ||
| ? { | ||
| name: args.actionName, | ||
| onClick: args.onActionClick || fn(), | ||
| } | ||
| : undefined, | ||
| style: args.style ? { root: args.style } : undefined, | ||
| }; |
There was a problem hiding this comment.
모든 스토리마다 option 설정하는 로직이 있어서 이를 ToastSample 내부에서 한번만 설정하도록 해서 반복을 줄일 수 있을 것 같아요
There was a problem hiding this comment.
interface가 피그마와 조금 다르긴 하네요!
prod 운영중이라 바꾸기는 어렵긴한데 디자이너와 소통해서 아이콘에는 아이콘만 들어가는지 or 텍스트 등 element 등도 들어갈 수 있는지 등이라도 맞춰주면 좋을 것 같아요. 이후로 디자이너도 확실한 rule을 가지고 변동이 없었으면 하는 마음에!
변경사항
Toast 컴포넌트 Story명을 변경하였으며, description을 구체화했습니다.
기존 toast prop 중 action이 객체 형태여서 스토리북에 나타낼 땐 이에 대한 기능을 구체적으로 확인하지 못했는데, 이 객체 부분을
actionName,onActionClick의 두 개의 prop으로 나타내어 평면화함으로써 각각의 prop이 어떤 역할을 하는지 명시하고, 제어 가능하도록 수정했습니다.또한 Toast 구현부에서
aria-live속성을 통해 동적인 변경 부분(Toast 문구)을 감지해서 알려줄 수 있도록 했으며,aria-atomic: true을 통해 변경된 부분 전체를 읽도록 속성을 추가했습니다시급한 정도
🏃♂️ 보통 : 최대한 빠르게 리뷰 부탁드립니다.