fix: token 스크립트 수정 #6
Conversation
Summary of ChangesHello @dasosann, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 PR은 디자인 토큰 시스템을 도입하여 UI 요소의 표준화를 목표로 합니다. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
이 PR은 디자인 토큰을 관리하기 위해 token.json 파일과 이를 CSS 변수 및 유틸리티 클래스로 변환하는 generate-tokens.js 스크립트를 도입했습니다. 이를 통해 일관된 UI를 구축할 수 있는 기반을 마련한 점이 좋습니다. page.tsx에 토큰을 시연하는 테스트 페이지를 추가한 것도 좋은 시도입니다.
다만, 스크립트의 안정성과 유지보수성을 높이기 위한 몇 가지 개선점과, 테스트 페이지에서 토큰이 올바르게 사용되지 않고 있는 문제점들이 발견되었습니다. 특히, 버튼 스타일링이 적용되지 않는 심각한 버그가 있으니 수정이 필요합니다. 자세한 내용은 각 파일에 남긴 코멘트를 확인해주세요.
| <div className="flex flex-wrap gap-4"> | ||
| <button className="btn-primary text-16-600 px-8 py-3 transition-opacity hover:opacity-90"> | ||
| Primary Button | ||
| </button> | ||
| <button className="btn-slate text-brand-black border-light text-16-600 rounded-2xl border px-8 py-3"> | ||
| Secondary Button | ||
| </button> | ||
| <button className="btn-disabled text-16-400 cursor-not-allowed px-8 py-3"> | ||
| Disabled Button | ||
| </button> | ||
| </div> |
There was a problem hiding this comment.
버튼 스타일링 문제 (버그 및 불일치)
버튼 컴포넌트에서 두 가지 주요 문제가 발견되었습니다.
-
존재하지 않는 클래스 사용 (버그):
btn-primary,btn-slate,btn-disabled클래스를 사용하고 있지만, 이 클래스들은tokens.css에 정의되어 있지 않습니다.generate-tokens.js스크립트는bg-button-primary와 같은 배경색 유틸리티만 생성합니다. 이로 인해 버튼에 의도한 스타일이 전혀 적용되지 않는 심각한 버그가 발생합니다. -
스타일 불일치: 버튼들의
border-radius스타일이 일관되지 않습니다. Secondary 버튼에만rounded-2xl이 적용되어 있고, 다른 버튼에는rounded-*클래스가 없습니다.token.json에 정의된button-border-radius토큰(radius-md, 16px)을 모든 버튼에 일관되게 적용해야 합니다.
제안:
tokens.css에 정의된 유틸리티 클래스들을 조합하여 모든 버튼에 일관된 스타일을 적용해주세요.
또한, 클래스 이름이 길어지고 있으므로, 코드 스타일 가이드라인(51번)에 따라 class-variance-authority (cva)를 사용하여 버튼 스타일을 컴포넌트 변형(variant)으로 관리하는 것을 적극적으로 고려해보세요. 이는 코드의 가독성과 유지보수성을 크게 향상시킬 것입니다.
// cva를 사용하지 않는 경우의 수정 예시
<button className="bg-button-primary text-white rounded-2xl text-16-600 px-8 py-3 transition-opacity hover:opacity-90">
Primary Button
</button>References
- Tailwind 클래스가 너무 길거나 복잡하면
class-variance-authority (cva)또는 논리적 그룹핑을 제안하세요. (link)
| <div className="bg-surface-base flex min-h-screen items-center justify-center font-sans"> | ||
| <main className="flex min-h-screen w-full max-w-3xl flex-col items-center gap-8 px-8 py-12"> | ||
| {/* 토큰 테스트 섹션 */} | ||
| <div className="w-full space-y-8 rounded-lg border border-gray-200 bg-white p-8 shadow-sm"> |
There was a problem hiding this comment.
토큰화되지 않은 클래스 사용
디자인 토큰 시스템을 도입했음에도 불구하고, border-gray-200, bg-white와 같이 토큰 시스템에 정의되지 않은 Tailwind 기본 클래스를 직접 사용하고 있습니다. 이는 디자인 시스템의 일관성을 해치고, 향후 테마(예: 다크 모드) 적용을 어렵게 만듭니다.
제안:
tokens.css에 정의된 유틸리티 클래스를 사용하도록 수정해주세요.
border-gray-200대신border-light사용을 고려해보세요.bg-white대신bg-surface-base또는 카드 배경을 위한 새로운 토큰 기반 클래스 사용을 고려해보세요. (token.json의theme.card.background에 해당하는 유틸리티가 필요해 보입니다.)
bg-gray-50(14번 줄) 등 다른 곳에서도 동일한 문제가 발견되니 함께 검토해주세요.
| // Grayscale 참조 | ||
| if (ref.startsWith("Grayscale.")) { | ||
| const key = ref.replace("Grayscale.", ""); | ||
| return `var(--${key})`; | ||
| } |
There was a problem hiding this comment.
resolveValue 함수의 안정성 문제
resolveValue 함수에서 토큰 참조를 해석할 때 sanitizeKey 함수를 사용하지 않아 잠재적인 버그가 발생할 수 있습니다.
현재 token.json의 참조 키(color-gray-64 등)는 이미 정제되어 있어 문제가 없지만, 만약 "My Color"와 같이 공백이나 대문자가 포함된 키가 참조될 경우 var(--My Color)와 같이 유효하지 않은 CSS가 생성됩니다.
제안:
resolveValue 함수 내부에서 키를 추출한 후 sanitizeKey를 적용하여 코드의 안정성을 높이는 것을 권장합니다. 이 변경을 Colors, Transparent 참조 해석에도 동일하게 적용해주세요.
| // Grayscale 참조 | |
| if (ref.startsWith("Grayscale.")) { | |
| const key = ref.replace("Grayscale.", ""); | |
| return `var(--${key})`; | |
| } | |
| // Grayscale 참조 | |
| if (ref.startsWith("Grayscale.")) { | |
| const key = ref.replace("Grayscale.", ""); | |
| return `var(--${sanitizeKey(key)})`; | |
| } |
| // Colors 참조 | ||
| if (ref.startsWith("Colors.Pink.")) { | ||
| const key = ref.replace("Colors.Pink.", ""); | ||
| return `var(--${key})`; | ||
| } | ||
| if (ref.startsWith("Colors.Flame.")) { | ||
| const key = ref.replace("Colors.Flame.", ""); | ||
| return `var(--${key})`; | ||
| } | ||
| if (ref.startsWith("Colors.Orange.")) { | ||
| const key = ref.replace("Colors.Orange.", ""); | ||
| return `var(--${key})`; | ||
| } |
There was a problem hiding this comment.
resolveValue 함수 중복 코드
resolveValue 함수 내에 Colors.Pink, Colors.Flame, Colors.Orange 등 각 색상 그룹에 대한 참조를 처리하는 로직이 중복되어 있습니다.
if (ref.startsWith("Colors.Pink.")) {
const key = ref.replace("Colors.Pink.", "");
return `var(--${sanitizeKey(key)})`;
}
if (ref.startsWith("Colors.Flame.")) {
// ...
}이러한 구조는 새로운 색상 그룹이 추가될 때마다 코드를 수정해야 하므로 유지보수성을 저해합니다. 정규식이나 split을 사용하여 이 로직을 일반화하면 코드를 더 간결하고 확장성 있게 만들 수 있습니다.
|
/describe |
|
PR Description updated to latest commit (354fd18)
|
User description
요약
구현 사항
📸 스크린샷
Need Review
Reference
📜 리뷰 규칙
Reviewer는 아래 P5 Rule을 참고하여 리뷰를 진행합니다.
P5 Rule을 통해 Reviewer는 Reviewee에게 리뷰의 의도를 보다 정확히 전달할 수 있습니다.
PR Type
Enhancement
Description
Implement automated token generation script from token.json
Create comprehensive CSS tokens file with design system variables
Update home page with design token showcase and examples
Configure VSCode to ignore unknown CSS at-rules
Diagram Walkthrough
File Walkthrough
generate-tokens.js
Token generation script implementationscripts/generate-tokens.js
token.jsonand generates CSS variablescustom properties
text-{size}-{weight}namingconvention
tokens.css
Generated design tokens CSS fileapp/tokens.css
opacity
weights
@layerfor proper cascade managementpage.tsx
Redesign home page with token examplesapp/page.tsx
buttons
globals.css
Import generated tokens into globalsapp/globals.css
tokens.cssfilesettings.json
Configure VSCode CSS linting settings.vscode/settings.json
"css.lint.unknownAtRules": "ignore"configurationgeneration
token.json
Remove old token.json from src directoryapp/src/token.json
app/token.jsonlocation