-
Notifications
You must be signed in to change notification settings - Fork 1
fix: token 스크립트 수정 #9
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은 디자인 토큰 시스템을 도입하여 프로젝트의 스타일 관리를 개선합니다. 기존 app/src/token.json 정의 파일을 제거하고, 새로운 스크립트를 통해 token.json 파일을 기반으로 CSS 변수와 Tailwind CSS와 유사한 유틸리티 클래스를 자동으로 생성하도록 변경했습니다. 이를 통해 디자인 시스템의 일관성을 유지하고 개발자가 미리 정의된 토큰을 사용하여 UI를 쉽게 구축할 수 있도록 합니다. 또한, app/page.tsx를 업데이트하여 새로운 토큰 기반 유틸리티 클래스의 실제 적용 사례를 보여줍니다. 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
|
|
/describe --pr_description.generate_title=true |
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
이번 PR은 token.json으로부터 CSS 변수와 유틸리티 클래스를 자동 생성하는 스크립트를 도입하여 디자인 시스템의 기반을 마련하는 중요한 변경입니다. 전반적으로 훌륭한 시도이며, 토큰 시각화를 위한 테스트 페이지를 추가한 점도 좋습니다.
제 리뷰는 주로 스크립트의 유지보수성과 안정성을 높이는 방향에 초점을 맞추었습니다. generate-tokens.js 파일에서 코드 중복을 줄이고, 잠재적인 버그를 수정하며, 확장성을 개선할 수 있는 몇 가지 리팩토링을 제안했습니다. 또한, page.tsx의 테스트 컴포넌트에서는 웹 표준과 접근성을 향상시키기 위한 개선점을 포함했습니다.
리뷰는 레포지토리 스타일 가이드의 웹 표준(규칙 70), 접근성(규칙 59), 코드 중복 방지(규칙 55) 항목을 기반으로 작성되었습니다. 제안된 변경들이 코드 품질을 한 단계 더 높이는 데 도움이 되기를 바랍니다.
| <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.
비활성화된 버튼의 접근성(Accessibility)을 향상시키기 위해 disabled 속성을 추가하는 것이 중요합니다. cursor-not-allowed 클래스는 시각적인 피드백만 제공할 뿐, 스크린 리더 사용자나 키보드 사용자에게는 버튼이 비활성화되었음을 알리지 못하고 여전히 포커스가 가능할 수 있습니다. disabled 속성을 추가하면 버튼이 상호작용 불가능한 상태임을 명확히 하여 접근성을 개선할 수 있습니다. 또한, 이 버튼에도 type="button"을 추가하는 것이 좋습니다.
| <button className="btn-disabled text-16-400 cursor-not-allowed px-8 py-3"> | |
| Disabled Button | |
| </button> | |
| <button type="button" className="btn-disabled text-16-400 cursor-not-allowed px-8 py-3" disabled> | |
| Disabled Button | |
| </button> |
References
- 규칙 59: A11y: ... 키보드 네비게이션을 확인하세요. (link)
| 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 함수는 새로운 토큰 카테고리가 추가될 때마다 수정이 필요한 구조로 되어 있어 확장성이 떨어집니다. 또한, 참조된 키를 var()로 변환할 때 sanitizeKey 함수를 사용하지 않아, token.json에 kebab-case가 아닌 키가 있을 경우 유효하지 않은 CSS 변수(var(--Some Key))를 생성할 수 있는 잠재적인 버그가 있습니다.
아래와 같이 리팩토링하여 코드 중복을 줄이고, sanitizeKey를 적용하여 안정성을 높이는 것을 제안합니다.
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];
const prefixes = [
"Grayscale.",
"Colors.Pink.",
"Colors.Flame.",
"Colors.Orange.",
"Transparent.",
"Colors.", // Must be last
];
for (const prefix of prefixes) {
if (ref.startsWith(prefix)) {
const key = ref.replace(prefix, "");
return `var(--${sanitizeKey(key)})`;
}
}
return value;
}References
- 규칙 55: 중복 코드를 발견하면 재사용 가능한 유틸리티나 컴포넌트로 추출할 것을 제안하세요. (link)
| <button className="btn-primary text-16-600 px-8 py-3 transition-opacity hover:opacity-90"> | ||
| Primary 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.
안녕하세요. 웹 표준 및 레포지토리 스타일 가이드(규칙 70) 준수를 위해 버튼에 type="button" 속성을 추가하는 것을 권장합니다. type이 명시되지 않은 버튼은 <form> 내에서 의도치 않게 폼을 제출할 수 있습니다.
| <button className="btn-primary text-16-600 px-8 py-3 transition-opacity hover:opacity-90"> | |
| Primary Button | |
| </button> | |
| <button type="button" className="btn-primary text-16-600 px-8 py-3 transition-opacity hover:opacity-90"> | |
| Primary Button | |
| </button> |
References
- 규칙 70: 태그 내에서 ... 제출 버튼은 type="submit"을 명시하는 등 폼 표준을 준수하는지 확인하세요. (link)
| <button className="btn-slate text-brand-black border-light text-16-600 rounded-2xl border px-8 py-3"> | ||
| Secondary 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.
안녕하세요. 웹 표준 및 레포지토리 스타일 가이드(규칙 70) 준수를 위해 이 버튼에도 type="button" 속성을 추가하는 것을 권장합니다.
| <button className="btn-slate text-brand-black border-light text-16-600 rounded-2xl border px-8 py-3"> | |
| Secondary Button | |
| </button> | |
| <button type="button" className="btn-slate text-brand-black border-light text-16-600 rounded-2xl border px-8 py-3"> | |
| Secondary Button | |
| </button> |
References
- 규칙 70: 태그 내에서 ... 제출 버튼은 type="submit"을 명시하는 등 폼 표준을 준수하는지 확인하세요. (link)
| const tokens = JSON.parse(fs.readFileSync(tokensPath, "utf-8")); | ||
|
|
||
| // Semantic tokens에서 실제 색상값 추출 | ||
| const semanticMode1 = tokens["Sementic/Mode 1"] || {}; |
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.
| // Colors (Pink, Flame, Orange) | ||
| if (colors.Pink) { | ||
| Object.entries(colors.Pink).forEach(([key, value]) => { | ||
| if (key.startsWith("color-pink-")) { | ||
| cssVariables.push(` --${sanitizeKey(key)}: ${value.$value};`); | ||
| } | ||
| }); | ||
| } | ||
| if (colors.Flame) { | ||
| Object.entries(colors.Flame).forEach(([key, value]) => { | ||
| if (key.startsWith("color-flame-")) { | ||
| cssVariables.push(` --${sanitizeKey(key)}: ${value.$value};`); | ||
| } | ||
| }); | ||
| } | ||
| if (colors.Orange) { | ||
| Object.entries(colors.Orange).forEach(([key, value]) => { | ||
| if (key.startsWith("color-orange-")) { | ||
| cssVariables.push(` --${sanitizeKey(key)}: ${value.$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.
Pink, Flame, Orange 색상 토큰을 처리하는 로직이 중복되고 있습니다. 스타일 가이드 55번(코드 중복) 항목에 따라, 이 부분을 배열과 반복문을 사용하여 리팩토링하면 코드를 더 간결하고 유지보수하기 쉽게 만들 수 있습니다.
// Colors (Pink, Flame, Orange)
['Pink', 'Flame', 'Orange'].forEach(colorName => {
const colorGroup = colors[colorName];
if (colorGroup) {
Object.entries(colorGroup).forEach(([key, value]) => {
if (key.startsWith(`color-${colorName.toLowerCase()}-`)) {
cssVariables.push(` --${sanitizeKey(key)}: ${value.$value};`);
}
});
}
});References
- 규칙 55: 중복 코드를 발견하면 재사용 가능한 유틸리티나 컴포넌트로 추출할 것을 제안하세요. (link)
token.json에서 색상, 타이포그래피, 반경 등을 자동으로 추출하여tokens.css생성구현 사항
generate-tokens.js
토큰 자동 생성 스크립트 구현scripts/generate-tokens.js
token.json에서 색상, 반경, 보더, 불투명도, 폰트 크기 등을 파싱하는 Node.js 스크립트 추가resolveValue)로 중첩된 토큰 참조 자동 해결text-{size}-{weight}형식의 타이포그래피 유틸리티 클래스 자동 생성app/tokens.css로 저장하고 실행 결과 로깅globals.css
토큰 CSS 임포트 추가app/globals.css
tokens.css임포트 추가하여 생성된 토큰 전역 적용page.tsx
토큰 테스트 페이지로 리팩토링app/page.tsx
text-24-700,bg-brand-secondary-pink등)Image임포트 제거tokens.css
자동 생성된 디자인 토큰 CSS 파일app/tokens.css
token.json에서 자동 생성된 CSS 변수 파일 (260줄)text-{size}-{weight}형식으로 16개 조합 제공settings.json
CSS 린트 설정 추가.vscode/settings.json
"css.lint.unknownAtRules": "ignore"설정@layer등 커스텀 at-rules 경고 무시token.json
레거시 토큰 파일 제거app/src/token.json
app/token.json으로 이동됨을 시사Need Review
Reference
📜 리뷰 규칙
Reviewer는 아래 P5 Rule을 참고하여 리뷰를 진행합니다.
P5 Rule을 통해 Reviewer는 Reviewee에게 리뷰의 의도를 보다 정확히 전달할 수 있습니다.