Conversation
📝 WalkthroughWalkthrough생성 페이지, 버튼, 카드 컴포넌트의 CSS 스타일과 DOM 구조를 개선합니다. 레이아웃 제약을 유연하게 조정하고, 스타일 exports를 추가하며, 중첩된 DOM 구조로 선택자 정확도를 높이고, 데이터 정규화 로직을 도입합니다. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
빌드 결과빌드 성공 🎊 |
🎨 Storybook 빌드 완료!📚 Storybook: https://686a831b8e000345a949970a-ofmzugfwrs.chromatic.com/ 📊 빌드 정보
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7a31daf53f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| furnitureProductOriginalPrice: | ||
| originalPrice ?? RESULT_CARD_UI_FALLBACK.originalPrice, | ||
| furnitureProductDiscountPrice: | ||
| discountPrice ?? RESULT_CARD_UI_FALLBACK.discountPrice, | ||
| furnitureProductDiscountRate: | ||
| discountRate ?? RESULT_CARD_UI_FALLBACK.discountRate, | ||
| furnitureProductColorHexes: normalizeColorHexes( | ||
| product.furnitureProductColorHexes | ||
| ) | ||
| ? product.furnitureProductColorHexes | ||
| : undefined, | ||
| furnitureProductSaveCount: Number.isFinite(saveCount) | ||
| ? saveCount | ||
| : undefined, | ||
| ), | ||
| furnitureProductSaveCount: | ||
| saveCount ?? RESULT_CARD_UI_FALLBACK.saveCount, |
There was a problem hiding this comment.
When the API omits price/discount/saveCount fields, this mapping now replaces them with 0. In CardProduct the price block renders whenever a numeric value exists and the save-count row renders for any finite number, so missing data will now appear as “0원/0%/0” instead of being hidden. This is misleading for products with incomplete data and is a behavioral regression compared to the previous undefined handling. Consider preserving undefined (or a distinct sentinel) for missing values so the UI stays blank rather than implying a real zero value.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
src/shared/components/card/cardProduct/CardProduct.tsx (1)
120-124: 🧹 Nitpick | 🔵 Trivial접근성 개선 제안:
alt텍스트에 상품명 포함현재
alt="카드 이미지"는 모든 카드에서 동일한 generic 텍스트입니다. 스크린 리더 사용자에게 의미 있는 정보를 제공하려면 상품명을 포함하는 것이 좋습니다. 이번 PR 범위 밖일 수 있으나 후속 개선으로 고려해 주세요.♿ 제안
<img className={styles.cardImage({ loaded: isLoaded })} src={imageUrl || CardImage} - alt="카드 이미지" + alt={`${title} 상품 이미지`} onLoad={() => setIsLoaded(true)} />src/pages/generate/pages/result/curationSheet/CurationSheet.tsx (2)
243-249: 🧹 Nitpick | 🔵 Trivial주석 처리된
LoadingDots코드 제거를 권장해요.사용하지 않는 주석 코드(Lines 243-249, 267)가 남아 있어요. 나중에 필요하면 git history에서 복원할 수 있으니, 정리해 주시면 좋겠어요.
128-131:⚠️ Potential issue | 🟡 Minor
furnitureProductIdfallback으로index + 1을 사용하면 React key가 불안정해질 수 있어요.
furnitureProductId가 유효하지 않을 때index + 1로 대체하면, 목록 순서가 바뀌거나 항목이 추가/삭제될 때 React reconciliation 문제가 발생할 수 있어요. 빈도가 낮은 edge case이긴 하지만, 고유 식별이 보장되지 않으면 렌더링 이상이 생길 수 있으니 주의해 주세요.Based on learnings, "React keys for furniture product lists should use the stable
furnitureProductIdrather than the ephemeralp.id(recommendId)."src/shared/components/button/saveButton/SaveButton.tsx (3)
14-21:⚠️ Potential issue | 🟠 Major
{...props}스프레드가className을 덮어쓸 수 있어요
SaveButtonProps가React.ComponentProps<'button'>을 확장하므로, 소비자가className을 전달하면 Line 21의{...props}가 Line 18-20의className을 완전히 덮어써서buttonWrapper/selected/unselected스타일이 모두 사라져요. 공유 컴포넌트라서 외부에서 className을 전달할 가능성이 있어요.🐛 className 병합 처리
-const SaveButton = ({ isSelected, onClick, ...props }: SaveButtonProps) => { +const SaveButton = ({ + isSelected, + onClick, + className, + ...props +}: SaveButtonProps) => { + const stateStyle = isSelected + ? styles.selected + : styles.unselected; return ( <button type="button" onClick={onClick} aria-pressed={isSelected} - className={`${styles.buttonWrapper} ${ - isSelected ? styles.selected : styles.unselected - }`} + className={`${styles.buttonWrapper} ${stateStyle}${ + className ? ` ${className}` : '' + }`} {...props} > <SaveIcon /> </button> ); };
7-10: 🧹 Nitpick | 🔵 Trivial
onClick이ComponentProps<'button'>과 중복 선언됨
React.ComponentProps<'button'>에는 이미onClick이 포함되어 있어요. 여기서onClick: () => void로 다시 선언하면 원래의MouseEventHandler<HTMLButtonElement>시그니처를 좁혀서, 이벤트 객체(e)에 접근할 수 없게 돼요. 의도적이라면 괜찮지만, 확장성을 위해 부모 타입을 그대로 쓰는 편이 나아요.♻️ 제안
interface SaveButtonProps extends React.ComponentProps<'button'> { isSelected: boolean; - onClick: () => void; }
12-26: 🧹 Nitpick | 🔵 Trivial공유 컴포넌트에
React.memo및forwardRef적용 검토이 컴포넌트는 카드 리스트 내에서 반복 렌더링될 수 있어요.
React.memo로 불필요한 리렌더를 방지하고,ref전달이 필요한 경우를 대비해forwardRef도 함께 적용하면 좋겠어요.As per coding guidelines: "성능 최적화 (memo, useMemo, useCallback)" 및 "ref 전달 시 forwardRef 사용 및 displayName 지정"
src/pages/generate/pages/result/components/GeneratedImgA.tsx (1)
198-208:⚠️ Potential issue | 🟡 Minor버튼
disabled와 아이콘 조건이 불일치해요.Line 202의
disabled조건은!swiper || currentSlideIndex === 0이지만, Line 206의 아이콘 조건은currentSlideIndex === 0만 확인합니다.swiper가 아직null인 초기 상태에서currentSlideIndex가 0이 아닌 경우(가능성 낮지만), 버튼은 disabled인데 활성 아이콘이 보일 수 있어요.Next 버튼(Line 252-265)도 동일한 패턴입니다.
🔧 아이콘 조건을 버튼 disabled 조건과 동기화하는 제안
- <button - type="button" - onClick={() => swiper?.slidePrev()} - className={styles.slidePrevBtn} - disabled={!swiper || currentSlideIndex === 0} - > - <span className={styles.slideBtnCircle} aria-hidden /> - <span className={styles.slideBtnIcon} aria-hidden> - {currentSlideIndex === 0 ? <SlidePrevDisabled /> : <SlidePrev />} - </span> - </button> + <button + type="button" + onClick={() => swiper?.slidePrev()} + className={styles.slidePrevBtn} + disabled={!swiper || currentSlideIndex === 0} + > + <span className={styles.slideBtnCircle} aria-hidden="true" /> + <span className={styles.slideBtnIcon} aria-hidden="true"> + {!swiper || currentSlideIndex === 0 ? ( + <SlidePrevDisabled /> + ) : ( + <SlidePrev /> + )} + </span> + </button>src/pages/generate/pages/result/components/GeneratedImg.css.ts (1)
106-152: 🧹 Nitpick | 🔵 Trivial버튼 위치의 매직 넘버에 주석을 달아주세요.
top: 'calc(50% + 0.8rem)'에서0.8rem오프셋이 슬라이드 번호 인디케이터 높이를 보정하는 것으로 보이는데, 의도를 주석으로 남기면 이후 유지보수에 도움이 됩니다.또한,
slidePrevBtn과slideNextBtn은left/right만 다르고 나머지가 동일하므로, 공통 베이스 스타일 추출도 고려해 보세요.
🤖 Fix all issues with AI agents
In `@src/pages/generate/pages/result/components/GeneratedImg.css.ts`:
- Around line 154-176: The opacity rules for slidePrevBtn and slideNextBtn are
duplicated; consolidate by creating a combined selector referencing slidePrevBtn
and slideNextBtn together with slideBtnCircle (e.g., `${slidePrevBtn},
${slideNextBtn}` then append ` ${slideBtnCircle}` and the state variants) and
replace the six globalStyle calls with three: default, :active:not(:disabled),
and :disabled, each using the combined selector so the default (0.3), active
(0.5), and disabled (0.04) opacities are applied to both buttons via a single
set of globalStyle calls.
- Line 4: Remove the unused commented import line in GeneratedImg.css.ts: delete
the commented statement "// import { zIndex } from
'@/shared/styles/tokens/zIndex';" so the file contains no dead/commented import;
ensure no other unused/commented imports remain in the same file (look for
imports at the top of GeneratedImg.css.ts and clean them) and run linting to
confirm.
In `@src/pages/generate/pages/result/components/GeneratedImgA.tsx`:
- Around line 204-207: Update the JSX in GeneratedImgA.tsx to use explicit
aria-hidden string values instead of bare attributes: replace occurrences on
elements with class names slideBtnCircle and slideBtnIcon (and the similar block
at the other occurrence) to use aria-hidden="true" or aria-hidden="false" as
appropriate; locate where currentSlideIndex is used to render
SlidePrevDisabled/SlidePrev (and the other slide-next block) and ensure the
aria-hidden values reflect visibility intent for screen readers.
In `@src/pages/generate/pages/result/curationSheet/CurationSheet.tsx`:
- Around line 57-60: The helper toFiniteNumber currently treats null as 0
because Number(null) === 0; update to treat null and undefined as "no value" by
checking for nullish first (e.g., value == null or value === null || value ===
undefined) and returning null in that case, otherwise coerce to Number and
return it only if Number.isFinite; modify the toFiniteNumber function
accordingly so null/undefined both fall through to the fallback path instead of
being converted to 0.
- Around line 133-136: The lines calling toFiniteNumber exceed 80 chars;
destructure the long property names from product first and then call
toFiniteNumber on each short variable to keep lines under 80. For example, in
CurationSheet.tsx extract const { furnitureProductOriginalPrice,
furnitureProductDiscountPrice, furnitureProductDiscountRate,
furnitureProductSaveCount } = product and then assign originalPrice,
discountPrice, discountRate, saveCount by calling toFiniteNumber on those
shorter identifiers (refer to symbols originalPrice, discountPrice,
discountRate, saveCount and function toFiniteNumber).
In `@src/shared/components/button/linkButton/LinkButton.css.ts`:
- Around line 24-32: The hover/active styles in the LinkButton styles currently
set backgroundColor to the same gray000 as the base so there's no visual
feedback; update the ':hover' and/or ':active' rules in the LinkButton CSS
object (the style block containing ':hover', ':active', ':visited') to provide
subtle feedback—for example use colorVars.color.gray050 for ':active' or apply a
small opacity change (e.g., reduce opacity on ':active' or slightly change
background on ':hover') so clicks and hovers are perceptible while keeping Figma
parity.
- Around line 36-45: The withText variant and related styles (withText,
linkContent, linkLabel) currently hard-code fixed dimensions (width, minWidth,
height, flex: '0 0 ...') which reduces reuse; change these rules to use flexible
sizing by removing fixed px/rem values and using auto/min/max or responsive
units, derive sizes from a new size prop or variant (e.g., add a sizeVariant or
prop to the LinkButton component and switch styles accordingly), replace flex:
'0 0 6.1rem' with flex: '0 1 auto' or appropriate flex-basis via the prop, and
expose CSS variables or tokens instead of hard-coded values so other pages can
override without editing withText/linkContent/linkLabel directly. Ensure
fontStyle usage remains but remove tight fixed dimensions to allow
content-driven width/height.
- Around line 67-78: The linkLabel style is missing textOverflow: 'ellipsis', so
add textOverflow: 'ellipsis' to the linkLabel style (keeping the existing
overflow: 'hidden' and whiteSpace: 'nowrap' and fixed width/flex settings) to
ensure long text shows an ellipsis; verify the element with the linkLabel class
remains constrained by its flex: '0 0 3.3rem' so the ellipsis can appear.
In `@src/shared/components/button/linkButton/LinkButton.tsx`:
- Around line 25-38: The LinkButton renders only an SVG (LinkIcon) when
typeVariant === 'onlyIcon', which lacks an accessible name; update the
LinkButton component to accept an aria-label prop (or derive a required label
when typeVariant is 'onlyIcon') and ensure that when rendering the icon-only
branch it applies that aria-label to the anchor/button element; validate inside
LinkButton that either children or aria-label is present (throw or warn) and use
the aria-label on the root element so screen readers can announce the link.
In `@src/shared/components/button/saveButton/SaveButton.css.ts`:
- Around line 17-29: Replace the empty style exports and globalStyle targeting
with a Vanilla Extract recipe: remove exported selected and unselected empty
styles and create a recipe (e.g., SaveButtonRecipe) with a boolean/variant like
selected: { true, false } (and any size/type variants if needed) and use the
recipe's class name (SaveButtonRecipe().selected) instead of
selected/unselected; update globalStyle selectors to target the recipe-generated
class for svg path (using the recipe's className for selected state) and keep
buttonWrapper globalStyle unchanged. Ensure references to the old exported
symbols (selected, unselected) are updated to the new recipe variant class where
the component applies classes.
- Around line 31-35: Replace the hardcoded fill 'rgba(0, 0, 0, 0.2)' used in the
globalStyle call for the unselected SVG path with a design token from colorVars
(update the globalStyle(`${unselected} svg path`, ...) usage). If a 20% black
opacity token doesn't exist, add a new token (e.g., gray999_20 or gray999_30 as
agreed with design) to color.css.ts and use that token via colorVars in the
SaveButton.css.ts globalStyle; confirm choice with the designer if using
gray999_30.
In `@src/shared/components/card/cardProduct/CardProduct.css.ts`:
- Around line 166-172: The 0.5px border in the exported style colorChipInner can
disappear on non-retina or older browsers; change the border from `0.5px solid
${colorVars.color.gray999_30}` to a reliable rendering approach such as `1px
solid ${colorVars.color.gray999_30}` or replace the thin border with a subtle
box-shadow (e.g., `boxShadow: '0 0 0 1px <color>'`) so the edge is consistently
visible across devices; update the border declaration inside the colorChipInner
style accordingly.
| import { globalStyle, style } from '@vanilla-extract/css'; | ||
| import { recipe } from '@vanilla-extract/recipes'; | ||
|
|
||
| // import { zIndex } from '@/shared/styles/tokens/zIndex'; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
주석 처리된 import를 제거해 주세요.
// import { zIndex } from '@/shared/styles/tokens/zIndex'; — 사용하지 않는 주석 코드는 정리하는 게 좋습니다.
🧹 제거 제안
-// import { zIndex } from '@/shared/styles/tokens/zIndex';📝 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.
| // import { zIndex } from '@/shared/styles/tokens/zIndex'; |
🤖 Prompt for AI Agents
In `@src/pages/generate/pages/result/components/GeneratedImg.css.ts` at line 4,
Remove the unused commented import line in GeneratedImg.css.ts: delete the
commented statement "// import { zIndex } from '@/shared/styles/tokens/zIndex';"
so the file contains no dead/commented import; ensure no other unused/commented
imports remain in the same file (look for imports at the top of
GeneratedImg.css.ts and clean them) and run linting to confirm.
| globalStyle(`${slidePrevBtn} ${slideBtnCircle}`, { | ||
| opacity: 0.3, | ||
| }); | ||
|
|
||
| globalStyle(`${slidePrevBtn}:active:not(:disabled) ${slideBtnCircle}`, { | ||
| opacity: 0.5, | ||
| }); | ||
|
|
||
| globalStyle(`${slidePrevBtn}:disabled ${slideBtnCircle}`, { | ||
| opacity: 0.04, | ||
| }); | ||
|
|
||
| globalStyle(`${slideNextBtn} ${slideBtnCircle}`, { | ||
| opacity: 0.3, | ||
| }); | ||
|
|
||
| globalStyle(`${slideNextBtn}:active:not(:disabled) ${slideBtnCircle}`, { | ||
| opacity: 0.5, | ||
| }); | ||
|
|
||
| globalStyle(`${slideNextBtn}:disabled ${slideBtnCircle}`, { | ||
| opacity: 0.04, | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Prev/Next 버튼의 globalStyle opacity 규칙이 완전히 중복돼요.
slidePrevBtn과 slideNextBtn의 세 가지 상태(default 0.3, active 0.5, disabled 0.04)가 동일합니다. 공통 셀렉터로 통합하면 유지보수가 편해져요.
♻️ 통합 제안
-globalStyle(`${slidePrevBtn} ${slideBtnCircle}`, {
- opacity: 0.3,
-});
-
-globalStyle(`${slidePrevBtn}:active:not(:disabled) ${slideBtnCircle}`, {
- opacity: 0.5,
-});
-
-globalStyle(`${slidePrevBtn}:disabled ${slideBtnCircle}`, {
- opacity: 0.04,
-});
-
-globalStyle(`${slideNextBtn} ${slideBtnCircle}`, {
- opacity: 0.3,
-});
-
-globalStyle(`${slideNextBtn}:active:not(:disabled) ${slideBtnCircle}`, {
- opacity: 0.5,
-});
-
-globalStyle(`${slideNextBtn}:disabled ${slideBtnCircle}`, {
- opacity: 0.04,
-});
+globalStyle(
+ `${slidePrevBtn} ${slideBtnCircle}, ${slideNextBtn} ${slideBtnCircle}`,
+ { opacity: 0.3 },
+);
+
+globalStyle(
+ `${slidePrevBtn}:active:not(:disabled) ${slideBtnCircle}, ${slideNextBtn}:active:not(:disabled) ${slideBtnCircle}`,
+ { opacity: 0.5 },
+);
+
+globalStyle(
+ `${slidePrevBtn}:disabled ${slideBtnCircle}, ${slideNextBtn}:disabled ${slideBtnCircle}`,
+ { opacity: 0.04 },
+);📝 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.
| globalStyle(`${slidePrevBtn} ${slideBtnCircle}`, { | |
| opacity: 0.3, | |
| }); | |
| globalStyle(`${slidePrevBtn}:active:not(:disabled) ${slideBtnCircle}`, { | |
| opacity: 0.5, | |
| }); | |
| globalStyle(`${slidePrevBtn}:disabled ${slideBtnCircle}`, { | |
| opacity: 0.04, | |
| }); | |
| globalStyle(`${slideNextBtn} ${slideBtnCircle}`, { | |
| opacity: 0.3, | |
| }); | |
| globalStyle(`${slideNextBtn}:active:not(:disabled) ${slideBtnCircle}`, { | |
| opacity: 0.5, | |
| }); | |
| globalStyle(`${slideNextBtn}:disabled ${slideBtnCircle}`, { | |
| opacity: 0.04, | |
| }); | |
| globalStyle( | |
| `${slidePrevBtn} ${slideBtnCircle}, ${slideNextBtn} ${slideBtnCircle}`, | |
| { opacity: 0.3 }, | |
| ); | |
| globalStyle( | |
| `${slidePrevBtn}:active:not(:disabled) ${slideBtnCircle}, ${slideNextBtn}:active:not(:disabled) ${slideBtnCircle}`, | |
| { opacity: 0.5 }, | |
| ); | |
| globalStyle( | |
| `${slidePrevBtn}:disabled ${slideBtnCircle}, ${slideNextBtn}:disabled ${slideBtnCircle}`, | |
| { opacity: 0.04 }, | |
| ); |
🤖 Prompt for AI Agents
In `@src/pages/generate/pages/result/components/GeneratedImg.css.ts` around lines
154 - 176, The opacity rules for slidePrevBtn and slideNextBtn are duplicated;
consolidate by creating a combined selector referencing slidePrevBtn and
slideNextBtn together with slideBtnCircle (e.g., `${slidePrevBtn},
${slideNextBtn}` then append ` ${slideBtnCircle}` and the state variants) and
replace the six globalStyle calls with three: default, :active:not(:disabled),
and :disabled, each using the combined selector so the default (0.3), active
(0.5), and disabled (0.04) opacities are applied to both buttons via a single
set of globalStyle calls.
| <span className={styles.slideBtnCircle} aria-hidden /> | ||
| <span className={styles.slideBtnIcon} aria-hidden> | ||
| {currentSlideIndex === 0 ? <SlidePrevDisabled /> : <SlidePrev />} | ||
| </span> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
aria-hidden에 명시적 값을 권장해요.
aria-hidden만 쓰면 JSX에서 true로 처리되지만, 접근성 속성은 aria-hidden="true"로 명시하는 게 가독성과 의도 전달에 좋습니다.
Also applies to: 258-265
🤖 Prompt for AI Agents
In `@src/pages/generate/pages/result/components/GeneratedImgA.tsx` around lines
204 - 207, Update the JSX in GeneratedImgA.tsx to use explicit aria-hidden
string values instead of bare attributes: replace occurrences on elements with
class names slideBtnCircle and slideBtnIcon (and the similar block at the other
occurrence) to use aria-hidden="true" or aria-hidden="false" as appropriate;
locate where currentSlideIndex is used to render SlidePrevDisabled/SlidePrev
(and the other slide-next block) and ensure the aria-hidden values reflect
visibility intent for screen readers.
| const toFiniteNumber = (value: unknown) => { | ||
| const numeric = Number(value); | ||
| return Number.isFinite(numeric) ? numeric : null; | ||
| }; |
There was a problem hiding this comment.
toFiniteNumber(null)은 0을 반환해요 — 의도된 동작인지 확인 필요.
Number(null) === 0이므로 toFiniteNumber(null)은 0(유효한 값)을 반환하고, Number(undefined)는 NaN이므로 null을 반환해요. API에서 null이 "값 없음"을 의미한다면, 0과 구별 없이 처리되는 셈이에요.
현재 fallback 값이 모두 0이라 실질적 영향은 없지만, 향후 fallback이 바뀌면 버그로 이어질 수 있어요.
🛡️ null/undefined 모두 fallback으로 빠지도록 수정 제안
const toFiniteNumber = (value: unknown) => {
+ if (value === null || value === undefined) return null;
const numeric = Number(value);
return Number.isFinite(numeric) ? numeric : null;
};🤖 Prompt for AI Agents
In `@src/pages/generate/pages/result/curationSheet/CurationSheet.tsx` around lines
57 - 60, The helper toFiniteNumber currently treats null as 0 because
Number(null) === 0; update to treat null and undefined as "no value" by checking
for nullish first (e.g., value == null or value === null || value === undefined)
and returning null in that case, otherwise coerce to Number and return it only
if Number.isFinite; modify the toFiniteNumber function accordingly so
null/undefined both fall through to the fallback path instead of being converted
to 0.
| const originalPrice = toFiniteNumber(product.furnitureProductOriginalPrice); | ||
| const discountPrice = toFiniteNumber(product.furnitureProductDiscountPrice); | ||
| const discountRate = toFiniteNumber(product.furnitureProductDiscountRate); | ||
| const saveCount = toFiniteNumber(product.furnitureProductSaveCount); |
There was a problem hiding this comment.
Prettier 포맷팅 위반 — 80자 초과.
정적 분석에서 133~134번 줄이 80자를 넘는다고 보고했어요. Prettier 규칙에 맞게 줄바꿈을 적용해 주세요.
🔧 포맷 수정 제안
- const originalPrice = toFiniteNumber(product.furnitureProductOriginalPrice);
- const discountPrice = toFiniteNumber(product.furnitureProductDiscountPrice);
+ const originalPrice = toFiniteNumber(
+ product.furnitureProductOriginalPrice
+ );
+ const discountPrice = toFiniteNumber(
+ product.furnitureProductDiscountPrice
+ );As per coding guidelines, "Maintain a maximum line width of 80 characters".
📝 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.
| const originalPrice = toFiniteNumber(product.furnitureProductOriginalPrice); | |
| const discountPrice = toFiniteNumber(product.furnitureProductDiscountPrice); | |
| const discountRate = toFiniteNumber(product.furnitureProductDiscountRate); | |
| const saveCount = toFiniteNumber(product.furnitureProductSaveCount); | |
| const originalPrice = toFiniteNumber( | |
| product.furnitureProductOriginalPrice | |
| ); | |
| const discountPrice = toFiniteNumber( | |
| product.furnitureProductDiscountPrice | |
| ); | |
| const discountRate = toFiniteNumber(product.furnitureProductDiscountRate); | |
| const saveCount = toFiniteNumber(product.furnitureProductSaveCount); |
🧰 Tools
🪛 ESLint
[error] 133-133: Replace product.furnitureProductOriginalPrice with ⏎········product.furnitureProductOriginalPrice⏎······
(prettier/prettier)
[error] 134-134: Replace product.furnitureProductDiscountPrice with ⏎········product.furnitureProductDiscountPrice⏎······
(prettier/prettier)
🤖 Prompt for AI Agents
In `@src/pages/generate/pages/result/curationSheet/CurationSheet.tsx` around lines
133 - 136, The lines calling toFiniteNumber exceed 80 chars; destructure the
long property names from product first and then call toFiniteNumber on each
short variable to keep lines under 80. For example, in CurationSheet.tsx extract
const { furnitureProductOriginalPrice, furnitureProductDiscountPrice,
furnitureProductDiscountRate, furnitureProductSaveCount } = product and then
assign originalPrice, discountPrice, discountRate, saveCount by calling
toFiniteNumber on those shorter identifiers (refer to symbols originalPrice,
discountPrice, discountRate, saveCount and function toFiniteNumber).
| export const linkLabel = style({ | ||
| width: '3.3rem', | ||
| height: '1.8rem', | ||
| lineHeight: '1.1rem', | ||
| textAlign: 'center', | ||
| flex: '0 0 3.3rem', | ||
| whiteSpace: 'nowrap', | ||
| display: 'flex', | ||
| alignItems: 'center', | ||
| justifyContent: 'center', | ||
| overflow: 'hidden', | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
overflow: 'hidden'에 textOverflow: 'ellipsis' 누락
linkLabel에 overflow: 'hidden'과 고정 너비(3.3rem)가 설정되어 있지만, textOverflow: 'ellipsis'가 없어서 긴 텍스트가 잘릴 때 사용자에게 시각적 힌트가 없어요.
♻️ 제안
export const linkLabel = style({
width: '3.3rem',
height: '1.8rem',
lineHeight: '1.1rem',
textAlign: 'center',
flex: '0 0 3.3rem',
whiteSpace: 'nowrap',
display: 'flex',
alignItems: 'center',
justifyContent: 'center',
overflow: 'hidden',
+ textOverflow: 'ellipsis',
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const linkLabel = style({ | |
| width: '3.3rem', | |
| height: '1.8rem', | |
| lineHeight: '1.1rem', | |
| textAlign: 'center', | |
| flex: '0 0 3.3rem', | |
| whiteSpace: 'nowrap', | |
| display: 'flex', | |
| alignItems: 'center', | |
| justifyContent: 'center', | |
| overflow: 'hidden', | |
| }); | |
| export const linkLabel = style({ | |
| width: '3.3rem', | |
| height: '1.8rem', | |
| lineHeight: '1.1rem', | |
| textAlign: 'center', | |
| flex: '0 0 3.3rem', | |
| whiteSpace: 'nowrap', | |
| display: 'flex', | |
| alignItems: 'center', | |
| justifyContent: 'center', | |
| overflow: 'hidden', | |
| textOverflow: 'ellipsis', | |
| }); |
🤖 Prompt for AI Agents
In `@src/shared/components/button/linkButton/LinkButton.css.ts` around lines 67 -
78, The linkLabel style is missing textOverflow: 'ellipsis', so add
textOverflow: 'ellipsis' to the linkLabel style (keeping the existing overflow:
'hidden' and whiteSpace: 'nowrap' and fixed width/flex settings) to ensure long
text shows an ellipsis; verify the element with the linkLabel class remains
constrained by its flex: '0 0 3.3rem' so the ellipsis can appear.
| {isWithText ? ( | ||
| <span className={styles.linkContent}> | ||
| <span className={styles.linkIconWrapper}> | ||
| <LinkIcon /> | ||
| </span> | ||
| {children ? ( | ||
| <span className={styles.linkLabel}>{children}</span> | ||
| ) : null} | ||
| </span> | ||
| ) : ( | ||
| <span className={styles.linkIconWrapper}> | ||
| <LinkIcon /> | ||
| </span> | ||
| )} |
There was a problem hiding this comment.
접근성(a11y): onlyIcon 모드에서 접근 가능한 레이블 누락
typeVariant='onlyIcon'일 때 <a> 내부에 텍스트 콘텐츠 없이 SVG 아이콘만 렌더링돼요. 스크린 리더가 이 링크의 용도를 파악할 수 없어요.
aria-label을 props로 받거나, 최소한 onlyIcon 모드에서 기본 aria-label을 제공하는 것을 권장해요.
♿ 제안
const LinkButton = ({
children,
typeVariant = 'withText',
+ 'aria-label': ariaLabel,
...props
}: LinkButtonProps) => {
const isWithText = typeVariant === 'withText';
return (
<a
{...props}
+ aria-label={!isWithText && !children ? ariaLabel ?? '링크 열기' : undefined}
target="_blank"
rel="noopener noreferrer"As per coding guidelines, 공통 컴포넌트 리뷰 시 접근성(a11y) 준수를 중점적으로 검토해야 해요: "접근성 준수: ARIA 속성, label 연결, 키보드 포커스".
🤖 Prompt for AI Agents
In `@src/shared/components/button/linkButton/LinkButton.tsx` around lines 25 - 38,
The LinkButton renders only an SVG (LinkIcon) when typeVariant === 'onlyIcon',
which lacks an accessible name; update the LinkButton component to accept an
aria-label prop (or derive a required label when typeVariant is 'onlyIcon') and
ensure that when rendering the icon-only branch it applies that aria-label to
the anchor/button element; validate inside LinkButton that either children or
aria-label is present (throw or warn) and use the aria-label on the root element
so screen readers can announce the link.
| export const selected = style({}); | ||
|
|
||
| export const unselected = style({}); | ||
|
|
||
| globalStyle(`${buttonWrapper} svg`, { | ||
| width: '2.4rem', | ||
| height: '2.4rem', | ||
| }); | ||
|
|
||
| globalStyle(`${selected} svg path`, { | ||
| fill: colorVars.color.primary, | ||
| stroke: 'none', | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
recipe 패턴 대신 빈 style({}) + globalStyle 조합 사용
selected/unselected를 빈 style({})로 선언하고 globalStyle로 SVG path를 타겟하는 방식은 동작하지만, 프로젝트 컨벤션에서는 버튼 상태 변형에 recipe 패턴을 권장하고 있어요. recipe의 variants를 활용하면 타입 안전성과 확장성이 높아지고, 빈 스타일 객체 export도 불필요해져요.
♻️ recipe 패턴 적용 예시
-import { globalStyle, style } from '@vanilla-extract/css';
+import { globalStyle, style } from '@vanilla-extract/css';
+import { recipe } from '@vanilla-extract/recipes';
import { colorVars } from '@/shared/styles/tokens/color.css';
export const buttonWrapper = style({
width: '2.8rem',
height: '2.8rem',
display: 'flex',
alignItems: 'center',
justifyContent: 'center',
background: 'none',
border: 'none',
padding: 0,
margin: 0,
});
-export const selected = style({});
-
-export const unselected = style({});
-
globalStyle(`${buttonWrapper} svg`, {
width: '2.4rem',
height: '2.4rem',
});
-globalStyle(`${selected} svg path`, {
- fill: colorVars.color.primary,
- stroke: 'none',
-});
-
-globalStyle(`${unselected} svg path`, {
- fill: 'rgba(0, 0, 0, 0.2)',
- stroke: colorVars.color.gray000,
- strokeWidth: 1,
-});
+export const heartVariant = recipe({
+ variants: {
+ state: {
+ selected: {},
+ unselected: {},
+ },
+ },
+});
+
+globalStyle(`${heartVariant.classNames.variants.state.selected} svg path`, {
+ fill: colorVars.color.primary,
+ stroke: 'none',
+});
+
+globalStyle(`${heartVariant.classNames.variants.state.unselected} svg path`, {
+ fill: colorVars.color.gray999_30,
+ stroke: colorVars.color.gray000,
+ strokeWidth: 1,
+});참고:
recipe의classNames를globalStyle과 조합하는 방식이 복잡하다면, 빈style({})접근도 실용적이에요. 다만, 팀 컨벤션과 일관성을 위해 검토해 주세요.
Based on learnings: "Button components in shared/components/button should use variant recipe pattern with Vanilla Extract to define state variations (active/inactive/selected), sizes, and types, reflecting designer tokens"
🤖 Prompt for AI Agents
In `@src/shared/components/button/saveButton/SaveButton.css.ts` around lines 17 -
29, Replace the empty style exports and globalStyle targeting with a Vanilla
Extract recipe: remove exported selected and unselected empty styles and create
a recipe (e.g., SaveButtonRecipe) with a boolean/variant like selected: { true,
false } (and any size/type variants if needed) and use the recipe's class name
(SaveButtonRecipe().selected) instead of selected/unselected; update globalStyle
selectors to target the recipe-generated class for svg path (using the recipe's
className for selected state) and keep buttonWrapper globalStyle unchanged.
Ensure references to the old exported symbols (selected, unselected) are updated
to the new recipe variant class where the component applies classes.
| globalStyle(`${unselected} svg path`, { | ||
| fill: 'rgba(0, 0, 0, 0.2)', | ||
| stroke: colorVars.color.gray000, | ||
| strokeWidth: 1, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
하드코딩된 색상값 rgba(0, 0, 0, 0.2) — 디자인 토큰 사용 권장
PR 목적 중 하나가 하드코딩된 색상을 중앙 토큰으로 대체하는 것인데, unselected 상태의 fill에 rgba(0, 0, 0, 0.2)가 직접 사용되고 있어요. 정확히 0.2 opacity 토큰이 없다면 color.css.ts에 토큰을 추가하거나, 가장 가까운 gray999_30을 디자이너와 확인 후 적용하는 것을 추천해요.
♻️ 토큰 적용 예시
globalStyle(`${unselected} svg path`, {
- fill: 'rgba(0, 0, 0, 0.2)',
+ fill: colorVars.color.gray999_30, // 또는 새 토큰 gray999_20 추가
stroke: colorVars.color.gray000,
strokeWidth: 1,
});As per coding guidelines: "디자인 토큰 활용 확인: colorVars 직접 사용"
📝 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.
| globalStyle(`${unselected} svg path`, { | |
| fill: 'rgba(0, 0, 0, 0.2)', | |
| stroke: colorVars.color.gray000, | |
| strokeWidth: 1, | |
| }); | |
| globalStyle(`${unselected} svg path`, { | |
| fill: colorVars.color.gray999_30, // 또는 새 토큰 gray999_20 추가 | |
| stroke: colorVars.color.gray000, | |
| strokeWidth: 1, | |
| }); |
🤖 Prompt for AI Agents
In `@src/shared/components/button/saveButton/SaveButton.css.ts` around lines 31 -
35, Replace the hardcoded fill 'rgba(0, 0, 0, 0.2)' used in the globalStyle call
for the unselected SVG path with a design token from colorVars (update the
globalStyle(`${unselected} svg path`, ...) usage). If a 20% black opacity token
doesn't exist, add a new token (e.g., gray999_20 or gray999_30 as agreed with
design) to color.css.ts and use that token via colorVars in the
SaveButton.css.ts globalStyle; confirm choice with the designer if using
gray999_30.
| export const colorChipInner = style({ | ||
| width: '1.2rem', | ||
| height: '1.2rem', | ||
| borderRadius: '50%', | ||
| border: `1px solid ${colorVars.color.gray999_30}`, | ||
| border: `0.5px solid ${colorVars.color.gray999_30}`, | ||
| boxSizing: 'border-box', | ||
| }); |
There was a problem hiding this comment.
0.5px border — 서브픽셀 렌더링 주의
Line 170의 0.5px solid 보더는 non-retina 디스플레이나 일부 브라우저(특히 구형 Android WebView)에서 0px로 렌더링되거나 1px로 반올림될 수 있습니다. 모바일 고정 레이아웃(375–440px)에서 대부분 retina이므로 큰 문제는 아니지만, QA 시 저해상도 기기에서 보더 누락 여부를 확인해 주세요.
🤖 Prompt for AI Agents
In `@src/shared/components/card/cardProduct/CardProduct.css.ts` around lines 166 -
172, The 0.5px border in the exported style colorChipInner can disappear on
non-retina or older browsers; change the border from `0.5px solid
${colorVars.color.gray999_30}` to a reliable rendering approach such as `1px
solid ${colorVars.color.gray999_30}` or replace the thin border with a subtle
box-shadow (e.g., `boxShadow: '0 0 0 1px <color>'`) so the edge is consistently
visible across devices; update the border declaration inside the colorChipInner
style accordingly.
1 participant
📌 Summary
결과 페이지 UI 불일치 항목을 브랜치 전체 기준으로 정리해 반영했습니다.
이미지 캐러셀 네비 버튼, 카드 사이트 버튼, 카드 간격/레이아웃, 컬러칩/하트 아이콘 스타일을 Figma 기준에 맞추고,
바텀시트 잔재 레이아웃을 인라인 큐레이션 구조로 정리했습니다.
또한 결과 카드가 API 누락 필드에서도 끊기지 않도록 normalize/fallback 로직을 보강했습니다.
마지막으로 카드 스켈레톤 색상 하드코딩을 중앙 컬러 토큰으로 치환했습니다.
📄 Tasks
🔍 To Reviewer
📸 Screenshot