-
Notifications
You must be signed in to change notification settings - Fork 0
Drag and Drop for Items #205
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR implements drag and drop functionality for inventory items, enabling users to move items between inventories and reorder items within a single inventory through an intuitive drag-and-drop interface.
Key changes:
- Added frontend drag event handlers to ItemRowDisplay and InventoryContainer components with visual ghost item feedback
- Implemented backend API endpoint for moving items between inventories with proper authorization checks
- Added sorting functionality that updates item order in the database with ORDER BY clause in queries
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/components/ItemRowDisplay.vue | Adds draggable attribute and drag event handlers to enable items to be dragged |
| frontend/src/components/InventoryContainer.vue | Implements drop zone logic, ghost item positioning, and orchestrates move/sort operations |
| frontend/src/components/GhostItem.vue | New component that provides visual feedback during drag operations |
| frontend/src/store/index.ts | Adds state management actions for changeItemSorting and moveItem operations |
| frontend/src/store/DatabaseHandler.ts | Adds API client methods for changeItemSorting and moveItem endpoints |
| backend/inventarwerk_api/src/routers/inventory_router.rs | Adds move_item_between_inventories endpoint with authorization validation |
| backend/inventarwerk_api/src/routers/mod.rs | Registers the new move endpoint in the route list |
| backend/repositories/src/repos/inventory_repository.rs | Implements move_inventory_item database operation and adds ORDER BY to item queries |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
frontend/src/store/index.ts
Outdated
| changeItemSorting(inventoryUuid: string, itemUuid: string, newSorting: number) { | ||
| this.inventories[inventoryUuid].items.find( | ||
| (item) => item.presetReference === itemUuid | ||
| )!.sorting = newSorting | ||
| this.inventories[inventoryUuid].items.sort((a, b) => a.sorting - b.sorting) | ||
| DatabaseHandler.getInstance().changeItemSorting(inventoryUuid, itemUuid, newSorting) |
Copilot
AI
Jan 5, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changeItemSorting method performs an optimistic update by modifying the local state and sorting the items array before the database operation completes. If the database operation fails, the local state will be out of sync with the server. Consider awaiting the database operation or implementing proper error handling to rollback local changes if the operation fails.
| changeItemSorting(inventoryUuid: string, itemUuid: string, newSorting: number) { | |
| this.inventories[inventoryUuid].items.find( | |
| (item) => item.presetReference === itemUuid | |
| )!.sorting = newSorting | |
| this.inventories[inventoryUuid].items.sort((a, b) => a.sorting - b.sorting) | |
| DatabaseHandler.getInstance().changeItemSorting(inventoryUuid, itemUuid, newSorting) | |
| async changeItemSorting(inventoryUuid: string, itemUuid: string, newSorting: number) { | |
| try { | |
| await DatabaseHandler.getInstance().changeItemSorting(inventoryUuid, itemUuid, newSorting) | |
| this.inventories[inventoryUuid].items.find( | |
| (item) => item.presetReference === itemUuid | |
| )!.sorting = newSorting | |
| this.inventories[inventoryUuid].items.sort((a, b) => a.sorting - b.sorting) | |
| } catch (error) { | |
| console.error('Failed to change item sorting:', error) | |
| } |
| async moveItem(sourceInventoryUuid: string, targetInventoryUuid: string, itemUuid: string) { | ||
| const item = this.inventories[sourceInventoryUuid].items.find( | ||
| (item) => item.presetReference === itemUuid | ||
| )! | ||
| this.inventories[targetInventoryUuid].items.push(item) | ||
| this.inventories[sourceInventoryUuid].items = this.inventories[ | ||
| sourceInventoryUuid | ||
| ].items.filter((item) => item.presetReference !== itemUuid) | ||
| await DatabaseHandler.getInstance().moveItem( | ||
| sourceInventoryUuid, | ||
| targetInventoryUuid, | ||
| itemUuid | ||
| ) |
Copilot
AI
Jan 5, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The moveItem method performs an optimistic update by modifying the local state before the database operation completes. If the database operation fails (e.g., due to constraint violations or network issues), the local state will be inconsistent with the server state. Consider awaiting the database operation and implementing proper error handling to rollback the state changes if the operation fails.
| ) { | ||
| ghostPosition.value = -1 | ||
| currentlyHovering.value = null | ||
| throw new Error(`This inventory already contains the target item!`) |
Copilot
AI
Jan 5, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Throwing an error in an async function without proper error handling can lead to unhandled promise rejections. The error thrown here may not be caught by the component's error boundary, potentially causing the application to crash or leave it in an inconsistent state. Consider using a more graceful error handling approach, such as displaying a user-friendly error message using a toast notification or alert dialog.
| throw new Error(`This inventory already contains the target item!`) | |
| alert('This inventory already contains the target item!') | |
| return |
Kr0nox
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer an actual preview instead of the simple + field. More importantly, the dragged one should disapear when dragging. I would suggest to just use a library like vue draggable next.
In addition after dragging the does not imideatly reflect the changes but takes a second. this should be instantly
I tried keeping the footprint as small as possible by using native HTML events, but I'll attempt to migrate to the library later today
I will investigate this further, this should not be the case, as (with all other inventory changes) the move should be emulated locally first, before sending it to the server. When exactly did you encounter this increased delay? |
This PR adds Drag and Drop for items, to both move them between inventories, and sorting inventories.
The initial commit (11029b7) implements a very basic version that just supports moving items between inventories, while the following commits add the sorting functionality, which turned out more complex in nature.
This feature should likely be tested more thoroughly before merging, local testing was difficult due to the live update feature not properly working on the main branch right now :)
Edit: Also closes #10