fix(ui): remove-all routes skips locked routes (#1298)#1370
fix(ui): remove-all routes skips locked routes (#1298)#1370iron-hope-shop wants to merge 2 commits intoAzgaar:masterfrom
Conversation
✅ Deploy Preview for afmg ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Looks good, thanks for the contribution! |
There was a problem hiding this comment.
Pull request overview
Updates the Routes Overview “Remove all” action to respect route locks, enabling users to clear generated/unwanted routes without deleting hand-pinned (locked) ones, aligning bulk-delete behavior with issue #1298.
Changes:
- Bulk delete now targets only unlocked routes and uses
Routes.removeper route to keep data + SVG in sync. - Adds user feedback when there are no routes or when all routes are locked (tip instead of confirmation).
- Updates confirmation wording and the trash button tooltip to clarify locked routes are retained.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/index.html | Updates the Routes Overview trash button tooltip to clarify it removes only unlocked routes. |
| public/modules/ui/routes-overview.js | Implements unlocked-only bulk deletion, adds empty/all-locked tips, and updates confirmation message/title accordingly. |
public/modules/ui/routes-overview.js
Outdated
| pack.routes = []; | ||
| routes.selectAll("path").remove(); | ||
|
|
||
| for (const route of [...toRemove]) { |
There was a problem hiding this comment.
toRemove is computed when opening the dialog and then captured in the Remove callback. Because #alert dialogs are non-modal by default, a user can lock/unlock routes while the confirmation is open; currently a route that becomes locked after opening can still be deleted. Consider recomputing the list of unlocked routes inside the Remove handler (or making the dialog modal) so the action always respects the current lock state.
| for (const route of [...toRemove]) { | |
| const routesToRemove = pack.routes.filter(route => !route.lock); | |
| if (!routesToRemove.length) { | |
| if (!pack.routes.length) { | |
| tip("There are no routes to remove", false, "error"); | |
| } else { | |
| tip("All routes are locked. Unlock routes to remove them, or use Lock all to unlock first.", false, "error"); | |
| } | |
| $(this).dialog("close"); | |
| return; | |
| } | |
| for (const route of [...routesToRemove]) { |
There was a problem hiding this comment.
that should be fine, Copilot. A very rare case.
| Remove: function () { | ||
| pack.cells.routes = {}; | ||
| pack.routes = []; | ||
| routes.selectAll("path").remove(); | ||
|
|
||
| for (const route of [...toRemove]) { | ||
| Routes.remove(route); | ||
| } | ||
| routesOverviewAddLines(); | ||
| $(this).dialog("close"); |
There was a problem hiding this comment.
After removing routes via Routes.remove, pack.cells.routes will keep many empty per-cell objects (since Routes.remove deletes only entries, not empty nodes). If all routes are removed, this leaves a large sparse object that bloats saves/exports (save.js JSON.stringify) and adds overhead to Routes.isConnected checks. Consider cleaning up empty pack.cells.routes[from] objects after the loop (or rebuilding links with Routes.buildLinks(pack.routes) / resetting to {} when pack.routes.length === 0).
|
@iron-hope-shop, could you please address the comments? I will be able to merge then. |
|
the remove action now recomputes unlocked routes at confirm time, and after bulk removal |
Summary
"Remove all routes" in the Routes Overview now deletes only unlocked routes. Locked routes and their geometry stay. Users who want a full wipe can unlock everything first (existing lock/unlock-all control).
Motivation
Implements the behavior requested in #1298: bulk delete should ignore locked routes so you can clear generated routes without losing hand-pinned or important ones.
Changes
How to test
#1298