[FEAT] 드래그 앤 드롭 디테일한 기능 추가 및 디자인 요구사항 반영 #297
Conversation
WalkthroughThis update migrates the PlanPage drag-and-drop functionality from a custom touch-based approach to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant PlanContent
participant DndContext (@dnd-kit/core)
participant useDragAndDrop
participant TimeBlockSection
participant TimeBlockItem
User->>TimeBlockItem: Drag start on todo
TimeBlockItem->>DndContext: Triggers drag start event
DndContext->>useDragAndDrop: Calls handleDragStart
useDragAndDrop->>PlanContent: Updates draggingTodo state
User->>TimeBlockSection: Drags over another path section
DndContext->>useDragAndDrop: Calls handleDragOver
useDragAndDrop->>PlanContent: Updates hoveredPathId, triggers scroll if needed
User->>TimeBlockSection: Drops todo
DndContext->>useDragAndDrop: Calls handleDragEnd
useDragAndDrop->>PlanContent: Updates todo path, resets dragging state
Note over PlanContent: UI updates overlays and previews accordingly
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes detected. Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
frontend/src/pages/PlanPage/PlanPage.styled.ts (1)
10-12: Consider adding positioning properties to StickyHeader.The StickyHeader component only defines
position: stickybut lacks essential properties liketopandz-indexfor proper sticky behavior.export const StickyHeader = styled.div` position: sticky; + top: 0; + z-index: 10; `;frontend/src/pages/PlanPage/hooks/useDragAndDrop.ts (1)
29-60: Add cleanup for pending operations on drag end.If the component unmounts or drag ends while an async mutation is in progress, it could lead to state updates on unmounted components.
Consider adding proper cleanup:
const handleDragEnd = async (event: DragEndEvent) => { const { active, over } = event; + + // Always stop auto-scroll on drag end + stopAutoScroll(); if (!active || !over || !draggingTodo) { setDraggingTodo(null); setHoveredPathId(null); - return; } // ... rest of the function };frontend/src/pages/PlanPage/components/TimeBlockContent/PlanContent/index.tsx (1)
68-72: Consider adding a drop animation for smoother UX.The
dropAnimation={null}makes the drop feel abrupt. Consider using the default animation or a custom one.- <DragOverlay dropAnimation={null}> + <DragOverlay> <TimeBlockItem todo={draggingTodo} draggingTodo={draggingTodo} isDragOverlay={true} /> </DragOverlay>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (15)
frontend/package.json(1 hunks)frontend/src/components/Tooltip/Tooltip.styled.ts(1 hunks)frontend/src/pages/PlanPage/PlanPage.styled.ts(1 hunks)frontend/src/pages/PlanPage/components/PlanHeader/PlanHeader.styled.ts(1 hunks)frontend/src/pages/PlanPage/components/PlanHeader/index.tsx(3 hunks)frontend/src/pages/PlanPage/components/TimeBlockContent/PlanContent/PlanContent.styled.ts(1 hunks)frontend/src/pages/PlanPage/components/TimeBlockContent/PlanContent/TimeBlockSection/TimeBlockSection.styled.ts(1 hunks)frontend/src/pages/PlanPage/components/TimeBlockContent/PlanContent/TimeBlockSection/index.tsx(1 hunks)frontend/src/pages/PlanPage/components/TimeBlockContent/PlanContent/index.tsx(1 hunks)frontend/src/pages/PlanPage/components/TimeBlockContent/TimeBlockList/TimeBlockItem/TimeBlockItem.styled.ts(4 hunks)frontend/src/pages/PlanPage/components/TimeBlockContent/TimeBlockList/TimeBlockItem/index.tsx(2 hunks)frontend/src/pages/PlanPage/components/TimeBlockContent/TimeBlockList/index.tsx(2 hunks)frontend/src/pages/PlanPage/components/TimeBlockContent/index.tsx(2 hunks)frontend/src/pages/PlanPage/hooks/useDragAndDrop.ts(1 hunks)frontend/src/pages/PlanPage/index.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
frontend/src/pages/PlanPage/components/PlanHeader/index.tsx (1)
Learnt from: rbgksqkr
PR: softeer5th/Team6-DuBu#37
File: frontend/src/components/Icon/icons/Alert.tsx:16-17
Timestamp: 2025-02-04T05:12:55.556Z
Learning: In React components, SVG attributes should use camelCase (e.g., strokeLinecap instead of stroke-linecap) to prevent console warning messages.
🧬 Code Graph Analysis (7)
frontend/src/pages/PlanPage/index.tsx (1)
frontend/src/pages/PlanPage/components/TimeBlockContent/PlanContent/PlanContent.styled.ts (1)
PlanContent(3-9)
frontend/src/pages/PlanPage/components/PlanHeader/PlanHeader.styled.ts (1)
frontend/src/pages/PlanPage/PlanPage.styled.ts (1)
StickyHeader(10-12)
frontend/src/pages/PlanPage/components/TimeBlockContent/PlanContent/TimeBlockSection/index.tsx (2)
frontend/src/api/plan.ts (1)
Path(15-25)frontend/src/pages/PlanPage/components/TimeBlockContent/PlanContent/TimeBlockSection/TimeBlockSection.styled.ts (1)
TimeBlockSection(3-7)
frontend/src/pages/PlanPage/PlanPage.styled.ts (1)
frontend/src/pages/PlanPage/components/PlanHeader/PlanHeader.styled.ts (1)
StickyHeader(3-5)
frontend/src/pages/PlanPage/components/TimeBlockContent/TimeBlockList/index.tsx (2)
frontend/src/api/plan.ts (1)
PathTodo(27-29)frontend/src/pages/PlanPage/components/TimeBlockContent/TimeBlockList/TimeBlockList.styled.ts (1)
TimeBlockList(3-9)
frontend/src/pages/PlanPage/components/TimeBlockContent/index.tsx (2)
frontend/src/api/plan.ts (1)
PathTodo(27-29)frontend/src/pages/PlanPage/components/TimeBlockContent/TimeBlockList/TimeBlockList.styled.ts (1)
TimeBlockList(3-9)
frontend/src/pages/PlanPage/components/TimeBlockContent/TimeBlockList/TimeBlockItem/index.tsx (1)
frontend/src/api/plan.ts (1)
PathTodo(27-29)
🔇 Additional comments (18)
frontend/src/components/Tooltip/Tooltip.styled.ts (1)
5-5: LGTM! Clean positioning adjustment.The 8px upward shift improves tooltip positioning relative to its trigger element.
frontend/src/pages/PlanPage/PlanPage.styled.ts (2)
6-8: LGTM! Proper flex layout for vertical stacking.The flex column layout is appropriate for the page structure and supports the new drag-and-drop functionality.
14-20: LGTM! Improved scrollable content design.The ScrollableContent component provides better UX with
overflow-y: autoinstead ofscroll, and the padding-top spacing enhances the layout.frontend/package.json (1)
18-18: @dnd-kit/core version and security status confirmed
- frontend/package.json pins
@dnd-kit/core@^6.3.1, which is the latest stable release (Dec 5, 2024).- Snyk reports no known vulnerabilities for 6.3.1.
- Note: the package hasn’t been updated in ~7 months; consider monitoring upstream for new releases.
frontend/src/pages/PlanPage/components/TimeBlockContent/PlanContent/PlanContent.styled.ts (1)
4-4: LGTM! Appropriate positioning for drag-and-drop functionality.Adding
position: relativeprovides the necessary positioning context for drag-and-drop overlays and interactions with the @dnd-kit/core library.frontend/src/pages/PlanPage/components/PlanHeader/index.tsx (3)
1-1: LGTM! Import optimization is correct.The removal of the explicit React import is appropriate since React 17+ JSX transform doesn't require React in scope when only using JSX syntax.
7-7: LGTM! Styled components import follows proper conventions.The aliased import pattern
* as Sis a good practice for styled components organization.
31-31: LGTM! Sticky header implementation is appropriate.The replacement of React fragment with
S.StickyHeaderaligns with the PR objectives to fix the top header when scrolling occurs during drag operations.Also applies to: 48-48
frontend/src/pages/PlanPage/index.tsx (1)
44-52: LGTM! ScrollableContent wrapper supports enhanced drag-and-drop functionality.The addition of
S.ScrollableContentwrapper aligns with the PR objectives to enable scrolling based on cursor position during dragging operations. The existing component structure is preserved appropriately.frontend/src/pages/PlanPage/components/TimeBlockContent/PlanContent/TimeBlockSection/TimeBlockSection.styled.ts (1)
1-7: LGTM! Clean styled component implementation.The
TimeBlockSectionstyled component uses semantic HTML (section) with appropriate flexbox layout and consistent padding. The implementation is clean and follows good practices.frontend/src/pages/PlanPage/components/TimeBlockContent/PlanContent/TimeBlockSection/index.tsx (1)
1-27: LGTM! Proper @dnd-kit/core integration.The
TimeBlockSectioncomponent correctly implements theuseDroppablehook:
- Uses
path.pathIdas a unique identifier- Properly assigns
setNodeRefto the styled component- Includes necessary data for drop handling
- The
keyprop on the root element matches the droppableid, which is good practiceThe implementation follows @dnd-kit/core best practices for droppable containers.
frontend/src/pages/PlanPage/components/TimeBlockContent/TimeBlockList/index.tsx (2)
10-10: LGTM! Prop type simplification aligns with @dnd-kit/core migration.The change from
DraggingTodo | nulltoPathTodo | nullstandardizes the drag state type and aligns with the migration to @dnd-kit/core library.
13-13: LGTM! Component simplification removes custom touch event handling.The removal of touch event handlers and related props simplifies the component by delegating drag-and-drop functionality to the @dnd-kit/core library. This is a good architectural improvement that reduces component complexity.
Also applies to: 26-26
frontend/src/pages/PlanPage/components/TimeBlockContent/index.tsx (1)
13-13: Clean migration to the new drag-and-drop system.The prop interface simplification and type change from
DraggingTodotoPathTodoaligns well with the centralized drag-and-drop handling using@dnd-kit/core.Also applies to: 16-16, 56-56
frontend/src/pages/PlanPage/components/TimeBlockContent/TimeBlockList/TimeBlockItem/TimeBlockItem.styled.ts (1)
3-34: Well-structured styling updates for the new drag system.The prop renaming and styling adjustments properly support the drag overlay states. The
touch-action: noneaddition prevents unintended touch interactions during drag operations.Also applies to: 50-73, 110-110
frontend/src/pages/PlanPage/components/TimeBlockContent/PlanContent/index.tsx (2)
30-37: Good sensor configuration for preventing accidental drags.The 300ms delay and 8px distance threshold effectively prevent unintended drag operations during scrolling or clicking.
50-54: Excellent UX with drag preview implementation.Showing the dragged todo as a preview in the target path provides clear visual feedback about where the item will be dropped.
frontend/src/pages/PlanPage/components/TimeBlockContent/TimeBlockList/TimeBlockItem/index.tsx (1)
24-29: Clean integration with @dnd-kit/core draggable system.The
useDraggablehook is properly configured with the todo data, and preventing todo viewing during drag operations improves UX consistency.Also applies to: 31-35
| export const StickyHeader = styled.div` | ||
| position: sticky; | ||
| `; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider consolidating duplicate StickyHeader components.
This StickyHeader component is identical to the one in frontend/src/pages/PlanPage/PlanPage.styled.ts (lines 10-12). Consider creating a shared styled component to avoid duplication and potential inconsistencies.
Consider moving the shared StickyHeader to a common styled components file:
// shared/styled/components.ts
export const StickyHeader = styled.div`
position: sticky;
top: 0;
z-index: 10;
`;Then import it in both files that need it.
🤖 Prompt for AI Agents
In frontend/src/pages/PlanPage/components/PlanHeader/PlanHeader.styled.ts lines
3 to 5, the StickyHeader styled component duplicates the one in
frontend/src/pages/PlanPage/PlanPage.styled.ts lines 10 to 12. To fix this,
create a shared styled component file (e.g., shared/styled/components.ts)
defining StickyHeader with the desired styles including position sticky, top 0,
and z-index 10. Then remove the duplicate definitions and import this shared
StickyHeader component in both PlanHeader.styled.ts and PlanPage.styled.ts to
avoid duplication and maintain consistency.
| // draggingTodo가 속한 현재 path 찾기 | ||
| const currentPath = paths?.find((path) => | ||
| path.todos.some((t) => t.todoId === draggingTodo.todoId), | ||
| ); | ||
| const currentPathId = currentPath?.pathId; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Extract duplicated path-finding logic into a helper function.
The logic for finding the current path containing the dragged todo is duplicated.
Add a helper function to eliminate duplication:
+ const findPathContainingTodo = (todoId: number) => {
+ return paths?.find((path) =>
+ path.todos.some((todo) => todo.todoId === todoId)
+ );
+ };
const handleDragEnd = async (event: DragEndEvent) => {
// ... existing code ...
- // draggingTodo가 속한 현재 path 찾기
- const currentPath = paths?.find((path) =>
- path.todos.some((t) => t.todoId === draggingTodo.todoId),
- );
- const currentPathId = currentPath?.pathId;
+ const currentPath = findPathContainingTodo(draggingTodo.todoId);
+ const currentPathId = currentPath?.pathId;Apply the same change in handleDragOver at lines 74-78.
Also applies to: 74-78
🤖 Prompt for AI Agents
In frontend/src/pages/PlanPage/hooks/useDragAndDrop.ts around lines 41-45 and
74-78, the logic to find the current path containing the dragged todo is
duplicated. Extract this logic into a reusable helper function that takes the
todoId and paths as arguments and returns the matching path. Replace the
duplicated code in both places with calls to this new helper function to improve
code maintainability and reduce redundancy.
| // 드래그 중인 요소가 화면 하단에 가까우면 자동 스크롤 시작 | ||
| const rect = over.rect; | ||
| const containerRect = containerRef.current?.getBoundingClientRect(); | ||
|
|
||
| if (containerRect) { | ||
| const threshold = 100; | ||
| const isNearBottom = rect.bottom > containerRect.bottom - threshold; | ||
|
|
||
| if (isNearBottom) { | ||
| startAutoScroll(); | ||
| } else { | ||
| stopAutoScroll(); | ||
| } | ||
| } | ||
| }; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Fix auto-scroll implementation to use container scrolling.
The auto-scroll logic checks bounds against containerRef but scrolls the window. This inconsistency could cause unexpected behavior. Additionally, the scroll continues indefinitely without checking bounds.
Apply this diff to fix the auto-scroll implementation:
const startAutoScroll = () => {
if (autoScrollIntervalRef.current) return;
autoScrollIntervalRef.current = setInterval(() => {
- window.scrollBy({
- top: 5,
- behavior: 'smooth',
- });
+ if (containerRef.current) {
+ const { scrollTop, scrollHeight, clientHeight } = containerRef.current;
+ const canScroll = scrollTop + clientHeight < scrollHeight;
+
+ if (canScroll) {
+ containerRef.current.scrollBy({
+ top: 5,
+ behavior: 'smooth',
+ });
+ } else {
+ stopAutoScroll();
+ }
+ }
}, 16);
};Alternatively, if window scrolling is intended, update the bounds checking logic in handleDragOver to use window dimensions:
Also applies to: 103-112
🏁 Script executed:
#!/bin/bash
# Check if the container uses overflow scrolling or if window scrolling is intended
rg -A 5 -B 5 "ScrollableContent|overflow.*scroll" frontend/src/pages/PlanPage/Length of output: 1993
Fix auto-scroll to target the scrollable container, not the window
The S.ScrollableContent wrapper in PlanPage uses overflow-y: auto, so containerRef points to that element. We need to scroll the container and stop when it can no longer scroll.
• Files to update:
– frontend/src/pages/PlanPage/hooks/useDragAndDrop.ts (around startAutoScroll in lines 87–101 and similar logic in 103–112)
Apply this diff:
--- a/frontend/src/pages/PlanPage/hooks/useDragAndDrop.ts
+++ b/frontend/src/pages/PlanPage/hooks/useDragAndDrop.ts
@@ const startAutoScroll = () => {
- autoScrollIntervalRef.current = setInterval(() => {
- window.scrollBy({
- top: 5,
- behavior: 'smooth',
- });
- }, 16);
+ autoScrollIntervalRef.current = setInterval(() => {
+ const container = containerRef.current;
+ if (!container) return;
+
+ const { scrollTop, scrollHeight, clientHeight } = container;
+ const canScrollMore = scrollTop + clientHeight < scrollHeight;
+ if (canScrollMore) {
+ container.scrollBy({ top: 5, behavior: 'smooth' });
+ } else {
+ stopAutoScroll();
+ }
+ }, 16);With this change, auto‐scroll will only advance within the bounds of your scrollable container and will cease at the bottom.
🤖 Prompt for AI Agents
In frontend/src/pages/PlanPage/hooks/useDragAndDrop.ts around lines 87 to 101
and similarly in lines 103 to 112, the auto-scroll logic currently scrolls the
window instead of the scrollable container referenced by containerRef. Update
the startAutoScroll and stopAutoScroll functions to scroll the container element
itself by adjusting its scrollTop property, and add checks to stop scrolling
when the container cannot scroll further. This ensures auto-scroll operates
within the container's bounds and stops at the bottom.
Issue Number
close #296
As-Is
To-Be
Check List
Test Screenshot
2025-07-17.12.17.12.mov
(Optional) Additional Description
드래그 중일 때 스크롤 처리하는 게 쉽지 않아서
dnd-kit라이브러리를 도입했습니다.기존에 touch action에 대해서만 처리되던 게 PointEvent로 관리되어 데스크톱과 모바일 기기 모두 대응이 가능합니다. 또한 기존에 top과 right를 직접 옮겨 관리하던 오버레이를 별도로 분리해서 성능 측면에서도 개선되었습니다. react-dnd보다 dnd-kit이 더 최신에 업데이트되고 있고, 다운로드 수도 훨씬 많았습니다.
가장 큰 문제는 react-dnd 에서 모바일 대응을 하기 어려운데, dnd-kit은 PointEvent로 되어있어서 대응하기 수월했습니다. 또한, TouchSensor를 추가적으로 등록해서 더 세밀한 제어도 가능하고 선언적으로 관리되고 있어서 선택하게 되었습니다.
Summary by CodeRabbit
New Features
Style
Refactor