-
Notifications
You must be signed in to change notification settings - Fork 1
fix: token 스크립트 수정 #7
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
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 일관성을 높이고 개발 효율성을 개선하는 것을 목표로 합니다. 기존의 디자인 토큰 정의를 스크립트를 통해 CSS 변수 및 유틸리티 클래스로 자동 변환하고, 이를 애플리케이션에 적용하여 디자인 시스템의 기반을 마련합니다. 또한, 새로운 토큰의 적용을 시각적으로 검증할 수 있는 데모 페이지를 제공합니다. 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
|
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
|||||||||||||||||||||||||
PR Code Suggestions ✨Explore these optional code suggestions:
|
|||||||||||||||||
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.
Code Review
디자인 토큰을 자동 생성하는 스크립트를 도입하여 CSS 변수와 유틸리티 클래스를 만드는 변경 사항 잘 보았습니다. 이를 통해 디자인 시스템의 일관성을 높이고 유지보수성을 개선하려는 방향은 매우 긍정적입니다. 다만, 새로 추가된 generate-tokens.js 스크립트의 토큰 참조 해석 로직이 특정 경로에 의존하고 있어 확장성이 떨어질 우려가 있습니다. 또한, 데모 페이지에서 아직 정의되지 않은 클래스를 사용하거나 하드코딩된 값을 사용하는 부분이 있어 수정이 필요해 보입니다. 아래에 몇 가지 구체적인 피드백을 남겼으니 확인 부탁드립니다.
| <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> |
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.
버튼에 사용된 btn-primary, btn-slate, btn-disabled 클래스가 tokens.css에 정의되어 있지 않아 버튼 스타일이 올바르게 적용되지 않습니다. generate-tokens.js 스크립트는 bg-button-* 형태의 배경 유틸리티 클래스를 생성하므로 이를 사용해야 합니다.
또한, 버튼들의 border-radius가 일관성 있게 적용되지 않았습니다. rounded-2xl을 공통으로 적용하고, text-white, text-disabled 같은 텍스트 색상 토큰을 사용하여 스타일을 완성하는 것을 제안합니다.
| <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> | |
| <button className="bg-button-primary text-white text-16-600 rounded-2xl px-8 py-3 transition-opacity hover:opacity-90"> | |
| Primary Button | |
| </button> | |
| <button className="bg-button-slate text-brand-black border-light text-16-600 rounded-2xl border px-8 py-3"> | |
| Secondary Button | |
| </button> | |
| <button className="bg-button-disabled text-disabled text-16-400 rounded-2xl cursor-not-allowed px-8 py-3"> | |
| Disabled Button | |
| </button> |
| function resolveValue(value, depth = 0) { | ||
| if (depth > 10 || typeof value !== "string") return value; | ||
|
|
||
| const match = value.match(/^\{(.+)\}$/); | ||
| if (!match) return value; | ||
|
|
||
| const ref = match[1]; | ||
|
|
||
| // Grayscale 참조 | ||
| if (ref.startsWith("Grayscale.")) { | ||
| const key = ref.replace("Grayscale.", ""); | ||
| return `var(--${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})`; | ||
| } | ||
|
|
||
| // Transparent 참조 | ||
| if (ref.startsWith("Transparent.")) { | ||
| const key = ref.replace("Transparent.", ""); | ||
| return `var(--${key})`; | ||
| } | ||
|
|
||
| // System/Mode 1의 Colors 참조 | ||
| if (ref.startsWith("Colors.")) { | ||
| const key = ref.replace("Colors.", ""); | ||
| return `var(--${key})`; | ||
| } | ||
|
|
||
| return value; | ||
| } |
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.
resolveValue 함수가 Grayscale., Colors.Pink. 등 특정 문자열 prefix에 의존하여 토큰 참조를 해석하고 있습니다. 이 방식은 새로운 토큰 그룹이 추가될 때마다 코드를 수정해야 하므로 확장성과 유지보수성이 떨어집니다.
점(.)으로 구분된 경로(a.b.c)를 파싱하여 토큰 객체를 동적으로 탐색하는 범용적인 해석 함수로 리팩터링하는 것을 강력히 권장합니다. 이렇게 하면 토큰 구조가 변경되거나 확장되어도 스크립트를 수정할 필요가 없어집니다.
References
- 중복 코드를 발견하면 재사용 가능한 유틸리티나 컴포넌트로 추출할 것을 제안하세요. 현재
resolveValue함수는 각 토큰 그룹에 대한 처리 로직이 중복되어 있어 유지보수가 어렵습니다. 이를 범용적인 함수로 추출하여 개선할 수 있습니다. (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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 컴포넌트에서 border-gray-200과 bg-white 같은 하드코딩된 스타일 값을 사용하고 있습니다. 디자인 토큰 시스템을 도입한 만큼, 모든 스타일은 토큰을 통해 관리하는 것이 일관성 및 유지보수 측면에서 바람직합니다. border-light와 같은 시맨틱 토큰을 사용하거나, border-gray-200에 해당하는 토큰을 추가하여 사용하시는 것을 권장합니다.
References
- 중복 코드를 발견하면 재사용 가능한 유틸리티나 컴포넌트로 추출할 것을 제안하세요. 이 경우, 하드코딩된 값을 사용하는 대신 토큰 기반의 유틸리티를 사용하여 코드의 일관성과 재사용성을 높여야 합니다. (link)
| {/* 1. Typography */} | ||
| <section className="space-y-4"> | ||
| <h2 className="text-20-600">1. Typography</h2> | ||
| <div className="space-y-2 rounded bg-gray-50 p-4"> |
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.
타이포그래피 섹션의 배경색으로 bg-gray-50이 하드코딩되어 있습니다. 이 또한 color-gray-50 토큰을 활용한 유틸리티 클래스(예: bg-gray-50)로 대체하여 일관성을 유지하는 것이 좋습니다. 현재 스크립트에서는 해당 유틸리티 클래스를 생성하지 않으므로, generate-tokens.js 스크립트에 Grayscale 색상에 대한 배경 유틸리티 클래스 생성 로직을 추가하는 것을 고려해 보세요.
References
- 중복 코드를 발견하면 재사용 가능한 유틸리티나 컴포넌트로 추출할 것을 제안하세요. 이 경우, 하드코딩된 값을 사용하는 대신 토큰 기반의 유틸리티를 사용하여 코드의 일관성과 재사용성을 높여야 합니다. (link)
| if (ref.startsWith("Grayscale.")) { | ||
| const key = ref.replace("Grayscale.", ""); | ||
| return `var(--${key})`; | ||
| } |
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.
resolveValue 함수 내에서 토큰 참조를 var() 함수로 변환할 때 sanitizeKey 함수를 사용하지 않고 있습니다. 현재 token.json의 참조 대상 키들은 이미 CSS 변수명으로 사용하기에 적합한 형태이지만, 만약 토큰 이름에 공백이나 특수문자가 포함될 경우 (예: {'My Color': ...}) 유효하지 않은 CSS 변수 참조(var(--My Color))가 생성될 수 있습니다. 코드의 안정성을 높이기 위해 var() 내부에서도 sanitizeKey를 적용하는 것을 권장합니다.
| if (ref.startsWith("Grayscale.")) { | |
| const key = ref.replace("Grayscale.", ""); | |
| return `var(--${key})`; | |
| } | |
| if (ref.startsWith("Grayscale.")) { | |
| const key = ref.replace("Grayscale.", ""); | |
| return `var(--${sanitizeKey(key)})`; | |
| } |
| tailwindUtilities.push(`${className} { | ||
| font-size: ${size}px; | ||
| font-weight: ${weight}; | ||
| line-height: 1.5; |
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.
| const tokensPath = path.join(__dirname, "../app/token.json"); | ||
| const tokens = JSON.parse(fs.readFileSync(tokensPath, "utf-8")); |
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.
Suggestion: Fix token JSON path
| const tokensPath = path.join(__dirname, "../app/token.json"); | |
| const tokens = JSON.parse(fs.readFileSync(tokensPath, "utf-8")); | |
| const tokensPath = path.join(__dirname, "../app/src/token.json"); | |
| const tokens = JSON.parse(fs.readFileSync(tokensPath, "utf-8")); |
요약
🤖 Generated by PR Agent at 354fd18
token.json에서 색상, 타이포그래피, 반경 등을 CSS 변수로 변환tokens.css를 전역 스타일에 통합하여 일관된 디자인 시스템 구축구현 사항
generate-tokens.js
토큰 JSON을 CSS 변수로 변환하는 생성 스크립트scripts/generate-tokens.js
token.json을 파싱하여 색상, 반경, 보더, 불투명도 등의 토큰을 추출{Grayscale.color-gray-0}형태)를 CSS 변수로 해석 및 변환text-{size}-{weight}형태의 타이포그래피 유틸리티 클래스 자동 생성app/tokens.css에 저장하는 Node.js 스크립트tokens.css
자동 생성된 CSS 변수 및 유틸리티 클래스app/tokens.css
--radius-xs~--radius-full), 보더, 불투명도 변수 설정text-{size}-{weight}형태의 타이포그래피 유틸리티 클래스 (10px ~ 24px)globals.css
토큰 CSS 임포트 추가 및 포맷 정리app/globals.css
@import "tailwindcss"따옴표 스타일 통일 (싱글 → 더블)@import "./tokens.css"추가하여 토큰 스타일 로드page.tsx
디자인 토큰 테스트 데모 페이지로 변경app/page.tsx
text-24-700,bg-brand-secondary-pink,bg-button-primary등 토큰 유틸리티 클래스사용
settings.json
CSS 커스텀 at-rule 린트 경고 비활성화.vscode/settings.json
"css.lint.unknownAtRules": "ignore"설정 추가@layer,@custom-variant등 커스텀 at-rule 경고 무시하도록 설정token.json
기존 토큰 파일 제거app/src/token.json
Need Review
Reference
📜 리뷰 규칙
Reviewer는 아래 P5 Rule을 참고하여 리뷰를 진행합니다.
P5 Rule을 통해 Reviewer는 Reviewee에게 리뷰의 의도를 보다 정확히 전달할 수 있습니다.