feat(planner): delete a dive plan from the canvas with confirm and undo#688
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds an in-canvas “Delete plan” action to the dive planner canvas overflow menu, including confirmation, deletion, navigation back to /planning, and an Undo flow that restores the captured plan snapshot.
Changes:
- Add a destructive overflow-menu action to delete the currently-open plan (gated to persisted plans /
:planIdroutes). - Implement confirm → delete → navigate → undo SnackBar behavior in
PlanCanvasPage. - Add localization strings + coverage via unit and widget tests.
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/features/planner/presentation/pages/plan_canvas_page.dart | Adds “Delete plan” overflow menu item, persistence gate helper, and confirm/delete/undo flow. |
| test/features/planner/plan_canvas_delete_test.dart | Widget tests covering delete, undo, cancel, and hidden-on-unsaved behaviors. |
| test/features/planner/plan_canvas_delete_gate_test.dart | Unit tests for planIsPersisted visibility gating helper. |
| lib/l10n/arb/app_en.arb | Adds new planner delete/undo strings (English source). |
| lib/l10n/arb/app_de.arb | Adds German translations for new planner delete/undo strings. |
| lib/l10n/arb/app_ar.arb | Adds Arabic translations for new planner delete/undo strings. |
| lib/l10n/arb/app_es.arb | Adds Spanish translations for new planner delete/undo strings. |
| lib/l10n/arb/app_fr.arb | Adds French translations for new planner delete/undo strings. |
| lib/l10n/arb/app_he.arb | Adds Hebrew translations for new planner delete/undo strings. |
| lib/l10n/arb/app_hu.arb | Adds Hungarian translations for new planner delete/undo strings. |
| lib/l10n/arb/app_it.arb | Adds Italian translations for new planner delete/undo strings. |
| lib/l10n/arb/app_nl.arb | Adds Dutch translations for new planner delete/undo strings. |
| lib/l10n/arb/app_pt.arb | Adds Portuguese translations for new planner delete/undo strings. |
| lib/l10n/arb/app_zh.arb | Adds Chinese translations for new planner delete/undo strings. |
| lib/l10n/arb/app_localizations.dart | Regenerates localization interface with new planner delete/undo keys. |
| lib/l10n/arb/app_localizations_en.dart | Regenerates English localization implementation with new planner delete/undo keys. |
| lib/l10n/arb/app_localizations_de.dart | Regenerates German localization implementation with new planner delete/undo keys. |
| lib/l10n/arb/app_localizations_ar.dart | Regenerates Arabic localization implementation with new planner delete/undo keys. |
| lib/l10n/arb/app_localizations_es.dart | Regenerates Spanish localization implementation with new planner delete/undo keys. |
| lib/l10n/arb/app_localizations_fr.dart | Regenerates French localization implementation with new planner delete/undo keys. |
| lib/l10n/arb/app_localizations_he.dart | Regenerates Hebrew localization implementation with new planner delete/undo keys. |
| lib/l10n/arb/app_localizations_hu.dart | Regenerates Hungarian localization implementation with new planner delete/undo keys. |
| lib/l10n/arb/app_localizations_it.dart | Regenerates Italian localization implementation with new planner delete/undo keys. |
| lib/l10n/arb/app_localizations_nl.dart | Regenerates Dutch localization implementation with new planner delete/undo keys. |
| lib/l10n/arb/app_localizations_pt.dart | Regenerates Portuguese localization implementation with new planner delete/undo keys. |
| lib/l10n/arb/app_localizations_zh.dart | Regenerates Chinese localization implementation with new planner delete/undo keys. |
Contributor
|
📦 Build artifacts for this PR · commit
Artifacts expire in 7 days. Downloading requires being signed in to GitHub. The macOS build is ad-hoc signed — right-click → Open on first launch. Updated automatically on each push. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…t locale Addresses PR #688 review: - _deletePlan no longer returns early when getPlan is null (plan removed via sync between open and confirm). It now deletes and navigates regardless, and only offers Undo when a snapshot was captured. - Pin plan_canvas_delete_test harness to Locale('en') so English label assertions match the planner test convention and don't depend on the environment locale. - Add a regression test for the already-deleted-elsewhere path.
…:false (#406) Addresses PR #688 follow-up review: - canDelete short-circuits on widget.planId != null before watching divePlanSummariesProvider, avoiding a DB subscription/fetch in the common edit-a-saved-plan case (the watch now runs only for a new plan). - The delete SnackBar sets duration 5s, persist: false, showCloseIcon: true so it auto-dismisses and can be closed without triggering Undo, matching the #406 convention (dive_edit_page.dart).
Addresses PR #688 review: during the loadPlanById() window the notifier state holds a freshly generated placeholder id, so _deletePlan reading planState.id could delete/tombstone the wrong id (and strand the real saved plan) on a fast tap. Now target widget.planId (fallback to notifier id only for a routeless plan) and fetch the plan from the store before the confirm dialog so the dialog, deletion, and undo snapshot all act on the same real plan. Add a test that deleting a routed plan leaves other stored plans intact.
…outcome Addresses PR #688 review: the undo snapshot restored the plan body from the store but its saved-plans summary numbers (maxDepth/runtime/tts) from planOutcomeProvider (transient in-memory edit state), so with unsaved edits or mid-load the restored list numbers could mismatch the restored plan. Add DivePlanRepository.getPlanSummary(id) (reads the persisted row like duplicatePlan) and capture the summary from it. capturedSummary is now nullable (persisted plan may have no computed summary), so drop the stale non-null assertion in the undo action. Add repository + widget tests.
github-project-automation
Bot
moved this from In review
to Done
in Submersion Release Tracker
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds an in-place way to delete the dive plan you are currently editing, from the plan canvas. Previously, deletion existed only in the saved-plans bottom sheet (reached via the canvas overflow menu → "Saved plans" → per-row trash icon), so a user inside an open plan had no direct affordance and had to back out to find it.
What changed
⋮overflow menu, rendered in the theme error color to read as distinct from the neutral items and from Reset (which only clears the working plan; it does not delete the saved record).divePlanSummariesProvider), with a fallback to the:planIdroute. A brand-new, never-saved plan has nothing to delete, so the item stays hidden.getPlan), then deleted (deletePlan, which already writes sync tombstones), and the editor navigates to/planningso it is never left on a now-dead:planIdroute.savePlan) — the same round-tripduplicatePlanrelies on. Undo re-inserts with fresh HLC events that supersede the tombstones, matching the app's existing dives/sites undo mechanism.No schema change, no migration, no new repository method — the feature reuses existing primitives.
Localization
Four new strings (
divePlanner_action_deletePlan,divePlanner_message_deleteConfirmation,divePlanner_message_planDeleted,divePlanner_undo) added to all 11 ARB locales and regenerated. Dialog buttons reuse the existingcommon_action_cancel/common_action_delete.Testing
planIsPersistedvisibility gate (in/absent/empty).flutter analyzeclean.Out of scope
The
/planninghub recent-plan tiles still lack an inline delete affordance (a separate discoverability gap); the saved-plans sheet's existing delete is unchanged.