feat(dialogs): skills, custom-section, and interests dialogs support draggable tags. #2244
Conversation
📝 WalkthroughWalkthroughThe changes add drag-and-drop functionality for managing keyword lists in three dialog components: Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Component
participant KeywordsList
User->>Component: Initiates drag (onDragStart)
Component->>Component: Set draggedIndex state
User->>Component: Drags over keyword (onDragOver)
Component->>Component: Prevents default behavior
User->>Component: Drops keyword (onDrop)
Component->>KeywordsList: Reorders keywords based on draggedIndex and drop target
Component->>Component: Resets draggedIndex state
Assessment against linked issues
Possibly related PRs
Poem
✨ Finishing Touches
🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/client/src/pages/builder/sidebars/left/dialogs/interests.tsx (2)
34-37: MovehandleDragOveroutside the component function.The ESLint error correctly identifies that the
handleDragOverfunction doesn't use any component state or props, so it can be moved outside the component for better performance.type FormValues = z.infer<typeof formSchema>; +const handleDragOver = (e: React.DragEvent) => { + e.preventDefault(); +}; + export const InterestsDialog = () => { const form = useForm<FormValues>({ defaultValues: defaultInterest, resolver: zodResolver(formSchema), }); const [pendingKeyword, setPendingKeyword] = useState(""); const [draggedIndex, setDraggedIndex] = useState<number | null>(null); - const handleDragOver = (e: React.DragEvent) => { - e.preventDefault(); - };🧰 Tools
🪛 ESLint
[error] 35-35: Move arrow function 'handleDragOver' to the outer scope.
(unicorn/consistent-function-scoping)
1-130: Consider applying consistent code structure across dialog components.For better maintainability, you should apply the same code structure improvement suggested for this file to the other dialog components (
SkillsDialogandCustomSectionDialog) - moving thehandleDragOverfunction outside of the component definition since it doesn't use any component state.🧰 Tools
🪛 ESLint
[error] 35-35: Move arrow function 'handleDragOver' to the outer scope.
(unicorn/consistent-function-scoping)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/client/src/pages/builder/sidebars/left/dialogs/custom-section.tsx(3 hunks)apps/client/src/pages/builder/sidebars/left/dialogs/interests.tsx(2 hunks)apps/client/src/pages/builder/sidebars/left/dialogs/skills.tsx(2 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
apps/client/src/pages/builder/sidebars/left/dialogs/skills.tsx (1)
libs/schema/src/sections/skill.ts (1) (1)
defaultSkill(17-23)
🪛 ESLint
apps/client/src/pages/builder/sidebars/left/dialogs/interests.tsx
[error] 35-35: Move arrow function 'handleDragOver' to the outer scope.
(unicorn/consistent-function-scoping)
🔇 Additional comments (11)
apps/client/src/pages/builder/sidebars/left/dialogs/skills.tsx (4)
28-30: Well-implemented drag handler function.The
handleDragOverfunction correctly prevents the default behavior, which is necessary for proper drag-and-drop functionality to work.
39-39: Good state implementation for drag tracking.Using a nullable number for tracking the dragged item's index is appropriate for this use case.
41-55: Well-structured drop handler with proper safeguards.The
handleDropfunction:
- Prevents default behavior
- Handles the null case safely
- Creates a new array to maintain immutability
- Correctly performs the array manipulation for reordering
- Resets the drag state after completion
This implementation follows React best practices for state updates.
146-156: Properly implemented drag event handlers.The drag-and-drop event handlers are correctly implemented on the motion.div component, enabling the reordering functionality while maintaining the component's animation capabilities.
apps/client/src/pages/builder/sidebars/left/dialogs/custom-section.tsx (4)
33-35: Well-implemented drag handler function.The
handleDragOverfunction correctly prevents the default behavior, which is necessary for proper drag-and-drop functionality to work.
47-47: Good state implementation for drag tracking.Using a nullable number for tracking the dragged item's index is appropriate for this use case.
49-63: Well-structured drop handler with proper safeguards.The
handleDropfunction:
- Prevents default behavior
- Handles the null case safely
- Creates a new array to maintain immutability
- Correctly performs the array manipulation for reordering
- Resets the drag state after completion
This implementation follows React best practices for state updates.
197-207: Properly implemented drag event handlers.The drag-and-drop event handlers are correctly implemented on the motion.div component, enabling the reordering functionality while maintaining the component's animation capabilities.
apps/client/src/pages/builder/sidebars/left/dialogs/interests.tsx (3)
34-34: Good state implementation for drag tracking.Using a nullable number for tracking the dragged item's index is appropriate for this use case.
39-53: Well-structured drop handler with proper safeguards.The
handleDropfunction:
- Prevents default behavior
- Handles the null case safely
- Creates a new array to maintain immutability
- Correctly performs the array manipulation for reordering
- Resets the drag state after completion
This implementation follows React best practices for state updates.
99-109: Properly implemented drag event handlers.The drag-and-drop event handlers are correctly implemented on the motion.div component, enabling the reordering functionality while maintaining the component's animation capabilities.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/client/src/pages/builder/sidebars/left/dialogs/interests.tsx (1)
100-110: Consider enhancing drag-and-drop accessibilityWhile the draggable implementation works well, consider adding:
- Visual feedback during drag operations (e.g., opacity changes or shadows)
- Keyboard navigation alternatives for users who cannot use drag-and-drop
- ARIA attributes for screen reader support
Example enhancement for visual feedback:
<motion.div key={item} layout draggable + className={draggedIndex === index ? "opacity-50" : ""} initial={{ opacity: 0, y: -50 }} animate={{ opacity: 1, y: 0, transition: { delay: index * 0.1 } }} exit={{ opacity: 0, x: -50 }} onDragStart={() => { setDraggedIndex(index); }} onDragOver={handleDragOver} onDrop={(e) => { handleDrop(e, index, field); }} + aria-grabbed={draggedIndex === index} + aria-dropeffect="move" >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/client/src/pages/builder/sidebars/left/dialogs/interests.tsx(2 hunks)
🔇 Additional comments (3)
apps/client/src/pages/builder/sidebars/left/dialogs/interests.tsx (3)
27-29: Clean implementation of handleDragOver functionThe function properly prevents the default dragover behavior, which is necessary to enable dropping elements.
38-38: Good state management for drag trackingThe draggedIndex state variable is appropriately implemented to track which item is currently being dragged.
40-54: Well-structured handleDrop implementationThe drop handler logic correctly:
- Creates a new array to maintain immutability
- Removes the item from its original position
- Inserts it at the new position
- Updates the field value
- Resets the drag state
This implementation follows React best practices for state updates.
closes #2243
Summary by CodeRabbit