Skip to content

fix(drivers): auto-dismiss DriverDetailSheet when driver disappears#84

Merged
variablefate merged 2 commits intomainfrom
fix/driver-detail-sheet-blank-62
Apr 29, 2026
Merged

fix(drivers): auto-dismiss DriverDetailSheet when driver disappears#84
variablefate merged 2 commits intomainfrom
fix/driver-detail-sheet-blank-62

Conversation

@variablefate
Copy link
Copy Markdown
Owner

Summary

Fixes #62. After PR #61 migrated DriverDetailSheet from let driver: FollowedDriver (with appState.getDriver(pubkey:) ?? driver fallback) to let pubkey: String consuming an Optional DriverDetailViewState, the sheet lost its stable fallback snapshot. When the backing driver was removed out-of-band — background Kind 30011 sync, logout from another session, account-deletion teardown — state went nil and every state?.foo read in the body short-circuited, leaving every section blank except the hardcoded pubkey. Note edits silently no-op'd against the missing driver.

Design choice

Option 2 from the issue (auto-dismiss) — endorsed by the issue body: "the sheet has no purpose without the backing driver and a silent close matches what the user gets when they delete the driver themselves."

  • Option 1 (snapshot fallback) rejected: keeps a stale view of a driver that no longer exists in any repo, and removeDriver/note edits would still silently no-op against the ghost — worse than the existing pre-feat(views): wire views through presentation types #61 behavior in subtle ways.
  • Option 3 ("Driver removed" placeholder) rejected: extra UI surface for a path that's already rare; "their detail sheet vanished" matches the user's mental model of "the driver no longer exists."

What changed

DriverDetailSheet.swift — added .onChange(of: state == nil, initial: true) that calls dismiss() when the lookup transitions to nil. initial: true also handles the (currently unreachable but cheap to guard) case of opening the sheet for a pubkey that's already gone.

Test approach

UI-layer auto-dismiss behavior is hard to assert directly in a unit test. Instead I added a regression test in AppStatePresentationTests.swiftAppStateDriverDetailViewStateTests.returnsNilAfterDriverRemoved() — that pins the contract the auto-dismiss depends on: removing a driver from FollowedDriversRepository flips appState.driverDetailViewState(pubkey:) from non-nil to nil. If that contract ever regresses, the auto-dismiss silently stops firing.

Verification

xcodebuild test -project RoadFlare.xcodeproj -scheme RoadFlare \
  -destination 'platform=iOS Simulator,name=iPhone 17 Pro' \
  -only-testing:RoadFlareTests/AppStateDriverDetailViewStateTests

TEST SUCCEEDED (5/5 passed, including new returnsNilAfterDriverRemoved)

Full project compiles clean (RoadFlare app + RoadFlareCore + RoadFlareTests + RoadFlareUITests targets).

Test plan

  • Open DriverDetailSheet for a followed driver
  • Trigger out-of-band removal (e.g. simulate background sync dropping the driver, or remove from another session)
  • Sheet auto-dismisses silently — no blank-state flash, no orphaned UI

🤖 Generated with Claude Code

After PR #61 migrated `DriverDetailSheet` to consume an Optional
`DriverDetailViewState` from `AppState`, the sheet lost its captured
`FollowedDriver` fallback snapshot. When a driver was removed
out-of-band — background Kind 30011 sync, logout from another session,
account-deletion teardown — `state` went nil and every `state?.foo`
read in the body short-circuited, leaving every section blank except
the hardcoded pubkey. Note edits silently no-op'd against the missing
driver.

Fix: watch `state == nil` via `.onChange(initial: true)` and call
`dismiss()` when it transitions to true. The sheet has no purpose
without backing state, and a silent close matches the UX of the user's
own "Remove Driver" flow. `initial: true` also covers the (currently
unreachable but cheap to guard) case of opening the sheet for a pubkey
that's already missing from the repo.

Add `AppState.driverDetailViewState` regression test that pins the
contract the auto-dismiss depends on: removing a driver from the repo
flips the lookup from non-nil to nil.

Closes #62

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@variablefate
Copy link
Copy Markdown
Owner Author

Code review — First pass

No actionable findings. Five parallel reviewers (CLAUDE.md compliance, shallow bug scan, git history, prior PR comments, code-comment compliance) surfaced three candidates; all verified as either false positives or intentional. Notes for posterity:

  1. Capture-once pattern (PR feat(views): wire views through presentation types #61): The body caches state once to avoid 6 lock acquisitions × 13 reads of appState.driverDetailViewState(pubkey:). The new .onChange(of: state == nil, initial: true) reads the captured state local — Swift evaluates the of: argument eagerly at body-evaluation time, so this does not add a 14th repo lookup. Cached pattern preserved.

  2. Explicit dismiss() after removeDriver() is not dead code. The "Remove Driver" button's local dismiss() runs synchronously for snappy UX; the new .onChange is the safety net for out-of-band removals (background sync, another-session logout). They're complementary, not redundant.

  3. .onAppear re-fetches state, but .onChange uses the captured local — by design. .onAppear runs after body settles so the captured local can be stale; .onChange(of:) is wired to the cached value and re-fires on body re-evaluation. Correct SwiftUI semantics for each.

🤖 Generated with Claude Code

If this code review was useful, please react with 👍. Otherwise, react with 👎.

@variablefate variablefate marked this pull request as ready for review April 29, 2026 19:18
@variablefate
Copy link
Copy Markdown
Owner Author

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

🤖 Generated with Claude Code

If this code review was useful, please react with 👍. Otherwise, react with 👎.

The only call site presenting `DriverDetailSheet` is `.sheet(item:
$selectedDriver)` in `DriversTab.swift:122`, where `selectedDriver` is
set exclusively from `onTap` of a list row whose driver exists in the
repo at that instant. The `roadflared:` deep-link path opens
`AddDriverSheet`, not this sheet. There is no path that presents
`DriverDetailSheet` for a pubkey that isn't already in the repo.

`initial: true` was guarding that unreachable case, so the modifier
fires once per body evaluation with `isMissing = false` for nothing.
Drop it (and the dead clause in the comment) per the rule against
defending scenarios that can't happen.

The transition trigger (non-nil → nil mid-session) is what actually
fixes #62, and that path is unaffected.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@variablefate
Copy link
Copy Markdown
Owner Author

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

🤖 Generated with Claude Code

If this code review was useful, please react with 👍. Otherwise, react with 👎.

@variablefate variablefate merged commit 72efee1 into main Apr 29, 2026
@variablefate variablefate deleted the fix/driver-detail-sheet-blank-62 branch April 29, 2026 21:51
variablefate added a commit that referenced this pull request Apr 29, 2026
Bumps CURRENT_PROJECT_VERSION from 1 to 2 across all 8 build
configurations in project.pbxproj. Build 1 was the live 1.0 build
uploaded 2026-04-16 20:25 PT; build 2 is the 1.0.1 candidate
shipping the merged work since (#66, #69, #71, #76, #84, #85, #86).

Per RELEASING.md, tag will land after App Store Connect confirms
upload acceptance, not before.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
variablefate added a commit that referenced this pull request May 7, 2026
Bumps MARKETING_VERSION from 1.0.1 to 1.0.2 across all 8 build
configurations (Debug/Release × app/extension targets) in
project.pbxproj. The next App Store release will ship as 1.0.2.

CURRENT_PROJECT_VERSION is intentionally left at 2 — bump that
immediately before the next archive, not now, per RELEASING.md.
Build numbers do not reset across marketing versions; they must
be unique and monotonically increasing for App Store Connect.

Anticipated 1.0.2 scope (already merged on main):
- fix(rider): subscribe to Kind 30173 for live driver vehicles (#93)
- feat(onboarding): publish-failure banner with retry (#95)
- fix(drivers): auto-dismiss DriverDetailSheet when driver disappears (#84)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DriverDetailSheet goes blank when driver is removed by background sync

1 participant