Unified Cylinders card on dive details#492
Merged
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Contributor
There was a problem hiding this comment.
Pull request overview
Unifies dive-detail cylinder information by replacing the separate Tanks card and the SAC by Cylinder subsection with a single always-expanded Cylinders card, while preserving the persisted section ID (DiveDetailSectionId.tanks) to avoid migrations.
Changes:
- Added a new
CylindersCardwidget that joins tanks with per-tank SAC results and optional multi-source attribution. - Rewired the dive detail page to render
CylindersCardin the existing tanks slot and removed the legacy tanks/SAC-by-cylinder builders and related expanded-state provider. - Updated l10n keys across all locales (added
..._cylinders, removed obsolete tanks/SAC-by-cylinder keys) and adjusted/added widget tests accordingly.
Reviewed changes
Copilot reviewed 34 out of 34 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test/features/settings/presentation/pages/dive_detail_sections_page_test.dart | Updates section-title expectation from “Tanks” to “Cylinders”. |
| test/features/dive_log/presentation/widgets/dive_profile_legend_test.dart | Updates legend UI test expectation to “Cylinders”. |
| test/features/dive_log/presentation/widgets/cylinders_card_test.dart | Adds focused widget tests for the new Cylinders card behavior/formatting. |
| test/core/constants/dive_detail_sections_test.dart | Updates section metadata expectations (display name + SAC segments description + tanks description). |
| lib/l10n/arb/app_ar.arb | Replaces removed tanks/SAC-by-cylinder keys with ..._cylinders and trims SAC-segments description. |
| lib/l10n/arb/app_de.arb | Same l10n key updates for German. |
| lib/l10n/arb/app_en.arb | Same l10n key updates for English (including metadata cleanup). |
| lib/l10n/arb/app_es.arb | Same l10n key updates for Spanish. |
| lib/l10n/arb/app_fr.arb | Same l10n key updates for French. |
| lib/l10n/arb/app_he.arb | Same l10n key updates for Hebrew. |
| lib/l10n/arb/app_hu.arb | Same l10n key updates for Hungarian. |
| lib/l10n/arb/app_it.arb | Same l10n key updates for Italian. |
| lib/l10n/arb/app_nl.arb | Same l10n key updates for Dutch. |
| lib/l10n/arb/app_pt.arb | Same l10n key updates for Portuguese. |
| lib/l10n/arb/app_zh.arb | Same l10n key updates for Chinese. |
| lib/l10n/arb/app_localizations.dart | Regenerated localization interface: removes obsolete getters and adds diveLog_detail_section_cylinders. |
| lib/l10n/arb/app_localizations_ar.dart | Regenerated locale implementation reflecting removed/added keys. |
| lib/l10n/arb/app_localizations_de.dart | Regenerated locale implementation reflecting removed/added keys. |
| lib/l10n/arb/app_localizations_en.dart | Regenerated locale implementation reflecting removed/added keys. |
| lib/l10n/arb/app_localizations_es.dart | Regenerated locale implementation reflecting removed/added keys. |
| lib/l10n/arb/app_localizations_fr.dart | Regenerated locale implementation reflecting removed/added keys. |
| lib/l10n/arb/app_localizations_he.dart | Regenerated locale implementation reflecting removed/added keys. |
| lib/l10n/arb/app_localizations_hu.dart | Regenerated locale implementation reflecting removed/added keys. |
| lib/l10n/arb/app_localizations_it.dart | Regenerated locale implementation reflecting removed/added keys. |
| lib/l10n/arb/app_localizations_nl.dart | Regenerated locale implementation reflecting removed/added keys. |
| lib/l10n/arb/app_localizations_pt.dart | Regenerated locale implementation reflecting removed/added keys. |
| lib/l10n/arb/app_localizations_zh.dart | Regenerated locale implementation reflecting removed/added keys. |
| lib/features/dive_log/presentation/widgets/dive_profile_legend.dart | Switches the legend section title to the new cylinders l10n key. |
| lib/features/dive_log/presentation/widgets/cylinders_card.dart | Introduces the unified Cylinders card UI (pressures, MOD/MND, per-tank SAC, attribution). |
| lib/features/dive_log/presentation/widgets/cylinder_sac_card.dart | Removes dead/unused legacy cylinder SAC card widget code. |
| lib/features/dive_log/presentation/providers/gas_analysis_providers.dart | Removes obsolete cylinderSacExpandedProvider. |
| lib/features/dive_log/presentation/pages/dive_detail_page.dart | Replaces tanks section rendering with CylindersCard and deletes legacy section builders/subsections. |
| lib/core/constants/dive_detail_sections.dart | Updates English fallback display name/description strings for tanks and SAC segments. |
| docs/superpowers/plans/2026-07-05-cylinders-card.md | Adds implementation plan documentation for the unified Cylinders card work. |
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
Merges the dive detail page's two overlapping cards — Tanks (always shown) and SAC by Cylinder (multi-tank only, buried in the SAC-segments area) — into one always-expanded Cylinders card. Both cards described the same physical cylinders keyed by the same tank IDs; the section metadata already promised "per-tank SAC" on the Tanks section but the implementation never delivered it.
Each cylinder row now shows: tank name + gas mix, volume/preset chip, start/end pressures with amount used, MOD/MND, per-tank SAC with gas consumed, and source-attribution badge on multi-source dives. Per-tank SAC now appears whenever computable, including single-tank dives (previously multi-tank only).
Design spec:
docs/superpowers/specs/2026-07-05-cylinders-card-design.mdImplementation plan:
docs/superpowers/plans/2026-07-05-cylinders-card.mdChanges
CylindersCardwidget (lib/features/dive_log/presentation/widgets/cylinders_card.dart) joiningdive.tankswithcylinderSacProviderresults by tank ID; settings/sacUnit injected via constructor so tests need noSettingsNotifiermockDiveDetailSectionId.tanksslot — enum value unchanged, so saved section order/visibility configs need no migration; section label renamed to "Cylinders" (English only; all 10 other locales already used their cylinder/bottle word)dive_detail_page.dart:_buildTanksSection,_buildCylinderSacSection,_formatCylinderSac,_resolveTankPressures, and the cylinder-SAC sub-block + fallback in the SAC-segments section (net -307 lines)cylinder_sac_card.dart(never referenced) andcylinderSacExpandedProviderdiveLog_detail_section_sacByCylinder,diveLog_detail_section_tanks,diveLog_detail_tankCount) from all 11 locales; profile legend now uses the newdiveLog_detail_section_cylinderskeyTest Plan
flutter testpasses (134 tests across affected suites)flutter analyzepasses