-
Notifications
You must be signed in to change notification settings - Fork 0
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
[YS-226] emotion -> vanilla extract 마이그레이션 세팅 #32
Conversation
워크스루이 풀 리퀘스트는 Emotion CSS-in-JS 라이브러리에서 Vanilla Extract로의 마이그레이션 작업을 수행합니다. Next.js 프로젝트에 Vanilla Extract 플러그인을 추가하고, 패키지 의존성을 업데이트하며, 기존 스타일 파일을 Vanilla Extract 방식으로 재작성했습니다. 주요 변경 사항은 스타일링 접근 방식의 전환과 관련 구성 파일 업데이트에 중점을 두고 있습니다. 변경 사항
이슈 대응 평가
관련 가능성 있는 PR
제안된 리뷰어
시
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 2
🧹 Nitpick comments (11)
src/app/example/page.tsx (2)
3-16
: 컴포넌트 타입 정의 추가 필요React 컴포넌트의 타입 안정성을 위해 다음과 같이 타입을 명시적으로 정의해주세요:
-function ExamplePage() { +function ExamplePage(): React.ReactElement {
6-7
: 주석 개선 제안현재 주석이 간단하여 이해하기 쉽지만, 스타일 변형에 대한 더 자세한 설명을 추가하면 좋을 것 같습니다.
- {/* 기본 스타일 */} + {/* 기본 스타일: 기본 색상(primary)과 중간 크기(medium)가 적용됨 */}src/app/example/example.css.ts (2)
9-14
: 상수 값 분리 필요크기와 관련된 매직 넘버들을 의미 있는 상수로 분리하면 좋을 것 같습니다:
+const DIMENSIONS = { + HEIGHT: '30rem', + BORDER_RADIUS: '1rem', +} as const; + export const exampleBox = recipe({ base: { - height: '30rem', + height: DIMENSIONS.HEIGHT, display: 'flex', alignItems: 'center', justifyContent: 'center', - borderRadius: '1rem', + borderRadius: DIMENSIONS.BORDER_RADIUS,
16-46
: variants 타입 문서화 필요각 variant에 대한 설명을 JSDoc으로 추가하면 더 명확할 것 같습니다:
variants: { + /** + * 배경색 변형 + * @property {string} primary - 기본 색상 + * @property {string} secondary - 보조 색상 + * @property {string} gray - 회색 + */ backgroundColor: { primary: { backgroundColor: colors.primaryTinted, }, // ... }, + /** + * 크기 변형 + * @property {string} small - 작은 크기 (20rem) + * @property {string} medium - 중간 크기 (30rem) + * @property {string} large - 큰 크기 (40rem) + */ size: { small: { width: '20rem',src/app/post/[post_id]/components/DeleteConfirmModal/DeleteConfirmModal.tsx (2)
25-27
: 접근성 개선 필요닫기 버튼의 aria-label을 더 명확하게 작성해주세요:
- <button className={closeButton} aria-label="모달 닫기"> + <button className={closeButton} aria-label="공고 삭제 확인 모달 닫기">
34-38
: 버튼 역할 명확화 필요삭제 확인 버튼에 위험 동작임을 나타내는 속성을 추가해주세요:
- <button className={`${modalButtonStyle} ${confirmButton}`}>삭제하기</button> + <button + className={`${modalButtonStyle} ${confirmButton}`} + aria-label="공고 영구 삭제" + role="button" + data-destructive="true" + > + 삭제하기 + </button>src/app/post/[post_id]/components/DeleteConfirmModal/DeleteConfirmModal.css.ts (3)
7-19
: dialogContent 스타일의 접근성 개선이 필요합니다.모달 다이얼로그의 접근성을 향상시키기 위해
role="dialog"
및aria-modal="true"
속성과 함께 사용될 수 있도록 스타일을 보완하는 것이 좋습니다.export const dialogContent = style({ width: '49rem', height: '16.4rem', backgroundColor: colors.field01, borderRadius: '1.2rem', padding: '3.2rem', position: 'fixed', top: '50%', left: '50%', transform: 'translate(-50%, -50%)', boxShadow: '0px 4px 16px rgba(53, 59, 61, 0.2)', zIndex: zIndex.dialogContent, + outline: 'none', // 포커스 표시를 위한 기본 아웃라인 제거 + ':focus': { + boxShadow: `0px 4px 16px rgba(53, 59, 61, 0.2), 0 0 0 2px ${colors.primaryMint}`, + }, });
35-43
: 버튼 스타일에 hover와 focus 상태를 추가하는 것이 좋습니다.사용자 경험 향상을 위해 버튼의 상호작용 상태에 대한 스타일을 추가하는 것이 좋습니다.
export const modalButtonStyle = style({ ...bodyNormalSB16, width: '100%', height: '4.8rem', borderRadius: '1.2rem', padding: '1.2rem 0', textAlign: 'center', cursor: 'pointer', + transition: 'background-color 0.2s ease-in-out', + ':hover': { + filter: 'brightness(0.95)', + }, + ':focus': { + outline: 'none', + boxShadow: `0 0 0 2px ${colors.primaryMint}`, + }, });
55-60
: dialogOverlay에 애니메이션 효과 추가를 고려해보세요.모달의 오버레이에 페이드 인/아웃 애니메이션을 추가하면 더 자연스러운 사용자 경험을 제공할 수 있습니다.
export const dialogOverlay = style({ background: 'rgba(0, 22, 54, 0.31)', position: 'fixed', inset: 0, zIndex: zIndex.dialogOverlay, + animation: 'fadeIn 0.2s ease-in-out', + '@keyframes': { + fadeIn: { + from: { opacity: 0 }, + to: { opacity: 1 }, + }, + }, });src/styles/fonts.css.ts (1)
1-32
: 공통 스타일을 변수로 추출하여 재사용성을 높이는 것이 좋습니다.반복되는 스타일 속성들을 변수로 추출하여 코드의 재사용성과 유지보수성을 향상시킬 수 있습니다.
+// Common styles +const normalFontStyle = { + fontStyle: 'normal', + letterSpacing: '0', +}; + +const body16Base = { + ...normalFontStyle, + fontSize: '1.6rem', + lineHeight: '2.4rem', +}; + // body - normal export const bodyNormalR16 = { - fontSize: '1.6rem', - fontStyle: 'normal', - fontWeight: '400', - lineHeight: '2.4rem', - letterSpacing: '0', + ...body16Base, + fontWeight: '400', };package.json (1)
24-26
: vanilla-extract 패키지들의 버전 관리에 대한 제안패키지들이 서로 호환되는 버전으로 잘 설정되어 있습니다. 다만, 프로젝트의 안정성을 위해 패키지 버전을 고정하는 것을 고려해보세요.
- "@vanilla-extract/css": "^1.17.0", - "@vanilla-extract/next-plugin": "^2.4.8", - "@vanilla-extract/recipes": "^0.5.5", + "@vanilla-extract/css": "1.17.0", + "@vanilla-extract/next-plugin": "2.4.8", + "@vanilla-extract/recipes": "0.5.5",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
next.config.mjs
(2 hunks)package.json
(1 hunks)src/app/example/example.css.ts
(1 hunks)src/app/example/page.tsx
(1 hunks)src/app/post/[post_id]/components/DeleteConfirmModal/DeleteConfirmModal.css.ts
(1 hunks)src/app/post/[post_id]/components/DeleteConfirmModal/DeleteConfirmModal.styles.ts
(0 hunks)src/app/post/[post_id]/components/DeleteConfirmModal/DeleteConfirmModal.tsx
(2 hunks)src/styles/fonts.css.ts
(1 hunks)
💤 Files with no reviewable changes (1)
- src/app/post/[post_id]/components/DeleteConfirmModal/DeleteConfirmModal.styles.ts
🔇 Additional comments (1)
next.config.mjs (1)
39-41
: 마이그레이션 전략에 대한 코멘트현재 emotion과 vanilla-extract가 공존하고 있습니다. 마이그레이션이 완료된 후에는 다음 사항들을 제거해야 합니다:
- compiler.emotion 설정
- @emotion 관련 의존성
마이그레이션 진행 상황을 추적하기 위해 다음 스크립트를 실행해보세요:
✅ Verification successful
마이그레이션 현황 확인 결과
코드베이스 분석 결과, 리뷰 코멘트가 정확합니다:
- 현재 emotion이 주된 스타일링 솔루션으로 광범위하게 사용되고 있습니다 (100개 이상의 사용처 확인)
- vanilla-extract는 일부 예제/실험 컴포넌트에서만 사용되고 있어 마이그레이션이 초기 단계임을 확인했습니다
- 마이그레이션 완료 후 emotion 관련 설정 및 의존성 제거가 필요합니다
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # emotion 사용 현황 확인 echo "Emotion 스타일 사용 현황:" rg "css prop|styled[.(\`]" --type ts --type tsx echo "\nVanilla Extract 스타일 사용 현황:" rg "import.*from.*css['\"]" --type ts --type tsxLength of output: 271
Script:
#!/bin/bash # 파일 타입 정의 및 스타일 사용 현황 확인 echo "Emotion 스타일 사용 현황:" rg --type-add 'tsx:*.tsx' --type-add 'ts:*.ts' "styled[\.(]\s*|css\s*`|css={" --type tsx --type ts echo -e "\nVanilla Extract 스타일 사용 현황:" rg --type-add 'tsx:*.tsx' --type-add 'ts:*.ts' "from ['\"](.*).css['\"]" --type tsx --type ts echo -e "\n의존성 확인:" cat package.json | grep -E "@emotion|vanilla-extract"Length of output: 163
Script:
#!/bin/bash echo "=== Emotion 스타일 사용 현황 ===" echo "1. styled 컴포넌트 검색:" rg --type-add 'tsx:*.tsx' --type-add 'ts:*.ts' "styled[.]\w+" --type tsx --type ts echo -e "\n2. css prop 사용 검색:" rg --type-add 'tsx:*.tsx' --type-add 'ts:*.ts' "css=" --type tsx --type ts echo -e "\n=== Vanilla Extract 스타일 사용 현황 ===" rg --type-add 'tsx:*.tsx' --type-add 'ts:*.ts' "from ['\"].*\.css['\"]" --type tsx --type ts echo -e "\n=== 패키지 의존성 확인 ===" if [ -f "package.json" ]; then echo "package.json에서 style 관련 의존성:" grep -E "(@emotion|vanilla-extract)" package.json || echo "관련 의존성을 찾을 수 없습니다." else echo "package.json 파일을 찾을 수 없습니다." fiLength of output: 33583
src/styles/fonts.css.ts
Outdated
export const labelLargeB14 = { | ||
fontSize: '1.4rem', | ||
fontStyle: 'normal', | ||
fontWeight: '700', | ||
lineHeight: '2rem', | ||
letterSpacing: '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.
🛠️ Refactor suggestion
labelLargeB14의 line-height가 다른 large 레이블들과 다릅니다.
다른 large 레이블들은 2.2rem의 line-height를 사용하는데 반해, labelLargeB14는 2rem을 사용하고 있습니다. 일관성 있게 수정하는 것이 좋습니다.
export const labelLargeB14 = {
fontSize: '1.4rem',
fontStyle: 'normal',
fontWeight: '700',
- lineHeight: '2rem',
+ lineHeight: '2.2rem',
letterSpacing: '0',
};
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
export const labelLargeB14 = { | |
fontSize: '1.4rem', | |
fontStyle: 'normal', | |
fontWeight: '700', | |
lineHeight: '2rem', | |
letterSpacing: '0', | |
}; | |
export const labelLargeB14 = { | |
fontSize: '1.4rem', | |
fontStyle: 'normal', | |
fontWeight: '700', | |
lineHeight: '2.2rem', | |
letterSpacing: '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.
네가 뭘 알아 우리 DS팀이 정해준거야 !! 👊
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.
@eeeyooon 죄송합니다! Design System 팀의 의도된 스펙이었네요. 제안을 철회하겠습니다. 🙏
(__))
(oo)
|| ∠ 미안해요...
||w||
✏️ Learnings added
Learnt from: eeeyooon
PR: YAPP-Github/25th-Web-Team-2-FE#32
File: src/styles/fonts.css.ts:152-158
Timestamp: 2025-01-27T11:18:44.810Z
Learning: The line-height value of 2rem for labelLargeB14 is specifically defined by the Design System team and should not be changed for consistency with other labels.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
src/styles/fonts.css.ts
Outdated
export const labelSmallSB12 = { | ||
fontSize: '1.2rem', | ||
fontStyle: 'normal', | ||
fontWeight: '600', | ||
lineHeight: '1.4rem', | ||
letterSpacing: '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.
🛠️ Refactor suggestion
labelSmallSB12의 line-height 비율이 다른 레이블들과 일관성이 없습니다.
다른 small 레이블들은 2rem의 line-height를 사용하는데 반해, labelSmallSB12만 1.4rem을 사용하고 있습니다. 일관성 있게 수정하는 것이 좋습니다.
export const labelSmallSB12 = {
fontSize: '1.2rem',
fontStyle: 'normal',
fontWeight: '600',
- lineHeight: '1.4rem',
+ lineHeight: '2rem',
letterSpacing: '0',
};
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
export const labelSmallSB12 = { | |
fontSize: '1.2rem', | |
fontStyle: 'normal', | |
fontWeight: '600', | |
lineHeight: '1.4rem', | |
letterSpacing: '0', | |
}; | |
export const labelSmallSB12 = { | |
fontSize: '1.2rem', | |
fontStyle: 'normal', | |
fontWeight: '600', | |
lineHeight: '2rem', | |
letterSpacing: '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.
이것도 우리 DS팀이 정해준거야~
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.
@eeeyooon 아, 네 이해했습니다! DS팀의 결정사항이라면 현재 값을 유지하는 것이 맞겠네요. 제안을 철회하겠습니다.
(__)/
(='.'=)
(")_(")
thoughtful bunny
✏️ Learnings added
Learnt from: eeeyooon
PR: YAPP-Github/25th-Web-Team-2-FE#32
File: src/styles/fonts.css.ts:94-100
Timestamp: 2025-01-27T11:19:02.265Z
Learning: The line-height values in font styles (fonts.css.ts) are specified by the Design System team and should not be modified for consistency without their approval, even if they appear inconsistent.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
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.
마이그레이션 너무 고생많으셨습니다 !! 익숙하지 않은 도구라 고민도 많고, 어떻게 바꿔야할지도 막막했는데 PR 에도 자세히 설명해주셔서 이해가 빠르게 되었습니다:)
궁금한 점만 남겨봤고, 예시 코드랑 설명한 내용 참고해서 제가 만든 컴포넌트에도 잘 반영해보겠습니당
src/styles/fonts.css.ts
Outdated
@@ -0,0 +1,158 @@ | |||
// body - 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.
💭 질문 💭
와우 다 바꾸느라 고생했겠는데요ㅠ
그런데 기존 방식에서 css 부분만 객체로 바꾸고 ...body.normal.R16
로도 될 것 같은데 변수로 바꾼 이유가 궁금합니다!
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.
변환은 chatGPT에 맡겨서 힘든 점은 없었습니다!!
변수로 바꾼 이유는..!! 사실 별 생각이 없었습니다 ㅎ..
그런데 기존 방식에서 emotion css만 제거하는게 더 편리할 거 같네요 ㅎㅎ;
반영해서 커밋 추가하겠습니다! 감사합니다!
src/app/example/example.css.ts
Outdated
}, | ||
}); | ||
|
||
export const exampleText = style(bodyNormalB16); |
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.
💭 질문 💭
오호 이렇게 따로 변수로 뺄 수도 있고, base 안에 ...bodyNormalB16
를 추가할 수도 있는거죠??
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.
네! recipe
를 사용할 경우 base 안에 추가할 수도 있습니다!!
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.
Actionable comments posted: 1
🧹 Nitpick comments (7)
src/styles/fonts.css.ts (2)
1-160
: 타입 안전성 향상을 위한 타입 정의가 필요합니다.현재
fonts
객체는 타입 정의 없이 export되어 있습니다. 타입스크립트를 사용하여 타입 안전성을 향상시키는 것이 좋습니다.+ type FontStyle = { + fontSize: string; + fontStyle: 'normal'; + fontWeight: string; + lineHeight: string; + letterSpacing: string; + }; + + type FontWeights = { + [key: string]: FontStyle; + }; + + type FontSizes = { + [key: string]: FontWeights; + }; + + type Fonts = { + body: { + normal: FontWeights; + }; + title: { + small: FontWeights; + medium: FontWeights; + large: FontWeights; + }; + label: { + small: FontWeights; + medium: FontWeights; + large: FontWeights; + }; + }; + - export const fonts = { + export const fonts: Fonts = { // ... existing code
1-34
: 중복 제거를 위한 공통 스타일 추출이 필요합니다.body 섹션의 각 스타일들이 동일한
fontSize
,fontStyle
,lineHeight
,letterSpacing
값을 공유하고 있습니다. 이를 기본 스타일로 추출하여 중복을 제거할 수 있습니다.+ const bodyBaseStyle = { + fontSize: '1.6rem', + fontStyle: 'normal' as const, + lineHeight: '2.4rem', + letterSpacing: '0', + }; + export const fonts = { body: { normal: { R16: { - fontSize: '1.6rem', - fontStyle: 'normal', + ...bodyBaseStyle, fontWeight: '400', - lineHeight: '2.4rem', - letterSpacing: '0', }, // Apply similar changes to M16, SB16, B16src/app/example/example.css.ts (2)
7-47
: recipe 타입 안전성 향상이 필요합니다.
recipe
의 variants에 대한 타입 정의가 없습니다. 타입스크립트를 활용하여 타입 안전성을 향상시키는 것이 좋습니다.+ type BackgroundColorVariant = 'primary' | 'secondary' | 'gray'; + type SizeVariant = 'small' | 'medium' | 'large'; + + type ExampleBoxVariants = { + backgroundColor: BackgroundColorVariant; + size: SizeVariant; + }; + - export const exampleBox = recipe({ + export const exampleBox = recipe<ExampleBoxVariants>({
49-51
: 상수로 추출된 스타일 객체 사용을 고려해보세요.단일 폰트 스타일을 적용하는 경우, 별도의 상수로 추출하여 재사용성을 높일 수 있습니다.
+ const bodyNormalB16 = fonts.body.normal.B16; + - export const exampleText = style({ - ...fonts.body.normal.B16, - }); + export const exampleText = style(bodyNormalB16);src/app/post/[post_id]/components/DeleteConfirmModal/DeleteConfirmModal.css.ts (3)
7-19
: 반응형 디자인 개선이 필요합니다.고정된 너비값은 모바일 환경에서 문제가 될 수 있습니다. 다음과 같은 개선을 제안드립니다:
export const dialogContent = style({ - width: '49rem', + width: '100%', + maxWidth: '49rem', + margin: '0 1.6rem', height: '16.4rem', backgroundColor: colors.field01, borderRadius: '1.2rem', padding: '3.2rem', position: 'fixed', top: '50%', left: '50%', transform: 'translate(-50%, -50%)', boxShadow: '0px 4px 16px rgba(53, 59, 61, 0.2)', zIndex: zIndex.dialogContent, });
35-43
: 상호작용 상태 스타일이 누락되었습니다.접근성과 사용자 경험 향상을 위해 hover, focus 상태를 추가하는 것이 좋습니다:
export const modalButtonStyle = style({ ...fonts.body.normal.SB16, width: '100%', height: '4.8rem', borderRadius: '1.2rem', padding: '1.2rem 0', textAlign: 'center', cursor: 'pointer', + ':hover': { + opacity: 0.8, + }, + ':focus': { + outline: `0.2rem solid ${colors.primaryMint}`, + outlineOffset: '0.2rem', + }, });
45-53
: 버튼 활성 상태 스타일이 필요합니다.사용자 피드백을 위해 active 상태를 추가하는 것이 좋습니다:
export const cancelButton = style({ backgroundColor: colors.field04, color: colors.text06, + ':active': { + backgroundColor: colors.field05, + }, }); export const confirmButton = style({ backgroundColor: colors.primaryMint, color: colors.text01, + ':active': { + backgroundColor: colors.primaryMintDark, + }, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/app/example/example.css.ts
(1 hunks)src/app/post/[post_id]/components/DeleteConfirmModal/DeleteConfirmModal.css.ts
(1 hunks)src/styles/fonts.css.ts
(1 hunks)
🔇 Additional comments (7)
src/styles/fonts.css.ts (2)
102-102
: 라인 높이가 다른 small 레이블과 다릅니다.
label.small.SB12
의lineHeight
가1.4rem
으로 설정되어 있는데, 이는 DS 팀의 의도된 스펙임을 이전 리뷰에서 확인했습니다.
155-155
: 라인 높이가 다른 large 레이블과 다릅니다.
label.large.B14
의lineHeight
가2rem
으로 설정되어 있는데, 이는 DS 팀의 의도된 스펙임을 이전 리뷰에서 확인했습니다.src/app/example/example.css.ts (1)
31-33
: 일관성 없는 폰트 스타일 사용을 확인해주세요.
small
variant에서는B16
을,large
variant에서는R16
을 사용하고 있습니다. 이러한 차이가 의도적인지 확인이 필요합니다.Also applies to: 38-40
src/app/post/[post_id]/components/DeleteConfirmModal/DeleteConfirmModal.css.ts (4)
1-6
: 스타일 유틸리티와 토큰 임포트가 잘 구성되어 있습니다!공유 스타일 토큰을 활용하여 일관된 디자인 시스템을 유지하고 있습니다.
21-26
: 타이포그래피 스타일링이 깔끔합니다!공유 폰트 스타일을 활용하여 일관성 있게 구현되었습니다.
28-33
: 버튼 그룹 레이아웃이 적절합니다!플렉스박스를 활용한 버튼 배치와 간격이 일관되게 구현되었습니다.
55-60
: 오버레이 구현이 적절합니다!반투명 배경과 z-index 설정이 모달의 계층 구조를 잘 표현하고 있습니다.
export const closeButton = style({ | ||
cursor: 'pointer', | ||
position: 'absolute', | ||
top: '1.2rem', | ||
right: '1.2rem', | ||
}); |
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.
🛠️ Refactor suggestion
닫기 버튼의 접근성 개선이 필요합니다.
키보드 사용자를 위한 포커스 상태와 시각적 피드백이 필요합니다:
export const closeButton = style({
cursor: 'pointer',
position: 'absolute',
top: '1.2rem',
right: '1.2rem',
+ padding: '0.8rem',
+ ':hover': {
+ opacity: 0.8,
+ },
+ ':focus': {
+ outline: `0.2rem solid ${colors.primaryMint}`,
+ outlineOffset: '0.2rem',
+ },
});
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
export const closeButton = style({ | |
cursor: 'pointer', | |
position: 'absolute', | |
top: '1.2rem', | |
right: '1.2rem', | |
}); | |
export const closeButton = style({ | |
cursor: 'pointer', | |
position: 'absolute', | |
top: '1.2rem', | |
right: '1.2rem', | |
padding: '0.8rem', | |
':hover': { | |
opacity: 0.8, | |
}, | |
':focus': { | |
outline: `0.2rem solid ${colors.primaryMint}`, | |
outlineOffset: '0.2rem', | |
}, | |
}); |
Issue Number
close #31
As-Is
To-Be
Check List
Test Screenshot
(Optional) Additional Description
vanilla-extract 마이그레이션을 위한 세팅 작업에 대해 공유드리겠습니다.
1. Vanilla Extract 초기 세팅
2. 타이포그래피 객체 생성
fonts.css.ts
파일을 생성하여 모든 타이포그래피 스타일을 객체로 관리하였습니다..css.ts
파일 내에서만 사용 가능합니다.fonts.css.ts
는 style을 사용하지 않고 순수 객체 형태로 작성되어서 반드시.css.ts
파일일 필요는 없지만 vanilla-extract에서 사용하는 파일임을 명확히 구분하고 기존 Emotion 기반의 fonts.ts와 충돌을 방지하기 위해.css.ts
파일로 생성하였습니다.style
에 인자로 넣으면 해당 타이포그래피 객체에 정의된 모든 CSS 속성을 가진 클래스를 생성합니다. 혹은 전개 연산자(...
)를 사용하여 스타일에 적용할 수 있습니다. (전개 연산자를 사용하는 이유는 본문 하단에서 후술하였습니다.)3. vanilla-extract의
recipe
사용더 편리한 동적 스타일링, 코드의 가독성과 재사용성을 고려하여
recipe
를 사용하였습니다.base
base
는 recipe의 모든variants
와 무관하게 항상 적용되는 기본 스타일입니다.recipe
로 생성된 클래스에 항상 포함됩니다.variants
에 상관없이 모든 상태에서 공통적으로 적용할 스타일을 정의합니다.base
스타일인display: 'flex', alignItems: 'center'
은 항상 적용됩니다.variants
size
,color
등)을 기준으로 서로 다른 스타일을 정의하고, 컴포넌트에서 호출하여 원하는 상태의 스타일을 적용할 수 있습니다.size
를 기준으로small
과large
를 정의하였습니다.display: 'flex', align-items: 'center', width: '20rem'
가 적용됩니다.defaultVariants
variants
의 기본 상태를 정의합니다.variants
를 명시하지 않으면defaultVarinats
의 스타일을 적용합니다.size
를 명시하지 않았으므로defaultVariants
에 정의된size: 'small'
이 적용됩니다.4.
example
코드 추가example
폴더를 추가하여 예시 코드를 작성하였습니다. 해당 코드에서 테스트를 해보며 사용해보시는 걸 추천드립니다!/example/page.tsx
'/example/example.css.ts`vanilla-extract의
createTheme
를 활용하여 theme를 설정하는 방법도 있지만 기존 타이포그래피 구조에서는 적용하기 어려운 점이 있었습니다. 또한 colors, fonts를 객체로 정의한 뒤 값을 불러와 사용하는 방식이createTheme
를 사용하는 방법과 비교해서 결과적으로 큰 차이가 없어보였고 설정이나 유지보수면에서 더 간단하다고 생각해 현재 pr에 올린 방식으로 진행하였습니다 (사용 방식 또한 객체로 정의해 사용하는 것이 더 익숙하였습니다. 👀 )colors
의 경우 css 속성으로 바로 할당할 수 있는 문자열이기 때문 직접 접근이 가능했으나 (ex.color: colors.text01
) fonsts의 경우 단순 문자열이 아닌 타이포그래피 속성들(fontSize, lineHeight등)이 포함된 객체인데, vanila-extract의style
은 각 css 속성을 개별적으로 받아야 하므로 전개 연산자(...
)를 사용하여 전달해야합니다.혹은 모든 개별 속성을 하나씩 풀어서 사용하여야 하는데 그러면 기존 스타일 코드를 마이그레이션 하는 과정에서 너무 번거로울 것 같아서 전개 연산자를 사용하는 방식으로 선택하였습니다.
처음
vanilla-extract
를 사용해보는 거라 설정 과정에서 고민이 많았습니다. 우선 제가 진행한 과정과 작업 내용에 공유 드렸고 관련해서 의견이 있다면 편하게 말씀해주세요 🙌+테스트용으로
DeleteConfirmModal
컴포넌트에만 마이그레이션을 진행했습니다.Summary by CodeRabbit
Summary by CodeRabbit
새로운 기능
ExamplePage
컴포넌트 추가의존성 업데이트
@vanilla-extract/css
,@vanilla-extract/next-plugin
,@vanilla-extract/recipes
)스타일링
DeleteConfirmModal
의 스타일링 방법 변경 (className 사용)