Skip to content
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

Merged
merged 5 commits into from
Jan 27, 2025

Conversation

eeeyooon
Copy link
Member

@eeeyooon eeeyooon commented Jan 27, 2025

Issue Number

close #31

As-Is

  • vanilla-extract 사용 x

To-Be

  • vanilla-extract 마이그레이션을 위한 세팅

Check List

  • 테스트가 전부 통과되었나요?
  • 모든 commit이 push 되었나요?
  • merge할 branch를 확인했나요?
  • Assignee를 지정했나요?
  • Label을 지정했나요?

Test Screenshot

(Optional) Additional Description

vanilla-extract 마이그레이션을 위한 세팅 작업에 대해 공유드리겠습니다.


1. Vanilla Extract 초기 세팅

  • vanilla-extract의 기본 패키지 및 Next.js 통합 플러그인, 동적 스타일링 구현을 위한 패키지(recipes)를 설치하였습니다.
pnpm add @vanilla-extract/css @vanilla-extract/next-plugin @vanilla-extract/recipes
  • 단계적으로 마이그레이션을 할 수 있도록 기존 Emotion 세팅은 남겨두었고, 마이그레이션이 완료되면 그때 Emotion 관련 패키지 및 코드는 제거할 예정입니다.

2. 타이포그래피 객체 생성

  • fonts.css.ts 파일을 생성하여 모든 타이포그래피 스타일을 객체로 관리하였습니다.
  • vanilla-extract의 style은 CSS 클래스 문자열을 반환하는 함수입니다. 따라서, vanilla-extract의 style은 .css.ts 파일 내에서만 사용 가능합니다.
  • 현재 fonts.css.ts는 style을 사용하지 않고 순수 객체 형태로 작성되어서 반드시 .css.ts 파일일 필요는 없지만 vanilla-extract에서 사용하는 파일임을 명확히 구분하고 기존 Emotion 기반의 fonts.ts와 충돌을 방지하기 위해 .css.ts 파일로 생성하였습니다.
  • 이 타이포그래피를 style에 인자로 넣으면 해당 타이포그래피 객체에 정의된 모든 CSS 속성을 가진 클래스를 생성합니다. 혹은 전개 연산자(...)를 사용하여 스타일에 적용할 수 있습니다. (전개 연산자를 사용하는 이유는 본문 하단에서 후술하였습니다.)
export const exampleText = style(bodyNormalB16);
export const exampleText = style({
  ...bodyNormalB16,
  color: colors.textPrimary,
});
size: {
      small: {
        width: '20rem',
        ...bodyNormalB16,
      },
      medium: {
        width: '30rem',
      },
      large: {
        width: '40rem',
        ...bodyNormalR16,
      },
    },

3. vanilla-extract의 recipe 사용

더 편리한 동적 스타일링, 코드의 가독성과 재사용성을 고려하여 recipe를 사용하였습니다.

export const exampleBox = recipe({
  base: {
    display: 'flex',
    alignItems: 'center',
  },
  variants: {
    size: {
      small: { width: '20rem' },
      large: { width: '40rem' },
    },
  },
  defaultVariants: {
    size: 'small',
  },
});

base

  • base는 recipe의 모든 variants와 무관하게 항상 적용되는 기본 스타일입니다. recipe로 생성된 클래스에 항상 포함됩니다. variants에 상관없이 모든 상태에서 공통적으로 적용할 스타일을 정의합니다.
  • base 스타일인 display: 'flex', alignItems: 'center'은 항상 적용됩니다.

variants

  • variants를 활용하여 다양한 상태별 스타일링을 동적으로 관리할 수 있습니다.
  • 특정 속성(size,color등)을 기준으로 서로 다른 스타일을 정의하고, 컴포넌트에서 호출하여 원하는 상태의 스타일을 적용할 수 있습니다.
  • 예시 코드에서는 size를 기준으로 smalllarge를 정의하였습니다.
<div className={exampleBox({ size: 'small' })}>Small Box</div>
  • 이 경우 display: 'flex', align-items: 'center', width: '20rem'가 적용됩니다.

defaultVariants

  • variants의 기본 상태를 정의합니다.
  • 컴포넌트가 호출될 때 별도의 variants를 명시하지 않으면 defaultVarinats의 스타일을 적용합니다.
<div className={exampleBox()}>Content</div>
  • 따로 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 속성을 개별적으로 받아야 하므로 전개 연산자(...)를 사용하여 전달해야합니다.

export const bodyText = style({
  ...bodyNormalR16, 
});

혹은 모든 개별 속성을 하나씩 풀어서 사용하여야 하는데 그러면 기존 스타일 코드를 마이그레이션 하는 과정에서 너무 번거로울 것 같아서 전개 연산자를 사용하는 방식으로 선택하였습니다.


처음 vanilla-extract를 사용해보는 거라 설정 과정에서 고민이 많았습니다. 우선 제가 진행한 과정과 작업 내용에 공유 드렸고 관련해서 의견이 있다면 편하게 말씀해주세요 🙌
+테스트용으로 DeleteConfirmModal 컴포넌트에만 마이그레이션을 진행했습니다.


Summary by CodeRabbit

Summary by CodeRabbit

  • 새로운 기능

    • Vanilla Extract를 사용한 고급 스타일링 시스템 도입
    • 동적 컴포넌트 스타일링 지원
    • 다양한 폰트 스타일 및 변형 추가
    • 새로운 ExamplePage 컴포넌트 추가
  • 의존성 업데이트

    • Vanilla Extract 관련 패키지 추가 (@vanilla-extract/css, @vanilla-extract/next-plugin, @vanilla-extract/recipes)
  • 스타일링

    • 새로운 CSS 모듈을 통한 컴포넌트 스타일 개선
    • 모달, 버튼, 텍스트 등에 대한 세밀한 스타일 정의
    • DeleteConfirmModal의 스타일링 방법 변경 (className 사용)

@eeeyooon eeeyooon added feat New feature or request chore 설정 파일, 라이브러리 설치 labels Jan 27, 2025
@eeeyooon eeeyooon requested a review from rbgksqkr January 27, 2025 10:40
@eeeyooon eeeyooon self-assigned this Jan 27, 2025
Copy link

coderabbitai bot commented Jan 27, 2025

워크스루

이 풀 리퀘스트는 Emotion CSS-in-JS 라이브러리에서 Vanilla Extract로의 마이그레이션 작업을 수행합니다. Next.js 프로젝트에 Vanilla Extract 플러그인을 추가하고, 패키지 의존성을 업데이트하며, 기존 스타일 파일을 Vanilla Extract 방식으로 재작성했습니다. 주요 변경 사항은 스타일링 접근 방식의 전환과 관련 구성 파일 업데이트에 중점을 두고 있습니다.

변경 사항

파일 변경 요약
next.config.mjs Vanilla Extract 플러그인 가져오기 및 구성 래핑
package.json Vanilla Extract 관련 의존성 추가 (@vanilla-extract/css, @vanilla-extract/next-plugin, @vanilla-extract/recipes)
src/app/example/example.css.ts 새로운 Vanilla Extract 스타일 모듈 생성
src/app/example/page.tsx Vanilla Extract 스타일 적용 예제 컴포넌트 추가
src/app/post/[post_id]/components/DeleteConfirmModal/DeleteConfirmModal.css.ts 모달 컴포넌트를 위한 새로운 Vanilla Extract 스타일 정의
src/app/post/[post_id]/components/DeleteConfirmModal/DeleteConfirmModal.styles.ts 기존 Emotion 스타일 파일 제거
src/app/post/[post_id]/components/DeleteConfirmModal/DeleteConfirmModal.tsx 스타일 적용 방식 css 프롭에서 className 프롭으로 변경
src/styles/fonts.css.ts 다양한 폰트 스타일 상수 추가

이슈 대응 평가

목표 대응 여부 설명
Vanilla Extract 설치 및 세팅
Vanilla Extract 테마 설정
Vanilla Extract로 스타일링 적용 (테스트)

관련 가능성 있는 PR

제안된 리뷰어

  • rbgksqkr

🐰 바닐라 추출의 마법
스타일이 춤추네
감정은 가고
새로운 색채가 피어나
코드의 토끼 점프! 🎨✨

✨ Finishing Touches
  • 📝 Generate Docstrings (Beta)

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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between 9c9c7d7 and cafdfe4.

⛔ 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 tsx

Length 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 파일을 찾을 수 없습니다."
fi

Length of output: 33583

Comment on lines 152 to 158
export const labelLargeB14 = {
fontSize: '1.4rem',
fontStyle: 'normal',
fontWeight: '700',
lineHeight: '2rem',
letterSpacing: '0',
};
Copy link

@coderabbitai coderabbitai bot Jan 27, 2025

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.

Suggested change
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',
};

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

네가 뭘 알아 우리 DS팀이 정해준거야 !! 👊

Copy link

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.

Comment on lines 94 to 100
export const labelSmallSB12 = {
fontSize: '1.2rem',
fontStyle: 'normal',
fontWeight: '600',
lineHeight: '1.4rem',
letterSpacing: '0',
};
Copy link

@coderabbitai coderabbitai bot Jan 27, 2025

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.

Suggested change
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',
};

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이것도 우리 DS팀이 정해준거야~

Copy link

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.

@YAPP-Github YAPP-Github deleted a comment from coderabbitai bot Jan 27, 2025
Copy link
Contributor

@rbgksqkr rbgksqkr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

마이그레이션 너무 고생많으셨습니다 !! 익숙하지 않은 도구라 고민도 많고, 어떻게 바꿔야할지도 막막했는데 PR 에도 자세히 설명해주셔서 이해가 빠르게 되었습니다:)
궁금한 점만 남겨봤고, 예시 코드랑 설명한 내용 참고해서 제가 만든 컴포넌트에도 잘 반영해보겠습니당

@@ -0,0 +1,158 @@
// body - normal
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💭 질문 💭

와우 다 바꾸느라 고생했겠는데요ㅠ
그런데 기존 방식에서 css 부분만 객체로 바꾸고 ...body.normal.R16 로도 될 것 같은데 변수로 바꾼 이유가 궁금합니다!

Copy link
Member Author

@eeeyooon eeeyooon Jan 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

변환은 chatGPT에 맡겨서 힘든 점은 없었습니다!!
변수로 바꾼 이유는..!! 사실 별 생각이 없었습니다 ㅎ..
그런데 기존 방식에서 emotion css만 제거하는게 더 편리할 거 같네요 ㅎㅎ;
반영해서 커밋 추가하겠습니다! 감사합니다! ☺️

},
});

export const exampleText = style(bodyNormalB16);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💭 질문 💭

오호 이렇게 따로 변수로 뺄 수도 있고, base 안에 ...bodyNormalB16 를 추가할 수도 있는거죠??

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

네! recipe를 사용할 경우 base 안에 추가할 수도 있습니다!!

@eeeyooon eeeyooon merged commit c55b030 into develop Jan 27, 2025
1 check was pending
Copy link

@coderabbitai coderabbitai bot left a 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, B16
src/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

📥 Commits

Reviewing files that changed from the base of the PR and between cafdfe4 and df88247.

📒 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.SB12lineHeight1.4rem으로 설정되어 있는데, 이는 DS 팀의 의도된 스펙임을 이전 리뷰에서 확인했습니다.


155-155: 라인 높이가 다른 large 레이블과 다릅니다.

label.large.B14lineHeight2rem으로 설정되어 있는데, 이는 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 설정이 모달의 계층 구조를 잘 표현하고 있습니다.

Comment on lines +62 to +67
export const closeButton = style({
cursor: 'pointer',
position: 'absolute',
top: '1.2rem',
right: '1.2rem',
});
Copy link

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.

Suggested change
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',
},
});

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore 설정 파일, 라이브러리 설치 feat New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[YS-226] emotion -> vanilla extract 마이그레이션 세팅
2 participants