Open
Conversation
…feat/styleDetailPage/#507
📝 WalkthroughSummary by CodeRabbit릴리스 노트
Walkthrough스타일 목록·상세 페이지와 관련 CSS·모의데이터를 추가하고, 색상 정규화 로직을 확장(문자열 및 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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-jvcczfpwvk.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: 6
🧹 Nitpick comments (2)
src/pages/style/StyleListPage.tsx (1)
24-31: 리스트 전체 카드에eager를 고정하지 않는 편이 좋겠습니다.이 화면은 여러 장의 L 카드가 세로로 이어지므로 전부 eager면 첫 렌더에서
이미지 요청이 한꺼번에 붙습니다. 첫 화면에 보이는 카드만 eager로 두고,
나머지는 lazy로 넘기는 쪽이 성능상 더 안전합니다. Based on learningsStyleCard들은 Explore 탭 첫 화면(above-the-fold) 영역에 표시되므로, imageLoading="eager"를 모든 카드에 적용하는 것이 LCP 성능에 유리하다. lazy loading은 뷰포트 밖 이미지에만 적합.예시 수정안
- {STYLELIST_MOCK.data.otherStyles.map((style) => ( + {STYLELIST_MOCK.data.otherStyles.map((style, index) => ( <StyleCard size="L" key={style.id} imageSrc={style.imageUrl} title={style.name} onClick={() => handleStyleClick(style.id)} - imageLoading="eager" + imageLoading={index === 0 ? 'eager' : 'lazy'} /> ))}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/style/StyleListPage.tsx` around lines 24 - 31, The StyleCard mapping currently forces imageLoading="eager" for every card (in the map over STYLELIST_MOCK.data.otherStyles); change the map to accept the index (map((style, idx) => ...)) and pass imageLoading conditionally (e.g., imageLoading="eager" only for the first visible card(s) such as idx === 0, otherwise "lazy") so only above-the-fold cards load eagerly; update the JSX for StyleCard (and any tests) to use that conditional prop and keep handleStyleClick, size, key, imageSrc, and title unchanged.src/pages/style/StyleDetailPage.tsx (1)
20-20: 색상 정규화 유틸은 shared 레이어로 올리는 편이 좋겠습니다.스타일 페이지가 generate/result 페이지 파일을 직접 참조하면 feature 경계가
뒤섞입니다.normalizeColorHexes는 공용 변환 로직이니shared/utils같은
중립 레이어로 옮겨 두면 재사용성과 의존 방향이 더 깔끔해집니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/style/StyleDetailPage.tsx` at line 20, The import of normalizeColorHexes in StyleDetailPage.tsx points to a feature page (generate/result/curationProducts) and should be moved to a neutral shared utility to preserve feature boundaries: extract the normalizeColorHexes function into the shared utils layer (e.g., shared/utils), export it from that module, update StyleDetailPage to import normalizeColorHexes from the new shared utils export, and update (or add) any other consumers to import from the shared utils; optionally leave a re-export in the old module for backward compatibility and run the build/tests to ensure no broken imports.
🤖 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/generate/pages/result/curationSection/curationProducts.ts`:
- Around line 79-91: The object branch that handles color values currently
returns (color as { value: string }).value without ensuring it's a string,
allowing non-string values (e.g., { value: 123 }) to slip through; update that
branch (the handler that inspects color) to check typeof color.value ===
'string' and then run the same trim/hex-detection and COLOR_NAME_TO_HEX_MAP
lookup logic used in the string branch so only valid hex strings or mapped names
are returned, and ensure the final .filter type guard (which narrows to
string[]) aligns with this stricter check.
In `@src/pages/style/mocks/styleDetail.ts`:
- Around line 47-48: The mock's styleDescription contains placeholder/repeated
text ("높였더높였다...") that will surface in QA; update the styleDescription string
in src/pages/style/mocks/styleDetail.ts to remove the repeated placeholder and
replace it with a coherent, final descriptive sentence (e.g., complete the
sentence about the high-ceiling loft and desk placement) so the mock no longer
exposes temporary text.
- Around line 7-17: The mock entries that set imageUrl to literal strings like
'@assets/v2/images/Imgbanner_01.png' must be replaced so the bundler resolves
them at build time: import the image files at the top of the module and use
those imports for imageUrl (or use fully-qualified public URLs), and ensure any
linkUrl fields point to real product paths (or use an empty string) and set
enableWholeCardLink={false} where appropriate; update the mock objects
(references: imageUrl, linkUrl) and add the corresponding import names at the
top of the file so Vite resolves assets instead of leaving '@assets/…' runtime
strings.
In `@src/pages/style/StyleDetailPage.css.ts`:
- Around line 12-17: The fixed CTA overlaps the last content because the
scrollable area lacks bottom spacing; update the CSS for the scroll container
(exported container in StyleDetailPage.css.ts) or the wrapper used for the page
content to add a padding-bottom equal to the CTA height (and possibly safe-area
inset), e.g. increase padding-bottom by the CTA's height value or a unitVars
constant so the last items aren't covered; apply the same change to the
corresponding container/wrapper styles referenced around lines 38-47 as well.
In `@src/pages/style/StyleDetailPage.tsx`:
- Around line 18-20: The page always renders the same data because
StyleDetailPage imports and uses STYLE_DETAIL_MOCK directly; change the
rendering to derive the displayed mock from the incoming styleId (route param or
prop) instead of fixed STYLE_DETAIL_MOCK: read the styleId (e.g., from useParams
or props) in StyleDetailPage, pick the appropriate mock entry (map/filter
STYLE_DETAIL_MOCK.data by id) or fall back to a sensible default, and pass that
selected object through the same normalization pipeline (normalizeColorHexes)
and to the components that currently consume STYLE_DETAIL_MOCK; update any logic
around the current constant usage (references between lines ~54-77) to use the
selectedMock variable so different cards render distinct details.
- Around line 29-33: The heart icon isn't updating because toggleJjym (from
useJjymMutation) updates useSavedItemsStore but ListProductCard reads
save.isSaved from the immutable prop item.isLiked; update handleToggleSave to
wait for the mutation success and then either (a) update the prop-fed data
source by syncing the changed saved flag into the parent list state (so
ListProductCard receives a new item.isLiked), or (b) subscribe ListProductCard
(or its parent) to useSavedItemsStore so it re-renders when the store changes;
specifically modify handleToggleSave / useJjymMutation to accept an onSuccess
handler that patches the parent list state or calls the store subscription
updater, and ensure ListProductCard uses the reactive saved value (save.isSaved
from the store or the updated local state) instead of the fixed item.isLiked.
---
Nitpick comments:
In `@src/pages/style/StyleDetailPage.tsx`:
- Line 20: The import of normalizeColorHexes in StyleDetailPage.tsx points to a
feature page (generate/result/curationProducts) and should be moved to a neutral
shared utility to preserve feature boundaries: extract the normalizeColorHexes
function into the shared utils layer (e.g., shared/utils), export it from that
module, update StyleDetailPage to import normalizeColorHexes from the new shared
utils export, and update (or add) any other consumers to import from the shared
utils; optionally leave a re-export in the old module for backward compatibility
and run the build/tests to ensure no broken imports.
In `@src/pages/style/StyleListPage.tsx`:
- Around line 24-31: The StyleCard mapping currently forces imageLoading="eager"
for every card (in the map over STYLELIST_MOCK.data.otherStyles); change the map
to accept the index (map((style, idx) => ...)) and pass imageLoading
conditionally (e.g., imageLoading="eager" only for the first visible card(s)
such as idx === 0, otherwise "lazy") so only above-the-fold cards load eagerly;
update the JSX for StyleCard (and any tests) to use that conditional prop and
keep handleStyleClick, size, key, imageSrc, and title unchanged.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0eeae932-e7ea-4663-bccc-e17cbeddd64e
📒 Files selected for processing (7)
src/pages/generate/pages/result/curationSection/curationProducts.tssrc/pages/style/StyleDetailPage.css.tssrc/pages/style/StyleDetailPage.tsxsrc/pages/style/StyleListPage.css.tssrc/pages/style/StyleListPage.tsxsrc/pages/style/mocks/styleDetail.tssrc/shared/components/v2/productCard/ListProductCard.css.ts
Comment on lines
+18
to
+20
| import { STYLE_DETAIL_MOCK } from './mocks/styleDetail'; | ||
| import * as styles from './StyleDetailPage.css'; | ||
| import { normalizeColorHexes } from '../generate/pages/result/curationSection/curationProducts'; |
There was a problem hiding this comment.
현재 상세 화면은 styleId와 무관하게 항상 같은 데이터를 보여줍니다.
리스트에서는 styleId를 바꿔서 이동시키는데, 이 페이지 본문은
STYLE_DETAIL_MOCK.data를 고정 참조하고 있어서 어떤 카드를 눌러도 같은
상세가 렌더링됩니다. styleId로 mock을 선택하거나, 실제 API 연동 전이라도
id별 분기 정도는 넣어 두셔야 합니다.
Also applies to: 54-77
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/style/StyleDetailPage.tsx` around lines 18 - 20, The page always
renders the same data because StyleDetailPage imports and uses STYLE_DETAIL_MOCK
directly; change the rendering to derive the displayed mock from the incoming
styleId (route param or prop) instead of fixed STYLE_DETAIL_MOCK: read the
styleId (e.g., from useParams or props) in StyleDetailPage, pick the appropriate
mock entry (map/filter STYLE_DETAIL_MOCK.data by id) or fall back to a sensible
default, and pass that selected object through the same normalization pipeline
(normalizeColorHexes) and to the components that currently consume
STYLE_DETAIL_MOCK; update any logic around the current constant usage
(references between lines ~54-77) to use the selectedMock variable so different
cards render distinct details.
빌드 결과빌드 성공 🎊 |
빌드 결과빌드 성공 🎊 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/generate/pages/result/curationSection/curationProducts.ts`:
- Around line 79-95: The color parsing logic is duplicated across the object and
string branches; extract a helper (e.g., parseColorToHex(value: unknown): string
| null) that performs "trim → hex regex test → lookup in COLOR_NAME_TO_HEX_MAP"
and use it for both the object branch (pass (color as {value?: unknown}).value)
and the string branch (pass color). Ensure the helper returns null for
non-strings and preserves the current hex regex and mapping behavior so both
branches delegate to this single function.
🪄 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: 7c508c33-576f-4561-b6e0-681ffde32cca
📒 Files selected for processing (1)
src/pages/generate/pages/result/curationSection/curationProducts.ts
Comment on lines
+79
to
+95
| // { name, value } 형태 | ||
| if (typeof color === 'object' && color !== null && 'value' in color) { | ||
| const rawValue = (color as { value?: unknown }).value; | ||
| if (typeof rawValue !== 'string') return null; | ||
| const trimmed = rawValue.trim(); | ||
| if (/^#([0-9a-fA-F]{3}|[0-9a-fA-F]{6})$/.test(trimmed)) { | ||
| return trimmed; | ||
| } | ||
| return COLOR_NAME_TO_HEX_MAP[trimmed] ?? null; | ||
| } | ||
| // string 형태 → 한글 이름이면 매핑 | ||
| if (typeof color === 'string') { | ||
| const trimmed = color.trim(); | ||
| if (/^#([0-9a-fA-F]{3}|[0-9a-fA-F]{6})$/.test(trimmed)) return trimmed; | ||
| return COLOR_NAME_TO_HEX_MAP[trimmed] ?? null; | ||
| } | ||
| return null; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
색상 파싱 로직 중복을 헬퍼로 추출해 유지보수성을 높여주세요.
객체 분기와 문자열 분기에서 trim → hex 검사 → 이름 매핑 로직이
반복됩니다. 규칙 변경 시 한쪽만 수정되는 리스크가 있습니다.
리팩터링 예시
+const resolveColorToHex = (raw: string) => {
+ const trimmed = raw.trim();
+ if (/^#([0-9a-fA-F]{3}|[0-9a-fA-F]{6})$/.test(trimmed)) {
+ return trimmed;
+ }
+ return COLOR_NAME_TO_HEX_MAP[trimmed] ?? null;
+};
+
export const normalizeColorHexes = (value: unknown) => {
if (!Array.isArray(value)) return [];
return value
.map((color) => {
- // { name, value } 형태
if (typeof color === 'object' && color !== null && 'value' in color) {
const rawValue = (color as { value?: unknown }).value;
if (typeof rawValue !== 'string') return null;
- const trimmed = rawValue.trim();
- if (/^#([0-9a-fA-F]{3}|[0-9a-fA-F]{6})$/.test(trimmed)) {
- return trimmed;
- }
- return COLOR_NAME_TO_HEX_MAP[trimmed] ?? null;
+ return resolveColorToHex(rawValue);
}
- // string 형태 → 한글 이름이면 매핑
if (typeof color === 'string') {
- const trimmed = color.trim();
- if (/^#([0-9a-fA-F]{3}|[0-9a-fA-F]{6})$/.test(trimmed)) return trimmed;
- return COLOR_NAME_TO_HEX_MAP[trimmed] ?? null;
+ return resolveColorToHex(color);
}
return null;
})
.filter((color): color is string => Boolean(color));
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/generate/pages/result/curationSection/curationProducts.ts` around
lines 79 - 95, The color parsing logic is duplicated across the object and
string branches; extract a helper (e.g., parseColorToHex(value: unknown): string
| null) that performs "trim → hex regex test → lookup in COLOR_NAME_TO_HEX_MAP"
and use it for both the object branch (pass (color as {value?: unknown}).value)
and the string branch (pass color). Ensure the helper returns null for
non-strings and preserves the current hex regex and mapping behavior so both
branches delegate to this single function.
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
v2.1 스타일 디테일 페이지 (styleList, styleDetail) UI를 구현했습니다.
📄 Tasks
infoSection의 최소 넓이 수정상품명을 한줄까지만 쓸 수 있게 하기 위해
infoSection의 최소 넓이를 26.1rem만큼(공컴 페이지 기준) 설정했는데, 뷰에서는 더 넓이가 작아 최소 넓이를 수정했습니다.normalizeColorHexes함수 수정서버에서 내려주는 컬러와 관련된 데이터가 마이페이지에선
colors: ['브라운'],같은 string 배열로 내려주는데, 다른 페이지들에선 (우선 저는 스타일 디테일 페이지)[{ name: '브라운', value: '#8B4513' }]같은 객체 배열로 내려주고 있어서normalizeColorHexes함수에서 두 가지 경우를 분기처리 했습니다.product={{ title: item.name, imageUrl: item.imageUrl, colorHexes: normalizeColorHexes(item.colors), }}ListProductCard에선 같은 형태로 사용하고 있습니다.
🔍 To Reviewer
📸 Screenshot