Open
Conversation
📝 WalkthroughSummary by CodeRabbit릴리스 노트
Walkthrough새로운 상품 카드 UI 컴포넌트( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
🚥 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. 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-krbzqyubdx.chromatic.com/ 📊 빌드 정보
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/pages/Test.tsx`:
- Around line 1-132: The Test.tsx demo page (ComponentTest component) should be
replaced by Storybook stories so the examples don't get bundled into production:
create stories under stories/ for CardProduct and ListCardProduct that reproduce
the variants shown (grid of CardProduct with cardType "shopping"/default,
different prop sets: title, brand, isSaved/onToggleSave interactive control,
linkHref/linkLabel, originalPrice/discountRate/discountPrice, colorHexes,
saveCount; and ListCardProduct stories for cardSize "s"/"m" with same prop
permutations), implement a simple local controls/state in the stories to toggle
isSaved (mirroring onToggleSave), add proper story meta (title, component,
argTypes) and remove or delete Test.tsx (ComponentTest) from the app entry so it
is not included in the production bundle.
In `@src/routes/router.tsx`:
- Around line 24-25: The route is statically importing ComponentTest which
prevents code-splitting; replace the top-level import of ComponentTest with a
dynamic import using React.lazy (e.g., const ComponentTest = lazy(() =>
import('@/pages/Test'))), remove the static import statement, and ensure the
route that renders ComponentTest is wrapped in a Suspense with an appropriate
fallback (or uses the app's existing suspense boundary) so the lazily-loaded
chunk loads correctly.
- Around line 30-33: The /test route (ROUTES.TEST) exposing ComponentTest is
included in publicRoutes and must not be shipped to production; change the
router registration so ComponentTest is only added when running in
non-production (e.g., guard inclusion using process.env.NODE_ENV !==
'production' or your build-time env var like import.meta.env.DEV), or remove the
route entirely before merge. Update the publicRoutes array (or the code that
constructs it) to conditionally push the ROUTES.TEST entry or omit it based on
the env flag, and ensure any references to ComponentTest are removed from
production bundles by using the conditional guard around the route definition.
In `@src/shared/components/v2/cardProduct/CardProduct.css.ts`:
- Line 11: The file currently imports and uses fontVars directly
(fontVars.font.*); update the import to use the shared helper token path and
replace direct fontVars references with the unified fontStyle() conversion
helper so font tokens are consumed via fontStyle() across this module;
specifically change the import of fontVars to the helper export and refactor all
usages (e.g., any fontVars.font.* occurrences) to call fontStyle(...) so styles
in CardProduct.css.ts consistently use the shared fontStyle helper (apply the
same change to the other blocks flagged in the review where fontVars.font.* is
used).
In `@src/shared/components/v2/cardProduct/CardProduct.tsx`:
- Around line 151-157: The Heart IconButton toggle currently only changes
visually; update the IconButton usage in CardProduct (the conditional rendering
block using isDefault, isSaved, disabled, onToggleSave) to include accessible
ARIA attributes: add an aria-pressed={isSaved} to reflect the toggle state and
supply a clear aria-label (e.g., "Save product" / "Unsave product" or a single
label like "Toggle save") that can be derived from isSaved so screen readers
receive state and intent; ensure the onToggleSave remains the click handler and
do not remove existing disabled handling.
- Around line 145-150: The overlay element styles.saveBtnOverlay in CardProduct
currently stops propagation but exposes a focusable/actionable control (the
shopping variant "ViewDetail") with no action or accessible name; decide whether
this is decorative or interactive: if decorative, replace the element with a
non-interactive Icon element (remove focusability and ARIA), otherwise implement
an actual button behavior by adding a proper onClick handler (e.g., call the
existing viewDetail function), an aria-label that describes the action, and
keyboard support (onKeyDown handling Enter/Space) and role="button" if you must
keep a div; ensure the change is made on the element(s) referenced
(styles.saveBtnOverlay and the ViewDetail control) so they are keyboard operable
and accessible per guidelines.
- Around line 78-92: Create a single safe link helper (e.g., safeOpenLink) that
validates linkHref protocol (allow only http/https), normalizes/returns early
for invalid URLs, and then opens the URL with window.open(..., '_blank',
'noopener,noreferrer') only when typeof window !== 'undefined'; replace direct
window.open calls in handleWrapperClick and the "사이트" button handler (the
onLinkClick usage) to call safeOpenLink(linkHref), and make handleWrapperKeyDown
use the same helper for Enter/Space activation; ensure onCardClick and
enableWholeCardLink behavior remains (call onCardClick before invoking
safeOpenLink), and apply the same helper refactor to ListCardProduct and the
legacy CardProduct components so all link openings share validation and opening
logic.
In `@src/shared/components/v2/cardProduct/ListCardProduct.css.ts`:
- Around line 158-161: The originalPriceText style lacks a strike-through and
uses the primary text color, making it visually indistinct from current price;
update the originalPriceText style (in ListCardProduct.css.ts) to add a
strike-through (textDecoration or equivalent) and switch its color to the
tertiary/auxiliary tone (e.g., colorVars.color.text.tertiary) to match the
existing CardProduct.css.ts convention for original prices so the discount UI is
clear.
- Line 10: The CSS module is directly accessing font tokens via fontVars.font.*
instead of using the shared fontStyle() helper; update every direct token access
in this file (including the other occurrences mentioned) to call fontStyle(...)
with the appropriate token keys, import/retain the fontStyle helper instead of
direct fontVars usage, and remove any unused fontVars references; make sure to
map each original token reference (e.g., fontVars.font.family,
fontVars.font.size, fontVars.font.weight) to the corresponding fontStyle(...)
call so the module's font declarations match the sibling CSS modules.
In `@src/shared/components/v2/cardProduct/ListCardProduct.tsx`:
- Around line 63-69: visibleColors and extraColorCount are computed from
different arrays causing incorrect "+N" when colorHexes contains falsy entries;
instead create a single cleaned array (e.g., cleanedColorHexes =
Array.isArray(colorHexes) ? colorHexes.filter(Boolean) : []) and derive both
visibleColors (cleanedColorHexes.slice(0, 3)) and extraColorCount (Math.max(0,
cleanedColorHexes.length - 3)) from that cleaned array so the logic matches
CardProduct and avoids overstating the overflow.
- Around line 180-185: The IconButton currently renders only an icon so screen
readers can't tell saved state; update the usage in ListCardProduct to pass
accessibility props: provide a descriptive aria-label (e.g., "Save product" /
"Unsave product") and set aria-pressed={isSaved}, and ensure disabled is passed
through; if IconButton does not accept native button props, extend its API
(props typing and spread) so it forwards aria-label and aria-pressed (and other
button attributes) to the underlying button element; keep onToggleSave and
isSaved as the source of truth for label and pressed state.
- Around line 71-85: The link icon button in ListCardProduct is rendered even
when linkHref is falsy, causing clicks to trigger only logging; update the JSX
that renders the IconButton (the link icon in ListCardProduct) to conditionally
render based on the linkHref prop (e.g., wrap the IconButton with {linkHref &&
(...)}) similar to CardProduct, ensuring the IconButton only appears when
linkHref is provided; keep existing onClick/aria behavior but hide the button
when linkHref is undefined or empty.
In `@src/shared/components/v2/linkButton/LinkButton.css.ts`:
- Around line 3-4: The V2 LinkButton CSS is still importing V1 tokens; update
the imports in src/shared/components/v2/linkButton/LinkButton.css.ts to use the
V2 token modules instead of '@styles/tokens/color.css' and '@styles/fontStyle'
(replace with '@/shared/styles/tokensV2/color.css' and
'@/shared/styles/tokensV2/font.css' or your project’s V2 equivalents), then
adjust any references to the imported symbols (e.g., colorVars and fontStyle) to
match the V2 token API and ensure the withText variant in the LinkButton styles
uses the V2 token names for colors and fonts.
In `@src/shared/components/v2/linkButton/LinkButton.tsx`:
- Around line 1-7: LinkButton.tsx is using the type React.ComponentProps<'a'>
(in the LinkButtonProps interface) but React isn't imported, causing TypeScript
errors; add a React import (e.g., import * as React from 'react' or import React
from 'react') at the top of the file so the LinkButtonProps type and any JSX
types resolve correctly.
- Around line 14-26: The LinkButton component currently spreads {...props} and
then sets className={styles.linkButton(...)} which overwrites any external
className; fix by destructuring className from props (e.g., function
LinkButton({ children, className, ...props })) and merge it with the generated
class using clsx (or classnames) so className={clsx(className,
styles.linkButton({ type: typeVariant }))}, and ensure you import clsx and
remove className from the spread to avoid duplicate props.
In `@src/shared/components/v2/saveButton/SaveButton.css.ts`:
- Around line 3-9: The buttonWrapper style contains redundant button-reset
properties already handled by globalStyle('button', { all: 'unset', ... });
remove margin, border, background and padding from the buttonWrapper style
definition and leave only the width: '100%' (i.e., update the exported
buttonWrapper to only include the unique width property).
In `@src/shared/components/v2/saveButton/SaveButton.tsx`:
- Around line 22-30: The SaveButton renders an icon-only button
(SaveOnIcon/SaveOffIcon) without an accessible name; update the SaveButton
component to provide an aria-label on the <button> using an explicit prop or
props['aria-label'] fallback so screen readers get a meaningful label (e.g. when
isSelected true -> "Saved", else "Save"); ensure the attribute is applied
alongside existing {...props} and allow callers to override it by passing their
own aria-label.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: b742e487-0932-44ec-8a97-9c8e502d96fb
⛔ Files ignored due to path filters (6)
src/shared/assets/v2/svg/ArrowLeft.svgis excluded by!**/*.svg,!**/*.svgand included bysrc/**src/shared/assets/v2/svg/ArrowRight.svgis excluded by!**/*.svg,!**/*.svgand included bysrc/**src/shared/assets/v2/svg/ChevronDown.svgis excluded by!**/*.svg,!**/*.svgand included bysrc/**src/shared/assets/v2/svg/ChevronUp.svgis excluded by!**/*.svg,!**/*.svgand included bysrc/**src/shared/assets/v2/svg/HeartStrokeGray.svgis excluded by!**/*.svg,!**/*.svgand included bysrc/**src/shared/assets/v2/svg/Link.svgis excluded by!**/*.svg,!**/*.svgand included bysrc/**
📒 Files selected for processing (14)
src/pages/Test.tsxsrc/routes/router.tsxsrc/shared/components/button/linkButton/LinkButton.css.tssrc/shared/components/v2/button/actionButton/ActionButton.css.tssrc/shared/components/v2/button/actionButton/ActionButton.tsxsrc/shared/components/v2/cardProduct/CardProduct.css.tssrc/shared/components/v2/cardProduct/CardProduct.tsxsrc/shared/components/v2/cardProduct/ListCardProduct.css.tssrc/shared/components/v2/cardProduct/ListCardProduct.tsxsrc/shared/components/v2/linkButton/LinkButton.css.tssrc/shared/components/v2/linkButton/LinkButton.tsxsrc/shared/components/v2/saveButton/SaveButton.css.tssrc/shared/components/v2/saveButton/SaveButton.tsxsrc/shared/styles/tokensV2/color.css.ts
Comment on lines
+1
to
+132
| // ComponentTest.tsx | ||
| import { useState } from 'react'; | ||
|
|
||
| import CardProduct from '@components/v2/cardProduct/CardProduct'; | ||
|
|
||
| import ListCardProduct from '@/shared/components/v2/cardProduct/ListCardProduct'; | ||
|
|
||
| const ComponentTest = () => { | ||
| const [isSaved1, setIsSaved1] = useState(false); | ||
| const [isSaved2, setIsSaved2] = useState(false); | ||
|
|
||
| return ( | ||
| <div | ||
| style={{ | ||
| padding: '10px', | ||
| display: 'flex', | ||
| flexDirection: 'column', | ||
| gap: '48px', | ||
| }} | ||
| > | ||
| {/* large */} | ||
| <div> | ||
| <div | ||
| style={{ | ||
| display: 'grid', | ||
| gridTemplateColumns: '1fr 1fr', | ||
| gap: '16px', | ||
| }} | ||
| > | ||
| <CardProduct | ||
| title="상품명은 최대 두 줄까지 쓸 수 있어요. 상품명은 최대 두 줄..." | ||
| brand="브랜드명은 최대 한 줄 까지 쓸 수..." | ||
| isSaved={isSaved1} | ||
| onToggleSave={() => setIsSaved1((prev) => !prev)} | ||
| linkHref="https://example.com" | ||
| linkLabel="사이트" | ||
| originalPrice={1000000} | ||
| discountRate={0} | ||
| discountPrice={1000000} | ||
| colorHexes={['#fff', '#999', '#666', '#333']} | ||
| saveCount={1000} | ||
| /> | ||
| <CardProduct | ||
| title="상품명은 최대 두 줄까지 쓸 수 있어요. 상품명은 최대 두 줄..." | ||
| brand="브랜드명은 최대 한 줄 까지 쓸 수..." | ||
| isSaved={isSaved2} | ||
| onToggleSave={() => setIsSaved2((prev) => !prev)} | ||
| linkHref="https://example.com" | ||
| linkLabel="사이트" | ||
| discountPrice={1000000} | ||
| colorHexes={['#ccc', '#999', '#666']} | ||
| /> | ||
|
|
||
| <CardProduct | ||
| cardType="shopping" | ||
| title="상품명은 최대 두 줄까지 쓸 수 있어요. 상품명은 최대 두 줄..." | ||
| isSaved={isSaved1} | ||
| onToggleSave={() => setIsSaved1((prev) => !prev)} | ||
| linkHref="https://example.com" | ||
| linkLabel="사이트" | ||
| discountRate={0} | ||
| discountPrice={1000000} | ||
| colorHexes={['#fff', '#999', '#666', '#333']} | ||
| saveCount={1000} | ||
| /> | ||
| <CardProduct | ||
| cardType="shopping" | ||
| title="상품명은 최대 두 줄까지 쓸 수 있어요. 상품명은 최대 두 줄..." | ||
| isSaved={isSaved2} | ||
| onToggleSave={() => setIsSaved2((prev) => !prev)} | ||
| linkHref="https://example.com" | ||
| linkLabel="사이트" | ||
| discountPrice={1000000} | ||
| colorHexes={['#ccc', '#999', '#666']} | ||
| /> | ||
| </div> | ||
| </div> | ||
|
|
||
| {/* List */} | ||
|
|
||
| <div | ||
| style={{ | ||
| display: 'flex', | ||
| flexDirection: 'column', | ||
| gap: '16px', | ||
| marginBottom: '100px', | ||
| }} | ||
| > | ||
| <h3>ListCardProduct</h3> | ||
| <ListCardProduct | ||
| cardSize="s" | ||
| title="리스트 상품명은 최대 한 줄까지 쓸 수 있어요." | ||
| isSaved={isSaved1} | ||
| onToggleSave={() => setIsSaved1((prev) => !prev)} | ||
| linkHref="https://example.com" | ||
| originalPrice={1000000} | ||
| /> | ||
| <ListCardProduct | ||
| cardSize="s" | ||
| title="리스트 상품명은 최대 한 줄까지 쓸 수 있어요." | ||
| isSaved={isSaved1} | ||
| onToggleSave={() => setIsSaved1((prev) => !prev)} | ||
| linkHref="https://example.com" | ||
| discountRate={0} | ||
| discountPrice={1000000} | ||
| /> | ||
|
|
||
| <ListCardProduct | ||
| cardSize="m" | ||
| title="리스트 상품명은 최대 한 줄까지 쓸 수 있어요리스트 상품명은 최대 한 줄까지 쓸 수 있어요리스트 상품명은 최대 한 줄까지 쓸 수 있어요리스트 상품명은 최대 한 줄까지 쓸 수 있어요" | ||
| isSaved={isSaved2} | ||
| onToggleSave={() => setIsSaved2((prev) => !prev)} | ||
| linkHref="https://example.com" | ||
| originalPrice={1000000} | ||
| colorHexes={['#fff', '#999', '#666', '#333']} | ||
| /> | ||
| <ListCardProduct | ||
| cardSize="m" | ||
| title="리스트 상품명은 최대 한 줄까지 쓸 수 있어요." | ||
| isSaved={isSaved2} | ||
| onToggleSave={() => setIsSaved2((prev) => !prev)} | ||
| linkHref="https://example.com" | ||
| discountRate={0} | ||
| discountPrice={1000000} | ||
| colorHexes={['#ccc', '#999', '#666']} | ||
| /> | ||
| </div> | ||
| </div> | ||
| ); | ||
| }; | ||
|
|
||
| export default ComponentTest; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Storybook 활용 권장
컴포넌트 데모/테스트용 페이지 대신 Storybook stories를 활용하면 프로덕션 번들에 포함되지 않고 체계적으로 컴포넌트를 문서화할 수 있어요.
Based on learnings: "Document UI components using Storybook stories in the stories/ directory"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/Test.tsx` around lines 1 - 132, The Test.tsx demo page
(ComponentTest component) should be replaced by Storybook stories so the
examples don't get bundled into production: create stories under stories/ for
CardProduct and ListCardProduct that reproduce the variants shown (grid of
CardProduct with cardType "shopping"/default, different prop sets: title, brand,
isSaved/onToggleSave interactive control, linkHref/linkLabel,
originalPrice/discountRate/discountPrice, colorHexes, saveCount; and
ListCardProduct stories for cardSize "s"/"m" with same prop permutations),
implement a simple local controls/state in the stories to toggle isSaved
(mirroring onToggleSave), add proper story meta (title, component, argTypes) and
remove or delete Test.tsx (ComponentTest) from the app entry so it is not
included in the production bundle.
Comment on lines
+24
to
+25
| import ComponentTest from '@/pages/Test'; | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
코드 스플리팅 적용 권장
다른 라우트들은 lazy를 사용해 동적 import를 적용했지만, ComponentTest는 직접 import되어 있어요. 번들 크기 최적화를 위해 lazy loading을 적용하는 것이 좋아요.
As per coding guidelines: "코드 스플리팅(dynamic import)과 적절한 청크 분리"
♻️ lazy loading 적용 제안
-import ComponentTest from '@/pages/Test'; {
path: ROUTES.TEST,
- element: <ComponentTest />,
+ lazy: async () => {
+ const { default: ComponentTest } = await import('@/pages/Test');
+ return { Component: ComponentTest };
+ },
},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/routes/router.tsx` around lines 24 - 25, The route is statically
importing ComponentTest which prevents code-splitting; replace the top-level
import of ComponentTest with a dynamic import using React.lazy (e.g., const
ComponentTest = lazy(() => import('@/pages/Test'))), remove the static import
statement, and ensure the route that renders ComponentTest is wrapped in a
Suspense with an appropriate fallback (or uses the app's existing suspense
boundary) so the lazily-loaded chunk loads correctly.
Comment on lines
+30
to
+33
| { | ||
| path: ROUTES.TEST, | ||
| element: <ComponentTest />, | ||
| }, |
There was a problem hiding this comment.
테스트 페이지가 프로덕션 빌드에 포함됨
/test 경로가 publicRoutes에 포함되어 인증 없이 누구나 접근할 수 있어요. PR 설명에 "test/demo components left in PR"로 언급되어 있지만, 프로덕션 환경에서 테스트 컴포넌트가 노출되는 것은 바람직하지 않아요.
환경 변수로 개발 환경에서만 활성화하거나, 머지 전에 제거를 고려해 주세요.
🛡️ 환경 변수 기반 조건부 라우트 제안
const publicRoutes = [
+ ...(import.meta.env.DEV
+ ? [
+ {
+ path: ROUTES.TEST,
+ element: <ComponentTest />,
+ },
+ ]
+ : []),
- {
- path: ROUTES.TEST,
- element: <ComponentTest />,
- },📝 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
| { | |
| path: ROUTES.TEST, | |
| element: <ComponentTest />, | |
| }, | |
| ...(import.meta.env.DEV | |
| ? [ | |
| { | |
| path: ROUTES.TEST, | |
| element: <ComponentTest />, | |
| }, | |
| ] | |
| : []), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/routes/router.tsx` around lines 30 - 33, The /test route (ROUTES.TEST)
exposing ComponentTest is included in publicRoutes and must not be shipped to
production; change the router registration so ComponentTest is only added when
running in non-production (e.g., guard inclusion using process.env.NODE_ENV !==
'production' or your build-time env var like import.meta.env.DEV), or remove the
route entirely before merge. Update the publicRoutes array (or the code that
constructs it) to conditionally push the ROUTES.TEST entry or omit it based on
the env flag, and ensure any references to ComponentTest are removed from
production bundles by using the conditional guard around the route definition.
| import { zIndex } from '@styles/tokens/zIndex'; | ||
|
|
||
| import { colorVars } from '@/shared/styles/tokensV2/color.css'; | ||
| import { fontVars } from '@/shared/styles/tokensV2/font.css'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
이 파일도 폰트 토큰은 helper 경로로 맞춰 주세요.
fontVars.font.* 직접 참조가 많아서 토큰 사용 방식이 형제 모듈과 분산됩니다. 공통 fontStyle()로 통일해 두면 이후 토큰 변경 시 수정 범위를 줄일 수 있습니다. As per coding guidelines, 'fontStyle() 함수를 통한 폰트 토큰 변환'.
Also applies to: 122-145, 149-183, 192-194
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/shared/components/v2/cardProduct/CardProduct.css.ts` at line 11, The file
currently imports and uses fontVars directly (fontVars.font.*); update the
import to use the shared helper token path and replace direct fontVars
references with the unified fontStyle() conversion helper so font tokens are
consumed via fontStyle() across this module; specifically change the import of
fontVars to the helper export and refactor all usages (e.g., any fontVars.font.*
occurrences) to call fontStyle(...) so styles in CardProduct.css.ts consistently
use the shared fontStyle helper (apply the same change to the other blocks
flagged in the review where fontVars.font.* is used).
Comment on lines
+78
to
+92
| const handleWrapperClick = (event: React.MouseEvent<HTMLDivElement>) => { | ||
| const target = event.target as HTMLElement | null; | ||
| const areaElement = target?.closest?.('[data-click-area]') as HTMLElement; | ||
| const area = areaElement?.dataset?.clickArea as CardClickArea | undefined; | ||
| const resolvedArea: CardClickArea = | ||
| area === 'image' || area === 'title' ? area : 'card'; | ||
|
|
||
| onCardClick?.(resolvedArea); | ||
|
|
||
| if (!enableWholeCardLink) return; | ||
| if (!linkHref) return; | ||
| if (typeof window === 'undefined') return; | ||
|
|
||
| window.open(linkHref, '_blank', 'noopener,noreferrer'); | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd CardProduct.tsx src/shared/componentsRepository: TEAM-HOUME/HOUME-CLIENT
Length of output: 231
🏁 Script executed:
wc -l src/shared/components/v2/cardProduct/CardProduct.tsxRepository: TEAM-HOUME/HOUME-CLIENT
Length of output: 123
🏁 Script executed:
cat -n src/shared/components/v2/cardProduct/CardProduct.tsxRepository: TEAM-HOUME/HOUME-CLIENT
Length of output: 9046
🏁 Script executed:
rg "window\.open" src/ --type ts --type tsx -B 2 -A 2Repository: TEAM-HOUME/HOUME-CLIENT
Length of output: 94
🏁 Script executed:
rg "window\.open" src/ -B 2 -A 2Repository: TEAM-HOUME/HOUME-CLIENT
Length of output: 2436
🏁 Script executed:
rg "validat|isValidUrl|safeUrl|validateUrl|http|https" src/ --type ts | grep -i "url\|http" | head -20Repository: TEAM-HOUME/HOUME-CLIENT
Length of output: 1723
🏁 Script executed:
fd "util\|helper\|lib" src/shared --type dRepository: TEAM-HOUME/HOUME-CLIENT
Length of output: 49
🏁 Script executed:
find src/shared -maxdepth 2 -name "*.ts" -o -name "*.tsx" | grep -E "util|helper|lib|service" | head -20Repository: TEAM-HOUME/HOUME-CLIENT
Length of output: 189
🏁 Script executed:
ls -la src/shared/utils/Repository: TEAM-HOUME/HOUME-CLIENT
Length of output: 435
🏁 Script executed:
cat -n src/shared/utils/*.tsRepository: TEAM-HOUME/HOUME-CLIENT
Length of output: 6659
래퍼와 "사이트" 버튼의 링크 오픈 방식을 통합하고 URL 검증을 추가하세요.
현재 wrapper는 window.open을 직접 호출하고, "사이트" 버튼은 onLinkClick 콜백만 실행해서 일관성이 없습니다. 또한 linkHref에 대한 프로토콜 검증이 없어 javascript: URL 같은 악의적인 값이 실행될 수 있습니다.
http/https 검증을 포함한 공통 helper를 만들고, 클릭(handleWrapperClick 라인 91), 키보드(handleWrapperKeyDown 라인 102), "사이트" 버튼이 모두 동일한 helper를 통해 안전하게 링크를 열도록 통일해 주세요. 같은 패턴이 ListCardProduct와 기존 CardProduct에도 있으니 함께 수정 부탁합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/shared/components/v2/cardProduct/CardProduct.tsx` around lines 78 - 92,
Create a single safe link helper (e.g., safeOpenLink) that validates linkHref
protocol (allow only http/https), normalizes/returns early for invalid URLs, and
then opens the URL with window.open(..., '_blank', 'noopener,noreferrer') only
when typeof window !== 'undefined'; replace direct window.open calls in
handleWrapperClick and the "사이트" button handler (the onLinkClick usage) to call
safeOpenLink(linkHref), and make handleWrapperKeyDown use the same helper for
Enter/Space activation; ensure onCardClick and enableWholeCardLink behavior
remains (call onCardClick before invoking safeOpenLink), and apply the same
helper refactor to ListCardProduct and the legacy CardProduct components so all
link openings share validation and opening logic.
Comment on lines
+3
to
+4
| import { fontStyle } from '@styles/fontStyle'; | ||
| import { colorVars } from '@styles/tokens/color.css'; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
V2 컴포넌트에서 V1 토큰 사용
src/shared/components/v2/linkButton/는 V2 디렉토리이지만 V1 토큰(@styles/tokens/color.css, @styles/fontStyle)을 사용하고 있어요. 반면 src/shared/components/button/linkButton/LinkButton.css.ts는 V2 토큰으로 업데이트되었어요.
일관성을 위해 V2 컴포넌트는 V2 토큰(@/shared/styles/tokensV2/color.css, @/shared/styles/tokensV2/font.css)을 사용하는 것이 좋아요.
♻️ V2 토큰 사용 제안
-import { fontStyle } from '@styles/fontStyle';
-import { colorVars } from '@styles/tokens/color.css';
+import { colorVars } from '@/shared/styles/tokensV2/color.css';
+import { fontVars } from '@/shared/styles/tokensV2/font.css';그리고 withText variant에서:
- ...fontStyle('caption_r_11'),
- color: colorVars.color.gray700,
+ ...fontVars.font.caption_r_11,
+ color: colorVars.color.text.secondary,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/shared/components/v2/linkButton/LinkButton.css.ts` around lines 3 - 4,
The V2 LinkButton CSS is still importing V1 tokens; update the imports in
src/shared/components/v2/linkButton/LinkButton.css.ts to use the V2 token
modules instead of '@styles/tokens/color.css' and '@styles/fontStyle' (replace
with '@/shared/styles/tokensV2/color.css' and
'@/shared/styles/tokensV2/font.css' or your project’s V2 equivalents), then
adjust any references to the imported symbols (e.g., colorVars and fontStyle) to
match the V2 token API and ensure the withText variant in the LinkButton styles
uses the V2 token names for colors and fonts.
Comment on lines
+1
to
+7
| import LinkIcon from '@assets/icons/icnLink.svg?react'; | ||
|
|
||
| import * as styles from './LinkButton.css'; | ||
| interface LinkButtonProps extends React.ComponentProps<'a'> { | ||
| children?: React.ReactNode; | ||
| typeVariant?: 'withText' | 'onlyIcon'; | ||
| } |
There was a problem hiding this comment.
React import 누락
React.ComponentProps<'a'>를 사용하지만 React가 import되지 않았어요. TypeScript 컴파일 에러가 발생할 수 있어요.
🐛 수정 제안
+import React from 'react';
+
import LinkIcon from '@assets/icons/icnLink.svg?react';📝 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
| import LinkIcon from '@assets/icons/icnLink.svg?react'; | |
| import * as styles from './LinkButton.css'; | |
| interface LinkButtonProps extends React.ComponentProps<'a'> { | |
| children?: React.ReactNode; | |
| typeVariant?: 'withText' | 'onlyIcon'; | |
| } | |
| import React from 'react'; | |
| import LinkIcon from '@assets/icons/icnLink.svg?react'; | |
| import * as styles from './LinkButton.css'; | |
| interface LinkButtonProps extends React.ComponentProps<'a'> { | |
| children?: React.ReactNode; | |
| typeVariant?: 'withText' | 'onlyIcon'; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/shared/components/v2/linkButton/LinkButton.tsx` around lines 1 - 7,
LinkButton.tsx is using the type React.ComponentProps<'a'> (in the
LinkButtonProps interface) but React isn't imported, causing TypeScript errors;
add a React import (e.g., import * as React from 'react' or import React from
'react') at the top of the file so the LinkButtonProps type and any JSX types
resolve correctly.
Comment on lines
+14
to
+26
| return ( | ||
| <a | ||
| {...props} | ||
| target="_blank" | ||
| rel="noopener noreferrer" // 새 탭으로 열기 | ||
| className={styles.linkButton({ | ||
| type: typeVariant, | ||
| })} | ||
| > | ||
| <LinkIcon /> | ||
| {children} | ||
| </a> | ||
| ); |
There was a problem hiding this comment.
외부 className이 무시됨
{...props}로 전달된 className이 styles.linkButton()으로 덮어씌워져요. clsx를 사용해 병합해야 외부에서 전달한 스타일이 적용돼요.
🐛 className 병합 수정 제안
+import clsx from 'clsx';
+
import LinkIcon from '@assets/icons/icnLink.svg?react';-interface LinkButtonProps extends React.ComponentProps<'a'> {
+interface LinkButtonProps extends Omit<React.ComponentProps<'a'>, 'className'> {
children?: React.ReactNode;
typeVariant?: 'withText' | 'onlyIcon';
+ className?: string;
}
const LinkButton = ({
children,
typeVariant = 'withText',
+ className,
...props
}: LinkButtonProps) => {
return (
<a
{...props}
target="_blank"
rel="noopener noreferrer"
- className={styles.linkButton({
- type: typeVariant,
- })}
+ className={clsx(
+ styles.linkButton({ type: typeVariant }),
+ className
+ )}
>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/shared/components/v2/linkButton/LinkButton.tsx` around lines 14 - 26, The
LinkButton component currently spreads {...props} and then sets
className={styles.linkButton(...)} which overwrites any external className; fix
by destructuring className from props (e.g., function LinkButton({ children,
className, ...props })) and merge it with the generated class using clsx (or
classnames) so className={clsx(className, styles.linkButton({ type: typeVariant
}))}, and ensure you import clsx and remove className from the spread to avoid
duplicate props.
Comment on lines
+3
to
+9
| export const buttonWrapper = style({ | ||
| margin: 0, | ||
| border: 'none', | ||
| background: 'none', | ||
| padding: 0, | ||
| width: '100%', | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if global button reset exists in the project
rg -n "globalStyle.*button" --type=tsRepository: TEAM-HOUME/HOUME-CLIENT
Length of output: 125
🏁 Script executed:
sed -n '115,125p' src/shared/styles/reset.css.tsRepository: TEAM-HOUME/HOUME-CLIENT
Length of output: 285
중복된 버튼 리셋 스타일 제거
src/shared/styles/reset.css.ts에서 globalStyle('button', { all: 'unset', ... })로 모든 버튼 기본 스타일을 이미 제거하고 있으므로, margin: 0, border: 'none', background: 'none', padding: 0은 중복입니다. width: '100%'만 유지하고 버튼 리셋 스타일은 제거하세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/shared/components/v2/saveButton/SaveButton.css.ts` around lines 3 - 9,
The buttonWrapper style contains redundant button-reset properties already
handled by globalStyle('button', { all: 'unset', ... }); remove margin, border,
background and padding from the buttonWrapper style definition and leave only
the width: '100%' (i.e., update the exported buttonWrapper to only include the
unique width property).
Comment on lines
+22
to
+30
| <button | ||
| {...props} | ||
| type="button" | ||
| onClick={onClick} | ||
| aria-pressed={isSelected} | ||
| className={clsx(styles.buttonWrapper, className)} | ||
| > | ||
| {isSelected ? <SaveOnIcon /> : <SaveOffIcon />} | ||
| </button> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
접근성 개선: aria-label 추가 권장
버튼에 텍스트 없이 아이콘만 있어서 스크린 리더 사용자를 위해 aria-label을 추가하면 좋아요.
As per coding guidelines: "접근성 준수: ARIA 속성, label 연결, 키보드 포커스"
♿ 접근성 개선 제안
<button
{...props}
type="button"
onClick={onClick}
aria-pressed={isSelected}
+ aria-label={isSelected ? '저장됨' : '저장하기'}
className={clsx(styles.buttonWrapper, className)}
>📝 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
| <button | |
| {...props} | |
| type="button" | |
| onClick={onClick} | |
| aria-pressed={isSelected} | |
| className={clsx(styles.buttonWrapper, className)} | |
| > | |
| {isSelected ? <SaveOnIcon /> : <SaveOffIcon />} | |
| </button> | |
| <button | |
| {...props} | |
| type="button" | |
| onClick={onClick} | |
| aria-pressed={isSelected} | |
| aria-label={isSelected ? '저장됨' : '저장하기'} | |
| className={clsx(styles.buttonWrapper, className)} | |
| > | |
| {isSelected ? <SaveOnIcon /> : <SaveOffIcon />} | |
| </button> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/shared/components/v2/saveButton/SaveButton.tsx` around lines 22 - 30, The
SaveButton renders an icon-only button (SaveOnIcon/SaveOffIcon) without an
accessible name; update the SaveButton component to provide an aria-label on the
<button> using an explicit prop or props['aria-label'] fallback so screen
readers get a meaningful label (e.g. when isSelected true -> "Saved", else
"Save"); ensure the attribute is applied alongside existing {...props} and allow
callers to override it by passing their own aria-label.
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
1 participant
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.
📌 Summary
📄 Tasks
cardProduct (
cardTypedefault, shopping)그리드 형태로 사용되는 상품 카드 컴포넌트입니다.
cardType prop으로 default와 shopping('선택' 버튼이 있는 카드)두 가지 타입을 지원합니다.
default — 컬러칩, 사이트 링크 버튼, 하트 저장 버튼, 좋아요 수 표시
shopping — 상세보기 버튼, 선택 버튼 표시
cardProduct (
sizes, m)리스트 형태로 사용되는 상품 카드 컴포넌트입니다.
cardSize prop으로 s와 m 두 가지 사이즈를 지원하며 s 사이즈는 크기가 고정이고, m 사이즈는 반응형(텍스트 길이와 아이콘 배치 위치가 달라짐)이 적용되어 있습니다.
사용방법
🔍 To Reviewer
📸 Screenshot