Drag Mode 추가 #18
Drag Mode 추가 #18
Conversation
# Conflicts: # package.json # src/components/thumbnail-maker/index.tsx # yarn.lock
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
""" 워크스루이 풀 리퀘스트는 썸네일 메이커 컴포넌트에 드래그 앤 드롭 기능을 추가하는 주요 변경 사항을 포함하고 있습니다. 새로운 라이브러리 변경 사항
시퀀스 다이어그램sequenceDiagram
participant User
participant UI
participant DragModeCanvas
participant TagContext
User->>UI: 드래그 모드 스위치 토글
UI->>DragModeCanvas: 드래그 모드 활성화
User->>DragModeCanvas: 태그 드래그
DragModeCanvas->>TagContext: 태그 순서 업데이트
TagContext-->>DragModeCanvas: 새 태그 순서 반환
DragModeCanvas-->>UI: 업데이트된 태그 렌더링
시
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
# Conflicts: # package.json # src/components/thumbnail-maker/SubMenu/SubActionMenu.tsx # src/components/thumbnail-maker/index.tsx
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🔭 Outside diff range comments (1)
src/components/thumbnail-maker/SubMenu/SubActionMenu.tsx (1)
Line range hint
19-73: 비동기 작업에 대한 로딩 상태 처리가 필요합니다.
getImageFile이 비동기 함수임에도 불구하고 로딩 상태 처리가 되어있지 않습니다. 사용자 경험 향상을 위해 로딩 상태를 추가하는 것이 좋습니다.export function SubActionMenu({ getImageFile, }: { getImageFile: () => Promise<Blob | null>; }) { + const [isLoading, setIsLoading] = useState(false); const [isSaveTemplateSheetOpen, setIsSaveTemplateSheetOpen] = useState(false); // ... other state + const handleSaveTemplate = async () => { + setIsLoading(true); + try { + await getImageFile(); + setIsSaveTemplateSheetOpen(true); + } finally { + setIsLoading(false); + } + }; return ( // ... existing JSX - <MenubarItem onClick={() => setIsSaveTemplateSheetOpen(true)}> + <MenubarItem + onClick={handleSaveTemplate} + disabled={isLoading} + > - Save Template + {isLoading ? "저장 중..." : "템플릿 저장"} </MenubarItem>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (9)
package.json(1 hunks)src/components/thumbnail-maker/DragMode/DragModeCanvas.tsx(1 hunks)src/components/thumbnail-maker/DragMode/SortableTagItem.tsx(1 hunks)src/components/thumbnail-maker/SubMenu/SubActionMenu.tsx(1 hunks)src/components/thumbnail-maker/Tag.context.tsx(4 hunks)src/components/thumbnail-maker/TagItem.tsx(2 hunks)src/components/thumbnail-maker/hooks/useThumbnailTagList.ts(2 hunks)src/components/thumbnail-maker/index.tsx(2 hunks)src/components/ui/switch.tsx(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/components/thumbnail-maker/TagItem.tsx
[error] 26-32: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.
(lint/a11y/useKeyWithClickEvents)
🔇 Additional comments (3)
src/components/thumbnail-maker/Tag.context.tsx (1)
15-15: 컨텍스트 구현이 잘 되었습니다!태그 순서 업데이트 기능이 컨텍스트에 잘 통합되었고, 기본값 처리도 적절히 되어 있습니다.
Also applies to: 27-27, 57-57, 79-79
package.json (2)
18-20: 드래그 앤 드롭 기능을 위한 의존성이 적절히 추가되었습니다.@dnd-kit 라이브러리들이 태그의 드래그 기능 구현을 위해 추가되었습니다. 이는 PR의 목적에 부합합니다.
28-28: @radix-ui/react-slot 버전 다운그레이드 검증이 필요합니다.버전이 1.1.1에서 1.0.2로 다운그레이드되었습니다. 이 변경이 의도적인지, 그리고 다른 Radix UI 컴포넌트들과의 호환성 문제는 없는지 확인이 필요합니다.
다음 스크립트를 실행하여 의존성 호환성을 확인해주세요:
✅ Verification successful
@radix-ui/react-slot 버전 다운그레이드는 안전합니다
다음과 같은 이유로 1.0.2 버전으로의 다운그레이드는 안전하다고 판단됩니다:
- Button 컴포넌트에서만 기본적인 Slot 기능을 사용하고 있습니다
- 해당 버전에서도 현재 구현된 기능이 모두 지원됩니다
- 다른 Radix UI 컴포넌트들은 자체적으로 번들링된 react-slot을 사용하므로 영향이 없습니다
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Radix UI 컴포넌트들 간의 peer dependency 호환성 검증 # package-lock.json이나 yarn.lock에서 @radix-ui 관련 의존성 검색 if [ -f "package-lock.json" ]; then echo "Checking package-lock.json for @radix-ui dependencies..." cat package-lock.json | jq '.packages | to_entries[] | select(.key | contains("@radix-ui")) | {package: .key, version: .value.version, peerDependencies: .value.peerDependencies}' elif [ -f "yarn.lock" ]; then echo "Checking yarn.lock for @radix-ui dependencies..." cat yarn.lock | grep -A 5 "@radix-ui" fi # npm info로 호환성 정보 확인 echo "Checking npm registry for @radix-ui/[email protected] peer dependencies..." npm info "@radix-ui/[email protected]" peerDependenciesLength of output: 23528
Script:
#!/bin/bash # Check if any Radix components explicitly require newer react-slot versions echo "Checking package.json for all @radix-ui dependencies..." cat package.json | jq '.dependencies | with_entries(select(.key | startswith("@radix-ui")))' echo -e "\nChecking for potential breaking changes in components using react-slot..." rg -A 5 "from '@radix-ui/react-slot'"Length of output: 886
Script:
#!/bin/bash # Search for react-slot usage in TypeScript/JavaScript files echo "Searching for react-slot usage in TS/JS files..." fd -e ts -e tsx -e js -e jsx | xargs rg -l "@radix-ui/react-slot" echo -e "\nChecking for Slot component usage..." fd -e ts -e tsx -e js -e jsx | xargs rg -l "Slot[^a-zA-Z]"Length of output: 380
Script:
#!/bin/bash echo "Checking button.tsx implementation..." cat src/components/ui/button.tsxLength of output: 2031
| return ( | ||
| <div | ||
| ref={setNodeRef} | ||
| {...attributes} | ||
| {...listeners} | ||
| style={{ | ||
| width: "auto", | ||
| height: "auto", | ||
| transform: transform | ||
| ? `translate3d(${transform.x}px, ${transform.y}px, 0)` | ||
| : undefined, | ||
| transition, | ||
| zIndex: isDragging ? 1 : undefined, | ||
| }} | ||
| className={cn("touch-none", isDragging && "opacity-50")} | ||
| > | ||
| {children} | ||
| </div> | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
접근성 개선이 필요합니다.
드래그 가능한 요소에 대한 접근성 속성이 누락되었습니다.
<div
ref={setNodeRef}
{...attributes}
{...listeners}
+ role="button"
+ aria-label="드래그 가능한 태그 항목"
+ aria-grabbed={isDragging}
+ tabIndex={0}
style={{📝 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.
| return ( | |
| <div | |
| ref={setNodeRef} | |
| {...attributes} | |
| {...listeners} | |
| style={{ | |
| width: "auto", | |
| height: "auto", | |
| transform: transform | |
| ? `translate3d(${transform.x}px, ${transform.y}px, 0)` | |
| : undefined, | |
| transition, | |
| zIndex: isDragging ? 1 : undefined, | |
| }} | |
| className={cn("touch-none", isDragging && "opacity-50")} | |
| > | |
| {children} | |
| </div> | |
| ); | |
| return ( | |
| <div | |
| ref={setNodeRef} | |
| {...attributes} | |
| {...listeners} | |
| role="button" | |
| aria-label="드래그 가능한 태그 항목" | |
| aria-grabbed={isDragging} | |
| tabIndex={0} | |
| style={{ | |
| width: "auto", | |
| height: "auto", | |
| transform: transform | |
| ? `translate3d(${transform.x}px, ${transform.y}px, 0)` | |
| : undefined, | |
| transition, | |
| zIndex: isDragging ? 1 : undefined, | |
| }} | |
| className={cn("touch-none", isDragging && "opacity-50")} | |
| > | |
| {children} | |
| </div> | |
| ); |
| interface SortableTagItemProps { | ||
| tag: { | ||
| id: string | number; | ||
| }; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
타입 정의를 더 구체적으로 개선하세요.
태그 인터페이스가 id 속성만 정의되어 있습니다. 태그와 관련된 다른 필수 속성들도 포함하는 것이 좋습니다.
interface SortableTagItemProps {
tag: {
id: string | number;
+ label: string;
+ position?: { x: number; y: number };
};
}📝 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.
| interface SortableTagItemProps { | |
| tag: { | |
| id: string | number; | |
| }; | |
| } | |
| interface SortableTagItemProps { | |
| tag: { | |
| id: string | number; | |
| label: string; | |
| position?: { x: number; y: number }; | |
| }; | |
| } |
src/components/ui/switch.tsx
Outdated
| const Switch = React.forwardRef< | ||
| React.ElementRef<typeof SwitchPrimitives.Root>, | ||
| React.ComponentPropsWithoutRef<typeof SwitchPrimitives.Root> | ||
| >(({ className, ...props }, ref) => ( | ||
| <SwitchPrimitives.Root | ||
| className={cn( | ||
| "peer inline-flex h-6 w-11 shrink-0 cursor-pointer items-center rounded-full border-2 border-transparent transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2 focus-visible:ring-offset-background disabled:cursor-not-allowed disabled:opacity-50 data-[state=checked]:bg-primary data-[state=unchecked]:bg-input", | ||
| className | ||
| )} | ||
| {...props} | ||
| ref={ref} | ||
| > | ||
| <SwitchPrimitives.Thumb | ||
| className={cn( | ||
| "pointer-events-none block h-5 w-5 rounded-full bg-background shadow-lg ring-0 transition-transform data-[state=checked]:translate-x-5 data-[state=unchecked]:translate-x-0" | ||
| )} | ||
| /> | ||
| </SwitchPrimitives.Root> | ||
| )); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
접근성 레이블 속성을 추가하세요.
스위치 컴포넌트에 기본 aria-label이 누락되었습니다. 사용자가 레이블을 제공하지 않았을 때 사용할 기본값이 필요합니다.
const Switch = React.forwardRef<
React.ElementRef<typeof SwitchPrimitives.Root>,
React.ComponentPropsWithoutRef<typeof SwitchPrimitives.Root>
->(({ className, ...props }, ref) => (
+>(({ className, "aria-label": ariaLabel = "토글 스위치", ...props }, ref) => (
<SwitchPrimitives.Root
className={cn(
"peer inline-flex h-6 w-11 shrink-0 cursor-pointer items-center rounded-full border-2 border-transparent transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2 focus-visible:ring-offset-background disabled:cursor-not-allowed disabled:opacity-50 data-[state=checked]:bg-primary data-[state=unchecked]:bg-input",
className
)}
+ aria-label={ariaLabel}
{...props}
ref={ref}
>Committable suggestion skipped: line range outside the PR's diff.
| const onUpdateTagOrder = (newTags: Tag[]) => { | ||
| setTags(newTags); | ||
| }; |
There was a problem hiding this comment.
이전 상태 백업이 누락되었습니다.
다른 상태 업데이트 함수들과 달리 onUpdateTagOrder 함수에서는 prevTags.current 업데이트가 누락되었습니다. 실행 취소 기능의 일관성을 위해 이전 상태를 저장해야 합니다.
const onUpdateTagOrder = (newTags: Tag[]) => {
+ prevTags.current = tags;
setTags(newTags);
};📝 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.
| const onUpdateTagOrder = (newTags: Tag[]) => { | |
| setTags(newTags); | |
| }; | |
| const onUpdateTagOrder = (newTags: Tag[]) => { | |
| prevTags.current = tags; | |
| setTags(newTags); | |
| }; |
| {tags.map((tag) => ( | ||
| <SortableTagItem key={tag.id} tag={tag}> | ||
| <TagItem tag={tag} /> | ||
| </SortableTagItem> | ||
| ))} |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
태그 렌더링 최적화 제안
태그 목록이 큰 경우 성능에 영향을 줄 수 있습니다. React.memo를 사용하여 TagItem 컴포넌트를 최적화하고, 필요한 경우 가상화를 고려하세요.
| const sensors = useSensors( | ||
| useSensor(PointerSensor), | ||
| useSensor(KeyboardSensor, { | ||
| coordinateGetter: sortableKeyboardCoordinates, | ||
| }) | ||
| ); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
센서 설정의 메모이제이션 권장
useSensors 호출과 센서 설정이 매 렌더링마다 재생성됩니다. 성능 최적화를 위해 useMemo를 사용하는 것이 좋습니다.
- const sensors = useSensors(
- useSensor(PointerSensor),
- useSensor(KeyboardSensor, {
- coordinateGetter: sortableKeyboardCoordinates,
- })
- );
+ const sensors = useMemo(
+ () =>
+ useSensors(
+ useSensor(PointerSensor),
+ useSensor(KeyboardSensor, {
+ coordinateGetter: sortableKeyboardCoordinates,
+ })
+ ),
+ []
+ );📝 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.
| const sensors = useSensors( | |
| useSensor(PointerSensor), | |
| useSensor(KeyboardSensor, { | |
| coordinateGetter: sortableKeyboardCoordinates, | |
| }) | |
| ); | |
| const sensors = useMemo( | |
| () => | |
| useSensors( | |
| useSensor(PointerSensor), | |
| useSensor(KeyboardSensor, { | |
| coordinateGetter: sortableKeyboardCoordinates, | |
| }) | |
| ), | |
| [] | |
| ); |
| const handleDragEnd = (event: DragEndEvent) => { | ||
| const { active, over } = event; | ||
|
|
||
| if (over && active.id !== over.id) { | ||
| const oldIndex = tags.findIndex((tag) => tag.id === active.id); | ||
| const newIndex = tags.findIndex((tag) => tag.id === over.id); | ||
|
|
||
| if (oldIndex !== -1 && newIndex !== -1) { | ||
| const newTags = arrayMove(tags, oldIndex, newIndex); | ||
|
|
||
| onUpdateTagOrder(newTags); | ||
| } | ||
| } | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
handleDragEnd 함수의 메모이제이션 필요
handleDragEnd 함수가 매 렌더링마다 재생성되어 불필요한 리렌더링을 유발할 수 있습니다. useCallback을 사용하여 최적화하는 것이 좋습니다.
- const handleDragEnd = (event: DragEndEvent) => {
+ const handleDragEnd = useCallback((event: DragEndEvent) => {
const { active, over } = event;
if (over && active.id !== over.id) {
const oldIndex = tags.findIndex((tag) => tag.id === active.id);
const newIndex = tags.findIndex((tag) => tag.id === over.id);
if (oldIndex !== -1 && newIndex !== -1) {
const newTags = arrayMove(tags, oldIndex, newIndex);
onUpdateTagOrder(newTags);
}
}
- };
+ }, [tags, onUpdateTagOrder]);📝 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.
| const handleDragEnd = (event: DragEndEvent) => { | |
| const { active, over } = event; | |
| if (over && active.id !== over.id) { | |
| const oldIndex = tags.findIndex((tag) => tag.id === active.id); | |
| const newIndex = tags.findIndex((tag) => tag.id === over.id); | |
| if (oldIndex !== -1 && newIndex !== -1) { | |
| const newTags = arrayMove(tags, oldIndex, newIndex); | |
| onUpdateTagOrder(newTags); | |
| } | |
| } | |
| }; | |
| const handleDragEnd = useCallback((event: DragEndEvent) => { | |
| const { active, over } = event; | |
| if (over && active.id !== over.id) { | |
| const oldIndex = tags.findIndex((tag) => tag.id === active.id); | |
| const newIndex = tags.findIndex((tag) => tag.id === over.id); | |
| if (oldIndex !== -1 && newIndex !== -1) { | |
| const newTags = arrayMove(tags, oldIndex, newIndex); | |
| onUpdateTagOrder(newTags); | |
| } | |
| } | |
| }, [tags, onUpdateTagOrder]); |
| {onRemove && ( | ||
| <div | ||
| className="absolute -right-1 -top-1 hidden h-6 w-6 cursor-default justify-center rounded-full bg-white/80 p-0.5 align-middle group-hover:flex" | ||
| onClick={(e) => { | ||
| e.stopPropagation(); | ||
| onRemove(); | ||
| }} | ||
| > | ||
| <X size={20} color="#454545" /> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
접근성 개선이 필요합니다
삭제 버튼에 키보드 이벤트가 없어 키보드 사용자가 접근할 수 없습니다. 다음과 같이 개선이 필요합니다:
{onRemove && (
<div
+ role="button"
+ tabIndex={0}
className="absolute -right-1 -top-1 hidden h-6 w-6 cursor-default justify-center rounded-full bg-white/80 p-0.5 align-middle group-hover:flex"
onClick={(e) => {
e.stopPropagation();
onRemove();
}}
+ onKeyDown={(e) => {
+ if (e.key === 'Enter' || e.key === ' ') {
+ e.stopPropagation();
+ onRemove();
+ }
+ }}
>
<X size={20} color="#454545" />
</div>
)}📝 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.
| {onRemove && ( | |
| <div | |
| className="absolute -right-1 -top-1 hidden h-6 w-6 cursor-default justify-center rounded-full bg-white/80 p-0.5 align-middle group-hover:flex" | |
| onClick={(e) => { | |
| e.stopPropagation(); | |
| onRemove(); | |
| }} | |
| > | |
| <X size={20} color="#454545" /> | |
| </div> | |
| )} | |
| {onRemove && ( | |
| <div | |
| role="button" | |
| tabIndex={0} | |
| className="absolute -right-1 -top-1 hidden h-6 w-6 cursor-default justify-center rounded-full bg-white/80 p-0.5 align-middle group-hover:flex" | |
| onClick={(e) => { | |
| e.stopPropagation(); | |
| onRemove(); | |
| }} | |
| onKeyDown={(e) => { | |
| if (e.key === 'Enter' || e.key === ' ') { | |
| e.stopPropagation(); | |
| onRemove(); | |
| } | |
| }} | |
| > | |
| <X size={20} color="#454545" /> | |
| </div> | |
| )} |
🧰 Tools
🪛 Biome (1.9.4)
[error] 26-32: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.
(lint/a11y/useKeyWithClickEvents)
| <div className="flex items-center justify-end gap-2"> | ||
| <Switch checked={isDragMode} onCheckedChange={setIsDragMode} /> | ||
| <p>{isDragMode ? "Drag Mode" : "Default Mode"}</p> | ||
| </div> |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
사용자 경험 개선 제안
드래그 모드 토글의 UX를 개선하기 위한 제안:
- 토글에 설명 툴팁 추가
- 모드 변경 시 사용자에게 피드백 제공
<div className="flex items-center justify-end gap-2">
+ <div className="relative">
<Switch checked={isDragMode} onCheckedChange={setIsDragMode} />
+ <div className="absolute -bottom-8 right-0 hidden whitespace-nowrap rounded bg-gray-800 px-2 py-1 text-sm group-hover:block">
+ 태그를 드래그하여 순서를 변경할 수 있습니다
+ </div>
+ </div>
<p>{isDragMode ? "Drag Mode" : "Default Mode"}</p>
</div>Committable suggestion skipped: line range outside the PR's diff.
| {isDragMode ? ( | ||
| <DragModeCanvas | ||
| previewRef={previewRef} | ||
| tagsContainerRef={tagsContainerRef} | ||
| /> | ||
| ) : ( | ||
| <CanvasContainer | ||
| previewRef={previewRef} | ||
| tagsContainerRef={tagsContainerRef} | ||
| > | ||
| <TagList setOpenTagSheet={onSelectTag} /> | ||
| </CanvasContainer> | ||
| )} |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
조건부 렌더링 성능 최적화
DragModeCanvas와 CanvasContainer 컴포넌트를 React.lazy를 사용하여 지연 로딩하면 초기 로딩 성능을 개선할 수 있습니다.
+ const DragModeCanvas = React.lazy(() => import('./DragMode/DragModeCanvas'));
+ const CanvasContainer = React.lazy(() => import('./CanvasContainer'));
{isDragMode ? (
+ <Suspense fallback={<div>로딩 중...</div>}>
<DragModeCanvas
previewRef={previewRef}
tagsContainerRef={tagsContainerRef}
/>
+ </Suspense>
) : (
+ <Suspense fallback={<div>로딩 중...</div>}>
<CanvasContainer
previewRef={previewRef}
tagsContainerRef={tagsContainerRef}
>
<TagList setOpenTagSheet={onSelectTag} />
</CanvasContainer>
+ </Suspense>
)}Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (3)
package.json(2 hunks)src/components/thumbnail-maker/SubMenu/SubActionMenu.tsx(2 hunks)src/components/thumbnail-maker/index.tsx(3 hunks)
🔇 Additional comments (3)
package.json (2)
30-30: Switch 컴포넌트 의존성 추가 확인드래그 모드 토글을 위한 Switch 컴포넌트가 적절히 추가되었습니다. 다만, 다른 Radix UI 컴포넌트들과 마찬가지로 버전 관리에 주의가 필요합니다.
다음 스크립트로 Radix UI 컴포넌트들 간의 버전 호환성을 확인해보세요:
18-20: @dnd-kit 의존성 버전 관리에 대한 제안드래그 앤 드롭 기능을 위한 @dnd-kit 라이브러리들의 버전이 캐럿(^)을 사용하고 있습니다. 이는 마이너 버전 업데이트로 인한 호환성 문제를 야기할 수 있습니다.
다음 스크립트를 실행하여 현재 버전들의 호환성을 확인해보세요:
✅ Verification successful
@dnd-kit 패키지 버전이 안정적입니다
현재 지정된 버전들이 최신 안정 버전과 일치하며 서로 호환됩니다. 하지만 더 안정적인 배포를 위해 다음과 같이 캐럿(^) 대신 정확한 버전을 사용하는 것을 권장드립니다:
"@dnd-kit/core": "6.3.1", "@dnd-kit/sortable": "10.0.0", "@dnd-kit/utilities": "3.2.2",🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # @dnd-kit 패키지들의 호환성 확인 curl -s https://registry.npmjs.org/@dnd-kit/core | jq '.versions | keys[]' | grep 6.3 curl -s https://registry.npmjs.org/@dnd-kit/sortable | jq '.versions | keys[]' | grep 10.0 curl -s https://registry.npmjs.org/@dnd-kit/utilities | jq '.versions | keys[]' | grep 3.2Length of output: 1240
src/components/thumbnail-maker/index.tsx (1)
96-99: Props 네이밍 일관성 유지
downloadImageprop의 네이밍이 일관성 있게 변경되었습니다. 👍
| import { Switch } from "../ui/switch"; | ||
| import { useState } from "react"; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
상태 관리 최적화 제안
isDragMode 상태는 앱의 전역 상태로 관리하는 것이 좋을 것 같습니다. 여러 컴포넌트에서 드래그 모드 상태를 참조해야 할 경우를 대비하여 Context나 상태 관리 라이브러리로 이동을 고려해보세요.
- const [isDragMode, setIsDragMode] = useState(false);
+ const { isDragMode, setIsDragMode } = useDragMode();Also applies to: 20-20
| {isDragMode ? ( | ||
| <DragModeCanvas | ||
| previewRef={previewRef} | ||
| tagsContainerRef={tagsContainerRef} | ||
| /> | ||
| ) : ( | ||
| <CanvasContainer | ||
| previewRef={previewRef} | ||
| tagsContainerRef={tagsContainerRef} | ||
| > | ||
| <TagList setOpenTagSheet={onSelectTag} /> | ||
| </CanvasContainer> | ||
| )} |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
조건부 렌더링 성능 최적화
DragModeCanvas와 CanvasContainer 컴포넌트의 불필요한 리렌더링을 방지하기 위해 메모이제이션을 적용하는 것이 좋습니다.
+ const MemoizedDragModeCanvas = useMemo(
+ () => (
+ <DragModeCanvas
+ previewRef={previewRef}
+ tagsContainerRef={tagsContainerRef}
+ />
+ ),
+ [previewRef, tagsContainerRef]
+ );
+ const MemoizedCanvasContainer = useMemo(
+ () => (
+ <CanvasContainer
+ previewRef={previewRef}
+ tagsContainerRef={tagsContainerRef}
+ >
+ <TagList setOpenTagSheet={onSelectTag} />
+ </CanvasContainer>
+ ),
+ [previewRef, tagsContainerRef, onSelectTag]
+ );
- {isDragMode ? (
- <DragModeCanvas
- previewRef={previewRef}
- tagsContainerRef={tagsContainerRef}
- />
- ) : (
- <CanvasContainer
- previewRef={previewRef}
- tagsContainerRef={tagsContainerRef}
- >
- <TagList setOpenTagSheet={onSelectTag} />
- </CanvasContainer>
- )}
+ {isDragMode ? MemoizedDragModeCanvas : MemoizedCanvasContainer}Committable suggestion skipped: line range outside the PR's diff.
| <Link to="/gallery"> | ||
| <Button variant="secondary"> | ||
| <LayoutTemplateIcon size={20} /> | ||
| </Button> | ||
| </Link> | ||
| <Button onClick={onDownload}> | ||
| <ImageIcon size={20} className="mr-2" /> Download Image | ||
| </Button> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
접근성 개선 필요
버튼에 적절한 aria-label이 없어 스크린 리더 사용자의 접근성이 떨어집니다.
<Link to="/gallery">
- <Button variant="secondary">
+ <Button variant="secondary" aria-label="갤러리로 이동">
<LayoutTemplateIcon size={20} />
</Button>
</Link>
- <Button onClick={onDownload}>
+ <Button onClick={onDownload} aria-label="이미지 다운로드">
<ImageIcon size={20} className="mr-2" /> Download Image
</Button>📝 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.
| <Link to="/gallery"> | |
| <Button variant="secondary"> | |
| <LayoutTemplateIcon size={20} /> | |
| </Button> | |
| </Link> | |
| <Button onClick={onDownload}> | |
| <ImageIcon size={20} className="mr-2" /> Download Image | |
| </Button> | |
| <Link to="/gallery"> | |
| <Button variant="secondary" aria-label="갤러리로 이동"> | |
| <LayoutTemplateIcon size={20} /> | |
| </Button> | |
| </Link> | |
| <Button onClick={onDownload} aria-label="이미지 다운로드"> | |
| <ImageIcon size={20} className="mr-2" /> Download Image | |
| </Button> |
| const onDownload = () => { | ||
| downloadImage(); | ||
| toast.message("Downloaded Successfully", { | ||
| duration: 5000, | ||
| description: | ||
| "Share your carefully crafted image with others through the gallery! 🎨", | ||
| action: { | ||
| label: "Upload", | ||
| onClick: () => { | ||
| setIsSaveTemplateSheetOpen(true); | ||
| }, | ||
| }, | ||
| }); | ||
| }; |
There was a problem hiding this comment.
다운로드 에러 처리 추가 필요
다운로드 실패 시 사용자에게 적절한 피드백이 없습니다. 에러 처리를 추가하는 것이 좋겠습니다.
const onDownload = () => {
+ try {
downloadImage();
toast.message("Downloaded Successfully", {
duration: 5000,
description:
"Share your carefully crafted image with others through the gallery! 🎨",
action: {
label: "Upload",
onClick: () => {
setIsSaveTemplateSheetOpen(true);
},
},
});
+ } catch (error) {
+ toast.error("다운로드 중 오류가 발생했습니다", {
+ description: "잠시 후 다시 시도해주세요",
+ });
+ }
};📝 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.
| const onDownload = () => { | |
| downloadImage(); | |
| toast.message("Downloaded Successfully", { | |
| duration: 5000, | |
| description: | |
| "Share your carefully crafted image with others through the gallery! 🎨", | |
| action: { | |
| label: "Upload", | |
| onClick: () => { | |
| setIsSaveTemplateSheetOpen(true); | |
| }, | |
| }, | |
| }); | |
| }; | |
| const onDownload = () => { | |
| try { | |
| downloadImage(); | |
| toast.message("Downloaded Successfully", { | |
| duration: 5000, | |
| description: | |
| "Share your carefully crafted image with others through the gallery! 🎨", | |
| action: { | |
| label: "Upload", | |
| onClick: () => { | |
| setIsSaveTemplateSheetOpen(true); | |
| }, | |
| }, | |
| }); | |
| } catch (error) { | |
| toast.error("다운로드 중 오류가 발생했습니다", { | |
| description: "잠시 후 다시 시도해주세요", | |
| }); | |
| } | |
| }; |
Summary by CodeRabbit
새로운 기능
의존성 업데이트
@dnd-kit/core,@dnd-kit/sortable,@dnd-kit/utilities) 추가개선 사항