fix: 14 audit fixes — concurrency, memory, performance, accessibility
Second audit round addressing data races, task stacking, unbounded caches, and VoiceOver gaps across 7 files. Concurrency: - Move NSItemProvider @State access into MainActor block (3 drop handlers) - Cancel stale ScheduleViewModel tasks on rapid filter changes Memory: - Replace unbounded image dict with LRUCache(countLimit: 50) - Replace demo-mode asyncAfter with cancellable Task Performance: - Wrap debug NBA print() in #if DEBUG - Cache visitsById via @State + onChange instead of per-render computed - Pre-compute sportProgressFractions in ProgressViewModel - Replace allGames computed property with hasGames bool check - Cache sortedTrips via @State + onChange in SavedTripsListView Accessibility: - Add combined VoiceOver label to progress ring - Combine away/home team text into single readable phrase - Hide decorative StadiumDetailSheet icon from VoiceOver - Add explicit accessibilityLabel to SportFilterChip - Add combined accessibilityLabel to GameRowView Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -7,6 +7,7 @@
|
||||
|
||||
import Foundation
|
||||
import UIKit
|
||||
import LRUCache
|
||||
|
||||
actor RemoteImageService {
|
||||
|
||||
@@ -32,7 +33,7 @@ actor RemoteImageService {
|
||||
// MARK: - Properties
|
||||
|
||||
private let urlSession: URLSession
|
||||
private var imageCache: [URL: UIImage] = [:]
|
||||
private let imageCache = LRUCache<URL, UIImage>(countLimit: 50)
|
||||
|
||||
// MARK: - Init
|
||||
|
||||
@@ -56,7 +57,7 @@ actor RemoteImageService {
|
||||
/// Fetch a single image from URL
|
||||
func fetchImage(from url: URL) async throws -> UIImage {
|
||||
// Check cache first
|
||||
if let cached = imageCache[url] {
|
||||
if let cached = imageCache.value(forKey: url) {
|
||||
return cached
|
||||
}
|
||||
|
||||
@@ -79,7 +80,7 @@ actor RemoteImageService {
|
||||
let scaledImage = scaleImage(image, maxWidth: 400)
|
||||
|
||||
// Cache it
|
||||
imageCache[url] = scaledImage
|
||||
imageCache.setValue(scaledImage, forKey: url)
|
||||
|
||||
return scaledImage
|
||||
}
|
||||
|
||||
@@ -479,10 +479,7 @@ struct SavedTripsListView: View {
|
||||
@State private var showDebugPoll = false
|
||||
#endif
|
||||
|
||||
/// Trips sorted by most cities (stops) first
|
||||
private var sortedTrips: [SavedTrip] {
|
||||
trips.sorted { ($0.trip?.stops.count ?? 0) > ($1.trip?.stops.count ?? 0) }
|
||||
}
|
||||
@State private var sortedTrips: [SavedTrip] = []
|
||||
|
||||
/// Trips as domain objects for poll creation
|
||||
private var tripsForPollCreation: [Trip] {
|
||||
@@ -501,6 +498,9 @@ struct SavedTripsListView: View {
|
||||
.padding(Theme.Spacing.md)
|
||||
}
|
||||
.themedBackground()
|
||||
.onChange(of: trips, initial: true) { _, newTrips in
|
||||
sortedTrips = newTrips.sorted { ($0.trip?.stops.count ?? 0) > ($1.trip?.stops.count ?? 0) }
|
||||
}
|
||||
.task {
|
||||
guard !hasLoadedPolls else { return }
|
||||
hasLoadedPolls = true
|
||||
|
||||
@@ -46,6 +46,9 @@ final class ProgressViewModel {
|
||||
/// Unvisited stadiums for the selected sport
|
||||
private(set) var unvisitedStadiums: [Stadium] = []
|
||||
|
||||
/// Pre-computed progress fractions per sport (avoids filtering visits per sport per render)
|
||||
private(set) var sportProgressFractions: [Sport: Double] = [:]
|
||||
|
||||
/// Stadiums for the selected sport
|
||||
var sportStadiums: [Stadium] {
|
||||
stadiums.filter { $0.sport == selectedSport }
|
||||
@@ -200,6 +203,19 @@ final class ProgressViewModel {
|
||||
}
|
||||
}
|
||||
stadiumVisitStatus = statusMap
|
||||
|
||||
// Pre-compute sport progress fractions for SportSelectorGrid
|
||||
var sportCounts: [Sport: Int] = [:]
|
||||
for visit in visits {
|
||||
if let sport = visit.sportEnum {
|
||||
sportCounts[sport, default: 0] += 1
|
||||
}
|
||||
}
|
||||
sportProgressFractions = Dictionary(uniqueKeysWithValues: Sport.supported.map { sport in
|
||||
let total = LeagueStructure.stadiumCount(for: sport)
|
||||
let visited = min(sportCounts[sport] ?? 0, total)
|
||||
return (sport, total > 0 ? Double(visited) / Double(total) : 0)
|
||||
})
|
||||
}
|
||||
|
||||
// MARK: - Progress Card Generation
|
||||
|
||||
@@ -19,11 +19,7 @@ struct ProgressTabView: View {
|
||||
@State private var selectedVisitId: UUID?
|
||||
|
||||
@Query private var visits: [StadiumVisit]
|
||||
|
||||
/// O(1) lookup for visits by ID (built from @Query results)
|
||||
private var visitsById: [UUID: StadiumVisit] {
|
||||
Dictionary(uniqueKeysWithValues: visits.map { ($0.id, $0) })
|
||||
}
|
||||
@State private var visitsById: [UUID: StadiumVisit] = [:]
|
||||
|
||||
var body: some View {
|
||||
Group {
|
||||
@@ -66,6 +62,9 @@ struct ProgressTabView: View {
|
||||
.accessibilityLabel("Add stadium visit")
|
||||
}
|
||||
}
|
||||
.onChange(of: visits, initial: true) { _, newVisits in
|
||||
visitsById = Dictionary(uniqueKeysWithValues: newVisits.map { ($0.id, $0) })
|
||||
}
|
||||
.task {
|
||||
viewModel.configure(with: modelContext.container)
|
||||
await viewModel.loadData()
|
||||
@@ -153,7 +152,7 @@ struct ProgressTabView: View {
|
||||
SportProgressButton(
|
||||
sport: sport,
|
||||
isSelected: viewModel.selectedSport == sport,
|
||||
progress: progressForSport(sport)
|
||||
progress: viewModel.sportProgressFractions[sport] ?? 0
|
||||
) {
|
||||
Theme.Animation.withMotion(Theme.Animation.spring) {
|
||||
viewModel.selectSport(sport)
|
||||
@@ -162,12 +161,6 @@ struct ProgressTabView: View {
|
||||
}
|
||||
}
|
||||
|
||||
private func progressForSport(_ sport: Sport) -> Double {
|
||||
let visitedCount = viewModel.visits.filter { $0.sportEnum == sport }.count
|
||||
let total = LeagueStructure.stadiumCount(for: sport)
|
||||
guard total > 0 else { return 0 }
|
||||
return Double(min(visitedCount, total)) / Double(total)
|
||||
}
|
||||
|
||||
// MARK: - Progress Summary Card
|
||||
|
||||
@@ -204,6 +197,8 @@ struct ProgressTabView: View {
|
||||
.foregroundStyle(Theme.textMuted(colorScheme))
|
||||
}
|
||||
}
|
||||
.accessibilityElement(children: .ignore)
|
||||
.accessibilityLabel("\(progress.visitedStadiums) of \(progress.totalStadiums) stadiums visited, \(Int(progress.completionPercentage)) percent complete")
|
||||
|
||||
VStack(alignment: .leading, spacing: Theme.Spacing.xs) {
|
||||
Text(viewModel.selectedSport.displayName)
|
||||
@@ -540,13 +535,10 @@ struct RecentVisitRow: View {
|
||||
.font(.body)
|
||||
.foregroundStyle(Theme.textPrimary(colorScheme))
|
||||
|
||||
// Date, Away @ Home on one line, left aligned
|
||||
VStack(alignment: .leading, spacing: 4) {
|
||||
Text(visit.shortDateDescription)
|
||||
if let away = visit.awayTeamName, let home = visit.homeTeamName {
|
||||
Text(away)
|
||||
Text("@")
|
||||
Text(home)
|
||||
Text("\(away) at \(home)")
|
||||
}
|
||||
}
|
||||
.font(.subheadline)
|
||||
@@ -596,6 +588,7 @@ struct StadiumDetailSheet: View {
|
||||
.font(.largeTitle)
|
||||
.foregroundStyle(visitStatus.isVisited ? .green : sport.themeColor)
|
||||
}
|
||||
.accessibilityHidden(true)
|
||||
|
||||
Text(stadium.name)
|
||||
.font(.headline)
|
||||
|
||||
@@ -28,6 +28,7 @@ final class ScheduleViewModel {
|
||||
private(set) var errorMessage: String?
|
||||
|
||||
private let dataProvider = AppDataProvider.shared
|
||||
private var loadTask: Task<Void, Never>?
|
||||
|
||||
// MARK: - Pre-computed Groupings (avoid computed property overhead)
|
||||
|
||||
@@ -117,6 +118,7 @@ final class ScheduleViewModel {
|
||||
logger.info("📅 \(sport.rawValue): \(count) games")
|
||||
}
|
||||
|
||||
#if DEBUG
|
||||
// Debug: Print all NBA games
|
||||
let nbaGames = games.filter { $0.game.sport == .nba }
|
||||
print("🏀 [DEBUG] All NBA games in schedule (\(nbaGames.count) total):")
|
||||
@@ -124,6 +126,7 @@ final class ScheduleViewModel {
|
||||
let dateStr = game.game.dateTime.gameDateTimeString(in: game.stadium.timeZone)
|
||||
print("🏀 \(dateStr): \(game.awayTeam.name) @ \(game.homeTeam.name) (\(game.game.id))")
|
||||
}
|
||||
#endif
|
||||
|
||||
} catch let cloudKitError as CloudKitError {
|
||||
self.error = cloudKitError
|
||||
@@ -151,9 +154,8 @@ final class ScheduleViewModel {
|
||||
selectedSports.insert(sport)
|
||||
}
|
||||
AnalyticsManager.shared.track(.scheduleFiltered(sport: sport.rawValue, dateRange: "\(startDate.formatted(.dateTime.month().day())) - \(endDate.formatted(.dateTime.month().day()))"))
|
||||
Task {
|
||||
await loadGames()
|
||||
}
|
||||
loadTask?.cancel()
|
||||
loadTask = Task { await loadGames() }
|
||||
}
|
||||
|
||||
func resetFilters() {
|
||||
@@ -161,17 +163,15 @@ final class ScheduleViewModel {
|
||||
searchText = ""
|
||||
startDate = Date()
|
||||
endDate = Calendar.current.date(byAdding: .day, value: 14, to: Date()) ?? Date()
|
||||
Task {
|
||||
await loadGames()
|
||||
}
|
||||
loadTask?.cancel()
|
||||
loadTask = Task { await loadGames() }
|
||||
}
|
||||
|
||||
func updateDateRange(start: Date, end: Date) {
|
||||
startDate = start
|
||||
endDate = end
|
||||
Task {
|
||||
await loadGames()
|
||||
}
|
||||
loadTask?.cancel()
|
||||
loadTask = Task { await loadGames() }
|
||||
}
|
||||
|
||||
// MARK: - Filtering & Grouping (pre-computed, not computed properties)
|
||||
|
||||
@@ -11,17 +11,17 @@ struct ScheduleListView: View {
|
||||
@State private var showDatePicker = false
|
||||
@State private var showDiagnostics = false
|
||||
|
||||
private var allGames: [RichGame] {
|
||||
viewModel.gamesBySport.flatMap(\.games)
|
||||
private var hasGames: Bool {
|
||||
!viewModel.gamesBySport.isEmpty
|
||||
}
|
||||
|
||||
var body: some View {
|
||||
Group {
|
||||
if viewModel.isLoading && allGames.isEmpty {
|
||||
if viewModel.isLoading && !hasGames {
|
||||
loadingView
|
||||
} else if let errorMessage = viewModel.errorMessage {
|
||||
errorView(message: errorMessage)
|
||||
} else if allGames.isEmpty {
|
||||
} else if !hasGames {
|
||||
emptyView
|
||||
} else {
|
||||
gamesList
|
||||
@@ -224,6 +224,7 @@ struct SportFilterChip: View {
|
||||
}
|
||||
.buttonStyle(.plain)
|
||||
.accessibilityIdentifier("schedule.sport.\(sport.rawValue.lowercased())")
|
||||
.accessibilityLabel(sport.rawValue)
|
||||
.accessibilityValue(isSelected ? "Selected" : "Not selected")
|
||||
.accessibilityAddTraits(isSelected ? .isSelected : [])
|
||||
}
|
||||
@@ -313,6 +314,18 @@ struct GameRowView: View {
|
||||
}
|
||||
.padding(.vertical, 4)
|
||||
.listRowBackground(isToday ? Color.orange.opacity(0.1) : nil)
|
||||
.accessibilityElement(children: .ignore)
|
||||
.accessibilityLabel(gameAccessibilityLabel)
|
||||
}
|
||||
|
||||
private var gameAccessibilityLabel: String {
|
||||
var parts = ["\(game.awayTeam.name) at \(game.homeTeam.name)"]
|
||||
parts.append(game.stadium.name)
|
||||
parts.append(game.localGameTimeShort)
|
||||
if showDate {
|
||||
parts.append(Self.dateFormatter.string(from: game.game.dateTime))
|
||||
}
|
||||
return parts.joined(separator: ", ")
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -35,6 +35,7 @@ struct TripDetailView: View {
|
||||
@State private var loadedGames: [String: RichGame] = [:]
|
||||
@State private var isLoadingGames = false
|
||||
@State private var hasAppliedDemoSelection = false
|
||||
@State private var demoSaveTask: Task<Void, Never>?
|
||||
|
||||
// Itinerary items state
|
||||
@State private var itineraryItems: [ItineraryItem] = []
|
||||
@@ -122,10 +123,10 @@ struct TripDetailView: View {
|
||||
// Demo mode: auto-favorite the trip
|
||||
if isDemoMode && !hasAppliedDemoSelection && !isSaved {
|
||||
hasAppliedDemoSelection = true
|
||||
DispatchQueue.main.asyncAfter(deadline: .now() + DemoConfig.selectionDelay + 0.5) {
|
||||
if !isSaved {
|
||||
saveTrip()
|
||||
}
|
||||
demoSaveTask = Task {
|
||||
try? await Task.sleep(for: .seconds(DemoConfig.selectionDelay + 0.5))
|
||||
guard !Task.isCancelled, !isSaved else { return }
|
||||
saveTrip()
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -137,7 +138,10 @@ struct TripDetailView: View {
|
||||
}
|
||||
recomputeSections()
|
||||
}
|
||||
.onDisappear { subscriptionCancellable?.cancel() }
|
||||
.onDisappear {
|
||||
subscriptionCancellable?.cancel()
|
||||
demoSaveTask?.cancel()
|
||||
}
|
||||
.onChange(of: itineraryItems) { _, newItems in
|
||||
handleItineraryItemsChange(newItems)
|
||||
recomputeSections()
|
||||
@@ -715,10 +719,10 @@ struct TripDetailView: View {
|
||||
|
||||
provider.loadObject(ofClass: NSString.self) { item, _ in
|
||||
guard let droppedId = item as? String,
|
||||
let itemId = UUID(uuidString: droppedId),
|
||||
let droppedItem = self.itineraryItems.first(where: { $0.id == itemId }) else { return }
|
||||
let itemId = UUID(uuidString: droppedId) else { return }
|
||||
|
||||
Task { @MainActor in
|
||||
guard let droppedItem = self.itineraryItems.first(where: { $0.id == itemId }) else { return }
|
||||
let day = self.findDayForTravelSegment(segment)
|
||||
// Place at beginning of day (sortOrder before existing items)
|
||||
let minSortOrder = self.itineraryItems
|
||||
@@ -743,11 +747,11 @@ struct TripDetailView: View {
|
||||
|
||||
provider.loadObject(ofClass: NSString.self) { item, _ in
|
||||
guard let droppedId = item as? String,
|
||||
let itemId = UUID(uuidString: droppedId),
|
||||
let droppedItem = self.itineraryItems.first(where: { $0.id == itemId }),
|
||||
droppedItem.id != targetItem.id else { return }
|
||||
let itemId = UUID(uuidString: droppedId) else { return }
|
||||
|
||||
Task { @MainActor in
|
||||
guard let droppedItem = self.itineraryItems.first(where: { $0.id == itemId }),
|
||||
droppedItem.id != targetItem.id else { return }
|
||||
// Place before target item using midpoint insertion
|
||||
let itemsInDay = self.itineraryItems.filter { $0.day == targetItem.day && $0.id != droppedItem.id }
|
||||
.sorted { $0.sortOrder < $1.sortOrder }
|
||||
@@ -772,10 +776,10 @@ struct TripDetailView: View {
|
||||
|
||||
provider.loadObject(ofClass: NSString.self) { item, _ in
|
||||
guard let droppedId = item as? String,
|
||||
let itemId = UUID(uuidString: droppedId),
|
||||
let droppedItem = self.itineraryItems.first(where: { $0.id == itemId }) else { return }
|
||||
let itemId = UUID(uuidString: droppedId) else { return }
|
||||
|
||||
Task { @MainActor in
|
||||
guard let droppedItem = self.itineraryItems.first(where: { $0.id == itemId }) else { return }
|
||||
// Calculate sortOrder: append at end of day's items
|
||||
let maxSortOrder = self.itineraryItems
|
||||
.filter { $0.day == day && $0.id != droppedItem.id }
|
||||
|
||||
Reference in New Issue
Block a user