feat(system): Text에 typography스타일 추가 #14
Conversation
qkrdmstlr3
requested review from
woo-jk,
Collection50 and
eunbeann
as code owners
| // NOTE: px정보를 typography token에서 직접 가져올 경우 tailwind에서 인식하지 못하는 문제가 있어 임시적으로 값을 넣어놓습니다. | ||
| // FIXME: 추후 수정해야합니다. | ||
| export const fontSize: Record<Typography, `text-[${number}px]`> = { |
There was a problem hiding this comment.
이런 문제가 있어, typography의 토큰을 복사해서 임시파일을 만들어 유지하였어요. 다른 해결책이 있다면 말씀주세요!
There was a problem hiding this comment.
혹시 NOTE에 대해 조금 더 설명해주실 수 있을까요??
className을 prop으로 넘겨주는 과정에서 오류가 난다는 말씀일까요??
There was a problem hiding this comment.
이게 검색을 좀 해보니 tailwind특성상 중간에 text-${something} 이런식으로 끼워넣으면 인식하지 못하는 것 같아요.
우선 컴포넌트를 사용하는 것이 급할 것 같아서 임시로 대응해두었습니닷..!
|
|
||
| // TODO: 자간 / 행간 높이 설정 | ||
| export const fontSize: Record<Typography, number> = { | ||
| Display1: 56, | ||
| Display2: 40, | ||
| Title1: 36, | ||
| Title2: 28, | ||
| Title3: 24, | ||
| Heading1: 22, | ||
| Heading2: 20, | ||
| Headline1: 18, | ||
| Headline2: 17, | ||
| Body1: 16, | ||
| Body2: 15, | ||
| Label1: 14, | ||
| Label2: 13, | ||
| Caption1: 12, | ||
| Caption2: 11, | ||
| export const fontSize: Record<Typography, `${number}px`> = { | ||
| title1: '36px', | ||
| title2: '28px', | ||
| title3: '24px', |
There was a problem hiding this comment.
letter-spacing은 em단위를 가지고 있어 px, em단위를 명시해줍니다
There was a problem hiding this comment.
이 부분은 tailwind.config에서 작성된 extend로 해결되지 않는 부분인걸까요-??
There was a problem hiding this comment.
토큰을 어딘가에 정의해둘 필요는 있다고 생각해서 파일로 만들어두었어요.
이것도 추후 tailwind.config에 통합될 예정입니다! 시간이 없어서.. 우선 asap으로 처리합니다..
There was a problem hiding this comment.
tailwind.config 얘기가 나와서 그런데,
이미 컴포넌트 만들어주신 시점에 너무 늦은 제안이긴 하지만,, 사실 tailwind 설정만 잘 해주면 Text 컴포넌트가 아예 필요 없을수도 있지 않을까 생각하는데,, 이 부분은 어떻게 생각하시나요!?
There was a problem hiding this comment.
엇 그렇네요? ㅋㅋㅋㅋㅋ
색상을 강제하고 있지도 않아서, 불필요한 컴포넌트가 될 수 있겠군요. 동의합니다!!
|
|
||
| // TODO: 자간 / 행간 높이 설정 | ||
| export const fontSize: Record<Typography, number> = { | ||
| Display1: 56, | ||
| Display2: 40, | ||
| Title1: 36, | ||
| Title2: 28, | ||
| Title3: 24, | ||
| Heading1: 22, | ||
| Heading2: 20, | ||
| Headline1: 18, | ||
| Headline2: 17, | ||
| Body1: 16, | ||
| Body2: 15, | ||
| Label1: 14, | ||
| Label2: 13, | ||
| Caption1: 12, | ||
| Caption2: 11, | ||
| export const fontSize: Record<Typography, `${number}px`> = { | ||
| title1: '36px', | ||
| title2: '28px', | ||
| title3: '24px', |
There was a problem hiding this comment.
tailwind.config 얘기가 나와서 그런데,
이미 컴포넌트 만들어주신 시점에 너무 늦은 제안이긴 하지만,, 사실 tailwind 설정만 잘 해주면 Text 컴포넌트가 아예 필요 없을수도 있지 않을까 생각하는데,, 이 부분은 어떻게 생각하시나요!?
|
|
||
| // TODO: Polymorphic하게 변경 | ||
| export type TextProps = ComponentProps<'p'> & { | ||
| as?: ElementType; | ||
| typography: Typography; | ||
| fontWeight: FontWeight; | ||
| color?: string; |
There was a problem hiding this comment.
color의 타입이 단순 string인 것 보다 피그마에서 제공되는 컬러 토큰을 자동완성 할 수 있다면 많이 간편해질 것 같은데, 어떻게 생각하시나요!
|
tailwind에게 스타일 컴포넌트 속성을 위임합니다. |
1. 무슨 이유로 코드를 변경했나요?
3. 관련 스크린샷을 첨부해주세요.
4. 완료 사항
5. 추가 사항 / 코드 리뷰 받고 싶은 부분