Skip to content

Show dive type badges in the dive detail header - #1276

Merged
ericgriffin merged 5 commits into
submersion-app:mainfrom
readme42:feature/dive-type-badges-1269
Aug 28, 2026
Merged

ericgriffin merged 5 commits into
submersion-app:mainfrom
readme42:feature/dive-type-badges-1269

Conversation

@readme42

@readme42 readme42 commented Aug 25, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Adds a row of type badges (Wreck, Night, Drift, etc.) directly below the star rating and OC/CCR badge in the dive detail header, matching that badge's shape but in a lighter, translucent tone so it stays quiet next to the mode indicator while still reading over the header's live map background. When more types are set than fit on one line, they collapse into a "+N" badge with a tooltip listing the rest instead of wrapping. The badge cap scales with the header's own width instead of a flat constant, so a wide detail pane can spell out more real labels before collapsing.

Built-in dive types get short-form abbreviations (Wreck, Tec, Rec, Wrack, Tief, Boot, Safari, ...) via new localized strings, translated across all eleven supported locales. Custom dive types can now set their own short name too (schema v173: dive_types.short_name), added as an optional field on the Add Dive Type dialog and previewed on the Manage Dive Types settings page.

The same badge row is now also shown on the compact and detailed dive list cards, right-aligned on the stat line next to depth/duration and sized to match the OC/CCR badge. Tags render on their own line below so a long tag name can never compete with the stat row or the badges for width.

Each dive type -- built-in or custom -- also gets two independent visibility toggles (schema v174: dive_types.show_in_detail_header / show_in_list_view, both defaulting to shown), editable via a new edit dialog reachable by tapping its row on the Manage Dive Types page. A type's name stays disabled for built-ins there, matching the existing "cannot update built-in dive types" protection; visibility is a separate per-diver display preference and bypasses that protection through a dedicated repository method.

image image

Dive type shortnames (on the right), also for custom dive types:
image

Dive types also in the list view:
image

Dive types also got 2 checkboxes to determine if they should be shown as badges in the list / header:
image

Closes #1269.

Test plan

  • flutter analyze clean on all touched files
  • dart format clean
  • New/updated unit and widget tests pass (dive_type_label_test.dart, dive_type_entity_test.dart, dive_detail_page_test.dart, dive_types_page_test.dart, dive_list_tile_type_badges_test.dart, dive_type_repository_short_name_test.dart, dive_type_repository_visibility_test.dart)
  • Migration coverage passes (migration_v173_dive_type_short_name_test.dart, migration_v174_dive_type_visibility_test.dart) and the full test/core/database/ suite passes
  • arb_parity_test.dart passes -- all eleven locales define every new key
  • Manual check on a narrow (embedded master-detail) window to confirm the badge row's width cap still looks right in both the header and the list cards

@readme42
readme42 marked this pull request as ready for review August 25, 2026 21:53
@readme42
readme42 requested a review from ericgriffin as a code owner August 25, 2026 21:53
@readme42 readme42 added enhancement New feature or request user interface labels Aug 25, 2026
@github-actions

github-actions Bot commented Aug 25, 2026 •

Copy link
Copy Markdown
Contributor

📦 Build artifacts for this PR · commit b962b42

Platform Download
Android (APK) android-apk
macOS macos-build
Windows windows-build
Linux linux-build

Artifacts expire in 7 days. Downloading requires being signed in to GitHub. macOS needs two extractions: unzip the downloaded artifact, then unzip the submersion-macos.zip inside it to get a runnable submersion.app. The build is ad-hoc signed — right-click → Open on first launch.

Updated automatically on each push.

@codecov

codecov Bot commented Aug 25, 2026 •

Copy link
Copy Markdown

@readme42
readme42 force-pushed the feature/dive-type-badges-1269 branch 2 times, most recently from ae0b3cd to 8214d8f Compare August 25, 2026 22:35
@ericgriffin
ericgriffin requested a lite review from Copilot August 26, 2026 01:01
@ericgriffin ericgriffin moved this from Backlog to In review in Submersion Release Tracker Aug 26, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds dive type badges to the Dive Detail header and introduces “short name” support for dive types (built-in via new localized abbreviations; custom via a new optional dive_types.short_name column + UI), including a v162 migration and accompanying tests.

Changes:

  • Render a single-line row of dive type badges in the Dive Detail header, collapsing overflow into a +N badge with tooltip.
  • Add optional short names for custom dive types (DB schema v162 + repository/provider wiring + add-dialog UI + preview on Dive Types page).
  • Extend localization keys for built-in short labels and add/update widget/unit/migration tests.

Reviewed changes

Copilot reviewed 38 out of 38 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
test/features/dive_types/presentation/pages/dive_types_page_test.dart Adds widget coverage for creating custom dive types with/without a short name and verifying badge behavior on the tile.
test/features/dive_types/data/repositories/dive_type_repository_short_name_test.dart New repository tests for persisting/round-tripping shortName.
test/features/dive_log/presentation/pages/dive_detail_page_test.dart Adds widget coverage asserting type badges appear (or not) on the Dive Detail header.
test/features/dive_log/presentation/formatters/dive_type_label_test.dart Adds unit coverage for diveTypeShortLabel behavior (built-in abbreviations vs custom entities).
test/core/database/migration_v162_dive_type_short_name_test.dart New migration coverage validating short_name column creation and ladder inclusion.
lib/l10n/arb/app_zh.arb Adds built-in short-label keys + add-dialog short-name strings (ZH).
lib/l10n/arb/app_pt.arb Adds built-in short-label keys + add-dialog short-name strings (PT).
lib/l10n/arb/app_nl.arb Adds built-in short-label keys + add-dialog short-name strings (NL).
lib/l10n/arb/app_localizations.dart Adds generated getters for new short-label and dialog strings.
lib/l10n/arb/app_localizations_zh.dart Adds generated ZH implementations for new localization getters.
lib/l10n/arb/app_localizations_pt.dart Adds generated PT implementations for new localization getters.
lib/l10n/arb/app_localizations_nl.dart Adds generated NL implementations for new localization getters.
lib/l10n/arb/app_localizations_it.dart Adds generated IT implementations for new localization getters.
lib/l10n/arb/app_localizations_hu.dart Adds generated HU implementations for new localization getters.
lib/l10n/arb/app_localizations_he.dart Adds generated HE implementations for new localization getters.
lib/l10n/arb/app_localizations_fr.dart Adds generated FR implementations for new localization getters.
lib/l10n/arb/app_localizations_es.dart Adds generated ES implementations for new localization getters.
lib/l10n/arb/app_localizations_en.dart Adds generated EN implementations for new localization getters.
lib/l10n/arb/app_localizations_de.dart Adds generated DE implementations for new localization getters.
lib/l10n/arb/app_localizations_ar.dart Adds generated AR implementations for new localization getters.
lib/l10n/arb/app_it.arb Adds built-in short-label keys + add-dialog short-name strings (IT).
lib/l10n/arb/app_hu.arb Adds built-in short-label keys + add-dialog short-name strings (HU).
lib/l10n/arb/app_he.arb Adds built-in short-label keys + add-dialog short-name strings (HE).
lib/l10n/arb/app_fr.arb Adds built-in short-label keys + add-dialog short-name strings (FR).
lib/l10n/arb/app_es.arb Adds built-in short-label keys + add-dialog short-name strings (ES).
lib/l10n/arb/app_en.arb Adds built-in short-label keys + add-dialog short-name strings (EN).
lib/l10n/arb/app_de.arb Adds built-in short-label keys + add-dialog short-name strings (DE).
lib/l10n/arb/app_ar.arb Adds built-in short-label keys + add-dialog short-name strings (AR).
lib/features/dive_types/presentation/providers/dive_type_providers.dart Extends add-by-name notifier API to accept optional shortName and trims/normalizes it.
lib/features/dive_types/presentation/pages/dive_types_page.dart Adds short-name input to the “Add Dive Type” dialog and previews short labels via DiveTypeBadge on the list tiles.
lib/features/dive_types/presentation/dive_type_display.dart Adds builtInDiveTypeShortName mapping for localized abbreviations.
lib/features/dive_types/domain/entities/dive_type_entity.dart Adds shortName field to the domain entity and propagates through constructors/copyWith/equatable props.
lib/features/dive_types/data/repositories/dive_type_repository.dart Persists/loads short_name in create/update/list/get mapping.
lib/features/dive_log/presentation/widgets/dive_type_badge.dart New badge widget matching DiveModeBadge styling with quieter visuals.
lib/features/dive_log/presentation/widgets/dive_type_badge_row.dart New single-line, overflow-collapsing badge row with tooltip listing hidden types.
lib/features/dive_log/presentation/pages/dive_detail_page.dart Renders the new dive type badge row in the header and wires label resolution via diveTypeShortLabel.
lib/features/dive_log/presentation/formatters/dive_type_label.dart Adds diveTypeShortLabel to resolve built-in abbreviations and custom short names safely.
lib/core/database/database.dart Adds dive_types.short_name column, bumps schema to v162, and introduces an idempotent backstop assertion + upgrade step.
Suppressed comments (1)

test/features/dive_log/presentation/pages/dive_detail_page_test.dart:1601

  • This test relies on the default locale but asserts English text (via DiveTypeBadge finders). Pinning the locale makes the test deterministic across different CI/device locale configurations.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread lib/features/dive_types/presentation/dive_type_display.dart Outdated
Comment thread lib/features/dive_log/presentation/widgets/dive_type_badge.dart Outdated
@readme42
readme42 marked this pull request as draft August 26, 2026 16:32
@readme42
readme42 force-pushed the feature/dive-type-badges-1269 branch 2 times, most recently from 1ec7ca8 to 45c8cfd Compare August 26, 2026 16:52
@ericgriffin

Copy link
Copy Markdown
Member

Merged main into this branch and resolved the conflicts; pushed to the fork branch.

The conflict was the migration-ladder collision. main landed v164 for media.manual_elapsed_seconds (#1090, PR #1287) while this branch also held v164 for dive_types.short_name, so this branch is renumbered to v167. v165 is claimed by the open PR #1290 and v166 by the open PR #1300, so 167 is the next free rung.

Worth knowing for anyone reading the diff: currentSchemaVersion itself did not carry a conflict marker. Both sides had independently written the literal 164, so git auto-merged that line, and the bare 164, ladder entry sat outside the markers as common ground too. Only the surrounding comments conflicted. Resolving just the marked hunks would have produced a database stamped v164 with two different migrations claiming it, which compiles and passes most tests.

All six sites of the claim moved together:

  1. currentSchemaVersion 164 -> 167
  2. the migrationVersions ladder entry (main's v164 entry kept intact)
  3. the DiveTypes.shortName column docstring and the _assertDiveTypeShortNameColumn() helper docstring
  4. the if (from < 167) onUpgrade guard and its reportProgress() twin, both now separate from main's v164 pair
  5. the beforeOpen backstop comment, with both backstops retained
  6. migration_v164_dive_type_short_name_test.dart -> migration_v167_dive_type_short_name_test.dart (git mv, so history follows), including its greaterThanOrEqualTo(167) and contains(167) assertions

minimumCompatibleSchemaVersion stays at 160: a new nullable column does not raise the compatibility floor.

Note the ladder now reads ... 161, 163, 164, 167 on this branch. 162 was already permanently skipped on main; 165 and 166 are the reservations held by #1290 and #1300. Whichever of the three lands last may need one more renumber, since a renumber is not a reservation.

Verification

  • Ladder audit (parsed the scalar, migrationVersions, every if (from < N) block, every reportProgress(), and the beforeOpen backstop calls): monotonic, no duplicates, scalar == max, every rung has a guard and a progress pair, both new helpers called in beforeOpen and defined once.
  • git diff origin/main -- lib/core/database/database.dart contains only this feature's added lines, so none of main's work was reverted in the resolution.
  • flutter analyze on the whole project: no issues.
  • flutter test test/core/database/: 469 passed (this is the suite that catches a clobbered ladder entry, not just the new migration test).
  • flutter test on dive_type_label_test.dart, dive_detail_page_test.dart, test/features/dive_types/, test/features/media/: 1865 passed.
  • flutter gen-l10n and dart run build_runner build produced no drift, so the auto-merged ARB and generated localization files are consistent.
  • The pre-push affected-test hook passed on the way out.

ericgriffin added a commit to alpheios-one/submersion that referenced this pull request Aug 26, 2026
main took v161 for diver_settings.default_show_o2_cell_mv (issue submersion-app#1235)
while this branch was open, so buddies.is_favorite moves to the next free
rung. v165, v166 and v167 are claimed by PRs submersion-app#1290, submersion-app#1300 and submersion-app#1276.
ericgriffin added a commit that referenced this pull request Aug 27, 2026
v168 was already claimed and pushed by PR #1237 (issue #638,
buddies.is_favorite). That claim was resolved locally and not yet pushed when
this branch picked its number, so the open-PR diff scan this plan prescribes
could not see it. Two branches writing the same scalar auto-merge with no
conflict marker, so the collision would have surfaced only as a database
silently skipping a rung.

Moves all six coupled sites together plus the test filename: the scalar, the
migrationVersions entry, the _assertTripDayWeatherSchema docstring, the
onUpgrade guard, its reportProgress twin, and the beforeOpen backstop comment.
The migration test is renamed to migration_v171_trip_day_weather_test.dart with
its greaterThanOrEqualTo and contains assertions updated. Its stranded-database
fixture moves from 168 to 171: 168 is now a real rung owned by #1237, so a
database stamped there upgrades normally and no longer exercises the backstop.

The design doc and plan are updated too, including the reasoning passages that
were wrong rather than merely stale: the plan recorded #1237 as a stale v161
claim when it was a live v168 one. Both now say to scan every worktree's
working-tree scalar alongside open PR diffs, and to re-run both immediately
before pushing rather than only when picking the number.

Ladder is left non-contiguous by design: 165 #1290, 166 #1300, 167 #1276,
168 #1237, 169 the dive-computer gear-twin branch, 170 #1322.

Verified: ladder monotonic, unique, scalar == max, 168 absent; helper defined
exactly once and referenced three times; guard and twin both at 171;
flutter analyze clean; flutter test test/core/database/ 471 passed.
ericgriffin added a commit that referenced this pull request Aug 27, 2026
…6 + v170)

Two conflicts, both from main moving under this branch:

- database.dart: the ladder took v166 (place_name_language, #1187). All
  three hunks are additive, so both rungs stay, ordered by version, with
  main's comment text verbatim; the scalar takes the max. v170's comment
  now names only the rungs still outstanding (167 #1276, 168 #1237, 169 a
  parallel worktree). The floor stays at 170: a defaulted column does not
  raise it, but this branch's synced-column rename does.
- dive_detail_page.dart: main extracted the Cylinders card into
  _cylindersCard so it can also render inside the Cylinders/Weights pair
  (#1306). Took main's structure and moved this branch's parameter change
  into the helper, so both call sites get it.
ericgriffin added a commit that referenced this pull request Aug 27, 2026
Resolves the schema-ladder conflict in database.dart: main landed v168
(#638 buddies.is_favorite) while this branch held v170, so main's ladder
entry, onUpgrade step and beforeOpen backstop are kept ahead of this
branch's v170 gas_consumption_display work and currentSchemaVersion stays
at 170. The v170 comment is corrected: 168 has landed, 167 is permanently
skipped now that main moved past PR #1276, and 169 belongs to PR #1320.
ericgriffin added a commit that referenced this pull request Aug 27, 2026
Resolves the schema-ladder conflict in database.dart: main landed v166
(#1187 place_name_language) and v168 (#638 buddies.is_favorite) while this
branch held v171, so both ladder entries, both onUpgrade steps and both
beforeOpen backstops are kept in ascending order and currentSchemaVersion
stays at 171. The v171 comment now names the gaps that are still open
(165 #1290, 169 #1320, 170 #1322) and records that 167 is permanently
skipped, since main landed 168 past PR #1276 and that PR moves up.
@ericgriffin

Copy link
Copy Markdown
Member

Merged main into this branch to clear the conflict, and moved the migration rung while doing it.

main is now at schema v168 (v166 = diver_settings.place_name_language from #1187, v168 = buddies.is_favorite from #638). A v167 step would sit below the shipped version, so if (from < 167) would never run for any database already at 168 and the short_name column would silently never be added.

So dive_types.short_name moves v167 -> v173, which is the next rung no open branch claims (169 #1320, 170 #1322, 171 #1319, 172 #1328). v167 is now permanently skipped, and the ladder comment records that.

Also in the merge commit:

  • test/core/database/migration_v167_dive_type_short_name_test.dart renamed to migration_v173_... with its ladder assertions updated.
  • main's v166 and v168 ladder entries, onUpgrade steps and beforeOpen backstops kept ahead of this branch's, in ascending order.

Verified locally: flutter analyze clean, and all 479 tests in test/core/database/ pass.

ericgriffin added a commit that referenced this pull request Aug 27, 2026
This branch was 1,957 commits behind. Two decisions worth recording:

Schema rung moved 138 -> 174. main reserved 138 for this branch but then
advanced to v168, and a rung below the shipped version never runs its
onUpgrade step: a database already at 168 would skip 'if (from < 138)' and
only pick up connected_accounts.diver_id from the beforeOpen backstop. 169
through 173 are claimed by PRs #1320, #1322, #1319, #1328 and #1276, so
this takes 174 and 138 becomes permanently skipped. main's reservation
comment is updated to say so, and
migration_v138_connected_accounts_diver_id_test.dart is renamed to
migration_v174_... with its tripwire updated.

The l10n conflicts were resolved at the source, not in the generated
output: all 11 ARB files were merged key-wise (main's entries in main's
order, then this branch's 112 new keys, verified free of duplicates and
valid JSON), then lib/l10n/arb/app_localizations*.dart was regenerated with
flutter gen-l10n rather than hand-merged.

connected_accounts_page keeps main's core/providers/provider.dart barrel;
the direct flutter_riverpod import it supersedes is dropped, since the
resulting unnecessary_import info is fatal in CI.

analyze clean; 2,413 tests pass across core/database, settings, l10n and
import_wizard, plus all 48 divelogs_sync tests.
ericgriffin added a commit that referenced this pull request Aug 27, 2026
Resolves the schema conflict in database.dart and moves this branch's rung
off v169.

main landed v170 (#1322, gas_consumption_display) while this branch held
169. main had reserved 169 here, but a reservation stops being safe the
moment main's scalar passes it: a database already at 170 skips
'if (from < 169)' entirely, so backfillDiveComputerGearTwins would never
run and no existing diver would get gear twins for the computers they
already own. The column alone would still arrive via the beforeOpen
backstop, which is exactly what would have made this hard to notice.

The rung therefore moves 169 -> 175, the next number no open branch claims
(171 #1319, 172 #1328, 173 #1276, 174 #603). 169 is now permanently
skipped, alongside 162 and 167. The v169 doc references across the gear
backfill, gear identity, sync service, buoyancy feature and dive-computer
repository are swept to v175, and
migration_v169_dive_computer_gear_test.dart is renamed to migration_v175_
with its ladder assertions and its stranded-at-the-rung PRAGMA updated.

analyze clean; 1,382 tests pass across core/database, core/buoyancy,
features/equipment and architecture.
ericgriffin added a commit that referenced this pull request Aug 27, 2026
Resolves the schema-ladder conflict in database.dart: main landed v170
(#1322, gas_consumption_display) while this branch held v171, so main's
ladder entry, onUpgrade step and beforeOpen backstop are kept ahead of this
branch's and currentSchemaVersion stays at 171. The v171 comment is
refreshed: 167 and 169 are now permanently skipped rather than claimed,
because main landed past both and PR #1276 moved to 173, PR #1320 to 175.

Also moves the existing no-tick marker on tripDayWeatherBackfillProvider
onto the declaration itself. main's provider_change_tick_test scans only
eleven lines above a declaration, and the marker sat above a thirteen-line
doc comment, so the guard never saw it and reported the provider as a
violation. The reasoning in the marker is unchanged and was already
correct; only its position moved.
@readme42

Copy link
Copy Markdown
Collaborator Author

Dive types also in the list view:
image

@readme42

Copy link
Copy Markdown
Collaborator Author

Dive types also got 2 checkboxes to determine if they should be shown as badges in the list / header
image

@readme42

Copy link
Copy Markdown
Collaborator Author

@ericgriffin As i added another migration for storing settings of dive types, maybe give it another copilot round.

@readme42
readme42 marked this pull request as ready for review August 27, 2026 22:37
Adds a row of type badges (Wreck, Night, Drift, etc.) directly below the
star rating and OC/CCR badge in the dive detail header, matching that
badge's shape but in a lighter, translucent tone so it stays quiet next to
the mode indicator while still reading over the header's live map
background. When more types are set than fit on one line, they collapse
into a "+N" badge with a tooltip listing the rest instead of wrapping.

Built-in dive types get short-form abbreviations (Wreck, Tec, Rec, Wrack,
Tief, Boot, Safari, ...) via new localized strings, translated across all
eleven supported locales. Custom dive types can now set their own short
name too (schema v164: dive_types.short_name), added as an optional field
on the Add Dive Type dialog and previewed on the Manage Dive Types settings
page.

Closes submersion-app#1269.
Adds the dive detail header's type-badge row (Wreck, Night, etc.) to both
the compact and detailed dive list cards, right-aligned on the stat row
next to depth/duration, reusing DiveTypeBadgeRow/DiveTypeBadge so styling
stays consistent across the app. Badges now support a dense variant that
matches DiveModeBadge's list-row size exactly.

Restructures the detailed card's stat/tag row so tags always get their own
line instead of sharing space with the stat row and badges -- a long tag
name (e.g. an auto-generated import-source tag) was previously able to
force a RenderFlex overflow once the badge row added an enclosing Row that
checks horizontal overflow, something nothing there checked before. Also
replaces a wasteful 50/50 Spacer+Flexible split with a single Expanded so
badges get the genuine remaining width instead of only half of it, and
makes the dive detail header's badge cap scale with the header's own width
instead of a flat 200px constant.
Adds show_in_detail_header and show_in_list_view columns to dive_types,
both defaulting to shown so existing dives keep their current badges.
Each type -- built-in or custom -- gets an edit dialog (tap its row on
the Manage Dive Types page) with two checkboxes controlling whether its
badge appears in the dive detail header, the dive list, both, or neither.
Name and short-name editing stay disabled for built-in types, which
remain protected from edits to their core definition; visibility is a
separate per-diver display preference and is editable on every type via
a new repository method that bypasses that protection.

The detail header and both list-view badge rows now filter dive.diveTypeIds
against these flags before building their label lists.
The nine new keys added for the visibility-toggle edit dialog (checkbox
labels/tooltips, dialog title/save button, built-in-name helper, and the
two update snackbars) only landed in English and German, which broke
arb_parity_test's requirement that every locale define every English key.
Translated them into the remaining nine locales.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 50 out of 50 changed files in this pull request and generated 3 comments.

Comment thread lib/features/dive_types/domain/entities/dive_type_entity.dart
Comment thread lib/features/dive_types/presentation/pages/dive_types_page.dart
Comment thread lib/core/database/database.dart
@readme42
readme42 force-pushed the feature/dive-type-badges-1269 branch from 5263ca7 to 027bc9c Compare August 27, 2026 23:34
Adopt the Diver.copyWith _unset/_resolve sentinel pattern so
DiveTypeEntity.copyWith(shortName: null) can actually clear an
existing short name instead of being swallowed by ?? this.value.

Show the localized name in the built-in edit dialog's disabled name
field instead of the seeded English DB value.
@ericgriffin
ericgriffin merged commit 929d713 into submersion-app:main Aug 28, 2026
25 checks passed
@github-project-automation github-project-automation Bot moved this from In review to Done in Submersion Release Tracker Aug 28, 2026
@readme42
readme42 deleted the feature/dive-type-badges-1269 branch August 28, 2026 06:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request user interface

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Automatic dive type tags

3 participants