feat: a table already existing can now be moved#222
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds the functionality to move existing tables in the POS table interface when in edit mode. It enables drag-and-drop repositioning of tables that have already been placed on the board.
- Integrates react-dnd drag functionality into existing DroppableTable components
- Updates the drop handler to differentiate between creating new tables and moving existing ones
- Adds visual feedback during dragging with opacity changes
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/Components/TablesView/DroppableTable.js | Adds useDrag hook and drag reference to enable existing tables to be draggable |
| src/Components/TablesView/TablesView.js | Updates drop handler to handle both new table creation and existing table movement |
| src/tests/Components/TableView/DroppableTable.test.js | Adds react-dnd mocks for testing the new drag functionality |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| const offset = monitor.getSourceClientOffset(); | ||
| if (!offset) return; | ||
|
|
||
| const containerRect = dropArea.getBoundingClientRect(); | ||
| const left = offset.x; | ||
| const top = offset.y; |
There was a problem hiding this comment.
The variable name offset is reused in both branches but represents different types of offsets (delta vs source client offset). Consider using more descriptive names like sourceOffset to distinguish their purposes and improve code clarity.
| return ( | ||
| <div onClick={() => onTableClick(table.orderId)} name={table.id} className={`${table.type === "circle" ? "rounded-full" : ""} border-4 absolute col-span-1 grid grid-flow-row ${inEdit === true ? `bg-grey-bg ${border} grid-rows-2` : table.time === "00:00" ? `bg-kitchen-green ${fuseBorder} grid-rows-3` : `bg-kitchen-yellow ${fuseBorder} grid-rows-3`} justify-center justify-items-center`} style={{height: `${table.h / tableSize}vw`, width: `${table.w / tableSize}vw`, top: table.top, left: table.left}}> | ||
| <div ref={inEdit ? drag : null} onClick={() => onTableClick(table.orderId)} name={table.id} className={`${table.type === "circle" ? "rounded-full" : ""} border-4 absolute col-span-1 grid grid-flow-row ${inEdit === true ? `bg-grey-bg ${border} grid-rows-2` : table.time === "00:00" ? `bg-kitchen-green ${fuseBorder} grid-rows-3` : `bg-kitchen-yellow ${fuseBorder} grid-rows-3`} justify-center justify-items-center`} style={{height: `${table.h / tableSize}vw`,width: `${table.w / tableSize}vw`,top: table.top,left: table.left,opacity: isDragging ? 0.5 : 1}}> |
There was a problem hiding this comment.
This line is extremely long and difficult to read. Consider breaking down the className construction and style object into separate variables or using template literals with line breaks for better maintainability.
JulesGresset
left a comment
JulesGresset
left a comment
There was a problem hiding this comment.
Features works as expected.
Some comments from Copilot to be resolved before merge.
| @@ -1,8 +1,18 @@ | |||
| /* eslint-disable react/prop-types */ | |||
There was a problem hiding this comment.
Why removing the linter in this file?
…-possibility-to-move-an-existing-table
Describe your changes
Add the possibility to move an existing table
How to test the feature
Go to POS table
Toggle edit mode
move an existing table
Issue ticket number and link
Closes #220
Checklist before requesting a review
If a single item in this checklist is not checked, the pull request cannot be accepted