scramble ticket T-1619 (chore) commits 1 files 5 (1 code, 4 docs) lines +144 / -1 production code 1 modifier

Pre-push review: T-1619 task-row-text-alignment

One-commit chore: vertically centre the trip-timeline task name with its checkbox, mirroring the packing row's nameColumn treatment from T-1587.

At a glance

  • Single-line task names now centre with their checkbox in the trip timeline; taller names still wrap top-aligned.
  • Mirrors the proven PackingItemRow.nameColumn pattern from T-1587 — same modifier, same geometry.
  • No automated test by design (recorded in decision_log.md); a manual visual check in-app is deferred to the author.
  • make build / make test-quick / make lint / make format all pass.

Verdict

Ready to push

The only production change is adding minHeight: 44 to the task name Text in TaskRow.swift — the identical, already-shipped modifier from PackingItemRow.nameColumn (T-1587). Build, unit tests, lint, and format are all green. No correctness, quality, or efficiency issues. One non-blocking reuse observation (duplicated 44 literal) deliberately left for later as it would touch out-of-scope code.

Review findings

1 raised · 0 fixed · 1 skipped

Jump to findings →

Commits

Three-level explanation

What changed

Each task row has a round checkbox and the task name beside it. A short name used to sit slightly above the checkbox's centre. We gave the name box the same minimum height as the checkbox (44 points) so a one-line name lines up with it.

Why it matters

A small visual tidy-up so the task list matches the packing list, which already had this fix.

Key concept

SwiftUI lays a row out from the top. The checkbox is centred inside a 44pt box; giving the text box the same 44pt minimum height makes both boxes the same height, so their centres meet.

Architecture

The task name Text sits in an HStack(alignment: .top) next to the checkbox, whose 24pt circle is centred in a 44×44 tap target. The name previously had only .frame(maxWidth: .infinity, alignment: .leading), so a single-line name hugged the top while the checkbox centre sat lower.

Pattern

Adding minHeight: 44 equalises the name box height to the checkbox box; two equal-height, top-aligned siblings share a vertical centre. Taller content grows past 44pt and the minHeight is a no-op (top-aligned), matching PackingItemRow.

Trade-offs

This is the same fix PackingItemRow.nameColumn got in T-1587; the task row just hadn't had it applied.

Deep dive

Alignment is pure box geometry under HStack(alignment: .top): equalising the min heights of two top-aligned siblings co-locates their centres. No alignment guide, custom Layout, or alignmentGuide needed — minHeight is the minimal lever.

Architecture impact

The minHeight is applied to the inner Text(task.name), not the enclosing VStack that also holds the conditional WhyDisclosureView. Sizing the Text's own box leaves the disclosure (a separate child beneath) unaffected, so opening it does not shift the name. Placing minHeight on the VStack would have stretched the whole column.

Edge cases

  • Multi-line / large Dynamic Type: intrinsic height dominates; reverts to top-alignment, consistent with PackingItemRow.
  • Tap target unchanged — the row already has its own .frame(minHeight: 44).
  • Strikethrough / opacity for completed or ghosted rows untouched.

Important changes — detailed

TaskRow: minHeight on the task name centres it with the checkbox

TaskRow.swift

Why it matters. User-visible alignment fix; the only production change in the branch.

What to look at. TaskRow.swift Text(task.name) frame — now .frame(maxWidth: .infinity, minHeight: 44, alignment: .leading)

Takeaway. Under HStack(alignment: .top), give two siblings equal min heights to co-locate their vertical centres — cheaper and more robust than custom alignment guides.
Rationale. Mirrors the proven PackingItemRow.nameColumn fix from T-1587; applying minHeight to the inner Text (not the VStack) keeps the WhyDisclosure position stable.

Key decisions

No automated test for the alignment.

Pure layout-constant change with no branching logic; the project tests view-driving logic, not view bodies, and has no snapshot infrastructure. Verified by build + lint + format + manual visual check. Consistent with T-1587, which shipped the same packing-row treatment untested. Recorded as Decision 1 in decision_log.md.

minHeight on the inner Text, not the enclosing VStack.

Sizing the name's own box centres it with the checkbox while leaving the sibling WhyDisclosureView position unchanged; stretching the VStack would have altered disclosure spacing.

Reference is the packing row, despite the ticket saying "task list".

The ticket description says "like what we already fixed for the task list," but the code shows the packing list received the fix (T-1587) and the task row is the one still lacking it — consistent with the ticket title "Make the tasks text inline with the checkbox." Validated against source before implementing.

Review findings

SeverityAreaFindingResolution
minorTaskRow.swift / PackingItemRow.swiftThe 44pt min-height literal is now duplicated across TaskRow and PackingItemRow (and each checkbox's own 44pt frame). A shared ViewModifier or layout constant could centralise it.Left as-is: only two call sites (below the rule-of-three), and a fix would edit PackingItemRow, which is out of scope for this smolspec. Consistent with the file already hardcoding 44 in multiple places.

Per-file diffs

Click to expand.

Scramble/Scramble/Components/TaskRow.swift Modified +3 / -1
diff --git a/Scramble/Scramble/Components/TaskRow.swift b/Scramble/Scramble/Components/TaskRow.swiftindex 7da706a..f87f9c1 100644--- a/Scramble/Scramble/Components/TaskRow.swift+++ b/Scramble/Scramble/Components/TaskRow.swift@@ -40,7 +40,9 @@ struct TaskRow: View {           .font(.body)           .strikethrough(task.isCompleted)           .foregroundStyle(variant.textPrimary)-          .frame(maxWidth: .infinity, alignment: .leading)+          // minHeight matches the checkbox's 44pt box so a single-line name+          // sits vertically centred with it (see PackingItemRow.nameColumn).+          .frame(maxWidth: .infinity, minHeight: 44, alignment: .leading)           .contentShape(Rectangle())           .onLongPressGesture(minimumDuration: 0.4) {             #if canImport(UIKit)
CHANGELOG.md Modified +2 / -0
diff --git a/CHANGELOG.md b/CHANGELOG.mdindex ad13aed..5393f55 100644--- a/CHANGELOG.md+++ b/CHANGELOG.md@@ -80,6 +80,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).  ### Changed +- `task-row-text-alignment` (T-1619) — the trip-timeline task name is given a 44pt min-height so a single-line name sits vertically centred with its checkbox, matching the packing row's `nameColumn` (the same treatment added for packing in T-1587). One modifier on `Text(task.name)` in `TaskRow.swift` (`minHeight: 44`); the row-level 44pt tap-target frame, the inline `WhyDisclosure`, and the assignee avatar are unchanged. Multi-line names and large Dynamic Type still wrap top-aligned (a no-op fallback, consistent with the packing row).+ - Pack-mode packing list moved from the **Departure day** phase to the **Day before** phase, so packing is prepared the day before departure (symmetric with repack on **Day before return**). The "is this a packing phase?" predicate and the phase→mode mapping are now centralised in a new `Phase.packingMode` computed property (single source of truth for `AccordionTimeline` and `TripDetailView.autoExpandPhase`); the summary block, packing-sheet entry, always-expandable gate, and the `PhaseNode` suitcase glyph all follow it onto `.dayBefore`. Departure day reverts to a plain task-only phase; on a 1-day trip the pack list lives on the prior day. The `phase4-pack-mode-trip` UI seed and the `AutoExpand` / `PhaseRow` / `PackingSheet` tests were updated, and `PhasePackingModeTests` pins the new mapping. Recorded as `specs/phase-4-packing-sheet/decision_log.md` Decision 11 (supersedes the Departure-day placement in the Phase 3/4 spec ACs); `CLAUDE.md`, `docs/scramble-ui-design-doc.md`, `docs/implementation-phases.md`, and `docs/agent-notes/packing-sheet.md` updated to match.  - `packing-item-subitems` (T-1587) — packing-sheet note / sub-item interaction reworked from the initial reveal-on-tap affordance row into compact trailing glyphs, after device review:
specs/task-row-text-alignment/smolspec.md Added +65
diff --git a/specs/task-row-text-alignment/smolspec.md b/specs/task-row-text-alignment/smolspec.mdnew file mode 100644index 0000000..37c43c6--- /dev/null+++ b/specs/task-row-text-alignment/smolspec.md@@ -0,0 +1,65 @@+# Task Row Text Alignment++## Overview++In the trip timeline, a single-line task name renders slightly higher than its+checkbox circle because the row's name `Text` has no minimum height while the+checkbox sits centred in its own 44pt box. This change vertically centres the+task name with the checkbox, matching the packing row, which already received+this treatment in T-1587. Reference ticket: T-1619.++## Requirements++- When the task name fits within the checkbox's height, the system MUST render+  it vertically centred with the checkbox in `TaskRow`; when the name is taller+  (multi-line, or large Dynamic Type), it MUST fall back to top-alignment.+- The vertical alignment of name-to-checkbox in `TaskRow` SHOULD match the+  alignment behaviour of `PackingItemRow` at every Dynamic Type size so the two+  list styles look consistent.+- The change MUST NOT alter the position of the `WhyDisclosureView` beneath the+  name, the assignee avatar, the 44pt tap targets, or the row's swipe /+  context-menu actions.++## Implementation Approach++- Key file: `Scramble/Scramble/Components/TaskRow.swift` (line numbers below are+  indicative — locate by symbol, as they drift with edits).+- The task name is rendered by `Text(task.name)` (around line 39) inside an+  `HStack(alignment: .top, spacing: 12)`. It currently has+  `.frame(maxWidth: .infinity, alignment: .leading)`.+- Mirror the established pattern in `Scramble/Scramble/Components/PackingItemRow.swift`+  (`nameColumn`, around line 219), which adds `minHeight: 44` to the name frame:+  `.frame(maxWidth: .infinity, minHeight: 44, alignment: .leading)`. Because the+  name box then matches the checkbox's 44pt box height and both top-align inside+  the HStack, their vertical centres coincide while the name fits within 44pt.+  Taller names grow past 44pt and wrap as before.+- The `minHeight` is applied to the `Text(task.name)` box itself, which sits+  above the `WhyDisclosureView` in the same `VStack`. Growing the name's own box+  to a 44pt minimum changes only that box's height; the disclosure is a separate+  child laid out beneath it, so its top edge is unaffected.+- The existing row-level `.frame(minHeight: 44)` (around line 65) sizes the whole+  row for the tap target and is unrelated; leave it as-is.+- Dependencies: none beyond the existing SwiftUI layout already in the file. No+  data-model, rules-engine, or sync change.+- Out of Scope: any change to `PackingItemRow`; the `WhyDisclosureView` content+  or its inline placement; the assignee avatar; the row-level tap-target frame.++## Risks and Assumptions++- Assumption: the intended reference is the packing row's `minHeight: 44`+  treatment from T-1587. The ticket description says "like what we already fixed+  for the task list," but in the code it was the packing list that received the+  fix; the task row is the one still lacking it, consistent with the ticket+  title "Make the tasks text inline with the checkbox." Validated against the+  current source before writing this spec.+- Risk: with very large Dynamic Type sizes a single-line name could exceed 44pt,+  in which case `minHeight` is a no-op and alignment falls back to the current+  top-aligned behaviour. | Mitigation: this matches `PackingItemRow` exactly, so+  the two rows stay consistent at every type size; no special handling added.+- Risk: this is a visual-only change with no unit-testable logic, so a future+  refactor could silently drop `minHeight: 44` with no failing test. |+  Mitigation: accepted trade-off (see `decision_log.md`); verification is by+  build + lint passing and a visual check in the simulator that the task name+  centres with the checkbox and matches the packing row.+- Prerequisite: `PackingItemRow.nameColumn` (the reference treatment) exists and+  is unchanged — confirmed at `PackingItemRow.swift:219`.
specs/task-row-text-alignment/decision_log.md Added +49
diff --git a/specs/task-row-text-alignment/decision_log.md b/specs/task-row-text-alignment/decision_log.mdnew file mode 100644index 0000000..5f1178c--- /dev/null+++ b/specs/task-row-text-alignment/decision_log.md@@ -0,0 +1,49 @@+# Decision Log: Task Row Text Alignment++## Decision 1: No automated test for the alignment change++**Date**: 2026-06-29+**Status**: accepted++### Context++T-1619 vertically centres the task name with its checkbox in `TaskRow` by+adding `minHeight: 44` to the name `Text`'s frame. This is a pure layout-constant+change with no branching logic. The project tests view-driving logic, not view+bodies (see the Swift testing rules: "Test the logic that drives the view, not+the view body"), and has no snapshot/pixel-comparison test infrastructure.++### Decision++Ship the change without a dedicated automated test. Verify by building, passing+swiftlint / swift-format, and a manual visual check in the simulator that the+task name centres with the checkbox and matches `PackingItemRow`.++### Rationale++There is no unit-testable behaviour: the change is a single layout modifier.+A UI test cannot meaningfully assert sub-pixel vertical centring, and adding+snapshot-test tooling for a one-line cosmetic change is disproportionate. The+identical treatment already shipped untested for `PackingItemRow` in T-1587,+so this is consistent with the established precedent.++### Alternatives Considered++- **Add a snapshot test**: Would catch a future regression that drops the+  `minHeight` - Rejected because the project has no snapshot infrastructure and+  introducing it for one cosmetic line is out of proportion to the change.+- **Assert layout frame in a UI test**: Inspect the rendered row geometry -+  Rejected because XCUITest exposes element frames unreliably for this purpose+  and the assertion would be brittle against theme/Dynamic-Type variation.++### Consequences++**Positive:**+- Keeps the change minimal and consistent with the T-1587 packing-row precedent.+- No new test tooling or maintenance burden.++**Negative:**+- A future refactor could remove `minHeight: 44` without any failing test;+  regression would only surface on visual inspection.++---
specs/task-row-text-alignment/tasks.md Added +25
diff --git a/specs/task-row-text-alignment/tasks.md b/specs/task-row-text-alignment/tasks.mdnew file mode 100644index 0000000..1842743--- /dev/null+++ b/specs/task-row-text-alignment/tasks.md@@ -0,0 +1,25 @@+---+references:+    - smolspec.md+    - decision_log.md+---+# Task Row Text Alignment++- [x] 1. Task name centres with its checkbox in TaskRow <!-- id:iczxcb3 -->+  - In Scramble/Scramble/Components/TaskRow.swift, the task name Text gains a 44pt minimum height so a single-line name sits vertically centred with the checkbox (mirroring PackingItemRow.nameColumn).+  - Verify: the Text(task.name) frame includes minHeight: 44 and a single-line name renders level with the checkbox circle.+  - See smolspec.md Implementation Approach.+  - References: smolspec.md++- [x] 2. Project builds and lint/format pass with the change <!-- id:iczxcb4 -->+  - make build succeeds and swiftlint + swift-format report no new issues on the change.+  - Verify: make build is green; make lint and make format are clean (or no-op if tools absent).+  - Blocked-by: iczxcb3 (Task name centres with its checkbox in TaskRow)+  - References: smolspec.md++- [-] 3. Alignment visually confirmed in simulator <!-- id:iczxcb5 -->+  - Run in the simulator and confirm a single-line task name is vertically centred with its checkbox and matches the packing row.+  - Confirm a multi-line task name still wraps top-aligned, the WhyDisclosure (long-press) still sits beneath the name, and the assignee avatar and tap targets are unchanged.+  - See smolspec.md Requirements.+  - Blocked-by: iczxcb4 (Project builds and lint/format pass with the change)+  - References: smolspec.md

Things to double-check

Visual confirmation in-app.

Alignment is reasoned from box geometry and matches the shipped packing row, but no screenshot was captured (no booted simulator / no sample data, and driving the UI to a populated timeline is disproportionate here). The author will eyeball it — tasks.md task 3 remains open.