Feat/global component #15
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! This pull request focuses on enhancing the application's visual foundation by introducing a global blur effect component and significantly improving the underlying design token system. The changes aim to provide a more dynamic and visually appealing user interface while establishing a more maintainable and extensible design token architecture. 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
이번 Pull Request는 전역 Blur 컴포넌트 추가 및 디자인 토큰 생성 스크립트 개선을 포함하고 있습니다. Blur 컴포넌트가 app/layout.tsx와 app/page.tsx에 성공적으로 통합되었으며, scripts/generate-tokens.js 파일의 리팩토링을 통해 토큰 관리의 유연성과 정확성이 향상되었습니다. 하지만, 생성된 CSS 파일에 유효하지 않은 CSS 변수 참조가 포함되어 있어 수정이 필요합니다. 이는 디자인 토큰 변환 로직의 일관성 부족 및 생성된 에셋이 예상과 일치하지 않는 문제와 관련이 있습니다. 또한, 일부 CSS 클래스명에 오타가 발견되었고, Tailwind CSS의 z-index 유틸리티 사용법에 개선의 여지가 있습니다.
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
이번 Pull Request는 토큰 생성 스크립트를 완전히 개선하고, CSS 변수 및 Tailwind 유틸리티를 자동화하여 디자인 시스템의 기반을 크게 강화했습니다. Blur 배경 컴포넌트 추가 및 레이아웃 통합을 통해 시각적인 개선도 이루어졌습니다. 전반적으로 코드의 유지보수성과 확장성이 향상되었으며, 자동화된 토큰 관리는 향후 개발에 큰 도움이 될 것입니다. 몇 가지 개선할 점이 발견되었으니, 아래 상세 피드백과 함께 관련 규칙을 참고해 주시면 감사하겠습니다.
| 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 parts = ref.split("."); | ||
| const lastPart = parts[parts.length - 1]; | ||
| const tokenName = sanitizeKey(lastPart); | ||
|
|
||
| // Grayscale 참조 | ||
| if (ref.startsWith("Grayscale.")) { | ||
| const key = ref.replace("Grayscale.", ""); | ||
| return `var(--${key})`; | ||
| // tokenMap에서 찾기 | ||
| for (const [key, token] of tokenMap.entries()) { | ||
| if (token.name === tokenName) { | ||
| return `var(--${token.name})`; | ||
| } | ||
| } |
There was a problem hiding this comment.
resolveValue 함수에서 토큰 참조를 해석할 때 token.name을 기준으로 tokenMap을 순회하고 있습니다. 하지만 token.name은 sanitizeKey를 거친 최종 이름으로, fullPath가 다른 여러 토큰이 동일한 token.name을 가질 수 있습니다 (예: color-gray-0와 color-gray-0-a30가 모두 color-gray-0으로 해석될 수 있음). 이 경우 올바른 토큰을 참조하지 못하거나 예측 불가능한 동작을 할 수 있습니다. tokenMap의 키는 uniqueKey (fullPath)이므로, tokenMap.get(ref)와 같이 fullPath를 사용하여 직접 참조를 해결하는 것이 더 정확하고 효율적입니다. 만약 ref가 fullPath 형태가 아니라면, fullPath를 생성하여 찾는 로직이 필요합니다.
References
- When transforming names or keys (e.g., from a design token to a CSS variable), ensure the transformation logic (e.g., sanitization) is applied consistently at both definition and reference sites. Using a non-unique
token.nameafter sanitization for reference can lead to incorrect token resolution.
| if (!uniqueVars.has(token.name)) { | ||
| uniqueVars.set(token.name, cssValue); |
There was a problem hiding this comment.
uniqueVars Map에 토큰을 저장할 때 token.name을 키로 사용하고 있습니다. resolveValue 함수와 마찬가지로, token.name이 전역적으로 고유하지 않을 경우 나중에 처리되는 토큰이 이전에 처리된 동일한 이름의 토큰 값을 덮어쓸 수 있습니다. 이는 의도치 않은 CSS 변수 값으로 이어질 수 있으므로, token.fullPath와 같이 전역적으로 고유한 식별자를 키로 사용하는 것을 고려해야 합니다.
References
- When transforming names or keys (e.g., from a design token to a CSS variable), ensure the transformation logic (e.g., sanitization) is applied consistently at both definition and reference sites. Using a non-unique
token.nameafter sanitization as a map key can lead to overwriting values and inconsistent CSS variables.
| <main> | ||
| <Blur /> |
There was a problem hiding this comment.
전역 레이아웃(app/layout.tsx)에 Blur 컴포넌트가 이미 추가되어 있습니다. page.tsx에 Blur 컴포넌트를 다시 추가하면 중복 렌더링이 발생하여 의도치 않은 결과를 초래할 수 있습니다. 전역 배경 효과라면 layout.tsx에서만 렌더링하는 것이 올바른 접근입니다.
| <main> | |
| <Blur /> | |
| <main> | |
| {/* <Blur /> */} | |
| </main> |
| const size = t.value; | ||
| if (typeof size === "number") { | ||
| weights.forEach((weight) => { | ||
| const className = `.text-${size}-${weight}`; | ||
| tailwindUtilities.push(`${className} { | ||
| font-size: ${size}px; | ||
| font-weight: ${weight}; | ||
| line-height: 1.5; | ||
| }`); | ||
| fontUtilitiesSet.add(` .text-${size}-${weight} { | ||
| font-size: ${size}px; | ||
| font-weight: ${weight}; | ||
| line-height: 1.5; | ||
| }`); |
There was a problem hiding this comment.
font-size 토큰은 token.json에서 $type: "number"로 정의되어 있습니다. 현재 스크립트는 dimension 또는 spacing 타입에 대해서만 px 단위를 추가하고 있습니다. 따라서 font-size CSS 변수(--font-size-16: 16;)에는 px 단위가 붙지 않지만, fontUtilities에서는 font-size: ${size}px;와 같이 px 단위를 명시하고 있습니다. 이로 인해 CSS 변수와 유틸리티 클래스 간에 단위 처리의 일관성이 부족합니다. font-size도 px 단위를 사용하는 것이 일반적이므로, CSS 변수 정의 시에도 px를 붙이거나, 유틸리티 클래스에서 px를 제거하여 일관성을 유지하는 것이 좋습니다.
References
- When a script generates assets (like CSS classes) for consumption elsewhere, verify that the generated names and structures match what the consuming code expects. Inconsistent unit handling between generated CSS variables and utility classes can lead to unexpected behavior.
| allTokens | ||
| .filter((t) => t.name.includes("border") && t.type === "color") | ||
| .forEach((t) => { | ||
| const className = t.name.replace(/^border-/, ""); |
There was a problem hiding this comment.
borderUtilities를 생성하는 필터 조건이 t.name.includes("border") && t.type === "color"로 되어 있습니다. border-width와 같은 토큰은 $type: "number"이므로, 현재 로직으로는 border-width 관련 유틸리티 클래스가 생성되지 않습니다. border-width 유틸리티도 필요하다면 필터 조건을 수정하여 number 타입의 border 관련 토큰도 포함하도록 해야 합니다.
References
- When a script generates assets (like CSS classes) for consumption elsewhere, verify that the generated names and structures match what the consuming code expects. Incorrect filtering logic can result in missing or incomplete sets of generated utility classes.
PR Type
Enhancement
Description
토큰 생성 스크립트 완전 개선 및 CSS 변수 자동화
Blur 배경 컴포넌트 추가 및 레이아웃 통합
CSS 변수 대폭 확장 (132개 토큰)
패키지 스크립트 추가 (pnpm run token)
Diagram Walkthrough
File Walkthrough
generate-tokens.js
토큰 생성 스크립트 완전 리팩토링scripts/generate-tokens.js
traverseTokens함수로 완전 개선isValidCSSVarName,isValidCSSValue) 추가tokens.css
CSS 변수 및 유틸리티 대폭 확장app/tokens.css
.bg-color-*,.bg-background-app-*).text-color-*).border-*)Blur.tsx
배경 블러 효과 컴포넌트 추가components/common/Blur.tsx
pointer-events-none으로 상호작용 방지layout.tsx
Blur 컴포넌트 통합 및 토큰 적용app/layout.tsx
bg-background-app-base토큰으로 변경page.tsx
페이지 구조 단순화app/page.tsx
package.json
토큰 생성 스크립트 명령어 추가package.json
token스크립트 추가 (node scripts/generate-tokens.js)✨ Describe tool usage guide:
Overview:
The
describetool scans the PR code changes, and generates a description for the PR - title, type, summary, walkthrough and labels. The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on a PR.When commenting, to edit configurations related to the describe tool (
pr_descriptionsection), use the following template:With a configuration file, use the following template:
Enabling\disabling automation
meaning the
describetool will run automatically on every PR.the tool will replace every marker of the form
pr_agent:marker_namein the PR description with the relevant content, wheremarker_nameis one of the following:type: the PR type.summary: the PR summary.walkthrough: the PR walkthrough.diagram: the PR sequence diagram (if enabled).Note that when markers are enabled, if the original PR description does not contain any markers, the tool will not alter the description at all.
Custom labels
The default labels of the
describetool are quite generic: [Bug fix,Tests,Enhancement,Documentation,Other].If you specify custom labels in the repo's labels page or via configuration file, you can get tailored labels for your use cases.
Examples for custom labels:
Main topic:performance- pr_agent:The main topic of this PR is performanceNew endpoint- pr_agent:A new endpoint was added in this PRSQL query- pr_agent:A new SQL query was added in this PRDockerfile changes- pr_agent:The PR contains changes in the DockerfileThe list above is eclectic, and aims to give an idea of different possibilities. Define custom labels that are relevant for your repo and use cases.
Note that Labels are not mutually exclusive, so you can add multiple label categories.
Make sure to provide proper title, and a detailed and well-phrased description for each label, so the tool will know when to suggest it.
Inline File Walkthrough 💎
For enhanced user experience, the
describetool can add file summaries directly to the "Files changed" tab in the PR page.This will enable you to quickly understand the changes in each file, while reviewing the code changes (diffs).
To enable inline file summary, set
pr_description.inline_file_summaryin the configuration file, possible values are:'table': File changes walkthrough table will be displayed on the top of the "Files changed" tab, in addition to the "Conversation" tab.true: A collapsable file comment with changes title and a changes summary for each file in the PR.false(default): File changes walkthrough will be added only to the "Conversation" tab.Utilizing extra instructions
The
describetool can be configured with extra instructions, to guide the model to a feedback tailored to the needs of your project.Be specific, clear, and concise in the instructions. With extra instructions, you are the prompter. Notice that the general structure of the description is fixed, and cannot be changed. Extra instructions can change the content or style of each sub-section of the PR description.
Examples for extra instructions:
Use triple quotes to write multi-line instructions. Use bullet points to make the instructions more readable.
More PR-Agent commands
See the describe usage page for a comprehensive guide on using this tool.