ViewStructure.md (4486B)
1 # SwiftUI UI Audit 2 3 This audit covers the SwiftUI code under `Crossmate/Views`, reviewed against the `$Swiftie` guidance. No code changes were made as part of the review. 4 5 ## Findings 6 7 ### Resolved: Custom Environment and Focused Values Carried Closures 8 9 The original audit found custom environment/focused values exposing behavior as closures. This has been addressed by: 10 11 - Replacing the app action closure entries in `Crossmate/Sync/SyncEngine.swift` with a stable `AppActions` object injected through `\.appActions`. 12 - Moving puzzle reveal confirmation state into `RevealConfirmation`, so `PuzzleActionTarget` stores stable references and methods rather than a `requestReveal` closure. 13 14 ### Resolved: GameListView Derived Sorted and Filtered Collections in the View Update Path 15 16 `GameListView.content(usesRoomierType:)` maps fetched Core Data entities into summaries, filters and sorts in-progress, revoked, and completed collections, slices completed games, builds blocked author IDs, and filters invites before rendering. 17 18 `GameSummaryCache` reduces summary construction cost, but the collection derivation still scales with the number of games and repeats whenever this view invalidates. Swiftie recommends caching sorted/filtered collections in a model or view-local state and recomputing them only when their inputs change. 19 20 This is most relevant because `GameListView` also reads several environment values and state values that can invalidate the whole view. 21 22 This has been addressed by caching a `GameListSections` snapshot in view-local 23 state. The snapshot is rebuilt when the managed object context changes or when 24 local player identity inputs change, while layout, announcement, Dynamic Type, 25 and other unrelated view invalidations reuse the prepared sections. 26 27 ### Medium: Large Computed Subviews Are Used as Structural Factoring 28 29 Several large screens use computed `some View` properties or `@ViewBuilder` helper functions to organize distinct sections: 30 31 - `GameListView.content`, `listLayout`, `gridLayout`, invite rows/cards, and section headers. 32 - `PuzzleView.phoneLayout`, `landscapePadLayout`, `portraitPadLayout`, `puzzleArea`, `controlsArea`, and `controlsPanel`. 33 - `NYTBrowseView.browserView`, `monthHeader`, `weekdayHeader`, `dayGrid`, random controls, and date picker popover. 34 - `ClueList.clueList`, `list`, `sidebarList`, row builders, and section headers. 35 36 Computed view properties and builder helpers improve readability but do not create independent SwiftUI invalidation boundaries. For the hottest screens, extracting meaningful sections into separate `View` structs with narrow inputs would let SwiftUI skip more work when unrelated state changes. 37 38 This does not need to be applied mechanically to every helper. It is most valuable for sections that are large, repeated, inside lazy containers, or depend on different subsets of state. 39 40 ### Low/Medium: Some User-Facing Text Formatting Is English-Shaped at Runtime 41 42 There are several localization-related patterns worth tightening: 43 44 - `PuzzleView.formattedList(_:)` manually joins names with comma and `and`. Prefer locale-aware list formatting such as `items.formatted()`. 45 - `ClueList.headingRow(_:)` and `sidebarSectionHeader(_:)` use `.textCase(.uppercase)`. Swiftie prefers putting the desired case in the localized string itself so translators can adjust casing per language. 46 - `NYTBrowseView.monthTitle` uses `DateFormatter.dateFormat = "MMMM yyyy"`. Prefer a locale-aware format style, or `setLocalizedDateFormatFromTemplate(_:)` if a `DateFormatter` is needed. 47 48 These are not correctness blockers, but they are useful cleanup targets before deeper localization work. 49 50 ## Positive Notes 51 52 - The views generally use Observation (`@Observable`, `@Bindable`, `@Environment(Model.self)`) rather than `ObservableObject` / `@ObservedObject`. 53 - `@State` properties in the scanned view files are private. 54 - Most `ForEach` usage has stable identity. The offset-based calendar/grid cases appear slot-based and low risk unless those rows later gain local state or animation-sensitive behavior. 55 - The code already has focused performance-oriented comments and caches in areas such as `GameSummaryCache`, which suggests the project is already paying attention to SwiftUI invalidation cost. 56 57 ## Suggested Order of Work 58 59 1. Extract the largest, hottest computed subviews into real `View` structs with narrow inputs. 60 2. Clean up the localization-shaped formatting issues as part of a dedicated localization pass.