feat(planner): name a saved dive plan on first save#722
Open
ericgriffin wants to merge 7 commits into
Open
Conversation
Prompt for a name on first save with a site + depth + date default, and make rename discoverable from the canvas title and the saved-plans overflow menu. No schema or sync work: dive_plans.name already exists and already syncs via HLC.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves the dive planner UX by prompting for a meaningful plan name on the first save (instead of persisting multiple indistinguishable “New Dive Plan” rows), adds a reusable name-entry dialog (fixing a TextEditingController lifecycle leak), and makes renaming more discoverable from both the canvas and the saved-plans sheet.
Changes:
- Gate first-time plan persistence behind a localized name dialog seeded with a generated “site + depth + date” default (unit-formatted depth, locale-aware date).
- Extract a reusable plan-name dialog widget (properly disposing its
TextEditingController) and add rename affordances (pencil icon in canvas title, Rename menu item in saved plans). - Add comprehensive unit/widget tests and new l10n keys across all locales (regenerated localizations).
Reviewed changes
Copilot reviewed 35 out of 35 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/features/planner/saved_plans_sheet_test.dart | Pins locale to English for string-based finders; adds overflow-menu rename test. |
| test/features/planner/plan_name_generator_test.dart | Adds unit tests for the default plan-name generator (note: needs Intl.defaultLocale pinning for determinism). |
| test/features/planner/plan_name_dialog_test.dart | Adds widget tests validating dialog contract (prefill, cancel/null, trim, disable/enable confirm). |
| test/features/planner/plan_canvas_page_test.dart | Updates existing save test to confirm the new first-save name dialog. |
| test/features/planner/plan_canvas_first_save_test.dart | Adds widget coverage for first-save gating behavior (open, cancel aborts, confirm persists, second save no reprompt). |
| lib/l10n/arb/app_ar.arb | Adds planner name-dialog title + fallback label strings (Arabic). |
| lib/l10n/arb/app_de.arb | Adds planner name-dialog title + fallback label strings (German). |
| lib/l10n/arb/app_en.arb | Adds planner name-dialog title + fallback label strings (English source). |
| lib/l10n/arb/app_es.arb | Adds planner name-dialog title + fallback label strings (Spanish). |
| lib/l10n/arb/app_fr.arb | Adds planner name-dialog title + fallback label strings (French). |
| lib/l10n/arb/app_he.arb | Adds planner name-dialog title + fallback label strings (Hebrew). |
| lib/l10n/arb/app_hu.arb | Adds planner name-dialog title + fallback label strings (Hungarian). |
| lib/l10n/arb/app_it.arb | Adds planner name-dialog title + fallback label strings (Italian). |
| lib/l10n/arb/app_nl.arb | Adds planner name-dialog title + fallback label strings (Dutch). |
| lib/l10n/arb/app_pt.arb | Adds planner name-dialog title + fallback label strings (Portuguese). |
| lib/l10n/arb/app_zh.arb | Adds planner name-dialog title + fallback label strings (Chinese). |
| lib/l10n/arb/app_localizations.dart | Regenerates base localization interface with new getters. |
| lib/l10n/arb/app_localizations_ar.dart | Regenerates Arabic localizations for new planner keys. |
| lib/l10n/arb/app_localizations_de.dart | Regenerates German localizations for new planner keys. |
| lib/l10n/arb/app_localizations_en.dart | Regenerates English localizations for new planner keys. |
| lib/l10n/arb/app_localizations_es.dart | Regenerates Spanish localizations for new planner keys. |
| lib/l10n/arb/app_localizations_fr.dart | Regenerates French localizations for new planner keys. |
| lib/l10n/arb/app_localizations_he.dart | Regenerates Hebrew localizations for new planner keys. |
| lib/l10n/arb/app_localizations_hu.dart | Regenerates Hungarian localizations for new planner keys. |
| lib/l10n/arb/app_localizations_it.dart | Regenerates Italian localizations for new planner keys. |
| lib/l10n/arb/app_localizations_nl.dart | Regenerates Dutch localizations for new planner keys. |
| lib/l10n/arb/app_localizations_pt.dart | Regenerates Portuguese localizations for new planner keys. |
| lib/l10n/arb/app_localizations_zh.dart | Regenerates Chinese localizations for new planner keys. |
| lib/features/planner/presentation/widgets/saved_plans_sheet.dart | Adds Rename action to saved-plan overflow menu, wired to the shared dialog. |
| lib/features/planner/presentation/widgets/plan_name_dialog.dart | New reusable plan-name dialog widget with proper controller disposal + validation. |
| lib/features/planner/presentation/pages/plan_canvas_page.dart | Adds title edit affordance; gates first save on dialog; reuses shared rename dialog. |
| lib/features/planner/domain/services/plan_name_generator.dart | New pure generator for default plan name (site/depth/date/fallback). |
| lib/features/dive_planner/presentation/providers/dive_planner_providers.dart | Exposes isPersisted to support “first save only” prompting logic. |
| docs/superpowers/specs/2026-07-25-name-saved-dive-plan-design.md | Adds design spec documenting UX, architecture, and rationale. |
| docs/superpowers/plans/2026-07-25-name-saved-dive-plan.md | Adds detailed implementation plan and verification checklist. |
Comment on lines
+1
to
+8
| import 'package:flutter_test/flutter_test.dart'; | ||
| import 'package:submersion/features/planner/domain/services/plan_name_generator.dart'; | ||
|
|
||
| void main() { | ||
| final date = DateTime(2026, 7, 25); | ||
|
|
||
| group('generateDefaultPlanName', () { | ||
| test('combines site, depth, and date', () { |
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❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
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.
Problem
Saving a dive plan was silent, so every new plan kept the hardcoded, non-localized default
New Dive Plan. A diver who saved three plans got three identical rows in the saved-plans sheet, separable only by the date/depth subtitle. Renaming was possible but undiscoverable: the canvas AppBar title was a bareInkWellwith no affordance suggesting it could be tapped.What changed
Prompt on first save. The first time a never-persisted plan is saved, a dialog asks for a name, pre-filled with a generated default. Cancel aborts the save entirely, so nothing is written and the plan stays dirty. Re-saves stay silent.
Generated default name.
Blue Hole 40m - Jul 25, composed from the plan's site, its unit-formatted max depth, and its start date. Absent parts drop out (Blue Hole - Jul 25,40m - Jul 25), and when neither a site nor a depth is available it falls back toDive Plan - Jul 25. An empty plan never produces0m.Discoverable rename. The canvas title gains a small pencil icon, and the saved-plans overflow menu gains a Rename entry alongside Duplicate and Share.
Also fixes a real leak: the old inline rename dialog created a
TextEditingControllerinside its builder closure and never disposed it. The extracted_PlanNameDialogis aStatefulWidgetthat disposes it properly.Scope
Presentation layer only.
dive_plans.namealready existed as a non-null column carrying anhlcand was already registered in the sync table map, so a rename propagates through the existing last-writer-wins path. No migration, no schema-version bump, no sync change — verified bygit diff main --stat -- lib/core/database/returning empty.Design decisions
The generated name embeds a unit-formatted depth, which freezes that unit into the stored string. That is deliberate: the name is a user-editable label, and regenerating it after a unit switch would overwrite names the diver typed.
One accepted consequence of not gating Convert-to-Dive: converting a never-persisted plan still writes a row named
New Dive Plan. That is recoverable through the new Rename entry.Testing
generateDefaultPlanNamecovering all four presence combinations, blank-site handling, and whitespace trimmingsave action clears the dirty flag) was updated to confirm the dialog, since the first-save contract deliberately changedFull suite: 14,051 passing, 14 skipped.
flutter analyzeclean,dart format .clean.Localization
Two new keys (
plannerCanvas_name_dialogTitle,plannerCanvas_name_defaultFallback) added toapp_en.arband all ten non-English locales, then regenerated.Docs
Design spec and implementation plan are committed under
docs/superpowers/.