Skip to content
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 #5

Merged
merged 5 commits into from
Apr 23, 2022
Merged

Feat/icon #5

merged 5 commits into from
Apr 23, 2022

Conversation

ghtea
Copy link
Collaborator

@ghtea ghtea commented Apr 23, 2022

Summary

add icon

Key Changes

  • add icon
  • add icon story
  • add svg files from Figma

Notes

  • usage
    • <Icon name={"close"} color={theme.colors.primary} size={48}/>
      • in most cases, you dont need to pass size because there is default size value (24)

Screenshots

Screen Shot 2022-04-23 at 11 51 41 AM

Checklist

  • lint-fix
  • ts-check

@ghtea ghtea requested review from hotbreakb and Hong-been April 23, 2022 02:56
@ghtea ghtea closed this Apr 23, 2022
@ghtea
Copy link
Collaborator Author

ghtea commented Apr 23, 2022

다른 방법도 한번 다시 시도해보고 reopen 할게요

@ghtea ghtea reopened this Apr 23, 2022
@Hong-been
Copy link
Collaborator

한 컴포넌트에서 모두 import하는게 걸리신다면.. 음..

저 다른프로젝트에서 이미지파일을 utils/icon.ts 같은 파일에서 모두 import 하고, 컴포넌트에서 이 유틸에 있는 객체를 Import해서 사용했었어요.

// 예시:  utils/icon.ts
import ArrowBack from "../public/icons/arrow-back.svg";
import ArrowRight from "../public/icons/arrow-right.svg";
...

export const ICONS = {
  ArrowBack,
  ArrowRight,
  ...
};
// conponents/icon/icon.tsx
import {ICONS} from "../../utils/icon";

const nameIconMap = {
  "arrow-back": ICONS.ArrowBack, 
  ...
}

이런식으로요... 장점은 컴포넌트내에서 줄줄이 import하는걸 깔끔히 할수있다?

@ghtea
Copy link
Collaborator Author

ghtea commented Apr 23, 2022

사실 저도 이부분 명확히 이해가 안되긴 했는데 (웹팩, ssr 등을 자세히 공부해야 할듯)
필요한 아이콘만 import 하는게 ssr 번들링상 나은 것 같기도 해서요.

보기좋게 하기 위해서는 아이콘이 많아지면 홍빈님 방식이든 다른 방식으로 나누는 것도 좋아보입니다~

Copy link
Collaborator

@Hong-been Hong-been left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏻

@ghtea
Copy link
Collaborator Author

ghtea commented Apr 23, 2022

@hotbreakb 이후 작업에 필요해서 먼저 머지할게요. 나중에라도 코멘트 달아주셔도 돼요~

@ghtea ghtea merged commit e90f4df into develop Apr 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants