[refactor] 대타 요청 UI 수정#37
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthrough대타 요청(목록/상세/수락·거절·취소) 기능을 추가하고, 교환 가능 일정 API를 연동해 캘린더 데이터를 서버 기반으로 구성합니다. 캘린더 렌더링을 grid로 리팩토링하고 네비게이션에 substitute 탭을 통합했습니다. Changes대타 요청 기능 통합
교환 가능 일정 조회 및 캘린더 통합
캘린더 UI 개선 (그리드화 및 주간 범위 조정)
네비게이션 및 라우팅 통합
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
시니어 프론트엔드 관점 핵심 이슈(우선순위별)
간결한 우선순위 중심 지적은 여기까지입니다. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
src/app/App.tsx (1)
25-25: ⚡ Quick win신규 페이지에 lazy loading 적용을 권장합니다.
SubstituteRequestPage가 일반 import로 추가되어 초기 번들에 포함됩니다. SignupPage처럼 lazy loading을 적용하면 초기 로딩 성능과 번들 크기를 개선할 수 있습니다.
♻️ lazy loading 적용 예시
-import { SubstituteRequestPage } from '`@/pages/user/substitute-request`' + +const SubstituteRequestPage = lazy(async () => { + const m = await import('`@/pages/user/substitute-request`') + return { default: m.SubstituteRequestPage } +})그리고 라우트에서 Suspense로 감싸기:
<Route path={ROUTES.USER.SUBSTITUTE_REQUEST_DETAIL_PATTERN} element={ <HomeRouteGuard expected="USER"> - <SubstituteRequestPage /> + <Suspense fallback={null}> + <SubstituteRequestPage /> + </Suspense> </HomeRouteGuard> } /> <Route path={ROUTES.USER.SUBSTITUTE_REQUEST} element={ <HomeRouteGuard expected="USER"> - <SubstituteRequestPage /> + <Suspense fallback={null}> + <SubstituteRequestPage /> + </Suspense> </HomeRouteGuard> } />🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/App.tsx` at line 25, SubstituteRequestPage is currently imported eagerly in App.tsx which increases initial bundle size; change the direct import to a dynamic React.lazy import (e.g., const SubstituteRequestPage = React.lazy(() => import('`@/pages/user/substitute-request`'))) and remove the existing eager import, then ensure the route rendering SubstituteRequestPage is wrapped with React.Suspense (provide a small fallback like a spinner or null) so the page is lazy-loaded on demand (mirror how SignupPage is implemented).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/features/user/home/workspace/hooks/useSubstituteRequestFlow.ts`:
- Around line 7-8: The hook useSubstituteRequestFlow currently imports peer
feature modules getExchangeableSchedules and
adaptExchangeableSchedulesToCalendar directly; remove these direct imports and
instead depend on the substitute feature's public API or shared/entities
contract (e.g., export adapter and API functions from a public index in the
substitute feature or define an interface in shared/entities and implement it in
substitute). Update useSubstituteRequestFlow to call the public API surface (or
injected/consumed service) rather than importing
'`@/features/user/substitute/`...'; ensure the hook references only entities or
shared-layer types and the substitute feature exposes the needed functions via
its public entrypoint.
In
`@src/features/user/substitute/hooks/useUserSubstituteRequestDetailViewModel.ts`:
- Around line 58-75: The mutations currently force a non-null requestId
(requestId!) when calling acceptUserSubstituteRequest,
rejectUserSubstituteRequest, and cancelUserSubstituteRequest, which can trigger
invalid API calls if requestId is undefined; update each useMutation.mutationFn
and any external action handlers to validate requestId first (e.g., return early
or throw a clear error and avoid calling the API) and ensure
options/onActionSuccess is only invoked when requestId is present and the API
call actually occurs; also consider disabling or not registering the mutation
when requestId is null to prevent accidental mutate() calls.
In `@src/features/user/substitute/hooks/useUserSubstituteRequestsViewModel.ts`:
- Line 70: getNextPageParam currently treats an empty string cursor as a valid
next cursor causing hasNextPage to stay true; update the getNextPageParam in
useUserSubstituteRequestsViewModel (the arrow returning
lastPage?.data?.page?.cursor ?? undefined) to return undefined when the cursor
is an empty string (e.g., check for cursor === '' or falsy) so empty cursors are
not considered valid next pages.
In `@src/features/user/substitute/lib/adaptExchangeableSchedules.ts`:
- Around line 1-6: The adapter imports feature-level utilities
(getDurationHours, toDateKey, toTimeLabel) and CalendarViewData from another
feature which violates layer boundaries; refactor by moving those common
utilities/types into the shared or entities layer (or ensure they already exist
there) and update adaptExchangeableSchedules.ts to import getDurationHours,
toDateKey, toTimeLabel and CalendarViewData from the shared/entities module
instead of '`@/features/home/common/`...'; keep the same identifiers
(getDurationHours, toDateKey, toTimeLabel, CalendarViewData) so
adaptExchangeableSchedules logic does not change, and run type checks to ensure
the new import paths are correct.
In `@src/features/user/substitute/types.ts`:
- Line 3: 현재 `src/features/user/substitute/types.ts`가 UI 컴포넌트 모듈의 타입
`WorkerRole`을 직접 임포트하여 도메인 타입이 UI에 결합되어 있습니다; 이 문제를 해결하려면 `WorkerRole` 타입을 비-UI
공통 모듈(예: `shared/types`)으로 이동하고 `WorkerRole`를 해당 모듈에서 export한 뒤
`src/features/user/substitute/types.ts`에서는 그 공통 모듈에서 임포트하도록 변경하세요; 또한
`WorkerRoleBadge` 컴포넌트는 새 위치(`shared/types`)에서 `WorkerRole`을 사용하도록 업데이트(혹은
re-export)하고 공통 타입 파일이 프로젝트에서 접근 가능하도록 index/export 정리를 확인하세요.
In `@src/pages/user/substitute-request/components/SubstituteRequestCard.tsx`:
- Around line 32-65: The root element in SubstituteRequestCard.tsx is a <button>
that contains inner interactive buttons (SubstituteRequestResponseActions),
causing nested interactive-element accessibility violations; change the card
root from a native <button> to a non-interactive container (e.g., <div> or <li>)
with role="button", tabIndex={0}, and keyboard handlers forwarding activation to
the existing onClick handler (handle Enter/Space in onKeyDown or onKeyPress) so
the card remains keyboard-activatable without wrapping native buttons, preserve
inner components (SubstituteRequestResponseActions,
SubstituteRequestStatusBadge, SubstituteProfileAvatar, WorkerRoleBadge) and keep
onAccept/onReject/actionsDisabled intact.
In `@src/pages/user/substitute-request/index.tsx`:
- Around line 56-57: The detail view currently derives its active direction from
the local state directionTab (useState<SubstituteDirectionTab> with initial
'sent'), which causes wrong fetching on direct deep-links/refresh; modify the
logic that chooses the direction for the detail route (where directionTab is
read) to prefer an explicit direction from the route (e.g., path param like
/substitute-request/received/:id or include direction in the detail route), or
restore it from location.state or the DTO payload if present, and only fall back
to the directionTab state if neither route param nor location.state/DTO provides
a direction; update all places that compute direction (the directionTab
setter/reader and wherever fetches use direction, including the blocks
referenced around the current useState and the detail fetch logic) so the
route-provided/restored direction is applied before the local state default.
---
Nitpick comments:
In `@src/app/App.tsx`:
- Line 25: SubstituteRequestPage is currently imported eagerly in App.tsx which
increases initial bundle size; change the direct import to a dynamic React.lazy
import (e.g., const SubstituteRequestPage = React.lazy(() =>
import('`@/pages/user/substitute-request`'))) and remove the existing eager
import, then ensure the route rendering SubstituteRequestPage is wrapped with
React.Suspense (provide a small fallback like a spinner or null) so the page is
lazy-loaded on demand (mirror how SignupPage is implemented).
🪄 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: CHILL
Plan: Pro
Run ID: c9e17f79-f405-412c-96e5-6df23a1308bc
⛔ Files ignored due to path filters (1)
src/assets/icons/doc/Substitute.svgis excluded by!**/*.svg
📒 Files selected for processing (36)
src/app/App.tsxsrc/features/home/common/schedule/ui/MonthlyCalendar.tsxsrc/features/home/common/schedule/ui/MonthlyDateCell.tsxsrc/features/user/home/schedule/lib/date.tssrc/features/user/home/workspace/hooks/useSubstituteRequestFlow.tssrc/features/user/substitute/api/exchangeableSchedules.tssrc/features/user/substitute/api/userSubstituteRequests.tssrc/features/user/substitute/hooks/useUserSubstituteRequestDetailViewModel.tssrc/features/user/substitute/hooks/useUserSubstituteRequestsViewModel.tssrc/features/user/substitute/lib/adaptExchangeableSchedules.tssrc/features/user/substitute/lib/adaptUserSubstituteRequest.tssrc/features/user/substitute/types.tssrc/pages/user/substitute-request/components/SubstituteCreateFab.tsxsrc/pages/user/substitute-request/components/SubstituteProfileAvatar.tsxsrc/pages/user/substitute-request/components/SubstituteRejectReasonModal.tsxsrc/pages/user/substitute-request/components/SubstituteRequestCard.tsxsrc/pages/user/substitute-request/components/SubstituteRequestDetailView.tsxsrc/pages/user/substitute-request/components/SubstituteRequestListSections.tsxsrc/pages/user/substitute-request/components/SubstituteRequestResponseActions.tsxsrc/pages/user/substitute-request/components/SubstituteRequestStatusBadge.tsxsrc/pages/user/substitute-request/components/SubstituteRequestTabs.tsxsrc/pages/user/substitute-request/components/SubstituteStoreSelectModal.tsxsrc/pages/user/substitute-request/index.tsxsrc/pages/user/workspace-detail/components/SubstituteCalendarPickerPanel.tsxsrc/pages/user/workspace-detail/components/SubstituteRequestModalFlow.tsxsrc/pages/user/workspace-detail/index.tsxsrc/shared/constants/routes.tssrc/shared/lib/queryKeys.tssrc/shared/stores/useDocStore.tssrc/shared/types/tab.tssrc/shared/ui/common/Docbar.tsxsrc/shared/ui/common/HamburgerMenuDrawer.tsxsrc/shared/ui/common/Navbar.tsxsrc/shared/ui/common/ScheduleCalendar.tsxsrc/shared/ui/home/WorkerRoleBadge.tsxstorybook/stories/Docbar.stories.tsx
Member
Author
|
@limtjdghks @kim3360 리뷰 부탁드립니다~! |
limtjdghks
approved these changes
May 20, 2026
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/pages/user/substitute-request/index.tsx (1)
46-62: 🏗️ Heavy lift페이지 레이어에 방향 결정 로직이 과도하게 들어와 있습니다.
parseDirectionTab/resolveDetailDirectionTab와 상세 방향 분기 로직은 페이지 조합 책임을 넘어선 비즈니스 규칙입니다. 이 로직은features/user/substitute쪽 ViewModel(또는 전용 훅)로 이동하고, 페이지는 결과 상태만 받아 렌더링하도록 분리하는 게 안전합니다.As per coding guidelines
src/pages/**: 페이지 컴포넌트가 비즈니스 로직 없이 조합(Composition)만 하는지.Also applies to: 84-89, 169-176, 180-190
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/user/substitute-request/index.tsx` around lines 46 - 62, Move the direction-parsing and resolution business logic out of the page component into the substitute feature layer: extract parseDirectionTab and resolveDetailDirectionTab and the related detail-direction branching (the logic around locationState?.receivedDto and use of 'sent'/'received' defaults referenced in lines ~84-89, 169-176, 180-190) into a ViewModel or a dedicated hook under features/user/substitute (e.g., useSubstituteDirection or SubstituteViewModel). The page should call that hook/VM to get a resolved SubstituteDirectionTab and any computed flags, and only render based on the returned state; remove any remaining conditional direction logic from the page so it contains composition-only code.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/pages/user/substitute-request/index.tsx`:
- Around line 46-62: Move the direction-parsing and resolution business logic
out of the page component into the substitute feature layer: extract
parseDirectionTab and resolveDetailDirectionTab and the related detail-direction
branching (the logic around locationState?.receivedDto and use of
'sent'/'received' defaults referenced in lines ~84-89, 169-176, 180-190) into a
ViewModel or a dedicated hook under features/user/substitute (e.g.,
useSubstituteDirection or SubstituteViewModel). The page should call that
hook/VM to get a resolved SubstituteDirectionTab and any computed flags, and
only render based on the returned state; remove any remaining conditional
direction logic from the page so it contains composition-only code.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f23c1c8a-8bcd-49c1-a523-637d51dfca09
📒 Files selected for processing (7)
src/features/user/home/workspace/hooks/useSubstituteRequestFlow.tssrc/features/user/substitute/hooks/useUserSubstituteRequestDetailViewModel.tssrc/features/user/substitute/hooks/useUserSubstituteRequestsViewModel.tssrc/features/user/substitute/index.tssrc/pages/user/substitute-request/components/SubstituteRejectReasonModal.tsxsrc/pages/user/substitute-request/components/SubstituteRequestCard.tsxsrc/pages/user/substitute-request/index.tsx
✅ Files skipped from review due to trivial changes (1)
- src/features/user/substitute/index.ts
Member
Author
|
꼬투리 잡지 마 |
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
2 participants
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.
ID
변경 내용
구현 사항
구현 시연 (필요 시)
2026-05-19.5.56.57.mov
Summary by CodeRabbit
New Features
UI Improvements