From ac84b2297773132d04c21812eaa3681fbe51ccbf Mon Sep 17 00:00:00 2001 From: Trey T Date: Mon, 18 May 2026 20:08:36 -0500 Subject: [PATCH] =?UTF-8?q?Books=20=E2=80=94=20fix=20infinite=20render=20l?= =?UTF-8?q?oop=20via=20value-based=20navigation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Opening a book chapter froze the app in an infinite render loop. Root cause: the books screens used the eager `NavigationLink { destination }` form inside `List`/`LazyVStack`. That form keeps the destination view structurally parented to the source row, so `BookReaderView`'s ScrollView got trapped inside a `List` row — a row sizes to intrinsic height, a ScrollView has none, so the two never converge and re-measure forever. Switch the whole books navigation chain to value-based navigation: - practiceHomeView, BookLibraryView, BookChapterListView use NavigationLink(value:). - PracticeView's NavigationStack declares the BooksRoute, Book, and BookChapter destinations once, at the stack root (mixing eager and value-based pushes in one path caused pushed screens to pop back). - BookReaderView is built from just a BookChapter; it resolves its Book by slug via @Query. Also: - BookChapter gains a stored paragraphCount so the chapter list no longer decodes the full paragraph JSON on every render (bookDataVersion -> 6 to re-seed). - BookSpeechController builds its AVSpeechSynthesizer lazily. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Conjuga/Services/BookSpeechController.swift | 12 ++++++++++-- Conjuga/Conjuga/Services/DataLoader.swift | 3 ++- .../Practice/Books/BookChapterListView.swift | 6 ++---- .../Views/Practice/Books/BookLibraryView.swift | 12 +++++++++--- .../Views/Practice/Books/BookReaderView.swift | 16 ++++++++++++++-- .../Conjuga/Views/Practice/PracticeView.swift | 17 ++++++++++++++--- .../Sources/SharedModels/BookChapter.swift | 5 +++++ 7 files changed, 56 insertions(+), 15 deletions(-) diff --git a/Conjuga/Conjuga/Services/BookSpeechController.swift b/Conjuga/Conjuga/Services/BookSpeechController.swift index 1aa0c7a..91c9b0d 100644 --- a/Conjuga/Conjuga/Services/BookSpeechController.swift +++ b/Conjuga/Conjuga/Services/BookSpeechController.swift @@ -25,7 +25,16 @@ final class BookSpeechController: NSObject, AVSpeechSynthesizerDelegate { // MARK: - Internals - private let synthesizer = AVSpeechSynthesizer() + /// Built on first use, not in `init`. `AVSpeechSynthesizer()` connects to + /// the system speech daemon, so allocating one per `BookReaderView` struct + /// construction (SwiftUI rebuilds the struct on every parent render) is a + /// real cost — deferring it keeps controller construction cheap. + @ObservationIgnored + private lazy var synthesizer: AVSpeechSynthesizer = { + let synth = AVSpeechSynthesizer() + synth.delegate = self + return synth + }() private var queue: [QueueEntry] = [] private var queueCursor: Int = 0 private var audioSessionConfigured = false @@ -38,7 +47,6 @@ final class BookSpeechController: NSObject, AVSpeechSynthesizerDelegate { override init() { super.init() - synthesizer.delegate = self } // MARK: - Public control diff --git a/Conjuga/Conjuga/Services/DataLoader.swift b/Conjuga/Conjuga/Services/DataLoader.swift index ac31440..704ec10 100644 --- a/Conjuga/Conjuga/Services/DataLoader.swift +++ b/Conjuga/Conjuga/Services/DataLoader.swift @@ -9,7 +9,7 @@ actor DataLoader { static let textbookDataVersion = 14 static let textbookDataKey = "textbookDataVersion" - static let bookDataVersion = 5 // bump: per-book glossary added + static let bookDataVersion = 6 // bump: BookChapter.paragraphCount added static let bookDataKey = "bookDataVersion" /// Quick check: does the DB need seeding or course data refresh? @@ -632,6 +632,7 @@ actor DataLoader { bookSlug: slug, number: number, title: chTitle, + paragraphCount: paragraphsES.count, paragraphsESJSON: esData, paragraphsENJSON: enData ) diff --git a/Conjuga/Conjuga/Views/Practice/Books/BookChapterListView.swift b/Conjuga/Conjuga/Views/Practice/Books/BookChapterListView.swift index 0be80fc..90b8570 100644 --- a/Conjuga/Conjuga/Views/Practice/Books/BookChapterListView.swift +++ b/Conjuga/Conjuga/Views/Practice/Books/BookChapterListView.swift @@ -19,9 +19,7 @@ struct BookChapterListView: View { var body: some View { List { ForEach(allChapters) { chapter in - NavigationLink { - BookReaderView(chapter: chapter, book: book) - } label: { + NavigationLink(value: chapter) { HStack(spacing: 12) { Text("\(chapter.number)") .font(.subheadline.weight(.bold).monospacedDigit()) @@ -31,7 +29,7 @@ struct BookChapterListView: View { VStack(alignment: .leading, spacing: 2) { Text(chapter.title) .font(.subheadline.weight(.medium)) - Text("\(chapter.paragraphsES().count) paragraph\(chapter.paragraphsES().count == 1 ? "" : "s")") + Text("\(chapter.paragraphCount) paragraph\(chapter.paragraphCount == 1 ? "" : "s")") .font(.caption2) .foregroundStyle(.tertiary) } diff --git a/Conjuga/Conjuga/Views/Practice/Books/BookLibraryView.swift b/Conjuga/Conjuga/Views/Practice/Books/BookLibraryView.swift index a36b1ae..b7e9efe 100644 --- a/Conjuga/Conjuga/Views/Practice/Books/BookLibraryView.swift +++ b/Conjuga/Conjuga/Views/Practice/Books/BookLibraryView.swift @@ -2,6 +2,14 @@ import SwiftUI import SharedModels import SwiftData +/// Route value for pushing the books library. Lets `PracticeView` use a +/// value-based link so the entire books navigation chain is consistent — +/// mixing a view-based push with value-based pushes deeper in the same +/// NavigationStack made pushed screens pop back immediately. +enum BooksRoute: Hashable { + case library +} + struct BookLibraryView: View { @Query(sort: \Book.title) private var books: [Book] @@ -17,9 +25,7 @@ struct BookLibraryView: View { ScrollView { LazyVStack(spacing: 12) { ForEach(books) { book in - NavigationLink { - BookChapterListView(book: book) - } label: { + NavigationLink(value: book) { BookCard(book: book) } .tint(.primary) diff --git a/Conjuga/Conjuga/Views/Practice/Books/BookReaderView.swift b/Conjuga/Conjuga/Views/Practice/Books/BookReaderView.swift index 2226638..598dc8f 100644 --- a/Conjuga/Conjuga/Views/Practice/Books/BookReaderView.swift +++ b/Conjuga/Conjuga/Views/Practice/Books/BookReaderView.swift @@ -1,10 +1,16 @@ import SwiftUI import SharedModels +import SwiftData import FoundationModels struct BookReaderView: View { let chapter: BookChapter - let book: Book + + /// The book this chapter belongs to, resolved by slug — used for the + /// pre-computed glossary. A @Query is safe here because the reader is + /// built lazily by `navigationDestination`: one instance, when opened. + @Query private var bookMatches: [Book] + private var book: Book? { bookMatches.first } @Environment(DictionaryService.self) private var dictionary @State private var speech = BookSpeechController() @@ -19,6 +25,12 @@ struct BookReaderView: View { @AppStorage("bookReaderVoiceId") private var storedVoiceId: String = "" @AppStorage("bookReaderRate") private var storedRate: Double = 0.45 + init(chapter: BookChapter) { + self.chapter = chapter + let slug = chapter.bookSlug + _bookMatches = Query(filter: #Predicate { $0.slug == slug }) + } + private var paragraphsES: [String] { chapter.paragraphsES() } private var paragraphsEN: [String] { chapter.paragraphsEN() } @@ -87,7 +99,7 @@ struct BookReaderView: View { speech.voiceIdentifier = storedVoiceId.isEmpty ? nil : storedVoiceId speech.rate = Float(storedRate) if glossary.isEmpty { - glossary = book.glossary() + glossary = book?.glossary() ?? [:] } } .onDisappear { diff --git a/Conjuga/Conjuga/Views/Practice/PracticeView.swift b/Conjuga/Conjuga/Views/Practice/PracticeView.swift index ececa6a..72a1c6a 100644 --- a/Conjuga/Conjuga/Views/Practice/PracticeView.swift +++ b/Conjuga/Conjuga/Views/Practice/PracticeView.swift @@ -21,6 +21,19 @@ struct PracticeView: View { practiceHomeView } } + // Book navigation is value-based and declared once here, at the + // stack root. Eager `NavigationLink { destination }` forms inside + // the List/LazyVStack of the book screens caused an infinite + // render loop; value-based links build destinations lazily. + .navigationDestination(for: BooksRoute.self) { _ in + BookLibraryView() + } + .navigationDestination(for: Book.self) { book in + BookChapterListView(book: book) + } + .navigationDestination(for: BookChapter.self) { chapter in + BookReaderView(chapter: chapter) + } .navigationTitle("Practice") .navigationBarTitleDisplayMode(.inline) .onAppear(perform: loadProgress) @@ -261,9 +274,7 @@ struct PracticeView: View { .padding(.horizontal) // Books - NavigationLink { - BookLibraryView() - } label: { + NavigationLink(value: BooksRoute.library) { HStack(spacing: 14) { Image(systemName: "books.vertical.fill") .font(.title3) diff --git a/Conjuga/SharedModels/Sources/SharedModels/BookChapter.swift b/Conjuga/SharedModels/Sources/SharedModels/BookChapter.swift index 3928a05..5f34723 100644 --- a/Conjuga/SharedModels/Sources/SharedModels/BookChapter.swift +++ b/Conjuga/SharedModels/Sources/SharedModels/BookChapter.swift @@ -10,6 +10,9 @@ public final class BookChapter { public var bookSlug: String = "" public var number: Int = 0 public var title: String = "" + /// Spanish paragraph count, stored at seed time so chapter lists can show + /// it without decoding the full `paragraphsESJSON` blob on every render. + public var paragraphCount: Int = 0 public var paragraphsESJSON: Data = Data() public var paragraphsENJSON: Data = Data() @@ -18,6 +21,7 @@ public final class BookChapter { bookSlug: String, number: Int, title: String, + paragraphCount: Int, paragraphsESJSON: Data, paragraphsENJSON: Data ) { @@ -25,6 +29,7 @@ public final class BookChapter { self.bookSlug = bookSlug self.number = number self.title = title + self.paragraphCount = paragraphCount self.paragraphsESJSON = paragraphsESJSON self.paragraphsENJSON = paragraphsENJSON }