-
Notifications
You must be signed in to change notification settings - Fork 1
[Init] vanilla extract 초기 세팅 & 디자인 시스템 적용 #27
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
Conversation
- global, reset css 파일 추가 및 적용 - theme 적용 - 예시 코드 추가
🚀 빌드 결과✅ 린트 검사 완료 |
odukong
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.
수고 많으셨습니다! 디자인시스템에 필요한 부분에 대해 거의 잘 적용해주신 것 같아요🙌🏻🩷
특히 FSD 구조에 맞춰 app과 shared로 역할을 명확히 분리한 점과, textRendering: "optimizeLegibility"로 텍스트 업로드 시 최적화까지 신경 쓰신 디테일에 감동....
제안해주신 z-index와 borderRadius 토큰화도 추후 고도화 단계에서 꼭 적용하면 좋을 것 같아요!
수정하거나 추가되면 좋을 부분들에 대한 코멘트 달아두었으니 확인 부탁드릴게요!>_<
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.
input 태그도 추가적인 커스텀을 진행하기 위해 적용되어 있는 기본 스타일은 reset.css에서 제거해주는게 좋을 것 같아요!
globalStyle("input", {
appearance: "none",
background: "transparent",
border: "none",
outline: "none",
});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.
html, body에 폰트(pretendard)가 지정되어도 form과 관련된 태그들은 font-family가 시스템 기본 폰트가 적용되는 문제가 있다고 해요. (참조링크) 그렇기 때문에 input, button, textarea, select 같은 form control 요소는 부모의 font 속성을 상속받을 수 있게 정의해주는 것이 안전할 것 같습니다!
globalStyle("input, button, textarea, select", {
font-family: "inherit",
});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.
커헉..또 하나 배워갑니다 👼
| title_b_24: { | ||
| fontSize: typography.fontSize[24], | ||
| fontWeight: typography.fontWeight.bold, | ||
| lineHeight: typography.lineHeight.normal, | ||
| letterSpacing: typography.letterSpacing.normal, | ||
| }, | ||
|
|
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.
title_m_24 폰트스타일이 하나 빠져있는 것 같아요! 추가 부탁드리겠습니다 ♪(´▽`)
| const tokens = { | ||
| color: color, | ||
| fontStyles: fontStyles, | ||
| }; | ||
|
|
||
| const properties = defineProperties({ | ||
| properties: tokens, | ||
| }); | ||
| const sprinkles = createSprinkles(properties); | ||
|
|
||
| const [themeClass, themeVars] = createTheme(tokens); |
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.
vanilla-extract에서 sprinkles는 color, display처럼 하나의 CSS 속성에 값(문자열 키)을 자동으로 매핑할 수 있게 하는 용도로 설계되었어요. 반면, 우리가 사용하려는 fontStyles는 size+weight+height 등 여러 속성이 묶여 있는 조합이기 때문에, sprinkles 토큰으로 넣어도 활용할 수 없는 속성일 것 같아요.
export const appContainer = sprinkles({
color: 'blue100', // 이렇게 1:1로 대응!
...themeVars.fontStyles.title_b_28,
});따라서 typography는 복합 스타일을 다루는 recipe에서 spread연산자로 정의해 사용하도록 하고, sprinkles는 순수하게 단일 속성(저희가 가지는 토큰에서는 color가 되겠죠?)만 담당하도록 하는 것이 좀 더 목적에 부합하지 않을까 생각합니다!
| const tokens = { | |
| color: color, | |
| fontStyles: fontStyles, | |
| }; | |
| const properties = defineProperties({ | |
| properties: tokens, | |
| }); | |
| const sprinkles = createSprinkles(properties); | |
| const [themeClass, themeVars] = createTheme(tokens); | |
| const tokens = { | |
| color: color, | |
| fontStyles: fontStyles, | |
| }; | |
| const [themeClass, themeVars] = createTheme(tokens); | |
| const properties = defineProperties({ | |
| properties: { | |
| color: themeVars.color, | |
| backgroundColor: themeVars.color, | |
| }, | |
| }); | |
| const sprinkles = createSprinkles(properties); | |
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.
여러 코드를 참고해가면서 작성한 코드라 sprinkles를 적용만 하고 실제로 사용하지 않고 있었네요 반성합니다 .. 💦💦 제가 지정한 fontStyles는 여러 개의 항목을 제어하고 있어서 1:1 매칭이 되지 않아 color: blue300 처럼 sprinkles 방식으로 적용할 수 없다는 점, 그리하여 color만 Sprinkles의 프로퍼티로 제어하고 fontStyle은 기존 방식대로 spread 연산자로 적용해야 한다는 점까지 모두 이해했습니다!
좋은 지적 너무 감사드리고 적용해서 수정하도록 하겠습니다 감사합니다 :)
(p.s. properties에서 backgroundColor까지 챙긴 거 완전 굿 ~ )
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.
음 그런데 color만 sprinkles로 제어하는 현재 방식이 오히려 복잡도를 증가시킬 수 있을 거 같다는 생각도 드네요 .. !! color: themeVars.color.blue300으로 끝낼 수 있는 코드를
export const buttonStyle = style([
sprinkles({
backgroundColor: "blue300",
}),
{
// .. 나머지 코드
}
]);이렇게 사용해야 하니까요! 그래서 일단은 sprinkles은 도입하지 않고 기존에 사용하던 대로 themeVars로 적용하되, 추후 디자인 시스템이 더 확장되어 정말로 도입이 필요하다 판단되면 그 때 확장하는 방향으로 해도 괜찮을 거 같은데 어떻게 생각하시나요? 🤔🤔
(cc. @u-zzn @qowjdals23 )
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.
사실 저도 현재 프로젝트 상에서는 themeVars만 우선 도입하는 것도 좋은 방향이라고 생각해요.
sprinkles를 사용해야 좋은 패턴인 경우도 분명히 존재하겠지만, 이전 프로젝트에서도 themeVars만으로 충분히 구현이 가능했고, sprinkles가 필요할 시점에 도입하는 것이 더 나은 선택일 수도 있을 것 같아요.
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.
찾아보니까 sprinkles가 defineProperties로 유틸(atomic) 클래스 세트를 만들어서 여기저기서 반복적으로 조합해 쓰는 구조라, 한두 개만 얹으면 세팅/학습 비용 대비 체감 이득이 크지 않을 수 있겠더라구요 🥹
도입할 거면 차라리 spacing/layout 쪽(padding/margin/gap/display/flex 정렬 등)처럼 실제로 자주 반복되는 단일 속성들을 같이 사용하는 게 이득이 훨씬 큰 방향인 것 같습니다!!
그래서 현재 그 정도로 넓게 쓸 계획이 아직 없다면, 지금은 themeVars만 도입해서 단순하게 가져가는 것도 충분히 좋은 것 같습니다 !
qowjdals23
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.
피그마 디자인 시스템이랑 컬러/타이포 토큰 값들 대조해서 꼼꼼히 봤는데, 제가 확인한 범위에서는 거의 완벽하게 들어가 있는 것 같습니다!!!!
app/styles 쪽에 global/reset/theme 역할 나눠둔 거, shared/styles에 토큰/폰트 스타일 모아둔 것도 깔끔해서, 이후에 컴포넌트 스타일 붙일 때 기준 잡기 훨씬 편할 것 같네요 !
fontSize 62.5%로 rem 기준 잡아둔 것도 넘 좋습니다 !
말씀하신대로 나중에 Modal 같은 레이어 들어가면 z-index 토큰화 / borderRadius 토큰화도 같이 가면 관리하기 더 수월해질 것 같습니다 ! 초기 세팅 수고 많으셨습니다 ❤️🔥
|
세팅 하시느라 수고 많으셨습니다아 reset / global 분리나 토큰 구조도 초기 세팅으로 깔끔하게 잡혀 있어서 이후 컴포넌트 단위로 확장해나가기에도 좋을 것 같아요 🙂 그리고 PR에 적어주신 것처럼 추후 z-index, border-Radius 값 등을 토큰으로 관리하는 것도 서비스 구현해나가면서 구체적으로 이야기 나눠보면 좋을 것 같습니다!! |
- 빠진 폰트 스타일 추가 - reset.css.ts. 파일 보강
src/app/providers/theme-provider.tsx
Outdated
| @@ -0,0 +1,20 @@ | |||
| import { type ReactNode } from "react"; | |||
|
|
|||
| import "../styles/reset.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.
현재 reset 파일이 reset.css.ts인데 여기서는 reset.css로 import 되고 있어서 경로나 확장자 기준이 살짝 헷갈릴 수 있을 것 같아요.
vanilla-extract는 보통 *.css.ts 파일을 엔트리로 가져와서 빌드 시 실제 CSS를 생성하는 흐름으로 알고 있어서, 이 경우에도 ../styles/reset.css.ts로 맞춰는 편이 동작과 의도를 더 명확하게 표현할 수 있지 않을까 생각합니다 🙂
특히 전역 스타일 성격의 파일인 만큼, import 기준을 한 번 정리해두면 이후에 혼란을 줄일 수 있을 것 같아요!
src/app/styles/global.css.ts
Outdated
| @@ -0,0 +1,46 @@ | |||
| import "./reset.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.
지금 구조를 보면 main.tsx에서 global 스타일을 import하고 있고, ThemeProvider / global.css.ts 쪽에서도 reset을 가져오는 형태라 전역 스타일의 엔트리 포인트가 혹시 분산될 수도 있지 않을까 걱정이 됩니다 🥲
동작 자체에는 당장 문제가 없을 것 같긴 한데, 혹시 모를 상황을 대비해 저희끼리 팀 컨벤션을 “전역 스타일은 main.tsx 한 곳에서만 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.
제가 두 군데에서 import 해버렸네요 💦 제미나이와 딥토크 해보니 reset스타일은 애플리케이션 전체에 일괄되게 적용되어야 하는 전역 레이어의 작업이기 때문에 provider보다는 global.css.ts에서 사용하는 것이 더 적절하다고 합니다! 해당 방향으로 수정하겠습니다 감사합니다 😊
src/app/styles/reset.css.ts
Outdated
| globalStyle("*", { | ||
| boxSizing: "border-box", | ||
| margin: "0", | ||
| padding: "0", | ||
| }); |
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.
box-sizing을 전역으로 border-box로 설정해주신 점 좋습니다 👍
다만 보통 reset 단계에서는 레이아웃 일관성을 위해 *, *::before, *::after까지 함께 포함하는 패턴을 많이 사용하는 편이라고 해서, 가상 요소를 사용하는 컴포넌트가 늘어나면 이 부분도 확장 고려해볼 수 있을 것 같아요.
지금 구조에서도 문제는 없지만, 추후 pseudo-element를 활용한 UI가 많아질 경우를 대비해서 적용 범위를 한 번 더 넓힐지 정도는 팀 차원에서 합의해봐도 좋을 것 같습니다 :)
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.
가상 요소까지 border-box 속성을 지정해주는 부분은 생각하지 못했는데 적절한 의견이라고 생각합니다! 논의 없이 바로 추가해도 괜찮을 거 같아요 감사합니다 😀
| globalStyle(":disabled", { | ||
| cursor: "default", | ||
| }); |
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.
:disabled 상태에서 cursor를 default로 설정한 것도 적절한 기본값이라 생각합니다!!
reset 단계에서 button outline 등을 따로 제거하지 않아서, 현재 접근성 측면에서도 안전한 상태로 유지되고 있는 점이 좋습니다.
추후 디자인 시스템이 구체화되면 :focus-visible 스타일 정도만 팀 컨벤션으로 합의해서 추가해주면 키보드 포커스 사용자까지 고려한 전역 접근성 세팅이 되지 않을까 생각합니다
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.
너무 좋습니다~
- reset.css import 위치 수정 - import 방식 수정 - border-box 옵션 대상 확대
✏️ Summary
vanilla extract 초기 세팅을 진행했습니다. 또한
global.css.ts파일과reset.css.ts파일을 통해 스타일 초기화 세팅 진행했으며 디자인 토큰 생성했습니다. 적용 방법은 하단 섹션에서 간단히 설명해두겠습니다😙📑 Tasks
vanilla-extract초기 세팅(폴더 구조)comfit은 런타임 디버깅, 디자인 토큰 관리 등의 측면에서 강한 이점이 있는 zero-runtime css 라이브러리 vanilla-extract을 선택했어요. 최종 폴더구조는 아래와 같아요.
fsd 구조에서 각각의 의미에 맞게, 초기화를 진행하는 reset, global css 파일은 app/styles에, 모든 페이지와 컴포넌트에 사용되는 디자인 토큰과 폰트 스타일은 shared/styles에 위치합니다.
global.css.ts&reset.css.tscss 기본 스타일 초기화, global 스타일 적용을 위해
global.css.ts&reset.css.ts파일을 세팅했습니다 .통용적으로 적용하는 것들을 적용해두었고 데스크탑 뷰이기 때문에 별도의 너비, 높이 설정은 하지 않았습니다global.css.ts주요 내용4. 토큰 적용
font 적용 방식전역 타이포그래피 일관성을 위해
PretendardVariable.woff2폰트를 적용했습니다. Variable font를 사용하여 하나의 폰트 파일로 다양한 font-weight 스타일을 유연하게 표현할 수 있도록 하였고,global.css.ts의 font-family에 적용해 버튼·텍스트 등 공통 UI 전반에서 일관된 렌더링이 이루어지도록 구성했습니다.⬆️ 폰트가 잘 적용된 것을 확인할 수 있습니다.
👀 To Reviewer
📸 Screenshot