-
Notifications
You must be signed in to change notification settings - Fork 4
feature(icons): icon story 파일 하드코딩 방식에서 map활용 랜더링 방식으로 변경 #312
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: main
Are you sure you want to change the base?
Conversation
|
|
constantly-dev
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.
수고하셨습니다! 몇 개 리뷰만 확인해주세요!
| } from '@sopt-makers/icons'; | ||
|
|
||
| const convertIconNameToKebabCase = (iconName: string): string => { | ||
| const withoutIcon = iconName.replace(/^Icon/, ''); |
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.
사소하지만 withoutIconPrefix로 네이밍하는 것은 어떨까요?
| {Object.values(category.icons).map((icon, index) => ( | ||
| <IconGenerator icon={icon} style={style} key={`${category.name}-${index}`} /> | ||
| ))} |
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.
여기를 Object.values가 아니라 Object.entries를 쓰면 여기서 컴포넌트 이름도 받을 수 있을 것 같아요.
그 컴포넌트 이름을 IconGenerator에 넘겨주면 아래 로직도 안써도 되고
const iconName = convertIconNameToKebabCase(icon.__docgenInfo.displayName);any type도 안써도 될 것 같아요.
| {Object.values(category.icons).map((icon, index) => ( | |
| <IconGenerator icon={icon} style={style} key={`${category.name}-${index}`} /> | |
| ))} | |
| {Object.entries(category.icons).map(([iconName, IconComponent]) => ( | |
| <IconGenerator key={iconName} icon={IconComponent} name={iconName} style={style} /> | |
| ))} |
또한 __docgenInfo 자체도 코드로 직접 작성하는 것보다 storybook이 내부적으로 docs를 위해서 사용하는 값 같아서 이게 실제 배포환경이나 빌드 됐을때 문제가 없을지 조금 걱정이 되기도 합니다.
(이 부분은 저도 지식이 부족해 아는 게 있으시다면 알려주세요!)
| // icon 카테고리 추가시 이곳에 추가 | ||
| const iconCategories = [ | ||
| { name: 'Communication', icons: Communication }, | ||
| { name: 'Editor', icons: Editor }, | ||
| { name: 'Feedback', icons: Feedback }, | ||
| { name: 'Files', icons: Files }, | ||
| { name: 'General', icons: General }, | ||
| { name: 'Interaction', icons: Interaction }, | ||
| { name: 'Media', icons: Media }, | ||
| { name: 'Time', icons: Time }, | ||
| { name: 'Users', icons: Users }, | ||
| { name: 'Logo', icons: Logo }, | ||
| ]; | ||
|
|
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.
위치가 여기있으면 render될 때마다 변수가 다시 선언될 것 같아요. iconCategories는 render 외부로 빼도 될 것 같습니다!
변경사항
이로인해 아이콘을 자동추출해주면 스토리북에 알아서 추출된 아이콘까지 포함됩니다.
링크
시급한 정도
🏃♂️ 보통 : 최대한 빠르게 리뷰 부탁드립니다.
기타 사항