feat(territories): Prévoir le mode 'DragNDrop' sur les territoires du … #468
Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements drag-and-drop functionality for territory views in the Territories control panel. The feature allows users to reorder territory views by dragging them, using the SortableJS library.
Key Changes:
- Added SortableJS integration for drag-and-drop reordering of territory views
- Improved error message handling by separating duplicate name and empty name validation messages
- Enhanced territory removal/restoration to preserve DOM elements instead of recreating them
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| TerritoriesDOM.js | Added SortableJS import, created draggable element factory function, and split error messages for better UX |
| Territories.js | Implemented drag-and-drop event handlers, territory reordering logic, and improved territory removal/restoration behavior |
| package.json | Updated version and date for the release |
| DRAFT_CHANGELOG.md | Documented the new drag-and-drop feature |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * ... | ||
| * @param {Event} e - ... |
There was a problem hiding this comment.
The JSDoc comment for onReorderTerritoriesViews is incomplete. It should describe what the method does (reorders territories based on drag-and-drop interaction) and specify the type of event parameter (e.g., Sortable event object).
| * ... | |
| * @param {Event} e - ... | |
| * Reorders the territories based on the current order of views in the container, | |
| * typically after a drag-and-drop interaction. | |
| * @param {Event} e - The drag-and-drop event object (e.g., from a Sortable or Draggable interaction). |
| for (let index = 0; index < this.territories.length; index++) { | ||
| const territory = this.territories[index]; |
There was a problem hiding this comment.
Iterating through this.territories while calling removeTerritory(territory, true) modifies the array during iteration (via splice at line 279), causing elements to be skipped. Use a reverse loop or iterate over a copy of the array.
| for (let index = 0; index < this.territories.length; index++) { | |
| const territory = this.territories[index]; | |
| for (const territory of this.territories.slice()) { |
| } | ||
| }); | ||
| this.territories = newOrderTerritories; | ||
| // mise à jour l’ordre des territoires dans le DOM principal |
There was a problem hiding this comment.
Corrected 'mise à jour' to 'mise à jour de' for proper grammar.
| // mise à jour l’ordre des territoires dans le DOM principal | |
| // mise à jour de l’ordre des territoires dans le DOM principal |
| } | ||
| // reordonne la liste des territoires selon l'index initial | ||
| this.territories.sort((a, b) => a.initialIndex - b.initialIndex); | ||
| // mise à jour le DOM principal |
There was a problem hiding this comment.
Corrected 'mise à jour le' to 'mise à jour du' for proper grammar.
| // mise à jour le DOM principal | |
| // mise à jour du DOM principal |
Fixes #465