fix: 13 audit fixes — memory, concurrency, performance, accessibility
Critical:
- ProgressViewModel: use single stored ModelContext instead of creating
new ones per operation (deleteVisit silently no-op'd)
- ProgressViewModel: convert expensive computed properties to stored
with explicit recompute after mutations (3x recomputation per render)
Memory:
- AnimatedSportsIcon: replace recursive GCD asyncAfter with Task loop,
cancelled in onDisappear (19 unkillable timer chains)
- ItineraryItemService: remove [weak self] from actor Task (semantically
wrong, silently drops flushPendingUpdates)
- VisitPhotoService: remove [weak self] from @MainActor Task closures
Concurrency:
- StoreManager: replace nested MainActor.run{Task{}} with direct await
in listenForTransactions (fire-and-forget race)
- VisitPhotoService: move JPEG encoding/file writing off MainActor via
nonisolated static helper + Task.detached
- SportsIconImageGenerator: replace GCD dispatch with Task.detached for
structured concurrency compliance
Performance:
- Game/RichGame: cache DateFormatters as static lets instead of
allocating per-call (hundreds of allocations in schedule view)
- TripDetailView: wrap ~10 routeWaypoints print() in #if DEBUG, remove
2 let _ = print() from TripMapView.body (fires every render)
Accessibility:
- GameRow: add combined VoiceOver label (was reading abbreviations
letter-by-letter)
- Sport badges: add accessibilityLabel to prevent SF symbol name readout
- SportsTimeApp: post UIAccessibility.screenChanged after bootstrap
completes so VoiceOver users know app is ready
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -29,83 +29,28 @@ final class ProgressViewModel {
|
||||
// MARK: - Dependencies
|
||||
|
||||
private var modelContainer: ModelContainer?
|
||||
private var modelContext: ModelContext?
|
||||
private let dataProvider = AppDataProvider.shared
|
||||
|
||||
// MARK: - Computed Properties
|
||||
// MARK: - Derived State (recomputed after mutations)
|
||||
|
||||
/// Overall progress for the selected sport
|
||||
var leagueProgress: LeagueProgress {
|
||||
// Filter stadiums by sport directly (same as sportStadiums)
|
||||
let sportStadiums = stadiums.filter { $0.sport == selectedSport }
|
||||
|
||||
let visitedStadiumIds = Set(
|
||||
visits
|
||||
.filter { $0.sportEnum == selectedSport }
|
||||
.compactMap { visit -> String? in
|
||||
// O(1) dictionary lookup via DataProvider
|
||||
dataProvider.stadium(for: visit.stadiumId)?.id
|
||||
}
|
||||
)
|
||||
|
||||
let visited = sportStadiums.filter { visitedStadiumIds.contains($0.id) }
|
||||
let remaining = sportStadiums.filter { !visitedStadiumIds.contains($0.id) }
|
||||
|
||||
return LeagueProgress(
|
||||
sport: selectedSport,
|
||||
totalStadiums: sportStadiums.count,
|
||||
visitedStadiums: visited.count,
|
||||
stadiumsVisited: visited,
|
||||
stadiumsRemaining: remaining
|
||||
)
|
||||
}
|
||||
private(set) var leagueProgress: LeagueProgress = LeagueProgress(sport: .mlb, totalStadiums: 0, visitedStadiums: 0, stadiumsVisited: [], stadiumsRemaining: [])
|
||||
|
||||
/// Stadium visit status indexed by stadium ID
|
||||
var stadiumVisitStatus: [String: StadiumVisitStatus] {
|
||||
var statusMap: [String: StadiumVisitStatus] = [:]
|
||||
private(set) var stadiumVisitStatus: [String: StadiumVisitStatus] = [:]
|
||||
|
||||
// Group visits by stadium
|
||||
let visitsByStadium = Dictionary(grouping: visits.filter { $0.sportEnum == selectedSport }) { $0.stadiumId }
|
||||
/// Visited stadiums for the selected sport
|
||||
private(set) var visitedStadiums: [Stadium] = []
|
||||
|
||||
for stadium in stadiums {
|
||||
if let stadiumVisits = visitsByStadium[stadium.id], !stadiumVisits.isEmpty {
|
||||
let summaries = stadiumVisits.map { visit in
|
||||
VisitSummary(
|
||||
id: visit.id,
|
||||
stadium: stadium,
|
||||
visitDate: visit.visitDate,
|
||||
visitType: visit.visitType,
|
||||
sport: selectedSport,
|
||||
homeTeamName: visit.homeTeamName,
|
||||
awayTeamName: visit.awayTeamName,
|
||||
score: visit.finalScore,
|
||||
photoCount: visit.photoMetadata?.count ?? 0,
|
||||
notes: visit.notes
|
||||
)
|
||||
}
|
||||
statusMap[stadium.id] = .visited(visits: summaries)
|
||||
} else {
|
||||
statusMap[stadium.id] = .notVisited
|
||||
}
|
||||
}
|
||||
|
||||
return statusMap
|
||||
}
|
||||
/// Unvisited stadiums for the selected sport
|
||||
private(set) var unvisitedStadiums: [Stadium] = []
|
||||
|
||||
/// Stadiums for the selected sport
|
||||
var sportStadiums: [Stadium] {
|
||||
stadiums.filter { $0.sport == selectedSport }
|
||||
}
|
||||
|
||||
/// Visited stadiums for the selected sport
|
||||
var visitedStadiums: [Stadium] {
|
||||
leagueProgress.stadiumsVisited
|
||||
}
|
||||
|
||||
/// Unvisited stadiums for the selected sport
|
||||
var unvisitedStadiums: [Stadium] {
|
||||
leagueProgress.stadiumsRemaining
|
||||
}
|
||||
|
||||
/// Count of trips for the selected sport (stub - can be enhanced)
|
||||
var tripCount: Int {
|
||||
// TODO: Fetch saved trips count from SwiftData
|
||||
@@ -141,6 +86,7 @@ final class ProgressViewModel {
|
||||
|
||||
func configure(with container: ModelContainer) {
|
||||
self.modelContainer = container
|
||||
self.modelContext = ModelContext(container)
|
||||
}
|
||||
|
||||
// MARK: - Actions
|
||||
@@ -159,8 +105,7 @@ final class ProgressViewModel {
|
||||
teams = dataProvider.teams
|
||||
|
||||
// Load visits from SwiftData
|
||||
if let container = modelContainer {
|
||||
let context = ModelContext(container)
|
||||
if let context = modelContext {
|
||||
let descriptor = FetchDescriptor<StadiumVisit>(
|
||||
sortBy: [SortDescriptor(\.visitDate, order: .reverse)]
|
||||
)
|
||||
@@ -171,11 +116,13 @@ final class ProgressViewModel {
|
||||
self.errorMessage = error.localizedDescription
|
||||
}
|
||||
|
||||
recomputeDerivedState()
|
||||
isLoading = false
|
||||
}
|
||||
|
||||
func selectSport(_ sport: Sport) {
|
||||
selectedSport = sport
|
||||
recomputeDerivedState()
|
||||
AnalyticsManager.shared.track(.sportSwitched(sport: sport.rawValue))
|
||||
}
|
||||
|
||||
@@ -187,13 +134,12 @@ final class ProgressViewModel {
|
||||
// MARK: - Visit Management
|
||||
|
||||
func deleteVisit(_ visit: StadiumVisit) async throws {
|
||||
guard let container = modelContainer else { return }
|
||||
guard let context = modelContext else { return }
|
||||
|
||||
if let sport = visit.sportEnum {
|
||||
AnalyticsManager.shared.track(.stadiumVisitDeleted(stadiumId: visit.stadiumId, sport: sport.rawValue))
|
||||
}
|
||||
|
||||
let context = ModelContext(container)
|
||||
context.delete(visit)
|
||||
try context.save()
|
||||
|
||||
@@ -201,6 +147,61 @@ final class ProgressViewModel {
|
||||
await loadData()
|
||||
}
|
||||
|
||||
// MARK: - Derived State Recomputation
|
||||
|
||||
private func recomputeDerivedState() {
|
||||
// Compute league progress once
|
||||
let sportStadiums = stadiums.filter { $0.sport == selectedSport }
|
||||
|
||||
let visitedStadiumIds = Set(
|
||||
visits
|
||||
.filter { $0.sportEnum == selectedSport }
|
||||
.compactMap { visit -> String? in
|
||||
dataProvider.stadium(for: visit.stadiumId)?.id
|
||||
}
|
||||
)
|
||||
|
||||
let visited = sportStadiums.filter { visitedStadiumIds.contains($0.id) }
|
||||
let remaining = sportStadiums.filter { !visitedStadiumIds.contains($0.id) }
|
||||
|
||||
leagueProgress = LeagueProgress(
|
||||
sport: selectedSport,
|
||||
totalStadiums: sportStadiums.count,
|
||||
visitedStadiums: visited.count,
|
||||
stadiumsVisited: visited,
|
||||
stadiumsRemaining: remaining
|
||||
)
|
||||
visitedStadiums = visited
|
||||
unvisitedStadiums = remaining
|
||||
|
||||
// Compute stadium visit status once
|
||||
var statusMap: [String: StadiumVisitStatus] = [:]
|
||||
let visitsByStadium = Dictionary(grouping: visits.filter { $0.sportEnum == selectedSport }) { $0.stadiumId }
|
||||
|
||||
for stadium in stadiums {
|
||||
if let stadiumVisits = visitsByStadium[stadium.id], !stadiumVisits.isEmpty {
|
||||
let summaries = stadiumVisits.map { visit in
|
||||
VisitSummary(
|
||||
id: visit.id,
|
||||
stadium: stadium,
|
||||
visitDate: visit.visitDate,
|
||||
visitType: visit.visitType,
|
||||
sport: selectedSport,
|
||||
homeTeamName: visit.homeTeamName,
|
||||
awayTeamName: visit.awayTeamName,
|
||||
score: visit.finalScore,
|
||||
photoCount: visit.photoMetadata?.count ?? 0,
|
||||
notes: visit.notes
|
||||
)
|
||||
}
|
||||
statusMap[stadium.id] = .visited(visits: summaries)
|
||||
} else {
|
||||
statusMap[stadium.id] = .notVisited
|
||||
}
|
||||
}
|
||||
stadiumVisitStatus = statusMap
|
||||
}
|
||||
|
||||
// MARK: - Progress Card Generation
|
||||
|
||||
func progressCardData(includeUsername: Bool = false) -> ProgressCardData {
|
||||
|
||||
@@ -240,14 +240,14 @@ struct SportsIconImageGeneratorView: View {
|
||||
isGenerating = true
|
||||
|
||||
// Generate on background thread to avoid UI freeze
|
||||
DispatchQueue.global(qos: .userInitiated).async {
|
||||
let image = SportsIconImageGenerator.generateImage()
|
||||
Task {
|
||||
let image = await Task.detached(priority: .userInitiated) {
|
||||
SportsIconImageGenerator.generateImage()
|
||||
}.value
|
||||
|
||||
DispatchQueue.main.async {
|
||||
withAnimation {
|
||||
generatedImage = image
|
||||
isGenerating = false
|
||||
}
|
||||
withAnimation {
|
||||
generatedImage = image
|
||||
isGenerating = false
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -457,6 +457,8 @@ struct TripDetailView: View {
|
||||
Text(sport.rawValue)
|
||||
.font(.caption2)
|
||||
}
|
||||
.accessibilityElement(children: .ignore)
|
||||
.accessibilityLabel(sport.rawValue)
|
||||
.padding(.horizontal, 10)
|
||||
.padding(.vertical, 5)
|
||||
.background(sport.themeColor.opacity(0.2))
|
||||
@@ -968,14 +970,7 @@ struct TripDetailView: View {
|
||||
private func fetchDrivingRoutes() async {
|
||||
// Use routeWaypoints which includes game stops + mappable custom items
|
||||
let waypoints = routeWaypoints
|
||||
print("🗺️ [FetchRoutes] Computing routes with \(waypoints.count) waypoints:")
|
||||
for (index, wp) in waypoints.enumerated() {
|
||||
print("🗺️ [FetchRoutes] \(index): \(wp.name) (custom: \(wp.isCustomItem))")
|
||||
}
|
||||
guard waypoints.count >= 2 else {
|
||||
print("🗺️ [FetchRoutes] Not enough waypoints, skipping")
|
||||
return
|
||||
}
|
||||
guard waypoints.count >= 2 else { return }
|
||||
|
||||
isLoadingRoutes = true
|
||||
var allCoordinates: [[CLLocationCoordinate2D]] = []
|
||||
@@ -1013,7 +1008,6 @@ struct TripDetailView: View {
|
||||
}
|
||||
|
||||
await MainActor.run {
|
||||
print("🗺️ [FetchRoutes] Setting \(allCoordinates.count) route segments")
|
||||
routeCoordinates = allCoordinates
|
||||
mapUpdateTrigger = UUID() // Force map to re-render with new routes
|
||||
isLoadingRoutes = false
|
||||
@@ -1051,6 +1045,7 @@ struct TripDetailView: View {
|
||||
// Items are ordered by (day, sortOrder) - visual order matches route order
|
||||
let itemsByDay = Dictionary(grouping: mappableCustomItems) { $0.day }
|
||||
|
||||
#if DEBUG
|
||||
print("🗺️ [Waypoints] Building waypoints. Mappable items by day:")
|
||||
for (day, items) in itemsByDay.sorted(by: { $0.key < $1.key }) {
|
||||
for item in items.sorted(by: { $0.sortOrder < $1.sortOrder }) {
|
||||
@@ -1058,11 +1053,14 @@ struct TripDetailView: View {
|
||||
print("🗺️ [Waypoints] Day \(day): \(title), sortOrder: \(item.sortOrder)")
|
||||
}
|
||||
}
|
||||
#endif
|
||||
|
||||
var waypoints: [(name: String, coordinate: CLLocationCoordinate2D, isCustomItem: Bool)] = []
|
||||
let days = tripDays
|
||||
|
||||
#if DEBUG
|
||||
print("🗺️ [Waypoints] Trip has \(days.count) days")
|
||||
#endif
|
||||
|
||||
for (dayIndex, dayDate) in days.enumerated() {
|
||||
let dayNumber = dayIndex + 1
|
||||
@@ -1077,7 +1075,9 @@ struct TripDetailView: View {
|
||||
return day >= arrival && day <= departure
|
||||
})?.city
|
||||
|
||||
#if DEBUG
|
||||
print("🗺️ [Waypoints] Day \(dayNumber): city=\(dayCity ?? "none"), games=\(gamesOnDay.count)")
|
||||
#endif
|
||||
|
||||
// Game stop for this day (only add once per city to avoid duplicates)
|
||||
if let city = dayCity {
|
||||
@@ -1098,16 +1098,15 @@ struct TripDetailView: View {
|
||||
if let stop = trip.stops.first(where: { $0.city == city }) {
|
||||
if let stadiumId = stop.stadium,
|
||||
let stadium = dataProvider.stadium(for: stadiumId) {
|
||||
print("🗺️ [Waypoints] Adding \(stadium.name) (stadium)")
|
||||
waypoints.append((stadium.name, stadium.coordinate, false))
|
||||
} else if let coord = stop.coordinate {
|
||||
// No stadium ID but stop has coordinate
|
||||
print("🗺️ [Waypoints] Adding \(city) (city coord)")
|
||||
waypoints.append((city, coord, false))
|
||||
}
|
||||
}
|
||||
} else {
|
||||
#if DEBUG
|
||||
print("🗺️ [Waypoints] \(city) already in waypoints, skipping")
|
||||
#endif
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1116,7 +1115,6 @@ struct TripDetailView: View {
|
||||
let sortedItems = items.sorted { $0.sortOrder < $1.sortOrder }
|
||||
for item in sortedItems {
|
||||
if let info = item.customInfo, let coord = info.coordinate {
|
||||
print("🗺️ [Waypoints] Adding \(info.title) (sortOrder: \(item.sortOrder))")
|
||||
waypoints.append((info.title, coord, true))
|
||||
}
|
||||
}
|
||||
@@ -1776,6 +1774,8 @@ struct GameRow: View {
|
||||
.padding(Theme.Spacing.sm)
|
||||
.background(Theme.cardBackgroundElevated(colorScheme))
|
||||
.clipShape(RoundedRectangle(cornerRadius: Theme.CornerRadius.small))
|
||||
.accessibilityElement(children: .ignore)
|
||||
.accessibilityLabel("\(game.game.sport.rawValue): \(game.awayTeam.name) at \(game.homeTeam.name), \(game.stadium.name), \(game.localGameTimeShort)")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1994,11 +1994,9 @@ struct TripMapView: View {
|
||||
}
|
||||
|
||||
var body: some View {
|
||||
let _ = print("🗺️ [TripMapView] Rendering with \(routeCoordinates.count) route segments, version: \(routeVersion)")
|
||||
Map(position: $cameraPosition, interactionModes: []) {
|
||||
// Routes (driving directions)
|
||||
ForEach(Array(routeCoordinates.enumerated()), id: \.offset) { index, coords in
|
||||
let _ = print("🗺️ [TripMapView] Drawing route \(index) with \(coords.count) points")
|
||||
if !coords.isEmpty {
|
||||
MapPolyline(MKPolyline(coordinates: coords, count: coords.count))
|
||||
.stroke(Theme.routeGold, lineWidth: 4)
|
||||
|
||||
Reference in New Issue
Block a user