[FIX]: 주문취소 배송지 수정 동선 및 닫기 버튼 복귀 경로 정리#63
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Walkthrough쿼리 파라미터(returnTo, from, selectedAddressId)를 각 페이지·컴포넌트로 전파해 네비게이션 컨텍스트를 보존하고, CloseButton에 Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant OrderPage as NextServer-OrderPage
participant ServerAction as changeOrderShippingAddress
participant OrdersAPI as OrdersAPI/DB
Browser->>OrderPage: 요청 (params + searchParams.selectedAddressId)
OrderPage->>ServerAction: 호출(changeOrderShippingAddress(orderId, selectedAddressId))
ServerAction->>OrdersAPI: 배송지 업데이트 요청
OrdersAPI-->>ServerAction: 성공/실패 응답
ServerAction-->>OrderPage: 결과 반환
alt 성공
OrderPage-->>Browser: redirect to /orders/{orderId}
else 실패
OrderPage-->>Browser: redirect to /orders/{orderId}?addressUpdateError=1
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
next.config.ts (1)
3-4:⚠️ Potential issue | 🟡 MinorNext.js 16에서
cacheComponents: true누락 - PPR 활성화 필요
reactCompiler: true의 최상위 위치는 Next.js 16에서 올바릅니다. 다만 PPR(Partial Prerendering)을 활성화하려면cacheComponents: true를 추가해야 합니다. Next.js 16에서는experimental.ppr플래그가 제거되고 Cache Components 모델로 변경되었습니다.const nextConfig: NextConfig = { reactCompiler: true, cacheComponents: true, };
🤖 Fix all issues with AI agents
In `@src/app/orders/`[orderId]/cancel/_components/cancel-form.tsx:
- Around line 84-89: The current parsing sets selectedAddressId via
Number(selectedAddressIdParam) and treats any numeric value (including 0) as
valid because hasSelectedAddressId checks only !== null and !Number.isNaN;
update hasSelectedAddressId to also validate the ID range (e.g.,
selectedAddressId > 0) if your address IDs are 1-based. Locate
searchParams.get('selectedAddressId'), selectedAddressId and
hasSelectedAddressId in cancel-form.tsx and change the validation to require a
positive integer (or another app-specific minimum) so invalid "0" IDs are
rejected.
- Around line 82-95: The CancelForm component uses the client-only hook
useSearchParams and must be isolated with a Suspense boundary to avoid forcing
the whole page into client-side rendering; update the page.tsx where CancelForm
is rendered to wrap the <CancelForm orderDetail={orderDetail}
defaultAddress={defaultAddress} /> JSX in a <Suspense fallback={...}> (provide a
simple fallback like a spinner or loading div) so only the CancelForm component
is client-rendered while the rest of the page remains server/PPR-rendered.
Ensure you import Suspense from React and keep the existing CloseXButton and
surrounding layout unchanged.
In `@src/app/orders/`[orderId]/page.tsx:
- Around line 38-41: Wrap the call to changeOrderShippingAddress(numericId,
selectedAddressId) in a try/catch inside the page component so failures are
handled gracefully: call changeOrderShippingAddress in try, on success call
redirect(`/orders/${numericId}`), and in catch log the error and surface a
user-visible fallback (e.g., set an error state or trigger a toast/flash
message) instead of letting the exception bubble to a 500; reference
selectedAddressId, numericId, changeOrderShippingAddress, and redirect to locate
the code to modify.
In `@src/components/address/address-item.tsx`:
- Around line 23-29: AddressList uses the client hook useSearchParams but is
being rendered directly from the server component AddressListPage without a
React Suspense boundary; update AddressListPage to wrap the AddressList element
in a <Suspense fallback={...}> (import Suspense from react) so AddressList can
hydrate correctly when using useSearchParams; locate the AddressList usage in
AddressListPage and replace the direct <AddressList ... /> render with a
Suspense-wrapped version while keeping props (addresses, showSelectButton,
initialSelectedAddressId) unchanged.
In `@src/components/product/product-header.tsx`:
- Line 25: The ternary for backLink is redundant: update the backLink assignment
in product-header to either remove the needless branch and use backHref ||
'/category' when you don't need categoryID, or if the intent was to include the
category id, build the path using categoryID (e.g., backHref ||
`/category/${categoryID}`) so the variable actually affects the URL; modify the
expression that sets backLink accordingly (referencing backLink, backHref, and
categoryID).
🧹 Nitpick comments (8)
next.config.ts (1)
27-42: 새 remote pattern에pathname제한 없음기존 항목도 마찬가지이긴 하나, 외부 이미지 호스트에
pathname를 지정하지 않으면 해당 도메인의 모든 경로가 Image Optimization API를 통해 프록시됩니다. 실제 사용하는 경로 패턴(예:/data/images/**)이 명확하다면 축소하는 것이 보안 표면을 줄이는 데 도움됩니다.현재 동작에 문제는 없으므로 당장 필수는 아닙니다.
src/components/ui/close-button.tsx (2)
28-38:window.history.length > 1체크는 신뢰하기 어렵습니다.브라우저마다
history.length초기값이 다르고, 특히 새 탭에서 직접 URL로 진입하면 Chrome에서1이 반환됩니다. 이 경우fallbackHref가 없으면 Line 38의router.back()이 실행되어 앱 외부로 이탈할 수 있습니다.
fallbackHref를 필수로 만들거나, fallback 없이back()을 호출하는 Line 38 분기를 제거하는 것을 권장합니다.제안
- if (typeof window !== 'undefined' && window.history.length > 1) { - router.back(); - return; - } - - if (fallbackHref) { - router.replace(fallbackHref); + if (fallbackHref) { + router.replace(fallbackHref); return; } router.back();
history.length분기를 제거하고,fallbackHref가 있으면 우선 사용하도록 단순화하면 예측 가능한 동작이 됩니다.
41-75:CloseButton과CloseXButton은 아이콘만 다릅니다.두 컴포넌트의 로직이 동일하고 아이콘(
arrow.svgvsX.svg)과 alt 텍스트만 다릅니다. prop으로 variant를 받는 단일 컴포넌트로 통합하면 중복을 줄일 수 있습니다. 급하지 않으므로 추후 리팩터 대상으로 고려해 주세요.src/app/orders/[orderId]/page.tsx (1)
31-36:selectedAddressId파싱 — 간결하게 정리 가능동작은 정확하지만, 배열 처리 + Number 변환을 좀 더 간결하게 표현할 수 있습니다.
♻️ 리팩터 제안
- const selectedAddressIdParam = Array.isArray(query.selectedAddressId) - ? query.selectedAddressId[0] - : query.selectedAddressId; - const selectedAddressId = selectedAddressIdParam - ? Number(selectedAddressIdParam) - : null; + const raw = Array.isArray(query.selectedAddressId) + ? query.selectedAddressId[0] + : query.selectedAddressId; + const selectedAddressId = raw ? Number(raw) : null;src/components/product/product-list.tsx (1)
13-17: Named export — 코딩 가이드라인과 불일치 가능성
src/components/**/*.tsx파일에 대해export default function패턴이 요구되지만,ProductList는 named export를 사용하고 있습니다. 기존 패턴 유지가 의도적이라면 무시해도 됩니다.As per coding guidelines, "
src/components/**/*.tsx: export default function 패턴 (화살표 함수 금지)".src/app/product/[id]/page.tsx (1)
16-18:from파라미터 검증 부재 — open redirect 가능성 낮지만 방어 코딩 권장
searchParams.from은 사용자 제어 값으로,backHref를 통해router.push()에 전달됩니다. Next.js App Router는 내부 경로만 처리하므로 외부 URL로의 redirect 위험은 낮지만, 경로가/로 시작하는지 정도의 검증을 추가하면 더 안전합니다.🛡️ 방어 코딩 제안
const backHref = Array.isArray(resolvedSearchParams.from) ? resolvedSearchParams.from[0] : resolvedSearchParams.from; + const safeBackHref = + backHref && backHref.startsWith('/') ? backHref : undefined;이후
safeBackHref를ProductDetailView에 전달.src/app/orders/[orderId]/cancel/_components/cancel-form.tsx (2)
97-102:useState초기값이searchParams에서 한 번만 읽힘 — soft navigation 시 stale 가능성
useState의 초기값은 컴포넌트 마운트 시 한 번만 평가됩니다. 같은 라우트에서 쿼리 파라미터만 변경되는 soft navigation이 발생하면step과selectedReason이 갱신되지 않습니다.현재 플로우상
/address로 이동 후 돌아오면 컴포넌트가 재마운트될 가능성이 높지만,router.replace로 같은 경로의 쿼리만 변경하는 케이스가 추가될 경우 버그로 이어질 수 있습니다.searchParams변경에 반응하는useEffect로 동기화하거나, 현재 플로우에서 항상 재마운트됨을 확인해 두는 것이 안전합니다.♻️ 동기화 예시
- const [step, setStep] = useState<Step>( - searchParams.get('step') === 'address' ? 'address' : 'reason', - ); - const [selectedReason, setSelectedReason] = useState<string | null>( - searchParams.get('step') === 'address' ? 'WRONG_ADDRESS' : null, - ); + const stepParam = searchParams.get('step'); + const [step, setStep] = useState<Step>( + stepParam === 'address' ? 'address' : 'reason', + ); + const [selectedReason, setSelectedReason] = useState<string | null>( + stepParam === 'address' ? 'WRONG_ADDRESS' : null, + ); + + useEffect(() => { + if (stepParam === 'address') { + setStep('address'); + setSelectedReason('WRONG_ADDRESS'); + } + }, [stepParam]);
80-128: 컴포넌트 책임이 과도하게 큼 — 단계별 분리 권장
CancelForm이 12개 이상의useState, 4개의 조건부 렌더 분기(alert,complete,confirm,address,reason)를 가지고 있어 인지 복잡도가 높습니다. 각 단계를 별도 컴포넌트로 분리하고, 상위에서step상태만 관리하는 구조를 고려해 주세요.예:
CancelReasonStep,CancelAddressStep,CancelConfirmStep,CancelCompleteStep이번 PR 범위를 넘어서므로 후속 리팩터링으로 진행해도 무방합니다.
| const searchParams = useSearchParams(); | ||
| const orderId = orderDetail.id; | ||
|
|
||
| const [step, setStep] = useState<Step>('reason'); | ||
| const [selectedReason, setSelectedReason] = useState<string | null>(null); | ||
| const selectedAddressIdParam = searchParams.get('selectedAddressId'); | ||
| const selectedAddressId = selectedAddressIdParam | ||
| ? Number(selectedAddressIdParam) | ||
| : null; | ||
| const hasSelectedAddressId = | ||
| selectedAddressId !== null && !Number.isNaN(selectedAddressId); | ||
| const returnToOrderDetail = `/orders/${orderId}`; | ||
| const encodedReturnTo = encodeURIComponent(returnToOrderDetail); | ||
| const addressListHref = `/address?mode=select&returnTo=${encodedReturnTo}`; | ||
| const orderDetailHref = hasSelectedAddressId | ||
| ? `/orders/${orderId}?selectedAddressId=${selectedAddressId}` | ||
| : `/orders/${orderId}`; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the cancel page and check its structure
fd "page.tsx" --full-path --glob "**/cancel/page.tsx" --type f | head -1 | xargs cat -nRepository: IT-Cotato/12th-OnGil-FE
Length of output: 172
🏁 Script executed:
# Also check the cancel-form.tsx file to confirm 'use client' directive
fd "cancel-form.tsx" --full-path --type f | head -1 | xargs head -20 -nRepository: IT-Cotato/12th-OnGil-FE
Length of output: 161
🏁 Script executed:
# Check the directory structure around the cancel page
fd --full-path --glob "**/orders/*/cancel/**" --type f | grep -E "\.(tsx|ts)$" | head -20Repository: IT-Cotato/12th-OnGil-FE
Length of output: 167
🏁 Script executed:
# Read the cancel page to check Suspense boundary
cat -n src/app/orders/\[orderId\]/cancel/page.tsxRepository: IT-Cotato/12th-OnGil-FE
Length of output: 1930
🏁 Script executed:
# Read the first 30 lines of cancel-form.tsx to check for 'use client' directive
head -30 src/app/orders/\[orderId\]/cancel/_components/cancel-form.tsx | cat -nRepository: IT-Cotato/12th-OnGil-FE
Length of output: 1156
CancelForm 컴포넌트를 <Suspense>로 감싸기
CancelForm은 클라이언트 컴포넌트에서 useSearchParams()를 사용하는데, page.tsx에서 <Suspense> 경계 없이 렌더링되고 있습니다. 이로 인해 전체 취소 페이지가 클라이언트 사이드 렌더링으로 전환되어 PPR의 이점을 잃게 됩니다.
return (
<div className="mx-auto min-h-screen max-w-2xl bg-white px-5 pb-20 leading-normal">
<header className="relative flex items-center justify-center py-8">
<h1 className="text-3xl font-semibold">주문 취소</h1>
<div className="absolute right-5">
<CloseXButton fallbackHref="/orders" />
</div>
</header>
<Suspense fallback={<div>로딩 중...</div>}>
<CancelForm orderDetail={orderDetail} defaultAddress={defaultAddress} />
</Suspense>
</div>
);page.tsx의 CancelForm 렌더링 부분을 <Suspense>로 감싸서 동적 클라이언트 컴포넌트를 올바르게 격리하세요.
🤖 Prompt for AI Agents
In `@src/app/orders/`[orderId]/cancel/_components/cancel-form.tsx around lines 82
- 95, The CancelForm component uses the client-only hook useSearchParams and
must be isolated with a Suspense boundary to avoid forcing the whole page into
client-side rendering; update the page.tsx where CancelForm is rendered to wrap
the <CancelForm orderDetail={orderDetail} defaultAddress={defaultAddress} /> JSX
in a <Suspense fallback={...}> (provide a simple fallback like a spinner or
loading div) so only the CancelForm component is client-rendered while the rest
of the page remains server/PPR-rendered. Ensure you import Suspense from React
and keep the existing CloseXButton and surrounding layout unchanged.
Seoje1405
added
✨ FEATURE
♻️ REFACTOR
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/app/orders/`[orderId]/page.tsx:
- Around line 45-57: The issue is that redirect() throws internally and is being
caught by the catch, causing success paths to be treated as failures; update the
flow in the block that checks selectedAddressId/numericId so that you await
changeOrderShippingAddress(numericId, selectedAddressId) inside try/catch but
move the successful redirect(`/orders/${numericId}`) out of the try so the
thrown NEXT_REDIRECT won't be caught; keep the catch to log the error and
perform redirect(`/orders/${numericId}?addressUpdateError=1`) only on failure,
referencing changeOrderShippingAddress, redirect, selectedAddressId and
numericId to locate the code to change.
🧹 Nitpick comments (3)
src/app/address/page.tsx (1)
72-84: Suspense 경계 자체는 올바르나, fallback 메시지가 실제 동작과 불일치합니다.
AddressList가useSearchParams()를 사용하므로 Next.js App Router에서 Suspense 경계가 필요합니다 — 이 부분은 정확합니다. 다만resolvedAddresses는 이미 서버에서 fetch된 데이터를 props로 전달하고 있으므로, 이 Suspense fallback이 실제로 표시되는 시점은 데이터 로딩이 아닌 클라이언트 hydration 단계입니다. "배송지 목록을 불러오는 중입니다"라는 문구는 사용자에게 오해를 줄 수 있습니다.♻️ fallback 문구 수정 제안
<Suspense fallback={ <div className="flex h-60 items-center justify-center rounded-3xl border border-[`#bdbdbd`] bg-white text-gray-500"> - 배송지 목록을 불러오는 중입니다. + 잠시만 기다려 주세요. </div> } >Based on learnings: "Wrap any Dynamic Component (performing data fetching or using dynamic functions like
cookies(),headers()) with<Suspense>and a properfallback." —useSearchParams()도 동적 함수이므로 Suspense 래핑은 적절합니다.src/app/orders/[orderId]/page.tsx (1)
89-93: 에러 배너 접근성 개선 필요에러 배너에
role="alert"속성이 없어, 스크린 리더 사용자에게 에러 상태가 즉시 전달되지 않습니다.♿ 접근성 속성 추가
- <div className="mb-4 rounded-lg border border-red-200 bg-red-50 px-4 py-3 text-sm text-red-700"> + <div role="alert" className="mb-4 rounded-lg border border-red-200 bg-red-50 px-4 py-3 text-sm text-red-700">src/app/orders/[orderId]/cancel/_components/cancel-form.tsx (1)
99-104:useState초기값으로searchParams사용 — 리마운트 전제 하에 정상 동작.
useState는 초기값만 캡처하므로, 동일 컴포넌트 인스턴스 내에서 URL이 변경되면step/selectedReason이 동기화되지 않습니다. 현재 플로우에서는 페이지 전환 시 컴포넌트가 리마운트되므로 문제가 없지만, 향후 shallow routing 등으로 URL만 변경하는 패턴이 도입되면useSyncExternalStore또는useEffect기반 동기화가 필요할 수 있습니다.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/app/orders/`[orderId]/page.tsx:
- Around line 45-61: The page is performing a PATCH mutation during GET
rendering by calling changeOrderShippingAddress when selectedAddressId is
present, which risks duplicate side effects; instead move this mutation out of
the render path by converting to a Server Action or a route handler: create a
POST/PATCH endpoint (e.g., app/api/orders/[orderId]/address/route.ts) that
accepts selectedAddressId and performs changeOrderShippingAddress, then have the
page submit via a <form> or fetch to that route and handle redirects
client/server-side; remove the direct call to changeOrderShippingAddress from
the page and only read query params for displaying status, using redirect only
after the route/action completes.
- Around line 93-97: The error banner rendered when showAddressUpdateError is
true is missing an accessibility role; update the div in
src/app/orders/[orderId]/page.tsx that renders the error message (the JSX block
checking showAddressUpdateError) to include role="alert" so screen readers
announce the failure immediately; ensure the same element that contains the
message text (the rounded red div) receives the attribute.
| if ( | ||
| selectedAddressId && | ||
| Number.isInteger(selectedAddressId) && | ||
| selectedAddressId > 0 | ||
| ) { | ||
| let addressChanged = false; | ||
| try { | ||
| await changeOrderShippingAddress(numericId, selectedAddressId); | ||
| addressChanged = true; | ||
| } catch (error) { | ||
| console.error('주문 배송지 변경 실패:', error); | ||
| redirect(`/orders/${numericId}?addressUpdateError=1`); | ||
| } | ||
| if (addressChanged) { | ||
| redirect(`/orders/${numericId}`); | ||
| } | ||
| } |
There was a problem hiding this comment.
GET 요청(페이지 렌더) 중 PATCH 뮤테이션 실행 — 부작용 위험
서버 컴포넌트의 렌더 경로에서 changeOrderShippingAddress (PATCH)를 호출하고 있습니다. 이 URL에 selectedAddressId 쿼리가 포함된 채로 브라우저 프리페치, 크롤러 접근, 사용자 새로고침이 발생하면 의도치 않게 뮤테이션이 재실행됩니다. HTTP GET의 안전성(safety) 원칙에 위배됩니다.
권장 대안:
- Server Action +
<form>패턴으로 전환하여 POST 요청으로 뮤테이션을 처리하거나, - Route Handler (
app/api/orders/[orderId]/address/route.ts)를 통해 PATCH → redirect 흐름을 구성.
현재 구조에서 최소한의 방어책으로는, redirect 후 클린 URL로 돌아오므로 중복 실행 가능성은 낮지만, 네트워크 지연 중 새로고침 시 이중 호출이 가능합니다.
🤖 Prompt for AI Agents
In `@src/app/orders/`[orderId]/page.tsx around lines 45 - 61, The page is
performing a PATCH mutation during GET rendering by calling
changeOrderShippingAddress when selectedAddressId is present, which risks
duplicate side effects; instead move this mutation out of the render path by
converting to a Server Action or a route handler: create a POST/PATCH endpoint
(e.g., app/api/orders/[orderId]/address/route.ts) that accepts selectedAddressId
and performs changeOrderShippingAddress, then have the page submit via a <form>
or fetch to that route and handle redirects client/server-side; remove the
direct call to changeOrderShippingAddress from the page and only read query
params for displaying status, using redirect only after the route/action
completes.
| {showAddressUpdateError ? ( | ||
| <div className="mb-4 rounded-lg border border-red-200 bg-red-50 px-4 py-3 text-sm text-red-700"> | ||
| 배송지 변경에 실패했습니다. 잠시 후 다시 시도해주세요. | ||
| </div> | ||
| ) : null} |
There was a problem hiding this comment.
에러 배너에 role="alert" 누락 — 스크린 리더 접근성
배송지 변경 실패 시 표시되는 에러 메시지가 시각적으로만 전달됩니다. 스크린 리더 사용자에게 즉시 알림이 전달되도록 role="alert"를 추가하세요.
♿ 접근성 개선 제안
- <div className="mb-4 rounded-lg border border-red-200 bg-red-50 px-4 py-3 text-sm text-red-700">
+ <div role="alert" className="mb-4 rounded-lg border border-red-200 bg-red-50 px-4 py-3 text-sm text-red-700">📝 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.
| {showAddressUpdateError ? ( | |
| <div className="mb-4 rounded-lg border border-red-200 bg-red-50 px-4 py-3 text-sm text-red-700"> | |
| 배송지 변경에 실패했습니다. 잠시 후 다시 시도해주세요. | |
| </div> | |
| ) : null} | |
| {showAddressUpdateError ? ( | |
| <div role="alert" className="mb-4 rounded-lg border border-red-200 bg-red-50 px-4 py-3 text-sm text-red-700"> | |
| 배송지 변경에 실패했습니다. 잠시 후 다시 시도해주세요. | |
| </div> | |
| ) : null} |
🤖 Prompt for AI Agents
In `@src/app/orders/`[orderId]/page.tsx around lines 93 - 97, The error banner
rendered when showAddressUpdateError is true is missing an accessibility role;
update the div in src/app/orders/[orderId]/page.tsx that renders the error
message (the JSX block checking showAddressUpdateError) to include role="alert"
so screen readers announce the failure immediately; ensure the same element that
contains the message text (the rounded red div) receives the attribute.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/orders/order-list-card.tsx (2)
57-63:⚠️ Potential issue | 🟡 MinorImage intrinsic 크기와 렌더 크기 불일치
width={110} height={110}으로 1:1 비율을 선언하지만,className="h-40 w-30"은 전혀 다른 비율(약 3:4)입니다. Next.jsImage는width/height를 aspect ratio 힌트 및 placeholder 크기로 사용하므로, CLS(Cumulative Layout Shift)가 발생할 수 있습니다. intrinsic 크기를 실제 렌더 비율에 맞추거나fill+ container sizing 패턴을 사용하세요.💡 fill 패턴 예시
- <Image - src={repItem?.productImage || '/placeholder.png'} - alt={repItem?.productName || '상품 이미지'} - width={110} - height={110} - className="h-40 w-30 shrink-0 object-contain" - /> + <div className="relative h-40 w-30 shrink-0"> + <Image + src={repItem?.productImage || '/placeholder.png'} + alt={repItem?.productName || '상품 이미지'} + fill + className="object-contain" + /> + </div>
106-122:⚠️ Potential issue | 🟠 Major커스텀 Alert 모달 접근성(a11y) 누락
현재 모달에
role="dialog",aria-modal="true",aria-label등이 없어 스크린 리더 사용자가 컨텍스트를 인지할 수 없습니다. 또한 포커스 트랩이 없어 키보드 네비게이션 시 모달 뒤의 요소로 포커스가 이동합니다. 백드롭 클릭으로 닫히지 않는 것도 기대 동작과 다릅니다.최소한 다음을 추가해 주세요:
♿ 접근성 개선 제안
- <div className="fixed inset-0 z-50 flex items-center justify-center bg-black/50"> - <div className="mx-5 w-full max-w-md rounded-2xl bg-white p-6"> + <div + className="fixed inset-0 z-50 flex items-center justify-center bg-black/50" + onClick={() => setShowAlert(false)} + > + <div + role="dialog" + aria-modal="true" + aria-label="주문 취소 안내" + className="mx-5 w-full max-w-md rounded-2xl bg-white p-6" + onClick={(e) => e.stopPropagation()} + >
🤖 Fix all issues with AI agents
In `@src/components/orders/order-list-card.tsx`:
- Around line 80-84: The "상품 문의하기" button in order-list-card.tsx is clickable
but has no handler; update the <button> (the one rendering "상품 문의하기 >") to
include type="button" and either wire it to the intended handler (e.g.,
onClick={openInquiryModal} or onClick={() => navigateTo('/inquiry')}—add/rename
the function in the component) or, if the feature isn't implemented yet, mark it
explicitly non-interactive: add disabled and aria-disabled="true" plus a visual
disabled style (e.g., add cursor-not-allowed/opacity classes) so it's not a dead
UI element.
In `@src/types/enums.ts`:
- Around line 26-30: OrderStatus enum has inconsistent value for ORDER_CONFIRMED
(key is ORDER_CONFIRMED but value is 'CONFIRMED'); update the enum so all
members follow the KEY = 'KEY' pattern (e.g., set ORDER_CONFIRMED =
'ORDER_CONFIRMED') or, if the backend truly returns 'CONFIRMED', add an explicit
mapping/transform in the component/service that consumes OrderStatus
(referencing OrderStatus and ORDER_CONFIRMED) so the codebase is consistent and
maintainable.
🧹 Nitpick comments (2)
src/components/orders/order-list-card.tsx (2)
22-29:order.items가 빈 배열일 경우 방어 처리 부재
order.items[0]이undefined일 때 Line 46의order.orderDate.split('T')[0]은 괜찮지만, Line 72의order.totalAmount.toLocaleString()과 이미지 영역이 빈 상태로 렌더됩니다.repItem이 없을 때의 fallback UI 또는 early return을 고려해 주세요.
26-30:canCancel로직과 alert 메시지 — 향후 상태 추가 시 취약
canCancel은ORDER_RECEIVED일 때만true이므로,CANCELED와ORDER_CONFIRMED모두 alert를 표시합니다. 현재 alert 메시지는CANCELED와 그 외(→ "주문 확정 상태")만 분기하므로, 향후SHIPPING등의 상태가 추가되면 메시지가 맞지 않습니다. 상태별 메시지 맵이나 exhaustive 분기를 권장합니다.
| <div className="my-8 flex justify-end"> | ||
| <button className="flex items-center leading-[18px] font-medium text-[#999999] transition-colors"> | ||
| 상품 문의하기 > | ||
| </button> | ||
| </div> |
There was a problem hiding this comment.
"상품 문의하기" 버튼에 onClick 핸들러 누락 — 데드 UI
<button>이 클릭 가능한 스타일이지만 아무런 동작도 바인딩되어 있지 않습니다. 사용자에게 혼란을 줄 수 있으므로, 아직 기능이 구현되지 않았다면 disabled 처리하거나 해당 요소를 제거하세요. 또한 type="button"을 명시하는 것이 PR 목표(폼 내부 오동작 방지)와도 일관됩니다.
💡 수정 제안
- <button className="flex items-center leading-[18px] font-medium text-[`#999999`] transition-colors">
+ <button
+ type="button"
+ disabled
+ className="flex items-center leading-[18px] font-medium text-[`#999999`] transition-colors disabled:opacity-50"
+ >
상품 문의하기 >
</button>📝 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.
| <div className="my-8 flex justify-end"> | |
| <button className="flex items-center leading-[18px] font-medium text-[#999999] transition-colors"> | |
| 상품 문의하기 > | |
| </button> | |
| </div> | |
| <div className="my-8 flex justify-end"> | |
| <button | |
| type="button" | |
| disabled | |
| className="flex items-center leading-[18px] font-medium text-[`#999999`] transition-colors disabled:opacity-50" | |
| > | |
| 상품 문의하기 > | |
| </button> | |
| </div> |
🤖 Prompt for AI Agents
In `@src/components/orders/order-list-card.tsx` around lines 80 - 84, The "상품
문의하기" button in order-list-card.tsx is clickable but has no handler; update the
<button> (the one rendering "상품 문의하기 >") to include type="button" and either
wire it to the intended handler (e.g., onClick={openInquiryModal} or onClick={()
=> navigateTo('/inquiry')}—add/rename the function in the component) or, if the
feature isn't implemented yet, mark it explicitly non-interactive: add disabled
and aria-disabled="true" plus a visual disabled style (e.g., add
cursor-not-allowed/opacity classes) so it's not a dead UI element.
| export enum OrderStatus { | ||
| ORDER_RECEIVED = 'ORDER_RECEIVED', | ||
| ORDER_CONFIRMED = 'CONFIRMED', | ||
| CANCELED = 'CANCELED', | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for usage of 'CONFIRMED' string literal
echo "=== Searching for 'CONFIRMED' string usage ==="
rg -n "CONFIRMED" --type ts --type tsx -C 2
echo ""
echo "=== Searching for OrderStatus enum imports and usage ==="
rg -n "OrderStatus" --type ts --type tsx -C 2
echo ""
echo "=== Searching for enum value assignments ==="
rg -n "ORDER_RECEIVED|ORDER_CONFIRMED|CANCELED" --type ts --type tsxRepository: IT-Cotato/12th-OnGil-FE
Length of output: 318
🏁 Script executed:
#!/bin/bash
# List TypeScript files in the repository to understand structure
echo "=== Finding TypeScript/TSX files ==="
fd -e ts -e tsx | head -20
echo ""
echo "=== Search for 'CONFIRMED' string ==="
rg -n "CONFIRMED"
echo ""
echo "=== Search for OrderStatus usage ==="
rg -n "OrderStatus" -A 2 -B 2
echo ""
echo "=== Search for backend API responses or test files ==="
fd -e ts -e tsx | xargs rg -l "ORDER_CONFIRMED\|'CONFIRMED'" 2>/dev/null || echo "No matches found"Repository: IT-Cotato/12th-OnGil-FE
Length of output: 3521
🏁 Script executed:
#!/bin/bash
echo "=== Checking order.ts domain types for API response ==="
cat -n src/types/domain/order.ts
echo ""
echo "=== Checking order actions for API calls ==="
head -100 src/app/actions/order.ts
echo ""
echo "=== Search for any response transformation or mocking ==="
rg -n "ORDER_RECEIVED|ORDER_CONFIRMED|CANCELED" -A 3 -B 3 src/app/actions/
echo ""
echo "=== Check for test files or mock data ==="
fd "\.test\.|\.spec\.|mock|fixture" -e ts -e tsxRepository: IT-Cotato/12th-OnGil-FE
Length of output: 6468
열거형 값 네이밍 불일치 개선 필요
ORDER_RECEIVED = 'ORDER_RECEIVED', CANCELED = 'CANCELED' 패턴과 달리 ORDER_CONFIRMED = 'CONFIRMED'로 정의되어 있습니다. 컴포넌트에서 별도의 변환 없이 직접 사용되므로 백엔드가 'CONFIRMED' 값을 반환하는 것으로 보이지만, 패턴 불일치는 유지보수성을 해칩니다. 모든 열거형 멤버를 KEY = 'KEY' 패턴으로 통일하거나, 백엔드 스펙이 다르다면 변환 로직을 명시적으로 추가하세요.
🤖 Prompt for AI Agents
In `@src/types/enums.ts` around lines 26 - 30, OrderStatus enum has inconsistent
value for ORDER_CONFIRMED (key is ORDER_CONFIRMED but value is 'CONFIRMED');
update the enum so all members follow the KEY = 'KEY' pattern (e.g., set
ORDER_CONFIRMED = 'ORDER_CONFIRMED') or, if the backend truly returns
'CONFIRMED', add an explicit mapping/transform in the component/service that
consumes OrderStatus (referencing OrderStatus and ORDER_CONFIRMED) so the
codebase is consistent and maintainable.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/address/page.tsx (1)
28-32:⚠️ Potential issue | 🟡 Minor
returnTo값에 대한 경로 검증 부재.
isSelectMode일 때params.returnTo가 그대로closeHref로 전달됩니다.address/new/page.tsx에서는returnTo?.startsWith('/')검증을 하고 있는데, 이 파일에서는 검증 없이 사용합니다. 일관성과 안전성을 위해 동일한 패턴을 적용하세요.제안
const closeHref = isManageMode ? '/me/edit' : isSelectMode - ? params.returnTo + ? (params.returnTo?.startsWith('/') ? params.returnTo : '/address') : undefined;
🤖 Fix all issues with AI agents
In `@src/app/orders/`[orderId]/cancel/_components/cancel-form.tsx:
- Around line 84-101: The component currently initializes step (and similarly
selectedReason) with useState once, so subsequent searchParams changes can leave
UI out-of-sync; update the code to synchronize state with searchParams by either
(A) replacing the useState-driven step/selectedReason with values derived
directly from searchParams (i.e., compute step = searchParams.get('step') ===
'address' ? 'address' : 'reason' and read selectedReason from searchParams each
render), or (B) keep the useState hooks (step, setStep and selectedReason,
setSelectedReason) but add a useEffect that watches searchParams (and relevant
params like selectedAddressId) and calls setStep(...) and setSelectedReason(...)
to update state whenever searchParams changes so the UI follows browser
navigation/popstate.
🧹 Nitpick comments (8)
src/app/address/page.tsx (1)
72-84:Suspense경계가useSearchParams()보호용으로 적절합니다.
AddressList가 내부에서useSearchParams()를 사용하므로 Next.js 정적 렌더링 시 이 Suspense 경계가 필요합니다. 다만 fallback 메시지("배송지 목록을 불러오는 중입니다")는 실제로는useSearchParams때문에 잠깐 노출될 수 있는데, 이미 서버에서resolvedAddresses를 fetch 완료한 상태이므로 사용자 입장에서 "불러오는 중"이라는 문구가 오해를 줄 수 있습니다.좀 더 정확한 fallback(예: 스피너 또는 스켈레톤)을 권장합니다.
src/app/address/new/page.tsx (1)
19-19:replace={true}— 불필요한={true}생략 가능.JSX에서 boolean prop은 이름만 전달해도
true로 평가됩니다.제안
- <CloseXButton href={closeHref} replace={true} /> + <CloseXButton href={closeHref} replace />src/app/orders/[orderId]/page.tsx (3)
86-91:header내absolute포지셔닝 주의Line 89의
absolute right-5는 부모header에position: relative가 없으므로 뷰포트 또는 가장 가까운 positioned ancestor 기준으로 배치됩니다.header에relative를 추가하거나, flexbox 레이아웃으로 닫기 버튼 위치를 잡는 것이 더 안정적입니다.♻️ flexbox 기반 레이아웃 제안
- <header className="flex items-center justify-center py-8"> + <header className="relative flex items-center justify-center py-8"> <h1 className="text-3xl font-semibold">주문 상세</h1> - <div className="absolute right-5"> + <div className="absolute right-0">
17-20:searchParams타입을 제네릭 유틸로 좁히는 것을 고려해 보세요현재
string | string[]유니온은 Next.js 15의 정확한 타입이지만, 여러 페이지에서 동일 패턴이 반복될 수 있습니다. 프로젝트 전반에SearchParam<T extends string>같은 유틸 타입을 두면 일관성이 높아집니다.// e.g., types/next.ts type SearchParamValue = string | string[] | undefined; type SearchParams<T extends string> = Promise<Partial<Record<T, SearchParamValue>>>;지금 당장 필요한 건 아니지만 페이지가 늘어나면 유용합니다.
45-49:selectedAddressId검증에서 첫 번째 truthiness 체크는 불필요
selectedAddressId가number | null이므로Number.isInteger(selectedAddressId) && selectedAddressId > 0만으로null,NaN,0, 소수점 값을 모두 걸러냅니다. 앞의selectedAddressId &&는 중복 조건입니다.♻️ 조건 단순화
if ( - selectedAddressId && Number.isInteger(selectedAddressId) && selectedAddressId > 0 ) {src/components/product/product-header.tsx (1)
29-33: CloseButton 분기를 단일 호출로 단순화 가능
backHref유무에 따라 두 개의<CloseButton>을 렌더링하고 있지만, props를 조건부로 넘기면 하나로 통합됩니다.CloseButton내부navigateByMode가href우선 →fallbackHref폴백 순서로 동작하므로 동일한 결과입니다.♻️ 단순화 제안
- {backHref ? ( - <CloseButton href={backHref} replace={true} /> - ) : ( - <CloseButton fallbackHref={fallback} /> - )} + <CloseButton + href={backHref} + replace={!!backHref} + fallbackHref={fallback} + />
href가undefined이면navigateByMode가fallbackHref로 폴백하므로 동작이 동일합니다. 분기 제거로 가독성이 올라갑니다.#!/bin/bash # CloseButton의 navigateByMode 로직 확인 ast-grep --pattern $'function navigateByMode($$$) { $$$ }'src/components/product-option-sheet/use-product-option.ts (1)
168-172:from파라미터 이중 인코딩 및 중첩 가능성 확인
searchParams에 이미from파라미터가 포함된 상태에서 다시from을 설정하면 중첩 경로가 점점 길어질 수 있습니다. 현재 동선에서는 문제가 없을 수 있지만,from을 재귀적으로 전파하지 않도록 원본from만 보존하는 방어 로직을 고려해볼 수 있습니다.♻️ 원본 from 보존 제안
- const currentSearch = searchParams.toString(); - const currentPath = currentSearch - ? `${pathname}?${currentSearch}` - : pathname; - params.set('from', currentPath); + const originalFrom = searchParams.get('from'); + params.set('from', originalFrom ?? pathname);이 변경이 의도된 동선과 맞는지 확인 후 적용하세요. 만약 결제 → 뒤로가기 시 상품 상세의 전체 상태(쿼리 포함)를 복원해야 한다면 현재 구현이 맞습니다.
src/app/orders/[orderId]/cancel/_components/cancel-form.tsx (1)
152-188: Effect 의존성 배열에 파생값 포함
hasSelectedAddressId는selectedAddressId에서 파생된 값이므로 의존성 배열에 둘 다 포함할 필요가 없습니다. 동작에는 영향이 없지만, 의도를 명확히 하려면 하나로 통일하는 편이 깔끔합니다.- }, [step, addressReloadTick, hasSelectedAddressId, selectedAddressId]); + }, [step, addressReloadTick, selectedAddressId]);
| const selectedAddressIdParam = searchParams.get('selectedAddressId'); | ||
| const selectedAddressId = selectedAddressIdParam | ||
| ? Number(selectedAddressIdParam) | ||
| : null; | ||
| const hasSelectedAddressId = | ||
| selectedAddressId !== null && | ||
| Number.isInteger(selectedAddressId) && | ||
| selectedAddressId > 0; | ||
| const returnToCancelAddressStep = `/orders/${orderId}/cancel?step=address`; | ||
| const encodedReturnTo = encodeURIComponent(returnToCancelAddressStep); | ||
| const addressListHref = `/address?mode=select&returnTo=${encodedReturnTo}`; | ||
| const orderDetailHref = hasSelectedAddressId | ||
| ? `/orders/${orderId}?selectedAddressId=${selectedAddressId}` | ||
| : `/orders/${orderId}`; | ||
|
|
||
| const [step, setStep] = useState<Step>( | ||
| searchParams.get('step') === 'address' ? 'address' : 'reason', | ||
| ); |
There was a problem hiding this comment.
useState 초기값이 searchParams 변경과 동기화되지 않음
useState의 초기값은 마운트 시 한 번만 평가됩니다. 같은 라우트 세그먼트 내에서 클라이언트 사이드 내비게이션(예: 뒤로가기)으로 searchParams가 변경되면 step과 selectedReason state는 갱신되지 않습니다.
현재 유저 플로우(배송지 목록 → 외부 페이지 경유 → 복귀)에서는 컴포넌트가 재마운트될 가능성이 높아 실질적 문제가 드물지만, 브라우저 popstate 등 엣지 케이스에서 URL과 UI 상태가 불일치할 수 있습니다.
searchParams 변경 시 state를 동기화하는 useEffect를 추가하거나, step을 state 대신 직접 searchParams에서 파생하는 방식을 고려해 주세요.
예시: searchParams 기반 파생
- const [step, setStep] = useState<Step>(
- searchParams.get('step') === 'address' ? 'address' : 'reason',
- );
- const [selectedReason, setSelectedReason] = useState<string | null>(
- searchParams.get('step') === 'address' ? 'WRONG_ADDRESS' : null,
- );
+ const urlStep = searchParams.get('step');
+ const [step, setStep] = useState<Step>(
+ urlStep === 'address' ? 'address' : 'reason',
+ );
+ const [selectedReason, setSelectedReason] = useState<string | null>(
+ urlStep === 'address' ? 'WRONG_ADDRESS' : null,
+ );
+
+ useEffect(() => {
+ if (urlStep === 'address') {
+ setStep('address');
+ setSelectedReason('WRONG_ADDRESS');
+ }
+ }, [urlStep]);🤖 Prompt for AI Agents
In `@src/app/orders/`[orderId]/cancel/_components/cancel-form.tsx around lines 84
- 101, The component currently initializes step (and similarly selectedReason)
with useState once, so subsequent searchParams changes can leave UI out-of-sync;
update the code to synchronize state with searchParams by either (A) replacing
the useState-driven step/selectedReason with values derived directly from
searchParams (i.e., compute step = searchParams.get('step') === 'address' ?
'address' : 'reason' and read selectedReason from searchParams each render), or
(B) keep the useState hooks (step, setStep and selectedReason,
setSelectedReason) but add a useEffect that watches searchParams (and relevant
params like selectedAddressId) and calls setStep(...) and setSelectedReason(...)
to update state whenever searchParams changes so the UI follows browser
navigation/popstate.
1 participant
📝 개요
주문 취소 화면의
배송지 잘못 입력분기에서 발생하던 이동 경로 불일치를 정리하고,배송지 선택/수정 결과가 실제 주문 상세에 반영되는 흐름으로 일관화한 작업입니다.
기존에는 주소 수정/선택 이후에도 취소 플로우로 되돌아가거나, 닫기 버튼(
X) 동작이 화면마다 달라 사용자가 맥락을 잃는 문제가 있었습니다.이번 PR은
선택 → 반영 → 결과 확인의 흐름을 기준으로 내비게이션을 재설계했습니다.관련 이슈 번호: #이슈번호
🎯 변경 의도
배송지 잘못 입력분기의 본질은 “취소 진행”이 아니라 “배송지 수정 반영”이므로, 최종 도착지를 주문 상세로 통일returnTo를 일관되게 전달🚀 주요 변경 사항
1) 주문 취소 → 배송지 변경 플로우 정리
배송지 잘못 입력선택 시 배송지 목록(mode=select)으로 이동returnTo를/orders/[orderId]로 설정selectedAddressId가 존재하면 주소 카드에 선택 주소를 우선 반영2) 주문 상세에서 배송지 반영 처리
/orders/[orderId]?selectedAddressId=...진입 시 주문 배송지 변경 API 실행revalidatePath ... during render오류 제거3) 배송지 화면 간 컨텍스트 유지
returnTo로 전달mode/returnTo컨텍스트 유지X는returnTo우선 복귀returnTo가 없을 경우 안전한 기본 경로로 복귀4) 주문 취소 완료 화면 정리
hideActions옵션 적용5) 공용 Close 버튼 정책 정리
href,replace,fallbackHref조합으로 이동 규칙 정리type="button"명시로 form 내부 오동작 방지🧪 테스트/검증 포인트
주문 취소
/orders/[orderId]/cancel진입배송지 잘못 입력선택 후 배송지 목록 이동 확인선택 완료시 주문 상세 이동 확인X클릭 시 back 우선/fallback 동작 확인배송지
X복귀 시 목록 컨텍스트 유지 확인mode,returnTo) 유지 확인완료 화면
공통
src/app/test/page.tsx)는 커밋 대상에서 제외했습니다.next.config.ts보정은 별도chore커밋으로 분리했습니다.✅ 체크리스트
이슈 해결 여부
Closes #이슈번호