[Feat] WTH-163-dialog alert 모달 구현#7
Open
Conversation
PR 검증 결과❌ TypeScript: 실패 |
📝 WalkthroughWalkthrough프로젝트에 Figma-to-component 및 PR 자동화 에이전트 문서가 추가되었고, CI/CD 워크플로우(GitHub Actions 및 Amplify)가 구성되었습니다. AlertDialog 컴포넌트에 상태 기반 변형(default/danger)이 추가되었으며, Button에 danger 변형이 추가되었습니다. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (5)
CLAUDE.md (2)
13-18: 코드 블록에 언어 식별자를 추가하세요.정적 분석 도구(markdownlint MD040)에서 경고가 발생했습니다. 코드 블록에 언어를 지정하면 린트 경고가 해소되고 구문 강조도 적용됩니다.
💡 제안
-``` +```text src/ app/globals.css # 디자인 토큰 (CSS variables, `@utility`) components/ui/ # 재사용 UI 컴포넌트, index.ts에서 re-export lib/cn.ts # className 병합 유틸 ```🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CLAUDE.md` around lines 13 - 18, Add a language identifier to the fenced code block that currently contains the "src/ … lib/cn.ts" tree (the block starting with ``` and the "src/" line) to satisfy markdownlint MD040 and enable syntax highlighting; update the opening fence to include a language token such as ```text (or ```bash) so the block becomes ```text followed by the existing lines and the closing ```.
51-53: Git 컨벤션 코드 블록에도 언어 식별자를 추가하세요.마찬가지로 markdownlint MD040 경고가 발생합니다.
💡 제안
-``` +```text feat / fix / style / refactor / ci / chore ```🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CLAUDE.md` around lines 51 - 53, The fenced code block that currently reads ``` feat / fix / style / refactor / ci / chore ``` should include a language identifier to satisfy markdownlint MD040; update the block delimiter to include an explicit language (for example change the opening fence to ```text) so the block becomes ```text … ```; locate the fenced block containing "feat / fix / style / refactor / ci / chore" in CLAUDE.md and add the language tag..github/workflows/ci.yml (1)
21-21:pnpm버전을 고정하세요.
amplify.yml과 동일하게version: latest대신 특정 버전을 지정하면 CI 환경의 재현성이 향상됩니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml at line 21, 현재 CI 워크플로의 pnpm 설정에서 "version: latest"로 되어 있어 재현성이 떨어집니다; .github/workflows/ci.yml에서 "version: latest"를 특정 버전 문자열(예: amplify.yml에 사용된 동일한 pnpm 버전)으로 고정하도록 변경하고, 해당 키("version" under pnpm setup) 값을 amplify.yml의 버전과 일치시키세요.amplify.yml (1)
7-7:pnpm@latest대신 특정 버전을 고정하는 것을 권장합니다.
pnpm@latest는 빌드 시점에 따라 다른 버전이 설치될 수 있어 재현 가능한 빌드를 보장하기 어렵습니다.package.json의packageManager필드에 버전을 명시하고, 여기서는 해당 버전을 참조하거나 고정 버전(예:pnpm@9.x.x)을 사용하세요.💡 제안
- - corepack prepare pnpm@latest --activate + - corepack prepare pnpm@9.15.0 --activate또는
package.json에"packageManager": "pnpm@9.15.0"필드를 추가하면corepack prepare없이도corepack enable만으로 올바른 버전이 활성화됩니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@amplify.yml` at line 7, Replace the unstable "corepack prepare pnpm@latest --activate" invocation with a pinned pnpm version or rely on package.json's packageManager field: either change the command to "corepack prepare pnpm@9.x.x --activate" (use the exact desired semver, e.g., 9.15.0) or remove the prepare call and ensure package.json contains "packageManager": "pnpm@9.15.0" so that "corepack enable" will activate the correct pnpm; update the line containing corepack prepare pnpm@latest --activate accordingly..claude/agents/figma-to-component.md (1)
7-9: 에이전트 가이드에 프론트엔드 스택 제약을 명시해 주세요.이 문서는 코드 생성 기준서 역할이라 Step 3 근처에
React 19 + Next.js 16(App Router) + Tailwind v4를 명시해 두면 결과 일관성이 더 좋아집니다.As per coding guidelines
**/*.{ts,tsx}: Use React 19 + TypeScript with Next.js 16 (App Router) for all frontend code; Use Tailwind CSS v4 and class-variance-authority (cva) for styling.Also applies to: 33-63
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/agents/figma-to-component.md around lines 7 - 9, Add explicit frontend stack constraints to the Figma-to-component agent guide: insert a short, clear statement near Step 3 that all generated frontend code must target "React 19 + Next.js 16 (App Router) + TypeScript" and use "Tailwind CSS v4 + class-variance-authority (cva)" for styling, and also update the coding-guidelines section referenced by the glob pattern "**/*.{ts,tsx}" so it explicitly requires React 19, Next.js 16 (App Router), TypeScript, Tailwind v4 and cva; locate the Step 3 heading and the coding-guidelines block in .claude/agents/figma-to-component.md (and the related paragraphs around lines 33-63) and add that exact stack constraint text to ensure consistency across generated components.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/agents/figma-to-component.md:
- Around line 1-70: The CI failure is due to Prettier formatting errors in the
Markdown file named figma-to-component (frontmatter and content). Reformat the
file with your project's Prettier configuration (run prettier --write on
.claude/agents/figma-to-component.md or use your editor's format command),
ensuring the YAML frontmatter, code blocks, and inline code are correctly
indented and wrapped; then stage and commit the reformatted file so the Prettier
check passes.
In @.github/workflows/ci.yml:
- Around line 32-50: Add a final "verdict" step that runs unconditionally (if:
always()) after the PR comment step and inspects the outcomes of the earlier
steps by their IDs (typescript, eslint, prettier, build); if any of
steps.typescript.outcome, steps.eslint.outcome, steps.prettier.outcome, or
steps.build.outcome is not "success", exit non‑zero to fail the workflow (give
the step an id like final-verdict and a clear error message). This preserves the
per-step continue-on-error behavior for comment/logging but enforces a blocking
CI failure if any check actually failed.
In `@src/components/ui/alert-dialog.tsx`:
- Around line 55-57: The conditional currently checks only for trigger !==
undefined, which still allows trigger === null to render <AlertDialogTrigger
asChild>{trigger}</AlertDialogTrigger> and cause a runtime error; update the
check to exclude null as well (e.g., use trigger != null or trigger !==
undefined && trigger !== null or Boolean(trigger)) so that AlertDialogTrigger is
only rendered when trigger is a valid non-null value, referencing the trigger
prop and AlertDialogTrigger/asChild usage.
- Around line 13-38: The component currently branches on AlertStatus across
AlertStatus, defaultTexts, and AlertDialogProps; refactor to use cva variants
and VariantProps by replacing the AlertStatus union and scattered defaultTexts
with a cva() call that defines variants (e.g., variant: { default, danger }) and
defaultVariants, update the props interface to extend VariantProps<typeof
yourCva> and React.ComponentProps<typeof AlertDialogPrimitive.Root>, and use the
generated variant classNames/props inside AlertDialogContext and the component
to derive title/description (move defaultTexts into variant defaults or a
mapping keyed by variant) so all status behavior is centralized in the cva
variants configuration.
- Around line 5-11: Replace the direct SVG imports InfoIcon and DeleteIcon with
lucide-react icon components: remove "import InfoIcon from
'@/assets/icons/info.svg'" and "import DeleteIcon from
'@/assets/icons/delete_forever.svg'" and instead import the proper lucide-react
icons (e.g., "import { Info, Trash2 } from 'lucide-react'"). Then update the JSX
usages that render <InfoIcon .../> and <DeleteIcon .../> (the instances
referenced in this file) to use <Info .../> and <Trash2 .../> (preserving
className, size, strokeWidth or other props passed through cn or Button props)
so styling and accessibility remain unchanged.
In `@src/components/ui/Button.tsx`:
- Around line 16-17: In src/components/ui/Button.tsx update the danger variant
string (the 'danger' key in the exported variants) to replace the arbitrary
class bg-[var(--state-error)] with the design-token utility class bg-state-error
so it matches other variants (e.g. bg-button-primary, bg-button-neutral) and
avoids arbitrary values; keep the rest of the class list (text-text-strong
hover/active/disabled classes) unchanged to preserve behavior.
---
Nitpick comments:
In @.claude/agents/figma-to-component.md:
- Around line 7-9: Add explicit frontend stack constraints to the
Figma-to-component agent guide: insert a short, clear statement near Step 3 that
all generated frontend code must target "React 19 + Next.js 16 (App Router) +
TypeScript" and use "Tailwind CSS v4 + class-variance-authority (cva)" for
styling, and also update the coding-guidelines section referenced by the glob
pattern "**/*.{ts,tsx}" so it explicitly requires React 19, Next.js 16 (App
Router), TypeScript, Tailwind v4 and cva; locate the Step 3 heading and the
coding-guidelines block in .claude/agents/figma-to-component.md (and the related
paragraphs around lines 33-63) and add that exact stack constraint text to
ensure consistency across generated components.
In @.github/workflows/ci.yml:
- Line 21: 현재 CI 워크플로의 pnpm 설정에서 "version: latest"로 되어 있어 재현성이 떨어집니다;
.github/workflows/ci.yml에서 "version: latest"를 특정 버전 문자열(예: amplify.yml에 사용된 동일한
pnpm 버전)으로 고정하도록 변경하고, 해당 키("version" under pnpm setup) 값을 amplify.yml의 버전과
일치시키세요.
In `@amplify.yml`:
- Line 7: Replace the unstable "corepack prepare pnpm@latest --activate"
invocation with a pinned pnpm version or rely on package.json's packageManager
field: either change the command to "corepack prepare pnpm@9.x.x --activate"
(use the exact desired semver, e.g., 9.15.0) or remove the prepare call and
ensure package.json contains "packageManager": "pnpm@9.15.0" so that "corepack
enable" will activate the correct pnpm; update the line containing corepack
prepare pnpm@latest --activate accordingly.
In `@CLAUDE.md`:
- Around line 13-18: Add a language identifier to the fenced code block that
currently contains the "src/ … lib/cn.ts" tree (the block starting with ``` and
the "src/" line) to satisfy markdownlint MD040 and enable syntax highlighting;
update the opening fence to include a language token such as ```text (or
```bash) so the block becomes ```text followed by the existing lines and the
closing ```.
- Around line 51-53: The fenced code block that currently reads ``` feat / fix /
style / refactor / ci / chore ``` should include a language identifier to
satisfy markdownlint MD040; update the block delimiter to include an explicit
language (for example change the opening fence to ```text) so the block becomes
```text … ```; locate the fenced block containing "feat / fix / style / refactor
/ ci / chore" in CLAUDE.md and add the language tag.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
src/assets/icons/delete_forever.svgis excluded by!**/*.svgsrc/assets/icons/info.svgis excluded by!**/*.svg
📒 Files selected for processing (11)
.claude/agents/figma-to-component.md.claude/agents/pr-writer.md.github/workflows/ci.ymlCLAUDE.mdamplify.ymlpackage.jsonsrc/app/(public)/(landing)/page.tsxsrc/app/globals.csssrc/components/ui/Button.tsxsrc/components/ui/alert-dialog.tsxsrc/components/ui/index.ts
Comment on lines
+32
to
+50
| - name: TypeScript | ||
| id: typescript | ||
| continue-on-error: true | ||
| run: pnpm typecheck | ||
|
|
||
| - name: ESLint | ||
| id: eslint | ||
| continue-on-error: true | ||
| run: pnpm lint | ||
|
|
||
| - name: Prettier | ||
| id: prettier | ||
| continue-on-error: true | ||
| run: pnpm format:check | ||
|
|
||
| - name: Build | ||
| id: build | ||
| continue-on-error: true | ||
| run: pnpm build |
There was a problem hiding this comment.
모든 단계가 continue-on-error: true이므로 CI가 실패를 차단하지 않습니다.
현재 TypeScript, ESLint, Prettier, Build 모든 단계에 continue-on-error: true가 설정되어 있어, 모든 검증이 실패하더라도 워크플로우는 항상 성공으로 표시됩니다. PR 병합을 차단하는 게이트 역할을 하지 못합니다.
워크플로우 마지막에 최종 판정 단계를 추가하세요:
💡 최종 실패 단계 추가 제안
+ - name: Fail if any check failed
+ if: always()
+ run: |
+ if [[ "${{ steps.typescript.outcome }}" == "failure" || \
+ "${{ steps.eslint.outcome }}" == "failure" || \
+ "${{ steps.prettier.outcome }}" == "failure" || \
+ "${{ steps.build.outcome }}" == "failure" ]]; then
+ echo "One or more checks failed"
+ exit 1
+ fi이 단계를 PR 코멘트 단계 뒤에 추가하면, 코멘트는 항상 작성되면서도 실패 시 워크플로우가 적절히 실패합니다.
📝 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
| - name: TypeScript | |
| id: typescript | |
| continue-on-error: true | |
| run: pnpm typecheck | |
| - name: ESLint | |
| id: eslint | |
| continue-on-error: true | |
| run: pnpm lint | |
| - name: Prettier | |
| id: prettier | |
| continue-on-error: true | |
| run: pnpm format:check | |
| - name: Build | |
| id: build | |
| continue-on-error: true | |
| run: pnpm build | |
| - name: TypeScript | |
| id: typescript | |
| continue-on-error: true | |
| run: pnpm typecheck | |
| - name: ESLint | |
| id: eslint | |
| continue-on-error: true | |
| run: pnpm lint | |
| - name: Prettier | |
| id: prettier | |
| continue-on-error: true | |
| run: pnpm format:check | |
| - name: Build | |
| id: build | |
| continue-on-error: true | |
| run: pnpm build | |
| - name: Fail if any check failed | |
| if: always() | |
| run: | | |
| if [[ "${{ steps.typescript.outcome }}" == "failure" || \ | |
| "${{ steps.eslint.outcome }}" == "failure" || \ | |
| "${{ steps.prettier.outcome }}" == "failure" || \ | |
| "${{ steps.build.outcome }}" == "failure" ]]; then | |
| echo "One or more checks failed" | |
| exit 1 | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/ci.yml around lines 32 - 50, Add a final "verdict" step
that runs unconditionally (if: always()) after the PR comment step and inspects
the outcomes of the earlier steps by their IDs (typescript, eslint, prettier,
build); if any of steps.typescript.outcome, steps.eslint.outcome,
steps.prettier.outcome, or steps.build.outcome is not "success", exit non‑zero
to fail the workflow (give the step an id like final-verdict and a clear error
message). This preserves the per-step continue-on-error behavior for
comment/logging but enforces a blocking CI failure if any check actually failed.
Comment on lines
+5
to
+11
| import Image from 'next/image'; | ||
|
|
||
| import { cn } from '@/lib/cn'; | ||
| import { Button, type ButtonProps } from '@/components/ui/Button'; | ||
|
|
||
| function AlertDialog({ ...props }: React.ComponentProps<typeof AlertDialogPrimitive.Root>) { | ||
| return <AlertDialogPrimitive.Root data-slot="alert-dialog" {...props} />; | ||
| import InfoIcon from '@/assets/icons/info.svg'; | ||
| import DeleteIcon from '@/assets/icons/delete_forever.svg'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
# 1) Check if SVG files exist
fd 'info\.svg|delete_forever\.svg'
# 2) Check for SVG type declarations
rg -n "declare module ['\"].*\\.svg['\"]"
# 3) Check lucide-react dependency
rg -n "\"lucide-react\"" package.jsonRepository: Team-Weeth/weeth-client
Length of output: 163
🏁 Script executed:
# Read the alert-dialog.tsx file to verify actual code state
wc -l src/components/ui/alert-dialog.tsxRepository: Team-Weeth/weeth-client
Length of output: 105
🏁 Script executed:
# Get the file to see lines 5-11 and around 127-139
head -20 src/components/ui/alert-dialog.tsxRepository: Team-Weeth/weeth-client
Length of output: 657
🏁 Script executed:
# Check full context around lines 127-139
sed -n '120,145p' src/components/ui/alert-dialog.tsxRepository: Team-Weeth/weeth-client
Length of output: 807
코딩 가이드라인: Lucide React 아이콘으로 변경 필요
현재 SVG 파일은 모두 존재하므로 빌드 문제는 없습니다. 다만 코딩 가이드라인에 따라 직접 SVG import 대신 lucide-react를 사용해야 합니다. 라인 10-11의 info.svg, delete_forever.svg import와 라인 131, 138의 사용 부분을 Lucide 아이콘으로 교체하세요.
-import Image from 'next/image';
-import InfoIcon from '@/assets/icons/info.svg';
-import DeleteIcon from '@/assets/icons/delete_forever.svg';
+import { Info, Trash2 } from 'lucide-react';- const Icon = status === 'danger' ? DeleteIcon : InfoIcon;
+ const Icon = status === 'danger' ? Trash2 : Info;- <Image src={Icon} alt="" width={48} height={48} />
+ <span className="bg-container-neutral-interaction text-icon-normal flex h-12 w-12 items-center justify-center rounded-full">
+ <Icon size={24} aria-hidden />
+ </span>🧰 Tools
🪛 GitHub Check: Lint & Build
[failure] 11-11:
Cannot find module '@/assets/icons/delete_forever.svg' or its corresponding type declarations.
[failure] 10-10:
Cannot find module '@/assets/icons/info.svg' or its corresponding type declarations.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/ui/alert-dialog.tsx` around lines 5 - 11, Replace the direct
SVG imports InfoIcon and DeleteIcon with lucide-react icon components: remove
"import InfoIcon from '@/assets/icons/info.svg'" and "import DeleteIcon from
'@/assets/icons/delete_forever.svg'" and instead import the proper lucide-react
icons (e.g., "import { Info, Trash2 } from 'lucide-react'"). Then update the JSX
usages that render <InfoIcon .../> and <DeleteIcon .../> (the instances
referenced in this file) to use <Info .../> and <Trash2 .../> (preserving
className, size, strokeWidth or other props passed through cn or Button props)
so styling and accessibility remain unchanged.
Comment on lines
+13
to
+38
| type AlertStatus = 'default' | 'danger'; | ||
|
|
||
| const AlertDialogContext = React.createContext<{ status: AlertStatus }>({ | ||
| status: 'default', | ||
| }); | ||
|
|
||
| const defaultTexts: Record<AlertStatus, { title: string; description: string }> = { | ||
| default: { | ||
| title: '변경 사항을 적용하시겠어요?', | ||
| description: "선택한 내용이 저장됩니다.\n진행하시려면 '확인'을 눌러주세요.", | ||
| }, | ||
| danger: { | ||
| title: '이 게시글을 삭제하시겠어요?', | ||
| description: '삭제된 게시글은 복구할 수 없습니다.\n신중히 확인 후 진행해 주세요.', | ||
| }, | ||
| }; | ||
|
|
||
| interface AlertDialogProps extends React.ComponentProps<typeof AlertDialogPrimitive.Root> { | ||
| status?: AlertStatus; | ||
| /** 간소화 모드: trigger 엘리먼트를 전달하면 내부에서 Content/Header/Footer를 자동 렌더링 */ | ||
| trigger?: React.ReactNode; | ||
| /** 기본값: status에 따라 자동 설정 */ | ||
| title?: string; | ||
| /** 기본값: status에 따라 자동 설정 */ | ||
| description?: string; | ||
| } |
There was a problem hiding this comment.
상태 변형(status)을 cva + VariantProps 패턴으로 통합해 주세요.
현재는 상태별 분기가 여러 위치에 흩어져 있어 확장 시 유지보수 비용이 큽니다. UI 컴포넌트 규약대로 variants/defaultVariants로 정리하는 편이 안전합니다.
리팩터링 방향 예시
+import { cva, type VariantProps } from 'class-variance-authority';
...
+const alertDialogVariants = cva('', {
+ variants: { status: { default: '', danger: '' } },
+ defaultVariants: { status: 'default' },
+});
+
-interface AlertDialogProps extends React.ComponentProps<typeof AlertDialogPrimitive.Root> {
- status?: AlertStatus;
+interface AlertDialogProps
+ extends React.ComponentProps<typeof AlertDialogPrimitive.Root>,
+ VariantProps<typeof alertDialogVariants> {
+ status?: AlertStatus;
...
}As per coding guidelines src/components/ui/**/*.{ts,tsx}: Implement component patterns using cva with variants, defaultVariants, and Props interface extending VariantProps and React.HTMLAttributes.
Also applies to: 179-188
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/ui/alert-dialog.tsx` around lines 13 - 38, The component
currently branches on AlertStatus across AlertStatus, defaultTexts, and
AlertDialogProps; refactor to use cva variants and VariantProps by replacing the
AlertStatus union and scattered defaultTexts with a cva() call that defines
variants (e.g., variant: { default, danger }) and defaultVariants, update the
props interface to extend VariantProps<typeof yourCva> and
React.ComponentProps<typeof AlertDialogPrimitive.Root>, and use the generated
variant classNames/props inside AlertDialogContext and the component to derive
title/description (move defaultTexts into variant defaults or a mapping keyed by
variant) so all status behavior is centralized in the cva variants
configuration.
Collaborator
There was a problem hiding this comment.
오호.. 이렇게 통합하는 게 유지 보수 시 더 좋겠네요!
There was a problem hiding this comment.
@JIN921 네, 맞습니다! cva 패턴으로 통합하면 variant가 추가될 때마다 한 곳에서만 관리할 수 있어서 훨씬 안전하고 명확해집니다. 😊
이 리팩토링을 제가 도와드릴까요? 원하신다면 ``를 통해 구체적인 변경 사항을 준비해드릴 수 있습니다. 아니면 직접 작업하시겠다면 제안드린 패턴을 참고하시면 됩니다!
∧＿∧
(◕‿◕)
/ づ💡 Great catch!
🧠 Learnings used
Learnt from: CR
Repo: Team-Weeth/weeth-client PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-20T05:00:54.945Z
Learning: Applies to src/components/ui/**/*.{ts,tsx} : Implement component patterns using cva with variants, defaultVariants, and Props interface extending VariantProps and React.HTMLAttributes
💡 For code changes across multiple files, use @coderabbitai plan in an issue comment
Comment on lines
+55
to
+57
| {trigger !== undefined ? ( | ||
| <> | ||
| <AlertDialogTrigger asChild>{trigger}</AlertDialogTrigger> |
There was a problem hiding this comment.
trigger={null} 케이스에서 asChild 런타임 오류가 날 수 있습니다.
undefined만 제외하면 null이 들어왔을 때도 간소화 모드로 진입합니다. null도 함께 배제해 주세요.
수정 제안
- {trigger !== undefined ? (
+ {trigger != null ? (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/ui/alert-dialog.tsx` around lines 55 - 57, The conditional
currently checks only for trigger !== undefined, which still allows trigger ===
null to render <AlertDialogTrigger asChild>{trigger}</AlertDialogTrigger> and
cause a runtime error; update the check to exclude null as well (e.g., use
trigger != null or trigger !== undefined && trigger !== null or
Boolean(trigger)) so that AlertDialogTrigger is only rendered when trigger is a
valid non-null value, referencing the trigger prop and
AlertDialogTrigger/asChild usage.
Comment on lines
+16
to
+17
| danger: | ||
| 'bg-[var(--state-error)] text-text-strong hover:opacity-90 active:opacity-80 disabled:bg-button-neutral disabled:text-text-disabled', |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check the Button.tsx file around lines 16-17
head -30 src/components/ui/Button.tsx | cat -nRepository: Team-Weeth/weeth-client
Length of output: 1603
🏁 Script executed:
# Check globals.css for state-error token registration
rg -n "state-error" src/app/globals.css -A 2 -B 2Repository: Team-Weeth/weeth-client
Length of output: 374
🏁 Script executed:
# Also check the `@theme` section in globals.css
rg -n "@theme" src/app/globals.css -A 10 -B 2Repository: Team-Weeth/weeth-client
Length of output: 756
🏁 Script executed:
# Check the complete `@theme` inline section to see all registered tokens
sed -n '275,350p' src/app/globals.css | cat -nRepository: Team-Weeth/weeth-client
Length of output: 3433
bg-[var(--state-error)] 대신 디자인 토큰 유틸리티 클래스 bg-state-error를 사용하세요.
globals.css의 @theme inline에 --color-state-error: var(--state-error)가 등록되어 있으므로, Tailwind v4가 자동으로 생성하는 bg-state-error 유틸리티 클래스를 직접 사용할 수 있습니다. 다른 variant들(bg-button-primary, bg-button-neutral, bg-button-primary-interaction 등)과의 일관성을 위해 arbitrary value 사용을 피하세요.
💡 제안
danger:
- 'bg-[var(--state-error)] text-text-strong hover:opacity-90 active:opacity-80 disabled:bg-button-neutral disabled:text-text-disabled',
+ 'bg-state-error text-text-strong hover:opacity-90 active:opacity-80 disabled:bg-button-neutral disabled:text-text-disabled',📝 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
| danger: | |
| 'bg-[var(--state-error)] text-text-strong hover:opacity-90 active:opacity-80 disabled:bg-button-neutral disabled:text-text-disabled', | |
| danger: | |
| 'bg-state-error text-text-strong hover:opacity-90 active:opacity-80 disabled:bg-button-neutral disabled:text-text-disabled', |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/ui/Button.tsx` around lines 16 - 17, In
src/components/ui/Button.tsx update the danger variant string (the 'danger' key
in the exported variants) to replace the arbitrary class bg-[var(--state-error)]
with the design-token utility class bg-state-error so it matches other variants
(e.g. bg-button-primary, bg-button-neutral) and avoids arbitrary values; keep
the rest of the class list (text-text-strong hover/active/disabled classes)
unchanged to preserve behavior.
dalzzy
changed the title
JIN921
approved these changes
Feb 26, 2026
Collaborator
JIN921
left a comment
JIN921
left a comment
There was a problem hiding this comment.
고생하셨습니다! 코드래빗 수정사항만 잘 적용하면 문제 없을 거 같아요! CI는 왜 자꾸.. 저러는지.. 일단 이전 pr에서 제가 다시 설정을 손 봐 놨으니까.. 그거 적용하고도 문제가 계속 생기면 설정을 다시 해 보겠습니다 ㅜ.ㅜ!!
| function AlertDialog({ ...props }: React.ComponentProps<typeof AlertDialogPrimitive.Root>) { | ||
| return <AlertDialogPrimitive.Root data-slot="alert-dialog" {...props} />; | ||
| import InfoIcon from '@/assets/icons/info.svg'; | ||
| import DeleteIcon from '@/assets/icons/delete_forever.svg'; |
Collaborator
There was a problem hiding this comment.
이거 빌드 에러 왜 뜰까요.....
Next.js가 next-env.d.ts를 통해 SVG 모듈을 자동 선언해주고 있어서, 파일만 있으면 TypeScript 타입 에러는 없습니다.
라고 하는데 로컬에서는 에러 안 나고 CI에서만 에러 나는 거면 경로에서 파일을 못 찾는 게 이유가 될 수 잇다구 해요,,
Comment on lines
+13
to
+38
| type AlertStatus = 'default' | 'danger'; | ||
|
|
||
| const AlertDialogContext = React.createContext<{ status: AlertStatus }>({ | ||
| status: 'default', | ||
| }); | ||
|
|
||
| const defaultTexts: Record<AlertStatus, { title: string; description: string }> = { | ||
| default: { | ||
| title: '변경 사항을 적용하시겠어요?', | ||
| description: "선택한 내용이 저장됩니다.\n진행하시려면 '확인'을 눌러주세요.", | ||
| }, | ||
| danger: { | ||
| title: '이 게시글을 삭제하시겠어요?', | ||
| description: '삭제된 게시글은 복구할 수 없습니다.\n신중히 확인 후 진행해 주세요.', | ||
| }, | ||
| }; | ||
|
|
||
| interface AlertDialogProps extends React.ComponentProps<typeof AlertDialogPrimitive.Root> { | ||
| status?: AlertStatus; | ||
| /** 간소화 모드: trigger 엘리먼트를 전달하면 내부에서 Content/Header/Footer를 자동 렌더링 */ | ||
| trigger?: React.ReactNode; | ||
| /** 기본값: status에 따라 자동 설정 */ | ||
| title?: string; | ||
| /** 기본값: status에 따라 자동 설정 */ | ||
| description?: string; | ||
| } |
Collaborator
There was a problem hiding this comment.
오호.. 이렇게 통합하는 게 유지 보수 시 더 좋겠네요!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
✅ PR 유형
어떤 변경 사항이 있었나요?
📌 관련 이슈번호
✅ Key Changes
📸 스크린샷 or 실행영상
2026-02-26.2.01.09.mov
🎸 기타 사항 or 추가 코멘트
유진님이 전에 작업하신거까지 올라와버렸는데 components/ui 안에 있는 파일들이랑 svg 파일들만 봐주시면 될 것 같습니다!!
피그마에 나와 있는 디자인대로 status별 기본 텍스트로 설정해두었습니다!
Default(기본 텍스트 사용 시)
Danger(기본 텍스트 사용 시)
텍스트 오버라이드
구조를 완전히 커스텀해야 하는 경우
trigger prop 없이 사용하면 기존 compound 패턴 그대로 동작합니다.