From d32daaab826fcbc6813110bbdbcef876c040446d Mon Sep 17 00:00:00 2001 From: jack Date: Sun, 9 Aug 2026 11:44:49 +0200 Subject: [PATCH] Review fixes: dismiss sheets on wipe; host the dialog durably - Codex P1: the App Info danger-zone wipe left its sheet presented, so the failed-wipe banner (and the success message) rendered underneath an opaque modal. panicClearAllData now dismisses all chrome-owned presentation first; pinned by a new architecture test. - Review nit: the confirmation dialog moved from the logo Text to the ContentView header stack, so a covered/removed header can't take a pending dialog down with it. Co-Authored-By: Claude Fable 5 --- bitchat/App/AppChromeModel.swift | 10 ++++++++++ bitchat/Views/ContentHeaderView.swift | 20 ++++---------------- bitchat/Views/ContentView.swift | 18 ++++++++++++++++++ bitchatTests/AppArchitectureTests.swift | 24 ++++++++++++++++++++++++ 4 files changed, 56 insertions(+), 16 deletions(-) diff --git a/bitchat/App/AppChromeModel.swift b/bitchat/App/AppChromeModel.swift index 67c34594..8d369c5b 100644 --- a/bitchat/App/AppChromeModel.swift +++ b/bitchat/App/AppChromeModel.swift @@ -125,6 +125,16 @@ final class AppChromeModel: ObservableObject { } func panicClearAllData() { + // A wipe invalidates everything on screen, and its outcome must be + // visible: the success message and the failed-wipe banner both live + // on the root timeline, so a sheet left up (the App Info danger-zone + // path keeps its sheet presented) would hide the one signal that says + // whether the wipe worked. + isAppInfoPresented = false + isLocationChannelsSheetPresented = false + isNoticesSheetPresented = false + showingFingerprintFor = nil + prepareForPanic?() onPanicWipe() chatViewModel.panicClearAllData() diff --git a/bitchat/Views/ContentHeaderView.swift b/bitchat/Views/ContentHeaderView.swift index a4bf074a..f857ba38 100644 --- a/bitchat/Views/ContentHeaderView.swift +++ b/bitchat/Views/ContentHeaderView.swift @@ -61,22 +61,10 @@ struct ContentHeaderView: View { .onTapGesture(count: 1) { appChromeModel.presentAppInfo() } - .confirmationDialog( - Text( - String(localized: "app_info.settings.danger.panic_confirm_title", defaultValue: "wipe all data?", comment: "Title of the confirmation dialog before a panic wipe") - ), - isPresented: $appChromeModel.showPanicConfirmation, - titleVisibility: .visible - ) { - Button(role: .destructive) { - appChromeModel.panicClearAllData() - } label: { - Text( - String(localized: "app_info.settings.danger.panic_confirm_action", defaultValue: "wipe everything", comment: "Destructive confirmation button that performs the panic wipe") - ) - } - Button("common.cancel", role: .cancel) {} - } + // The confirmation dialog itself is hosted on ContentView + // (next to the failed-wipe banner), not on this Text: a host + // that can be covered or removed could take the pending + // dialog down with it. // This is the only entry point to App Info, but it reads as // static text; surface the tap. (The triple-tap panic wipe // stays undiscoverable on purpose — it's destructive.) diff --git a/bitchat/Views/ContentView.swift b/bitchat/Views/ContentView.swift index a04ea03e..6ef956ed 100644 --- a/bitchat/Views/ContentView.swift +++ b/bitchat/Views/ContentView.swift @@ -503,6 +503,24 @@ struct ContentView: View { PanicWipeBlockedBanner() } } + // Hosted here rather than on the logo Text so the pending dialog + // survives whatever happens to the header chrome. + .confirmationDialog( + Text( + String(localized: "app_info.settings.danger.panic_confirm_title", defaultValue: "wipe all data?", comment: "Title of the confirmation dialog before a panic wipe") + ), + isPresented: $appChromeModel.showPanicConfirmation, + titleVisibility: .visible + ) { + Button(role: .destructive) { + appChromeModel.panicClearAllData() + } label: { + Text( + String(localized: "app_info.settings.danger.panic_confirm_action", defaultValue: "wipe everything", comment: "Destructive confirmation button that performs the panic wipe") + ) + } + Button("common.cancel", role: .cancel) {} + } } private var publicMessageList: some View { diff --git a/bitchatTests/AppArchitectureTests.swift b/bitchatTests/AppArchitectureTests.swift index 941ef461..6b202d5a 100644 --- a/bitchatTests/AppArchitectureTests.swift +++ b/bitchatTests/AppArchitectureTests.swift @@ -482,6 +482,30 @@ struct AppArchitectureTests { #expect(viewModel.messages.map(\.id) == ["keep-1"]) } + @Test("Panic wipe dismisses chrome sheets so its outcome is visible") + @MainActor + func panicWipeDismissesChromePresentation() { + let viewModel = makeArchitectureViewModel() + let privateInboxModel = PrivateInboxModel(conversations: ConversationStore()) + let chromeModel = AppChromeModel(chatViewModel: viewModel, privateInboxModel: privateInboxModel) + + // The App Info danger-zone path wipes while its own sheet is up; the + // success message and the failed-wipe banner both render on the root + // timeline, so any sheet left presented would hide the one signal + // that says whether the wipe worked. + chromeModel.presentAppInfo() + chromeModel.isLocationChannelsSheetPresented = true + chromeModel.presentNotices() + chromeModel.showFingerprint(for: PeerID(str: "peer-3")) + + chromeModel.panicClearAllData() + + #expect(!chromeModel.isAppInfoPresented) + #expect(!chromeModel.isLocationChannelsSheetPresented) + #expect(!chromeModel.isNoticesSheetPresented) + #expect(chromeModel.showingFingerprintFor == nil) + } + @Test("PrivateConversationModel resolves canonical header state for the selected DM") @MainActor func privateConversationModelResolvesSelectedHeaderState() async {