diff --git a/.agents/skills/building-ui/SKILL.md b/.agents/skills/building-ui/SKILL.md index 7781abc63..54fd6f130 100644 --- a/.agents/skills/building-ui/SKILL.md +++ b/.agents/skills/building-ui/SKILL.md @@ -113,6 +113,13 @@ module-owned stylesheets. than remain at zero opacity. - Keep the visual structure consistent across states and variants unless the difference is intentional and modeled by the component style. +- Put a large declarative subtree behind a nominal child `View` before passing + it through a custom generic container that stores or repeatedly transforms + its `Content`. In particular, do not pass a multi-section `Form` or `List` + directly into such a wrapper: SwiftUI may copy the full concrete value on the + stack while applying environment or navigation updates. Treat a DEBUG-only + content-footprint guard as a heuristic tripwire for extraction, not a + shipping layout contract or a threshold to retune around. ## Build for accessibility and localization diff --git a/Where/WhereUI/AGENTS.md b/Where/WhereUI/AGENTS.md index 1148b3879..a1afa4f2a 100644 --- a/Where/WhereUI/AGENTS.md +++ b/Where/WhereUI/AGENTS.md @@ -83,6 +83,15 @@ Layering, localization, preview, and testing conventions live in the feature ([`Sources/Shared/MotionIsStatic.swift`](Sources/Shared/MotionIsStatic.swift)) for its static end-state — never hand-roll the `\.accessibilityReduceMotion` + `\.isCapturingSnapshot` pair. +- Pass large `Form`/`List` subtrees through a nominal child before + `SettingsFocusScope`; `SettingsRouteViewTests` exercises every push + destination and its nested heterogeneous routes against the DEBUG-only + value-size tripwire. +- Keep the Settings content-footprint tripwire DEBUG-only; Release has no + footprint precondition. +- Diagnostic: `EXC_BAD_ACCESS` / `___chkstk_darwin` followed by + `View.environment` during a navigation push means inspect concrete content + stored by generic wrappers. - A step joins `WhereLaunch`'s plan through `.measured()` and so must declare a `budget` (`BudgetedLaunchStep`) — see [Spans](../AGENTS.md#spans). WhereUI also owns log retention: `LogHistoryPruner` bounds the store by age *and* event diff --git a/Where/WhereUI/Sources/Settings/FeaturePreviews/Evidence/ShareEvidenceFeaturesView.swift b/Where/WhereUI/Sources/Settings/FeaturePreviews/Evidence/ShareEvidenceFeaturesView.swift index dc69959c8..9c51bc917 100644 --- a/Where/WhereUI/Sources/Settings/FeaturePreviews/Evidence/ShareEvidenceFeaturesView.swift +++ b/Where/WhereUI/Sources/Settings/FeaturePreviews/Evidence/ShareEvidenceFeaturesView.swift @@ -116,7 +116,7 @@ struct ShareEvidenceFeaturesView: View { } } - private enum Route: Hashable { + enum Route: Hashable { case archive } } diff --git a/Where/WhereUI/Sources/Settings/SettingsRouteView.swift b/Where/WhereUI/Sources/Settings/SettingsRouteView.swift new file mode 100644 index 000000000..f4ee599c1 --- /dev/null +++ b/Where/WhereUI/Sources/Settings/SettingsRouteView.swift @@ -0,0 +1,72 @@ +import SwiftUI + +/// Builds one pushed Settings destination from the root's typed route and +/// view-scoped collaborators. Keeping route rendering in a nominal child lets +/// tests exercise the same destination tree without a test-only Settings API. +struct SettingsRouteView: View { + let route: SettingsRoute + let report: YearReportModel + let backup: BackupModel + let reminders: RemindersSettingsModel + + @Environment(WhereSession.self) private var session + + var body: some View { + switch route.destination { + case .attachments: + EvidenceListView(report: report) + case .loggedDays: + LoggedDaysView(report: report) + case .devices: + DevicesSettingsView(session: session, focus: route.focus) + case .regions: + // Regions is presented as a sheet (`isSheet`), so it's never + // routed here; this arm only keeps the switch exhaustive. + EmptyView() + case .alerts: + AlertsSettingsView(report: report, reminders: reminders, focus: route.focus) + case .appearance: + AppearanceSettingsView(report: report, focus: route.focus) + case .year: + VisibleYearSettingsView(report: report, focus: route.focus) + case .siri: + SiriFeaturesView( + focus: route.focus, + presentation: featureDiscoveryPresentation, + ) + case .widgets: + WidgetFeaturesView( + focus: route.focus, + presentation: featureDiscoveryPresentation, + ) + case .shareEvidence: + ShareEvidenceFeaturesView( + report: report, + focus: route.focus, + presentation: featureDiscoveryPresentation, + ) + case .estimatedTime: + EstimatedTimeFeaturesView(report: report, focus: route.focus) + case .insightsAccuracy: + InsightsAccuracyFeaturesView( + report: report, + focus: route.focus, + ) + case .personalization: + PersonalizationFeaturesView(report: report, focus: route.focus) + case .data: + DataSettingsView(report: report, backup: backup, focus: route.focus) + case .about: + AboutSettingsView(focus: route.focus) + } + } + + private var featureDiscoveryPresentation: FeatureDiscoveryPresentation { + FeatureDiscoveryPresentation( + report: report.report, + selectedYear: report.selectedYear, + referenceDate: report.referenceDate, + calendar: report.calendar, + ) + } +} diff --git a/Where/WhereUI/Sources/Settings/SettingsRow.swift b/Where/WhereUI/Sources/Settings/SettingsRow.swift index d5d2fa2f8..3677f2614 100644 --- a/Where/WhereUI/Sources/Settings/SettingsRow.swift +++ b/Where/WhereUI/Sources/Settings/SettingsRow.swift @@ -97,6 +97,16 @@ struct SettingsRowModifier: ViewModifier { /// (no fade — still scrolls and shows a brief static highlight). Flashes once per /// appearance so returning to the screen doesn't re-flash. struct SettingsFocusScope: View { + #if DEBUG + /// A DEBUG-only heuristic separating the largest known-safe Settings form + /// (46,912 bytes) from the 62,680-byte concrete value that overflowed a + /// device stack while SwiftUI applied this scope's environment during a + /// navigation push. Large trees belong behind a small nominal child view. + static var maximumContentFootprint: Int { + 56 * 1024 + } + #endif + let focus: SettingsFocus? let isReady: Bool let content: Content @@ -113,6 +123,14 @@ struct SettingsFocusScope: View { ) { self.focus = focus self.isReady = isReady + #if DEBUG + let contentFootprint = MemoryLayout.size + precondition( + contentFootprint <= Self.maximumContentFootprint, + "SettingsFocusScope Content is \(contentFootprint) bytes; extract its large " + + "Form/List subtree behind a nominal child View.", + ) + #endif self.content = content() } diff --git a/Where/WhereUI/Sources/Settings/SettingsSearch.swift b/Where/WhereUI/Sources/Settings/SettingsSearch.swift index 9473e80d1..6a416d0ad 100644 --- a/Where/WhereUI/Sources/Settings/SettingsSearch.swift +++ b/Where/WhereUI/Sources/Settings/SettingsSearch.swift @@ -3,8 +3,8 @@ import SwiftUI /// The top-level Settings groups. Each drills into its own sub-screen; the /// top-level list and `SettingsRoute` route on these, and the -/// `navigationDestination` switch (in `SettingsView`) builds a screen for every -/// case with no `default:`, so adding a case is a compile error until wired. +/// `SettingsRouteView` switch builds a screen for every case with no `default:`, +/// so adding a case is a compile error until wired. enum SettingsDestination: Hashable, CaseIterable { case attachments case loggedDays diff --git a/Where/WhereUI/Sources/Settings/SettingsView.swift b/Where/WhereUI/Sources/Settings/SettingsView.swift index ed2cd5c7a..cc5daeaf0 100644 --- a/Where/WhereUI/Sources/Settings/SettingsView.swift +++ b/Where/WhereUI/Sources/Settings/SettingsView.swift @@ -104,7 +104,12 @@ struct SettingsView: View { } } .navigationDestination(for: SettingsRoute.self) { route in - destination(for: route) + SettingsRouteView( + route: route, + report: report, + backup: backup, + reminders: reminders, + ) } .sheet(isPresented: $showRegions) { RegionsSettingsView(usedThisYear: regionsUsedThisYear) @@ -254,57 +259,6 @@ struct SettingsView: View { } } - @ViewBuilder - private func destination(for route: SettingsRoute) -> some View { - switch route.destination { - case .attachments: - EvidenceListView(report: report) - case .loggedDays: - LoggedDaysView(report: report) - case .devices: - DevicesSettingsView(session: session, focus: route.focus) - case .regions: - // Regions is presented as a sheet (`isSheet`), so it's never - // routed here; this arm only keeps the switch exhaustive. - EmptyView() - case .alerts: - AlertsSettingsView(report: report, reminders: reminders, focus: route.focus) - case .appearance: - AppearanceSettingsView(report: report, focus: route.focus) - case .year: - VisibleYearSettingsView(report: report, focus: route.focus) - case .siri: - SiriFeaturesView( - focus: route.focus, - presentation: featureDiscoveryPresentation, - ) - case .widgets: - WidgetFeaturesView( - focus: route.focus, - presentation: featureDiscoveryPresentation, - ) - case .shareEvidence: - ShareEvidenceFeaturesView( - report: report, - focus: route.focus, - presentation: featureDiscoveryPresentation, - ) - case .estimatedTime: - EstimatedTimeFeaturesView(report: report, focus: route.focus) - case .insightsAccuracy: - InsightsAccuracyFeaturesView( - report: report, - focus: route.focus, - ) - case .personalization: - PersonalizationFeaturesView(report: report, focus: route.focus) - case .data: - DataSettingsView(report: report, backup: backup, focus: route.focus) - case .about: - AboutSettingsView(focus: route.focus) - } - } - /// Regions with days in the selected report year, so the region editor can /// surface a "used this year" group (grouping order only — it doesn't affect /// what's saved). `.other` isn't a pickable region, so it's dropped. @@ -312,15 +266,6 @@ struct SettingsView: View { guard let totals = report.report?.totals else { return [] } return Set(totals.filter { $0.key != .other && $0.value > 0 }.map(\.key)) } - - private var featureDiscoveryPresentation: FeatureDiscoveryPresentation { - FeatureDiscoveryPresentation( - report: report.report, - selectedYear: report.selectedYear, - referenceDate: report.referenceDate, - calendar: report.calendar, - ) - } } #if DEBUG diff --git a/Where/WhereUI/Tests/SettingsRouteViewTests.swift b/Where/WhereUI/Tests/SettingsRouteViewTests.swift new file mode 100644 index 000000000..625df2ab1 --- /dev/null +++ b/Where/WhereUI/Tests/SettingsRouteViewTests.swift @@ -0,0 +1,193 @@ +import SwiftUI +import TestHostSupport +import Testing +@testable import WhereUI + +/// Exercises every typed Settings push through the production route renderer, +/// including heterogeneous values pushed by child destinations. +@MainActor +@Suite(.serialized) +struct SettingsRouteViewTests { + @Test(arguments: SettingsDestination.allCases.filter { !$0.isSheet }) + func everyPushDestinationHosts(_ destination: SettingsDestination) throws { + let world = try World() + let rootView = world.host(route: SettingsRoute(destination)) + + try show(UIHostingController(rootView: rootView)) { hosted in + try waitFor { + hosted.viewIfLoaded?.window != nil + && navigationController(in: hosted)?.viewControllers.count == 2 + } + } + } + + @Test func shareEvidenceArchivePushesASecondHeterogeneousRoute() throws { + let world = try World() + let rootView = world.host( + route: SettingsRoute(.shareEvidence), + nested: ShareEvidenceFeaturesView.Route.archive, + ) + + try show(UIHostingController(rootView: rootView)) { hosted in + try waitFor { + hosted.viewIfLoaded?.window != nil + && navigationController(in: hosted)?.viewControllers.count == 3 + } + } + } + + @Test func attachmentDetailPushesASecondHeterogeneousValue() throws { + let evidence = try #require(PreviewSupport.sampleEvidence().first) + let world = try World() + let rootView = world.host( + route: SettingsRoute(.attachments), + nested: evidence, + ) + + try show(UIHostingController(rootView: rootView)) { hosted in + try waitFor { + hosted.viewIfLoaded?.window != nil + && navigationController(in: hosted)?.viewControllers.count == 3 + } + } + } + + private func navigationController( + in viewController: UIViewController, + ) -> UINavigationController? { + if let navigationController = viewController as? UINavigationController { + return navigationController + } + for child in viewController.children { + if let navigationController = navigationController(in: child) { + return navigationController + } + } + return nil + } +} + +@MainActor +private struct World { + let model: WhereModel + let session: WhereSession + let report: YearReportModel + let backup: BackupModel + let reminders: RemindersSettingsModel + + init() throws { + let model = PreviewSupport.loadedModel() + let session = try #require( + model.session, + "PreviewSupport.loadedModel() should create its session.", + ) + + self.model = model + self.session = session + report = YearReportModel( + services: session.services, + details: model.initialYearDetails, + selectedYear: model.initialSelectedYear, + preferences: session.preferences, + now: session.now, + ) + backup = BackupModel(services: session.services) + reminders = RemindersSettingsModel( + services: session.services, + preferences: session.preferences, + now: session.now, + ) + } + + func host(route: SettingsRoute) -> some View { + NavigationHost( + route: route, + report: report, + backup: backup, + reminders: reminders, + ) + .environment(model) + .environment(session) + } + + func host(route: SettingsRoute, nested: some Hashable) -> some View { + NavigationHost( + route: route, + nested: nested, + report: report, + backup: backup, + reminders: reminders, + ) + .environment(model) + .environment(session) + } +} + +@MainActor +private struct NavigationHost: View { + let report: YearReportModel + let backup: BackupModel + let reminders: RemindersSettingsModel + + @State private var path: NavigationPath + + init( + route: SettingsRoute, + report: YearReportModel, + backup: BackupModel, + reminders: RemindersSettingsModel, + ) { + var path = NavigationPath() + path.append(route) + self.init( + path: path, + report: report, + backup: backup, + reminders: reminders, + ) + } + + init( + route: SettingsRoute, + nested: some Hashable, + report: YearReportModel, + backup: BackupModel, + reminders: RemindersSettingsModel, + ) { + var path = NavigationPath() + path.append(route) + path.append(nested) + self.init( + path: path, + report: report, + backup: backup, + reminders: reminders, + ) + } + + private init( + path: NavigationPath, + report: YearReportModel, + backup: BackupModel, + reminders: RemindersSettingsModel, + ) { + _path = State(initialValue: path) + self.report = report + self.backup = backup + self.reminders = reminders + } + + var body: some View { + NavigationStack(path: $path) { + Color.clear + .navigationDestination(for: SettingsRoute.self) { route in + SettingsRouteView( + route: route, + report: report, + backup: backup, + reminders: reminders, + ) + } + } + } +}