[feat] 도면 선택 페이지 UI 및 내부 비즈니스 로직 구현 + 일부 공컴 수정#494
Open
[feat] 도면 선택 페이지 UI 및 내부 비즈니스 로직 구현 + 일부 공컴 수정#494
Conversation
- useFloorPlanStore: 공간 선택 페이지의 모든 상태를 한 곳에 정리 - useFloorPlanSelect: 공간 선택 페이지 훅 - useFloorPlanSheet: 도면 바텀시트 관리 훅 - floorPlan: 도면선택 페이지/API에 필요한 타입(추후 수정 필요)
- 바텀시트 primary/secondary 버튼 공통 스타일 추출 - FloorPlanSelectTest.tsx 제거 (개발용 테스트 페이지)
# Conflicts: # src/shared/components/v2/roomTypeCard/RoomTypeCard.tsx
- '저장된 내 공간을 불러왔어요'
- suffixIcon css 수정 - Chip 전체를 button 컴포넌트로 변경
- 가로 스크롤 컴포넌트 내에서는 16rem으로 고정(반응형 대응X)
- id에 keyof FloorPlanFilters 적용으로 사용처에서 타입 단언 제거
- 최적화 미적용으로 인해 연산 오버헤드 발생 시 추가할 예정 - API 연동 시 불필요해지는 함수들에 주석 추가
- FloorPlanSelectStep에서 필요한 store을 훅에 추가 - 훅 반환부에서 도면필터 그리드, FilterSheet, FloorPlanSheet, RecentSheet 에서 사용되는 훅을 객체로 묶어서 반환
📝 WalkthroughWalkthrough플로어플랜 선택 v2 기능(타입, 더미 데이터, Zustand 스토어, 훅, 그리드/필터/시트 UI, 테스트 페이지, 라우트)을 추가했고, BottomSheet·ActionButton·Chip·TitleNavBar·RoomTypeCard·PageLayout 등 공유 컴포넌트의 스타일·API를 조정했습니다. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 |
jstar000
changed the title
빌드 결과빌드 성공 🎊 |
🎨 Storybook 빌드 완료!📚 Storybook: https://686a831b8e000345a949970a-lqtzguytfr.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: 9fe788fe8f
ℹ️ 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".
![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 (2)
src/shared/components/v2/navBar/TitleNavBar.css.ts (1)
27-45:⚠️ Potential issue | 🟠 Major
backButton자체의 터치 영역은 유지해 주세요.지금은 여백이
label로만 이동해서 버튼 클릭 영역이 아이콘/텍스트 크기로 줄어듭니다. 이 상태면 모바일에서 뒤로가기 탭 타깃과 포커스 영역이 너무 작아집니다. 패딩이나min-width/min-height는backButton에 남기고,label은 아이콘-텍스트 간격만 담당하게 두는 편이 안전합니다.🔧 제안 수정안
export const backButton = style({ display: 'inline-flex', alignItems: 'center', justifyContent: 'center', + minWidth: '4.4rem', + minHeight: '4.4rem', transition: 'transform 120ms ease', border: 0, background: 'transparent', color: colorVars.color.text.tertiary, ...fontVars.font.title_r_15, selectors: { '&:active': { transform: 'scale(0.95)', }, }, }); export const label = style({ - padding: `${unitVars.unit.gapPadding['000']} ${unitVars.unit.gapPadding['100']}`, + paddingLeft: unitVars.unit.gapPadding['100'], + paddingRight: unitVars.unit.gapPadding['100'], });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/components/v2/navBar/TitleNavBar.css.ts` around lines 27 - 45, The touch target padding/minimum size should stay on backButton rather than on label; update the styles so backButton retains padding and a comfortable min-width/min-height (or padding values from unitVars) and remove any overall padding from label so label only controls icon–text spacing (e.g., use margin or gap inside label). Modify backButton (export const backButton) to include the padding/min-size and simplify label (export const label) to only handle spacing between the icon and text.src/shared/components/v2/chip/Chip.css.ts (1)
73-98: 🧹 Nitpick | 🔵 Trivial
suffix/suffixButton공통 스타일 중복을 줄이면 유지보수가 좋아집니다.패딩/정렬 속성이 두 블록에 중복되어 있어 추후 수정 시 drift가 생기기 쉽습니다. 공통 base style을 추출하고 interactive 변형만 분리하는 구조를 권장합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/components/v2/chip/Chip.css.ts` around lines 73 - 98, The suffix and suffixButton styles duplicate padding and alignment; extract a shared base style object (e.g., suffixBase or baseSuffixStyle) that contains display, alignItems, justifyContent, and the padding entries (unitVars.unit.gapPadding['200'|'300'|'000']), then have suffix reuse that base and suffixButton compose/extend it to add transition, color, and the selectors (&:active transform scale). Update the export names suffix and suffixButton to reference the base so future padding/alignment changes only need to be made in one place.
🤖 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/home/components/explore/RoomTypeSection/RoomTypeSection.tsx`:
- Line 61: The empty onClick={() => {}} in RoomTypeSection is a placeholder that
should be addressed: either implement the intended click behavior in the
RoomTypeSection component (e.g., call a prop handler or perform
navigation/filtering) or remove the prop and add a clear TODO comment linking to
an issue; locate the JSX element with onClick inside RoomTypeSection.tsx and
replace the noop with the real handler (call an existing prop like
onSelectRoomType or dispatch the appropriate action) or add a concise TODO
comment referencing an issue ID so this placeholder is tracked.
In `@src/pages/imageSetup/stores/useFunnelStore.ts`:
- Around line 5-6: 현재 주석 TODO는 요구사항 불확실성을 남겨두므로 useFunnelStore의 계약을 명시하세요: 제거할
주석 대신에 FunnelStore(또는 useFunnelStore) 내부에 단계 타입(예: enum Steps {MoodBoard,
ActivityInfo, ...}) 또는 각 스텝에 대한 필수 여부를 담은 플래그(예: requiredSteps: Record<Steps,
boolean>)를 추가하고, moodBoard와 activityInfo를 해당 타입/플래그로 표현해 분기 로직이 타입에 의해 결정되도록
변경하세요; 결정 보류가 필요하면 관련 이슈 번호를 참조하는 필드(issueId: string) 또는 주석을 남겨 결정 시점을 고정하세요.
In `@src/pages/imageSetup/v2/hooks/useFloorPlanSelect.ts`:
- Around line 124-144: handleConfirmRecentFloorPlan currently calls
useFunnelStore.getState().setFloorPlan(...) directly; refactor it to extract the
setFloorPlan action via the hook (e.g., const setFloorPlan = useFunnelStore(s =>
s.setFloorPlan)) and call setFloorPlan(floorPlanData) instead, remove the direct
getState usage, and update the hook dependency array to include setFloorPlan
(and remove store if no longer needed) to mirror the pattern used in
handleConfirmFloorPlan for consistency.
- Around line 102-121: handleConfirmFloorPlan currently reads store.isMirror and
calls useFunnelStore.getState().setFloorPlan(...) while only listing
store.isMirror in the deps, which bypasses React reactivity and misses
dependencies; fix by replacing direct getState() usage with selectors: read
isMirror and the setFloorPlan setter via useFunnelStore(selector) (e.g., const
setFloorPlan = useFunnelStore(s => s.setFloorPlan) and const storeIsMirror =
useFunnelStore(s => s.isMirror)), use those values inside handleConfirmFloorPlan
(instead of useFunnelStore.getState() and store.isMirror), and include the
selected selector results (setFloorPlan and storeIsMirror) in the dependency
array along with selectedFloorPlan, savedHouseInfo, context, and onNext so
handleConfirmFloorPlan has correct dependencies.
- Around line 93-99: handleCardClick가 전체 store 객체를 의존성으로 사용해 불필요한 재생성을 유발할 수
있습니다; 대신 useStore에서 selectFloorPlan와 openFloorPlanSheet 같은 개별 액션(또는 해당 함수 참조들)을
구조 분해해 가져와 handleCardClick의 클로저에 직접 사용하고, 의존성 배열에는 그 개별 액션들만 넣어 참조 안정성을 보장하도록
변경하세요 (참조: handleCardClick, selectFloorPlan, openFloorPlanSheet).
- Around line 41-48: The effect currently disables ESLint instead of addressing
missing dependencies: ensure store.openRecentSheet and notify are handled safely
rather than ignoring them. Either include them in the dependency array and guard
execution with a mountedRef/ranRef so the body runs only once, or capture their
initial references via useRef (e.g., const notifyRef = useRef(notify); const
openRef = useRef(store.openRecentSheet)) and use those refs inside useEffect
together with recentFloorPlan (or just read recentFloorPlan into a ref at mount)
so you avoid unstable notify re-references while keeping the effect logically
run-once; update useEffect accordingly and remove the eslint-disable comment.
In `@src/pages/imageSetup/v2/steps/floorPlanSelect/FilterSheet.tsx`:
- Around line 38-51: The code reads pendingFilters[category.id as keyof
FloorPlanFilters] into currentValues and then accesses currentValues.length,
which can throw if undefined; change the assignment in FilterSheet so
currentValues defaults to an empty array (and validate it's an array) before use
(e.g., set currentValues = pendingFilters[category.id as keyof FloorPlanFilters]
?? [] or coerce non-arrays to []), and then use that safe currentValues in the
Chip selected logic and any other places that rely on length/includes to prevent
runtime errors.
In `@src/pages/imageSetup/v2/steps/floorPlanSelect/FloorPlanSelectGrid.css.ts`:
- Around line 39-44: The divider style uses inconsistent key access on
unitVars.unit.gapPadding (using gapPadding[800] vs gapPadding['500']); update
the access to use string keys consistently (change gapPadding[800] to
gapPadding['800']) in the divider definition so both uses of gapPadding use the
same string-key format, referencing the divider export and
unitVars.unit.gapPadding to locate the change.
In `@src/pages/imageSetup/v2/steps/floorPlanSelect/FloorPlanSelectStep.tsx`:
- Around line 51-80: The sheet lifecycle is being managed in this step component
(FilterSheet and FloorPlanSheet) while open/close state and selection logic are
split between the store and hooks; move sheet lifecycle into the overlay-kit per
the TODO: remove direct rendering of FilterSheet and FloorPlanSheet from this
step and instead register these sheets with overlay-kit (using the same props
currently passed: open state props like store.isFilterSheetOpen,
store.isFloorPlanSheetOpen, store.isRecentSheetOpen; handlers like
store.closeFilterSheet, store.closeRecentSheet, store.setPendingFilter,
store.applyFilters, store.resetFilters; selection callbacks
handleConfirmFloorPlan and handleConfirmRecentFloorPlan; and data values
selectedFloorPlan, selectedDetailViews, recentFloorPlan), then replace local
mounting with overlay-kit show/register calls so overlay-kit owns open/close,
stacking and lifecycle, and update any callers to trigger overlay-kit actions
rather than relying on this step to render sheets.
In `@src/pages/imageSetup/v2/steps/floorPlanSelect/FloorPlanSheet.tsx`:
- Around line 61-109: The mirroring class styles.mirrored is applied to the
outer wrapper (styles.mirrorWrapper) which flips the entire .swiperContainer
including navigation IconButton components; restrict the mirror transform to
only the slide content by moving styles.mirrored (or applying a new class) to
the element that renders the slide image (e.g., the <SwiperSlide> or the <img>
inside it) so that Swiper, navigation buttons (IconButton) and their click
handlers (swiperRef.current.slidePrev/slideNext) remain unflipped; update the
JSX to remove styles.mirrored from the wrapper and apply it to the slide content
element (keeping Swiper, SwiperSlide, swiperRef, and setViewIndex usage
unchanged).
In `@src/shared/components/v2/chip/Chip.tsx`:
- Around line 37-52: The nested interactive suffix (the span using
role="button"/tabIndex with onSuffixClick inside the interactive root in
Chip.tsx) breaks accessibility; if onSuffixClick represents an independent
action, refactor so the suffix is rendered as a sibling <button> (or a focusable
native element) rather than a nested element with role="button", move aria-label
and keyboard handling to that sibling button, remove role/tabIndex from the
inner span, and ensure you call stopPropagation or otherwise prevent the root
action when the suffix button is activated; locate the suffix rendering block
that uses onSuffixClick and styles.suffixButton/styles.suffix and change it to a
separate button element alongside the root click handler.
In `@src/shared/components/v2/navBar/TitleNavBar.tsx`:
- Around line 29-31: The span with className styles.label is rendered even when
backLabel is undefined, leaving an empty DOM node; update TitleNavBar so the
span is conditionally rendered only when backLabel is not null/undefined (e.g.,
check backLabel !== undefined/null or truthy) around the existing <span
className={styles.label}>{backLabel}</span> inside the back button (the element
containing Icon name="ArrowLeft"), so no empty <span> remains when backLabel is
absent.
---
Outside diff comments:
In `@src/shared/components/v2/chip/Chip.css.ts`:
- Around line 73-98: The suffix and suffixButton styles duplicate padding and
alignment; extract a shared base style object (e.g., suffixBase or
baseSuffixStyle) that contains display, alignItems, justifyContent, and the
padding entries (unitVars.unit.gapPadding['200'|'300'|'000']), then have suffix
reuse that base and suffixButton compose/extend it to add transition, color, and
the selectors (&:active transform scale). Update the export names suffix and
suffixButton to reference the base so future padding/alignment changes only need
to be made in one place.
In `@src/shared/components/v2/navBar/TitleNavBar.css.ts`:
- Around line 27-45: The touch target padding/minimum size should stay on
backButton rather than on label; update the styles so backButton retains padding
and a comfortable min-width/min-height (or padding values from unitVars) and
remove any overall padding from label so label only controls icon–text spacing
(e.g., use margin or gap inside label). Modify backButton (export const
backButton) to include the padding/min-size and simplify label (export const
label) to only handle spacing between the icon and text.
🪄 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: 5e8a73be-4c82-45a5-af1d-73a4121edf51
⛔ Files ignored due to path filters (2)
src/shared/assets/v2/images/ImgEmpty.pngis excluded by!**/*.png,!**/*.pngand included bysrc/**src/shared/assets/v2/svg/Close.svgis excluded by!**/*.svg,!**/*.svgand included bysrc/**
📒 Files selected for processing (29)
src/pages/home/components/explore/RoomTypeSection/RoomTypeSection.css.tssrc/pages/home/components/explore/RoomTypeSection/RoomTypeSection.tsxsrc/pages/imageSetup/stores/useFunnelStore.tssrc/pages/imageSetup/v2/constants/floorPlanDummy.tssrc/pages/imageSetup/v2/hooks/useFloorPlanSelect.tssrc/pages/imageSetup/v2/hooks/useFloorPlanSheet.tssrc/pages/imageSetup/v2/steps/floorPlanSelect/FilterSheet.css.tssrc/pages/imageSetup/v2/steps/floorPlanSelect/FilterSheet.tsxsrc/pages/imageSetup/v2/steps/floorPlanSelect/FloorPlanSelectGrid.css.tssrc/pages/imageSetup/v2/steps/floorPlanSelect/FloorPlanSelectGrid.tsxsrc/pages/imageSetup/v2/steps/floorPlanSelect/FloorPlanSelectStep.css.tssrc/pages/imageSetup/v2/steps/floorPlanSelect/FloorPlanSelectStep.tsxsrc/pages/imageSetup/v2/steps/floorPlanSelect/FloorPlanSheet.css.tssrc/pages/imageSetup/v2/steps/floorPlanSelect/FloorPlanSheet.tsxsrc/pages/imageSetup/v2/stores/useFloorPlanStore.tssrc/pages/imageSetup/v2/types/floorPlan.tssrc/shared/components/pageLayout/PageLayout.css.tssrc/shared/components/pageLayout/PageLayout.tsxsrc/shared/components/v2/bottomSheet/BottomSheetBase.css.tssrc/shared/components/v2/bottomSheet/BottomSheetBase.tsxsrc/shared/components/v2/bottomSheet/CloseBottomSheet.tsxsrc/shared/components/v2/button/actionButton/ActionButton.css.tssrc/shared/components/v2/button/actionButton/ActionButton.tsxsrc/shared/components/v2/chip/Chip.css.tssrc/shared/components/v2/chip/Chip.tsxsrc/shared/components/v2/navBar/TitleNavBar.css.tssrc/shared/components/v2/navBar/TitleNavBar.tsxsrc/shared/components/v2/roomTypeCard/RoomTypeCard.css.tssrc/shared/components/v2/roomTypeCard/RoomTypeCard.tsx
Comment on lines
+5
to
+6
| // TODO: FunnelStore 수정 | ||
| // moodBoard, activityInfo 스텝은 이미지 생성에 필요할 수도 있고 아닐 수도 있음 |
There was a problem hiding this comment.
불확실한 요구사항 TODO는 스토어 계약으로 명시해 주세요.
Line 5~6의 TODO처럼 필수 여부가 미정인 상태를 주석으로만 남기면,
후속 단계에서 분기 해석이 달라질 수 있습니다. moodBoard/activityInfo의
필수 여부를 타입(예: 단계 enum/플래그)으로 명시하거나 이슈 번호를 연결해
결정 시점을 고정하는 쪽이 안전합니다. 원하시면 현재 플로우 기준으로
타입 계약안까지 바로 제안드릴게요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/imageSetup/stores/useFunnelStore.ts` around lines 5 - 6, 현재 주석
TODO는 요구사항 불확실성을 남겨두므로 useFunnelStore의 계약을 명시하세요: 제거할 주석 대신에 FunnelStore(또는
useFunnelStore) 내부에 단계 타입(예: enum Steps {MoodBoard, ActivityInfo, ...}) 또는 각 스텝에
대한 필수 여부를 담은 플래그(예: requiredSteps: Record<Steps, boolean>)를 추가하고, moodBoard와
activityInfo를 해당 타입/플래그로 표현해 분기 로직이 타입에 의해 결정되도록 변경하세요; 결정 보류가 필요하면 관련 이슈 번호를
참조하는 필드(issueId: string) 또는 주석을 남겨 결정 시점을 고정하세요.
Comment on lines
+102
to
+121
| const handleConfirmFloorPlan = useCallback(() => { | ||
| if (!selectedFloorPlan) return; | ||
|
|
||
| const floorPlanData = { | ||
| floorPlanId: selectedFloorPlan.id, | ||
| isMirror: store.isMirror, | ||
| }; | ||
|
|
||
| useFunnelStore.getState().setFloorPlan(floorPlanData); | ||
|
|
||
| const payload: CompletedFloorPlan = { | ||
| houseType: savedHouseInfo?.houseType ?? context.houseType, | ||
| roomType: savedHouseInfo?.roomType ?? context.roomType, | ||
| areaType: savedHouseInfo?.areaType ?? context.areaType, | ||
| houseId: savedHouseInfo?.houseId ?? context.houseId, | ||
| floorPlan: floorPlanData, | ||
| }; | ||
|
|
||
| onNext(payload); | ||
| }, [selectedFloorPlan, store.isMirror, savedHouseInfo, context, onNext]); |
There was a problem hiding this comment.
handleConfirmFloorPlan 의존성 배열에 store 누락
Line 107에서 store.isMirror를 사용하고 있지만 의존성 배열에는 store.isMirror만 포함되어 있어요. 또한 useFunnelStore.getState()를 컴포넌트 외부에서 직접 호출하는 패턴이 사용되고 있는데, 이는 React의 반응성 시스템을 우회해요.
🔧 수정 제안
+ const setFloorPlan = useFunnelStore((state) => state.setFloorPlan);
const handleConfirmFloorPlan = useCallback(() => {
if (!selectedFloorPlan) return;
const floorPlanData = {
floorPlanId: selectedFloorPlan.id,
isMirror: store.isMirror,
};
- useFunnelStore.getState().setFloorPlan(floorPlanData);
+ setFloorPlan(floorPlanData);
// ...
- }, [selectedFloorPlan, store.isMirror, savedHouseInfo, context, onNext]);
+ }, [selectedFloorPlan, store.isMirror, savedHouseInfo, context, onNext, setFloorPlan]);📝 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
| const handleConfirmFloorPlan = useCallback(() => { | |
| if (!selectedFloorPlan) return; | |
| const floorPlanData = { | |
| floorPlanId: selectedFloorPlan.id, | |
| isMirror: store.isMirror, | |
| }; | |
| useFunnelStore.getState().setFloorPlan(floorPlanData); | |
| const payload: CompletedFloorPlan = { | |
| houseType: savedHouseInfo?.houseType ?? context.houseType, | |
| roomType: savedHouseInfo?.roomType ?? context.roomType, | |
| areaType: savedHouseInfo?.areaType ?? context.areaType, | |
| houseId: savedHouseInfo?.houseId ?? context.houseId, | |
| floorPlan: floorPlanData, | |
| }; | |
| onNext(payload); | |
| }, [selectedFloorPlan, store.isMirror, savedHouseInfo, context, onNext]); | |
| const setFloorPlan = useFunnelStore((state) => state.setFloorPlan); | |
| const handleConfirmFloorPlan = useCallback(() => { | |
| if (!selectedFloorPlan) return; | |
| const floorPlanData = { | |
| floorPlanId: selectedFloorPlan.id, | |
| isMirror: store.isMirror, | |
| }; | |
| setFloorPlan(floorPlanData); | |
| const payload: CompletedFloorPlan = { | |
| houseType: savedHouseInfo?.houseType ?? context.houseType, | |
| roomType: savedHouseInfo?.roomType ?? context.roomType, | |
| areaType: savedHouseInfo?.areaType ?? context.areaType, | |
| houseId: savedHouseInfo?.houseId ?? context.houseId, | |
| floorPlan: floorPlanData, | |
| }; | |
| onNext(payload); | |
| }, [selectedFloorPlan, store.isMirror, savedHouseInfo, context, onNext, setFloorPlan]); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/imageSetup/v2/hooks/useFloorPlanSelect.ts` around lines 102 - 121,
handleConfirmFloorPlan currently reads store.isMirror and calls
useFunnelStore.getState().setFloorPlan(...) while only listing store.isMirror in
the deps, which bypasses React reactivity and misses dependencies; fix by
replacing direct getState() usage with selectors: read isMirror and the
setFloorPlan setter via useFunnelStore(selector) (e.g., const setFloorPlan =
useFunnelStore(s => s.setFloorPlan) and const storeIsMirror = useFunnelStore(s
=> s.isMirror)), use those values inside handleConfirmFloorPlan (instead of
useFunnelStore.getState() and store.isMirror), and include the selected selector
results (setFloorPlan and storeIsMirror) in the dependency array along with
selectedFloorPlan, savedHouseInfo, context, and onNext so handleConfirmFloorPlan
has correct dependencies.
src/pages/imageSetup/v2/steps/floorPlanSelect/FloorPlanSelectGrid.css.ts
Show resolved
Hide resolved
src/pages/imageSetup/v2/steps/floorPlanSelect/FloorPlanSheet.tsx
Outdated
Show resolved
Hide resolved
Comment on lines
+37
to
52
| <span | ||
| role={onSuffixClick ? 'button' : undefined} | ||
| tabIndex={onSuffixClick ? 0 : undefined} | ||
| aria-label={suffixAriaLabel} | ||
| className={onSuffixClick ? styles.suffixButton : styles.suffix} | ||
| onClick={ | ||
| onSuffixClick | ||
| ? (e) => { | ||
| e.stopPropagation(); | ||
| onSuffixClick(); | ||
| } | ||
| : undefined | ||
| } | ||
| > | ||
| <span aria-hidden="true">{suffixIcon}</span> | ||
| </span> |
There was a problem hiding this comment.
중첩 인터랙티브 요소로 접근성 동작이 깨질 수 있습니다.
루트가 <button>인데 suffix를 role="button"/tabIndex로 또 인터랙티브하게 만들어 키보드 포커스/스크린리더 동작이 불안정해질 수 있습니다. suffix가 독립 액션이면 루트 액션과 형제 <button> 구조로 분리해주세요.
예시 리팩터링 방향
- <button ...>
- <span className={styles.label({ selected, hasSuffix })}>{children}</span>
- {hasSuffix && (
- <span
- role={onSuffixClick ? 'button' : undefined}
- tabIndex={onSuffixClick ? 0 : undefined}
- ...
- >
- <span aria-hidden="true">{suffixIcon}</span>
- </span>
- )}
- </button>
+ <div className={chipClassName}>
+ <button type={type} aria-pressed={selected} onClick={onClick} {...props}>
+ <span className={styles.label({ selected, hasSuffix })}>{children}</span>
+ </button>
+ {hasSuffix &&
+ (onSuffixClick ? (
+ <button
+ type="button"
+ aria-label={suffixAriaLabel}
+ className={styles.suffixButton}
+ onClick={onSuffixClick}
+ >
+ <span aria-hidden="true">{suffixIcon}</span>
+ </button>
+ ) : (
+ <span className={styles.suffix} aria-hidden="true">
+ {suffixIcon}
+ </span>
+ ))}
+ </div>As per coding guidelines "접근성 준수: ARIA 속성, label 연결, 키보드 포커스".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/shared/components/v2/chip/Chip.tsx` around lines 37 - 52, The nested
interactive suffix (the span using role="button"/tabIndex with onSuffixClick
inside the interactive root in Chip.tsx) breaks accessibility; if onSuffixClick
represents an independent action, refactor so the suffix is rendered as a
sibling <button> (or a focusable native element) rather than a nested element
with role="button", move aria-label and keyboard handling to that sibling
button, remove role/tabIndex from the inner span, and ensure you call
stopPropagation or otherwise prevent the root action when the suffix button is
activated; locate the suffix rendering block that uses onSuffixClick and
styles.suffixButton/styles.suffix and change it to a separate button element
alongside the root click handler.
Comment on lines
+29
to
31
| <Icon name="ArrowLeft" size="16" /> | ||
| <span className={styles.label}>{backLabel}</span> | ||
| </button> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
backLabel이 undefined일 때 빈 <span> 렌더링
backLabel이 undefined인 경우에도 <span className={styles.label}>{backLabel}</span>가 렌더링되어 빈 요소가 DOM에 남아요. 의도된 동작이라면 문제없지만, 불필요한 DOM 노드를 줄이려면 조건부 렌더링을 고려해 주세요.
♻️ 조건부 렌더링 제안
<Icon name="ArrowLeft" size="16" />
- <span className={styles.label}>{backLabel}</span>
+ {backLabel && <span className={styles.label}>{backLabel}</span>}📝 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
| <Icon name="ArrowLeft" size="16" /> | |
| <span className={styles.label}>{backLabel}</span> | |
| </button> | |
| <Icon name="ArrowLeft" size="16" /> | |
| {backLabel && <span className={styles.label}>{backLabel}</span>} | |
| </button> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/shared/components/v2/navBar/TitleNavBar.tsx` around lines 29 - 31, The
span with className styles.label is rendered even when backLabel is undefined,
leaving an empty DOM node; update TitleNavBar so the span is conditionally
rendered only when backLabel is not null/undefined (e.g., check backLabel !==
undefined/null or truthy) around the existing <span
className={styles.label}>{backLabel}</span> inside the back button (the element
containing Icon name="ArrowLeft"), so no empty <span> remains when backLabel is
absent.
빌드 결과빌드 실패 ❌ |
빌드 결과빌드 실패 ❌ |
빌드 결과빌드 성공 🎊 |
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (2)
src/pages/imageSetup/v2/steps/floorPlanSelect/FilterSheet.tsx (1)
37-51:⚠️ Potential issue | 🟡 Minor
currentValuesundefined 방어가 필요해요.
category.id와pendingFilters키가 어긋나면currentValues.length와currentValues.includes()에서 런타임 에러가 발생할 수 있어요.🛡️ 제안 수정
{filterCategories.map((category) => { - const currentValues = pendingFilters[category.id]; + const currentValues = pendingFilters[category.id] ?? []; return (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/imageSetup/v2/steps/floorPlanSelect/FilterSheet.tsx` around lines 37 - 51, Guard against pendingFilters[category.id] being undefined in FilterSheet.tsx: when mapping filterCategories and computing currentValues, default it to an empty array (or otherwise handle the absent key) before using currentValues.length or currentValues.includes; update the logic that sets selected on the Chip (the ternary that checks option.id === 'ALL') to work with the guarded currentValues so no runtime error occurs if the key is missing.src/pages/imageSetup/v2/hooks/useFloorPlanSelect.ts (1)
86-98: 🧹 Nitpick | 🔵 Trivial핸들러 함수들의 메모이제이션을 고려해 주세요.
handleCardClick,handleConfirmFloorPlan함수들이 매 렌더링마다 새로 생성되어 자식 컴포넌트에 전달되고 있어요.useCallback으로 감싸면 불필요한 리렌더링을 방지할 수 있어요.♻️ 제안 수정
+import { useCallback, useEffect } from 'react'; - const handleCardClick = (floorPlanId: number) => { + const handleCardClick = useCallback((floorPlanId: number) => { store.selectFloorPlan(floorPlanId); store.openFloorPlanSheet(); - }; + }, [store]); - const handleConfirmFloorPlan = () => { + const handleConfirmFloorPlan = useCallback(() => { if (!selectedFloorPlan) return; confirmFloorPlan({ floorPlanId: selectedFloorPlan.id, isMirror: store.isMirror, }); - }; + }, [selectedFloorPlan, store.isMirror, confirmFloorPlan]);Based on learnings: "In TypeScript/React projects using Zustand: prefer splitting Zustand selectors over wrapping components with React.memo."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/imageSetup/v2/hooks/useFloorPlanSelect.ts` around lines 86 - 98, handleCardClick and handleConfirmFloorPlan are recreated each render and should be memoized to avoid child re-renders; wrap them with React's useCallback and include the correct dependency arrays: for handleCardClick depend on store.selectFloorPlan and store.openFloorPlanSheet (or the whole store object if selectors aren't stable), and for handleConfirmFloorPlan depend on selectedFloorPlan, confirmFloorPlan, and store.isMirror; import useCallback and replace the inline functions with memoized callbacks using those dependencies.
🤖 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/imageSetup/v2/FloorPlanSelectTest.tsx`:
- Around line 18-40: Remove the unused imports and state to fix the TypeScript
build: delete the imports of DragHandleBottomSheet and ActionButton and remove
the unused React state declarations isDragSheetOpen and setIsDragSheetOpen from
FloorPlanSelectTest; ensure that only used symbols like FloorPlanSelectStep,
MOCK_CONTEXT and handleNext remain so the component compiles without unused
identifier errors.
In `@src/pages/imageSetup/v2/steps/floorPlanSelect/FloorPlanSelectGrid.tsx`:
- Around line 90-94: The string with '\n' inside the <p
className={styles.emptyDescription}> in FloorPlanSelectGrid.tsx won't produce a
visible line break; update the rendering or styles: either split the text into
separate JSX lines/paragraphs or replace the '\n' with an explicit <br /> node,
or add whiteSpace: 'pre-line' to the emptyDescription style in
FloorPlanSelectGrid.css.ts so the newline is honored; modify the component or
the emptyDescription style (reference: FloorPlanSelectGrid,
styles.emptyDescription) accordingly.
In `@src/pages/imageSetup/v2/steps/floorPlanSelect/FloorPlanSelectStep.tsx`:
- Around line 18-32: Add a cleanup that calls the store reset when the component
unmounts: in useFloorPlanSelect (the hook used by FloorPlanSelectStep) import
the useFloorPlanStore reset action and invoke it in a useEffect cleanup (or add
a useEffect cleanup in FloorPlanSelectStep) so that useFloorPlanStore.reset()
runs on unmount to clear
filterCategories/selectedFloorPlan/selectedDetailViews/recentFloorPlan state.
In `@src/pages/imageSetup/v2/types/floorPlan.ts`:
- Around line 27-28: The fields equilibrium and view in the FloorPlan type are
currently plain strings but annotated as "Enum (값 미정)"; when the API spec
stabilizes, replace those string types with a string literal union or TypeScript
enum to tighten the type. Locate the interface/type that defines equilibrium and
view (e.g., FloorPlanDetailView or the type in
src/pages/imageSetup/v2/types/floorPlan.ts) and change equilibrium: string and
view: string to concrete types such as export type Equilibrium = '...|'... and
export type ViewType = '...|'... (or an enum) and update the property types to
Equilibrium and ViewType respectively; ensure any usages and parsing/validation
of API responses are adjusted to accept only the new literal values or to
map/validate unknown values.
- Around line 76-80: DEFAULT_FILTERS is inferred as mutable arrays; update its
declaration to preserve literal/readonly types by appending as const (or use as
const with satisfies Record<keyof FloorPlanFilters, readonly string[]> if you
need explicit type checking) so DEFAULT_FILTERS keeps immutable literal types
while still conforming to FloorPlanFilters; change the declaration of
DEFAULT_FILTERS to use "as const" and ensure usages handle readonly string[]
where applicable.
---
Duplicate comments:
In `@src/pages/imageSetup/v2/hooks/useFloorPlanSelect.ts`:
- Around line 86-98: handleCardClick and handleConfirmFloorPlan are recreated
each render and should be memoized to avoid child re-renders; wrap them with
React's useCallback and include the correct dependency arrays: for
handleCardClick depend on store.selectFloorPlan and store.openFloorPlanSheet (or
the whole store object if selectors aren't stable), and for
handleConfirmFloorPlan depend on selectedFloorPlan, confirmFloorPlan, and
store.isMirror; import useCallback and replace the inline functions with
memoized callbacks using those dependencies.
In `@src/pages/imageSetup/v2/steps/floorPlanSelect/FilterSheet.tsx`:
- Around line 37-51: Guard against pendingFilters[category.id] being undefined
in FilterSheet.tsx: when mapping filterCategories and computing currentValues,
default it to an empty array (or otherwise handle the absent key) before using
currentValues.length or currentValues.includes; update the logic that sets
selected on the Chip (the ternary that checks option.id === 'ALL') to work with
the guarded currentValues so no runtime error occurs if the key is missing.
🪄 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: 1eabb640-adb2-4d7b-a4e5-999d3ebebd33
📒 Files selected for processing (9)
src/pages/imageSetup/v2/FloorPlanSelectTest.tsxsrc/pages/imageSetup/v2/hooks/useFloorPlanSelect.tssrc/pages/imageSetup/v2/steps/floorPlanSelect/FilterSheet.tsxsrc/pages/imageSetup/v2/steps/floorPlanSelect/FloorPlanSelectGrid.tsxsrc/pages/imageSetup/v2/steps/floorPlanSelect/FloorPlanSelectStep.tsxsrc/pages/imageSetup/v2/steps/floorPlanSelect/FloorPlanSheet.tsxsrc/pages/imageSetup/v2/stores/useFloorPlanStore.tssrc/pages/imageSetup/v2/types/floorPlan.tssrc/routes/router.tsx
Comment on lines
+90
to
+94
| <p className={styles.emptyDescription}> | ||
| { | ||
| '하우미는 순차적으로 공간 유형을 확장하고 있어요.\n비슷한 공간을 선택해 이미지를 생성해보세요!' | ||
| } | ||
| </p> |
There was a problem hiding this comment.
\n이 줄바꿈으로 렌더링되지 않을 수 있어요.
JSX에서 문자열 내 \n은 기본적으로 공백으로 렌더링돼요. CSS에서 white-space: pre-line 또는 <br /> 태그를 사용해야 의도한 줄바꿈이 표시돼요.
🔧 제안 수정 (방법 1: JSX로 분리)
<p className={styles.emptyDescription}>
- {
- '하우미는 순차적으로 공간 유형을 확장하고 있어요.\n비슷한 공간을 선택해 이미지를 생성해보세요!'
- }
+ 하우미는 순차적으로 공간 유형을 확장하고 있어요.
+ <br />
+ 비슷한 공간을 선택해 이미지를 생성해보세요!
</p>🔧 제안 수정 (방법 2: CSS로 처리)
FloorPlanSelectGrid.css.ts에서 emptyDescription 스타일에 추가:
whiteSpace: 'pre-line',📝 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
| <p className={styles.emptyDescription}> | |
| { | |
| '하우미는 순차적으로 공간 유형을 확장하고 있어요.\n비슷한 공간을 선택해 이미지를 생성해보세요!' | |
| } | |
| </p> | |
| <p className={styles.emptyDescription}> | |
| 하우미는 순차적으로 공간 유형을 확장하고 있어요. | |
| <br /> | |
| 비슷한 공간을 선택해 이미지를 생성해보세요! | |
| </p> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/imageSetup/v2/steps/floorPlanSelect/FloorPlanSelectGrid.tsx` around
lines 90 - 94, The string with '\n' inside the <p
className={styles.emptyDescription}> in FloorPlanSelectGrid.tsx won't produce a
visible line break; update the rendering or styles: either split the text into
separate JSX lines/paragraphs or replace the '\n' with an explicit <br /> node,
or add whiteSpace: 'pre-line' to the emptyDescription style in
FloorPlanSelectGrid.css.ts so the newline is honored; modify the component or
the emptyDescription style (reference: FloorPlanSelectGrid,
styles.emptyDescription) accordingly.
Comment on lines
+18
to
+32
| const FloorPlanSelectStep = ({ context, onNext }: FloorPlanSelectStepProps) => { | ||
| const { | ||
| filterCategories, | ||
| filteredFloorPlans, | ||
| selectedFloorPlan, | ||
| selectedDetailViews, | ||
| recentFloorPlan, | ||
| handleCardClick, | ||
| handleConfirmFloorPlan, | ||
| handleConfirmRecentFloorPlan, | ||
| grid, | ||
| filterSheet, | ||
| floorPlanSheet, | ||
| recentSheet, | ||
| } = useFloorPlanSelect(context, onNext); |
There was a problem hiding this comment.
컴포넌트 언마운트 시 store 상태 초기화가 필요해요.
useFloorPlanStore에 reset() 액션이 있고 주석에 "페이지 이탈 시 모든 상태 초기화"라고 명시되어 있지만, 현재 FloorPlanSelectStep에서 언마운트 시 reset()을 호출하지 않아요. 라우트 재진입 시 이전 필터/선택 상태가 그대로 남을 수 있어요.
🔧 제안 수정 (useFloorPlanSelect.ts에 추가)
+ import { useEffect } from 'react';
+ import { useFloorPlanStore } from '../stores/useFloorPlanStore';
export const useFloorPlanSelect = (
context: ImageSetupSteps['FloorPlan'],
onNext: (data: CompletedFloorPlan) => void
) => {
const store = useFloorPlanStore();
+
+ // 컴포넌트 언마운트 시 store 상태 초기화
+ useEffect(() => {
+ return () => {
+ store.reset();
+ };
+ }, [store]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/imageSetup/v2/steps/floorPlanSelect/FloorPlanSelectStep.tsx` around
lines 18 - 32, Add a cleanup that calls the store reset when the component
unmounts: in useFloorPlanSelect (the hook used by FloorPlanSelectStep) import
the useFloorPlanStore reset action and invoke it in a useEffect cleanup (or add
a useEffect cleanup in FloorPlanSelectStep) so that useFloorPlanStore.reset()
runs on unmount to clear
filterCategories/selectedFloorPlan/selectedDetailViews/recentFloorPlan state.
Comment on lines
+27
to
+28
| equilibrium: string; // Enum — 평형 (값 미정) | ||
| view: string; // Enum — 뷰 (값 미정) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
향후 개선: Enum 값 확정 시 string literal union 적용
주석에 "Enum (값 미정)"으로 표시되어 있어요. 현재는 일반 string 타입이 적절하지만, API 스펙이 확정되면 string literal union이나 enum으로 타입을 더 구체화할 수 있어요.
💡 값 확정 후 적용 가능한 예시
// 예시: 값이 확정되면
export type Equilibrium = '10평대' | '20평대' | '30평대' | '30평 이상';
export type ViewType = '정면 뷰' | '측면 뷰' | '평면도';
export interface FloorPlanDetailView {
id: number;
name: string;
imageUrl: string;
equilibrium: Equilibrium;
view: ViewType;
}API 응답 값이 확정되면 타입을 구체화하는 작업이 필요할 때 도움이 필요하시면 말씀해 주세요!
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/imageSetup/v2/types/floorPlan.ts` around lines 27 - 28, The fields
equilibrium and view in the FloorPlan type are currently plain strings but
annotated as "Enum (값 미정)"; when the API spec stabilizes, replace those string
types with a string literal union or TypeScript enum to tighten the type. Locate
the interface/type that defines equilibrium and view (e.g., FloorPlanDetailView
or the type in src/pages/imageSetup/v2/types/floorPlan.ts) and change
equilibrium: string and view: string to concrete types such as export type
Equilibrium = '...|'... and export type ViewType = '...|'... (or an enum) and
update the property types to Equilibrium and ViewType respectively; ensure any
usages and parsing/validation of API responses are adjusted to accept only the
new literal values or to map/validate unknown values.
Comment on lines
+76
to
+80
| export const DEFAULT_FILTERS: FloorPlanFilters = { | ||
| residenceType: [], | ||
| layoutType: [], | ||
| areaSize: [], | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
가이드라인: 상수에 as const 적용 필요
코딩 가이드라인에서 "상수 리터럴은 as const로 리터럴 타입 유지"를 명시하고 있어요. 현재는 빈 배열들이 mutable string[]로 추론되는데, as const를 적용하면 더 정확한 타입을 보장할 수 있어요.
♻️ 가이드라인 준수를 위한 개선안
-export const DEFAULT_FILTERS: FloorPlanFilters = {
+export const DEFAULT_FILTERS = {
residenceType: [],
layoutType: [],
areaSize: [],
-};
+} as const;또는 satisfies 연산자를 함께 활용하여 타입 체크를 유지할 수도 있어요:
export const DEFAULT_FILTERS = {
residenceType: [],
layoutType: [],
areaSize: [],
} as const satisfies Record<keyof FloorPlanFilters, readonly string[]>;단, 이 경우 사용처에서 readonly 배열을 처리해야 하므로, 프로젝트 전반의 불변성 전략과 함께 고려해 주세요.
코딩 가이드라인에 따라 "상수 리터럴은 as const로 리터럴 타입 유지" 규칙을 적용해야 해요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/imageSetup/v2/types/floorPlan.ts` around lines 76 - 80,
DEFAULT_FILTERS is inferred as mutable arrays; update its declaration to
preserve literal/readonly types by appending as const (or use as const with
satisfies Record<keyof FloorPlanFilters, readonly string[]> if you need explicit
type checking) so DEFAULT_FILTERS keeps immutable literal types while still
conforming to FloorPlanFilters; change the declaration of DEFAULT_FILTERS to use
"as const" and ensure usages handle readonly string[] where applicable.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/imageSetup/v2/FloorPlanSelectTest.tsx`:
- Around line 18-37: 테스트용 컴포넌트 FloorPlanSelectTest와 임시 라우트가 남아 있어 배포 전 반드시 제거해야
합니다; delete the FloorPlanSelectTest component (FloorPlanSelectTest) and remove
the corresponding '/test/floor-plan' entry from the publicRoutes array in router
(publicRoutes) and then remove the associated TODO comment ("확인 완료 후 삭제"),
ensuring no other modules import FloorPlanSelectTest or reference the test route
before committing.
In `@src/pages/imageSetup/v2/steps/floorPlanSelect/FloorPlanSelectGrid.css.ts`:
- Line 40: Break the long template literal used for the margin declaration in
FloorPlanSelectGrid (the line that currently reads margin:
`${unitVars.unit.gapPadding['800']} calc(${unitVars.unit.gapPadding['500']} *
-1)`,) into shorter parts to respect the 80-character guideline — either assign
unitVars.unit.gapPadding['800'] and the calc(...) expression to intermediate
constants (e.g., const topPadding = unitVars.unit.gapPadding['800']; const
negativeCalc = `calc(${unitVars.unit.gapPadding['500']} * -1)` and then use
margin: `${topPadding} ${negativeCalc}`,) or break the template literal across
multiple lines so each line stays under 80 chars; apply the same treatment to
the similar long template at the other occurrence (the one around line 90).
- Around line 73-74: Replace direct spreads of font tokens (e.g.
...fontVars.font.title_m_16 and the other fontVars.font.* occurrences) with the
fontStyle helper: locate the places in FloorPlanSelectGrid.css.ts where
fontVars.font tokens are being spread and change them to use the spread of
...fontStyle('tokenName') (for example replace ...fontVars.font.title_m_16 with
...fontStyle('title_m_16') and do the same for the other token names), keeping
the rest of the style object intact.
🪄 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: 919d7987-9bc7-46cd-94ed-bf4a4a1a5f4d
📒 Files selected for processing (2)
src/pages/imageSetup/v2/FloorPlanSelectTest.tsxsrc/pages/imageSetup/v2/steps/floorPlanSelect/FloorPlanSelectGrid.css.ts
Comment on lines
+18
to
+37
| import FloorPlanSelectStep from './steps/floorPlanSelect/FloorPlanSelectStep'; | ||
|
|
||
| import type { CompletedFloorPlan } from '../types/funnel/steps'; | ||
|
|
||
| const MOCK_CONTEXT = { | ||
| houseType: 'OFFICETEL', | ||
| roomType: 'ONE_ROOM', | ||
| areaType: 'TENS', | ||
| houseId: 1, | ||
| }; | ||
|
|
||
| const handleNext = (data: CompletedFloorPlan) => { | ||
| console.log('[FloorPlanSelectTest] onNext 호출됨:', data); | ||
| }; | ||
|
|
||
| const FloorPlanSelectTest = () => { | ||
| return <FloorPlanSelectStep context={MOCK_CONTEXT} onNext={handleNext} />; | ||
| }; | ||
|
|
||
| export default FloorPlanSelectTest; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: 테스트 경로가 프로덕션 빌드에 포함되어 있는지 확인
# router.tsx에서 /test/floor-plan 경로 검색
rg -n 'test/floor-plan' --type ts --type tsx
# FloorPlanSelectTest 파일 존재 여부 확인
fd 'FloorPlanSelectTest' --type fRepository: TEAM-HOUME/HOUME-CLIENT
Length of output: 159
🏁 Script executed:
find . -name "router.tsx" -o -name "router.ts" -o -name "*router*" -type f | head -20Repository: TEAM-HOUME/HOUME-CLIENT
Length of output: 90
🏁 Script executed:
rg "test.*floor.*plan|floor.*plan.*test" -i --type ts --max-count 20Repository: TEAM-HOUME/HOUME-CLIENT
Length of output: 695
🏁 Script executed:
rg "FloorPlanSelectTest" --type ts --type tsx --max-count 20Repository: TEAM-HOUME/HOUME-CLIENT
Length of output: 94
🏁 Script executed:
rg "publicRoutes|publicRoute" -A 5 -B 5 --type ts --type tsx | head -50Repository: TEAM-HOUME/HOUME-CLIENT
Length of output: 94
🏁 Script executed:
cat -n src/routes/router.tsx | head -100Repository: TEAM-HOUME/HOUME-CLIENT
Length of output: 3482
🏁 Script executed:
rg -A 20 -B 5 "/test/floor-plan" src/routes/router.tsxRepository: TEAM-HOUME/HOUME-CLIENT
Length of output: 671
🏁 Script executed:
rg -A 50 "publicRoutes" src/routes/router.tsxRepository: TEAM-HOUME/HOUME-CLIENT
Length of output: 3126
코드 구현이 깔끔하고 가이드라인을 잘 준수해요.
- 이전 리뷰에서 지적된 미사용 import/state가 모두 제거되었어요.
MOCK_CONTEXT는ImageSetupSteps['FloorPlan']타입의 필수 필드(houseType,roomType,areaType,houseId)를 모두 충족하고 있어요.- 네이밍 컨벤션(상수 BIG_SNAKE_CASE, 함수 camelCase, 컴포넌트 PascalCase)과 trailing comma 규칙이 잘 지켜졌어요.
배포 전 필수 확인: src/routes/router.tsx의 publicRoutes에 /test/floor-plan 경로가 포함되어 있어 인증 없이 누구나 접근할 수 있어요. TODO 주석(확인 완료 후 삭제)이 있으니 검증 완료 후 FloorPlanSelectTest.tsx 파일과 라우트 정의를 반드시 제거해 주세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/imageSetup/v2/FloorPlanSelectTest.tsx` around lines 18 - 37, 테스트용
컴포넌트 FloorPlanSelectTest와 임시 라우트가 남아 있어 배포 전 반드시 제거해야 합니다; delete the
FloorPlanSelectTest component (FloorPlanSelectTest) and remove the corresponding
'/test/floor-plan' entry from the publicRoutes array in router (publicRoutes)
and then remove the associated TODO comment ("확인 완료 후 삭제"), ensuring no other
modules import FloorPlanSelectTest or reference the test route before
committing.
| }); | ||
|
|
||
| export const divider = style({ | ||
| margin: `${unitVars.unit.gapPadding['800']} calc(${unitVars.unit.gapPadding['500']} * -1)`, |
There was a problem hiding this comment.
한 줄 길이(80자) 초과 가능성이 높은 선언은 줄바꿈해 주세요.
Line 40, Line 90의 템플릿 리터럴이 길어 가독성과 포매터 일관성이
떨어집니다. 중간 변수로 분리하거나 멀티라인으로 나누는 편이 좋습니다.
As per coding guidelines: "Maintain a maximum line width of 80 characters".
Also applies to: 90-90
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/imageSetup/v2/steps/floorPlanSelect/FloorPlanSelectGrid.css.ts` at
line 40, Break the long template literal used for the margin declaration in
FloorPlanSelectGrid (the line that currently reads margin:
`${unitVars.unit.gapPadding['800']} calc(${unitVars.unit.gapPadding['500']} *
-1)`,) into shorter parts to respect the 80-character guideline — either assign
unitVars.unit.gapPadding['800'] and the calc(...) expression to intermediate
constants (e.g., const topPadding = unitVars.unit.gapPadding['800']; const
negativeCalc = `calc(${unitVars.unit.gapPadding['500']} * -1)` and then use
margin: `${topPadding} ${negativeCalc}`,) or break the template literal across
multiple lines so each line stays under 80 chars; apply the same treatment to
the similar long template at the other occurrence (the one around line 90).
Comment on lines
+73
to
+74
| ...fontVars.font.title_m_16, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
fontVars.font.* 직접 스프레드 대신 fontStyle() 사용으로 통일해주세요.
Line 73, Line 81, Line 96에서 폰트 토큰을 직접 스프레드하고 있어
리포 규칙과 다릅니다. ...fontStyle('토큰명') 형태로 통일하는 게
맞습니다.
As per coding guidelines: "fontStyle() 함수 사용법: ...fontStyle('토큰명') 형태로 스프레드 연산자와 함께 사용".
Also applies to: 81-82, 96-97
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/imageSetup/v2/steps/floorPlanSelect/FloorPlanSelectGrid.css.ts`
around lines 73 - 74, Replace direct spreads of font tokens (e.g.
...fontVars.font.title_m_16 and the other fontVars.font.* occurrences) with the
fontStyle helper: locate the places in FloorPlanSelectGrid.css.ts where
fontVars.font tokens are being spread and change them to use the spread of
...fontStyle('tokenName') (for example replace ...fontVars.font.title_m_16 with
...fontStyle('title_m_16') and do the same for the other token names), keeping
the rest of the style object intact.
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
해당 PR에 대한 작업 내용을 요약하여 작성해주세요.
📄 Tasks
구현요약
작업 영역
리뷰 가이드 (읽는 순서)
1: 공통 컴포넌트 변경 관련 파일
fullWidthprop 추가 +<span>래퍼 제거<span>이 fullWidth를 막음 → span 제거 및width: 100%를 적용해주는fullWidthprop 추가Drawer.Overlay→div로 교체 및 바텀시트 닫기 버튼IconButton로 교체div로 교체, Overlay 효과는 커스텀함stopPropagation) + 피그마 반영(body_m_13,scale(0.9)적용)IconButton→Icon으로 변경 및 미사용 아이콘 스타일 제거IcnArrowLeftM→Iconvaul(바텀시트 라이브러리) 관련 이슈
바텀시트 구현을 위해 추가한 라이브러리인 vaul의
Drawer.Overlay는 내부적으로 radix Dialog의 overlay를 사용하는데, 내부적으로 body에Dialog.Overlay에서 강제하는 css가 주입돼 데스크탑에서 뒷배경 레이아웃이 깨지는 문제가 발생 (모바일에서는 문제 X)⇒ 커스텀
div로 교체하고 뒷배경 dim 적용과 pointer-events: ‘auto’ 적용으로Drawer.Overlay와동일하게 동작하도록 구현+) vaul에서 제공하는 기능들 중 현재
Drawer.Root/Portal/Content만 사용되고 핵심 기능(드래그해서 닫기, 스냅 포인트)을 커스텀으로 구현해 사용하고 있어서, 장기적으로는 제거를 검토해야 할 것 같아요.2: 도면 선택 비즈니스 로직 (이번 작업에서 새로 추가된 파일들)
onSlideChange와 연동useFloorPlanSelect 주요 설계
도면 확정:
handleConfirmFloorPlan(도면 선택 이벤트) /handleConfirmRecentFloorPlan(저장된 기존 도면 선택 이벤트) 두 핸들러가 동일한 payload(다음 스텝으로 넘어가기 전 저장할 데이터) 생성 로직을 쓰므로confirmFloorPlan헬퍼로 추출.savedHouseInfo가 있으면 우선 사용, 없으면 funnel의 context로 폴백훅 반환값 관리: useFloorPlanSelect 훅에서 컴포넌트별로 상태/액션을 묶어서 반환(
grid,filterSheet,floorPlanSheet,recentSheet).-> 사용처에서 가독성 개선
도면 선택, 스와이핑 등에 메모이제이션 미적용: 자식 리렌더 비용이 크지 않고, 대량의 데이터를 필터링하거나 정렬해야되는 등 연산이 무겁지 않다고 생각해 메모이제이션은 별도로 적용하지 않았어요. API 연동한 뒤에 성능 이슈가 생기면 도입을 고려해봐도 좋을 것 같아요.
3: 도면 선택 UI (이번 작업에서 새로 추가된 파일들)
4: 기타 수정
width: 100%)가 부모의max-content에 의해 무한 확장되는 문제 발생 →cardItemcss를 추가해 가로 스크롤 영역 안에서는 RoomTypeCard의 너비16rem고정해당 PR에 수행한 작업을 작성해주세요.
🔍 To Reviewer
스토리북 전역 세팅이 잘못된건지 브라우저에서 실제로 보이는 UI와 스토리북에서 보이는 UI가 다르네요.. 저도 테스트 파일 추가해뒀어요
/test/floor-plan으로 접근하면 도면 선택 플로우 테스트 페이지 확인 가능합니다!현재는 바텀시트를 도면 선택 페이지 내부 비즈니스 로직으로 관리하고 있는데, 다음 작업(E2E 전체 플로우 스켈레톤 구현)에서
FloorPlanSheet를toss/use-overlay의funnel.Render.overlay()로 관리할 예정이에요. 바텀시트를 퍼널에 포함시킴으로써 퍼널에서 뒤로가기 시 바텀시트가 뜨도록, 즉 일반 스텝들처럼 바텀시트도 퍼널의 히스토리에 넣어 관리할 수 있어요.작업 볼륨이 많이 커졌네요....
리뷰어에게 요청하는 내용을 작성해주세요.
📸 Screenshot
2026-03-25.4.00.47.mov
작업한 내용에 대한 스크린샷을 첨부해주세요.