-
Notifications
You must be signed in to change notification settings - Fork 3
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] Icon 컴포넌트 생성 #29
Conversation
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.
고생하셨습니다! 통째로 svg 전체를 가져오는게 아니라 해당 프로젝트에서 사용하는 유효한 녀석들만 빌드하도록 최적화시키는 작업도 그렇고 매번 날카롭게 최적화하려 노력하는 모습이 큰 귀감이 됩니다.
제안해주신 radix-ui
아이콘 교체의 경우, radix-ui
보다 우리 디자인시스템에 갖춰진 아이콘을 더 우선적으로 적용해야하지 않을까 싶습니다. 제가 피그마 와이어프레임 디자인할 때 거의 디자인시스템 내 아이콘을 가져다 사용했고 만져본 결과 그 자체로 정사각형 viewbox가 갖춰져있어서 말씀하신 치우침 현상도 사라지지 않을까 싶고, 일관성도 챙길 수 있을 것 같습니다. 이 부분은 동의하신다면 제가 관객앱에서 사용된 디자인시스템 아이콘 목록을 리스트업해놓겠습니다.
packages/icons/src/type.d.ts
Outdated
@@ -0,0 +1,11 @@ | |||
declare module '*.svg' { | |||
import * as React from 'react'; |
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.
여기 react 모듈 전체를 import한 의도가 있을까요? 다른 icon 패키지 내 모듈 불러오는 부분들에는 필요한 모듈만 선택적으로 불러왔기 때문에 여기도 일관성있게 type.d.ts
에서 사용하는 모듈만 import하는게 낫지않나 싶습니다!
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.
오 그러네요! 반영하겠습니다ㅎㅎ
추가로 아이콘 관련해서 디자인 시스템에 있는 아이콘이 사용하기에 충분하다면 그렇게 하는 것도 좋습니다!! 👍
packages/ui/src/icon/icon.css.ts
Outdated
@@ -0,0 +1,17 @@ | |||
import { theme } from '@hcc/styles/dist/theme.css'; |
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.
theme
를 @hcc/styles
로 불러와 사용하는 것도 좋은 방법일 거 같습니다.
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.
헉 그러네요 나중에 수정해야지 하고 있던 건데 까먹고 같이 올렸네요ㅠㅠ 수정하겠습니다!
🌍 이슈 번호
✅ 작업 내용
svg 파일용 패키지 추가
manager
와spectator
라는 두 개의 페이지 워크스페이스가 존재합니다. 두 워크스페이스는 모두 아이콘 컴포넌트를 사용하고 있습니다.spectator
에서는 총 15개의 svg 파일 중 5개 정도의 아이콘을 사용하고 있습니다.rollup
을 이용하여 구현하였습니다.webpack
을 이용해서 트리쉐이킹 할 수 있지만, 설정이 비교적 간단하다고 판단하여rollup
을 채택했습니다.svg를 실제 렌더링 할 수 있는 Icon 컴포넌트 추가
사용
📝 참고 자료
번들 결과 트리쉐이킹에 성공했음
실제 spectator 워크스페이스를 빌드한 후, 생성된 번들의 용량을 분석한 것입니다. 약 15개 가량의 svg 파일 중, spectator에서 사용하고 있는 것은 HccLogo, BackgroundLogoIcon, ProfileIcon, HamburgerIcon, CrossIcon까지 총 5개입니다. 그래서 이미지에서 보시는 것처럼 언급한 5개의 svg만 번들에 포함된 것을 확인할 수 있습니다.
♾️ 기타
packages/icons/src/svg
경로에 svg 파일을 추가해주기만 하면 됩니다.<svg />
태그의 속성으로fill="currentColor"
를 추가해주세요. color style을 적용하기 위함입니다.현재 아이콘을 사용하기 위해서는 위의 예시처럼 import 구문이 최소 두 줄이 필요합니다. 만약 이것이 불필요한 분리라고 생각이 들 수 있다고 생각하며, 이 부분에 대해 의견이 있다면 언제든 말씀해주세요! 저도 막상 분리를 했지만 import를 하고 보니 "굳이인가?" 라는 생각을 실제로 하기도 했네요.
어떤 정적 svg 파일을 사용할 지에 대한 논의도 필요해보입니다. 일부 svg 파일의 경우 viewbox가 한쪽으로 치우쳐져 있어 요소의 정가운데에 위치하지 않고 있습니다. 그래서 특정 ui 라이브러리의 icons를 그대로 가져와서 사용하는 게 아이콘 일관성에도 도움이 될 것 같다고 생각이 드네요. 지금 일부 아이콘의 경우
radix-ui
의 것들을 사용하고 있다고 알고 있는데, 아래의 svg 파일 역시radix-ui
의 svg로 교체하는 것은 어떨까요?