From 49c9b7871ced854489598f2db18ee64b687a0447 Mon Sep 17 00:00:00 2001 From: Yusuf Suleman Date: Fri, 3 Apr 2026 21:17:41 -0500 Subject: [PATCH] =?UTF-8?q?fix:=20Reader=20architecture=20overhaul=20?= =?UTF-8?q?=E2=80=94=20persistent=20WKWebView,=20stable=20layout,=20local-?= =?UTF-8?q?first=20state?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root-cause investigation identified 5 architectural issues. This commit fixes all of them with structural changes, not patches. ## 1. Persistent ArticleRenderer (fixes first-article freeze) BEFORE: Every article tap created a new WKWebView with a new WKWebViewConfiguration and a new WKProcessPool. Each spawned a WebContent process (~3s). The "warmer" used a different config, warming a different process — useless. AFTER: Single ArticleRenderer singleton owns one WKWebView with one shared WKProcessPool + WKWebViewConfiguration. Created at app launch via `_ = ArticleRenderer.shared` in ContentView.task. ArticleWebView wraps the shared WKWebView in a container UIView. SwiftUI owns the container lifecycle, not the WKWebView's. Zero process launches after first warm-up. ## 2. Stable Reader layout (fixes tab jitter) BEFORE: Sub-tabs and feed chips were conditionally rendered (`if !vm.isLoading || !vm.entries.isEmpty`). When loading finished, ~80px of UI appeared suddenly, causing layout shift that rippled to the tab bar. AFTER: Sub-tabs and feed chip bar ALWAYS render. Feed chip bar has fixed height (44px). No conditional wrappers in the layout hierarchy. Content area shows LoadingView during fetch. Chrome never changes shape. ## 3. Local-first state updates (fixes mark-read lag) BEFORE: markAsRead made 3 sequential API calls (mark, re-fetch entry, re-fetch counters). toggleRead and toggleStar did the same. Each action had 3 network round-trips before UI updated. AFTER: Mutate local entries array immediately (status/starred are now var). API sync happens in background via Task.detached. UI updates instantly. Counter refresh happens async. ## 4. Atomic list replacement (fixes empty flash) BEFORE: loadEntries(reset:true) set `entries = []` then `entries = newList`. Two mutations = empty state flash + full LazyVStack teardown/rebuild. AFTER: Never clear entries. Fetch completes, then single atomic `entries = newList`. SwiftUI diffs by Identifiable.id — only changed rows update. ## 5. Reserved thumbnail space (fixes card layout jump) BEFORE: AsyncImage default case was EmptyView() (0px). When image loaded, 180px appeared. Cards jumped. AFTER: Default case renders a placeholder Rectangle at 180px. Card height is stable from first render. ## Additional: Pre-load moved off TabView `.task { await readerVM.loadInitial() }` moved from TabView (caused observable mutations during TabView body evaluation, contributing to tab bar jitter) to the outer ZStack. Co-Authored-By: Claude Opus 4.6 (1M context) --- ios/Platform/Platform/ContentView.swift | 9 +- .../Features/Reader/Models/ReaderModels.swift | 4 +- .../Reader/ViewModels/ReaderViewModel.swift | 75 ++++++---- .../Reader/Views/ArticleWebView.swift | 77 +++++----- .../Features/Reader/Views/EntryListView.swift | 5 +- .../Features/Reader/Views/ReaderTabView.swift | 132 +++++++++--------- 6 files changed, 163 insertions(+), 139 deletions(-) diff --git a/ios/Platform/Platform/ContentView.swift b/ios/Platform/Platform/ContentView.swift index 4304e36..5ef9fea 100644 --- a/ios/Platform/Platform/ContentView.swift +++ b/ios/Platform/Platform/ContentView.swift @@ -45,10 +45,6 @@ struct MainTabView: View { } .tint(Color.accentWarm) .modifier(TabBarMinimizeModifier()) - .task { - // Pre-fetch reader data in background while user is on Home/Fitness - await readerVM.loadInitial() - } // Floating buttons VStack { @@ -89,6 +85,11 @@ struct MainTabView: View { .sheet(isPresented: $showAssistant) { AssistantSheetView(onFoodAdded: foodAdded) } + .task { + // Pre-warm WebKit process pool + pre-fetch reader data + _ = ArticleRenderer.shared + await readerVM.loadInitial() + } } private func foodAdded() { diff --git a/ios/Platform/Platform/Features/Reader/Models/ReaderModels.swift b/ios/Platform/Platform/Features/Reader/Models/ReaderModels.swift index 167d5f2..65b50a8 100644 --- a/ios/Platform/Platform/Features/Reader/Models/ReaderModels.swift +++ b/ios/Platform/Platform/Features/Reader/Models/ReaderModels.swift @@ -15,8 +15,8 @@ struct ReaderEntry: Codable, Identifiable, Hashable { let fullContent: String? let author: String? let publishedAt: String? - let status: String - let starred: Bool + var status: String // mutable for local-first read/unread updates + var starred: Bool // mutable for local-first star updates let readingTime: Int let thumbnail: String? let feed: ReaderFeedRef? diff --git a/ios/Platform/Platform/Features/Reader/ViewModels/ReaderViewModel.swift b/ios/Platform/Platform/Features/Reader/ViewModels/ReaderViewModel.swift index dd102aa..d496655 100644 --- a/ios/Platform/Platform/Features/Reader/ViewModels/ReaderViewModel.swift +++ b/ios/Platform/Platform/Features/Reader/ViewModels/ReaderViewModel.swift @@ -38,11 +38,10 @@ final class ReaderViewModel { private var offset = 0 private let pageSize = 30 private var hasMore = true + private var hasLoadedOnce = false // MARK: - Load - private var hasLoadedOnce = false - func loadInitial() async { guard !hasLoadedOnce else { return } hasLoadedOnce = true @@ -73,7 +72,7 @@ final class ReaderViewModel { if reset { offset = 0 hasMore = true - entries = [] + // DO NOT set entries = [] — causes full list teardown + empty flash } guard !isLoading else { return } isLoading = true @@ -81,6 +80,7 @@ final class ReaderViewModel { do { let list = try await fetchEntries(offset: 0) + // Atomic swap — SwiftUI diffs by Identifiable.id entries = list.entries total = list.total offset = list.entries.count @@ -111,7 +111,6 @@ final class ReaderViewModel { isRefreshing = true do { _ = try await api.refreshAllFeeds() - // Wait briefly for feeds to update try await Task.sleep(for: .seconds(2)) counters = try await api.getCounters() await loadEntries(reset: true) @@ -121,41 +120,51 @@ final class ReaderViewModel { isRefreshing = false } - // MARK: - Entry Actions + // MARK: - Entry Actions (local-first, API in background) func markAsRead(_ entry: ReaderEntry) async { guard !entry.isRead else { return } - do { - try await api.markEntries(ids: [entry.id], status: "read") - if let idx = entries.firstIndex(where: { $0.id == entry.id }) { - let updated = try await api.getEntry(id: entry.id) - entries[idx] = updated - } - counters = try await api.getCounters() - } catch {} + + // Update local state IMMEDIATELY — no API round-trip needed for UI + if let idx = entries.firstIndex(where: { $0.id == entry.id }) { + entries[idx].status = "read" + } + + // Sync with API in background (fire-and-forget) + Task.detached { [api] in + try? await api.markEntries(ids: [entry.id], status: "read") + } + + // Refresh counters in background + Task { + counters = try? await api.getCounters() + } } func toggleRead(_ entry: ReaderEntry) async { let newStatus = entry.isRead ? "unread" : "read" - do { - try await api.markEntries(ids: [entry.id], status: newStatus) - if let idx = entries.firstIndex(where: { $0.id == entry.id }) { - let updated = try await api.getEntry(id: entry.id) - entries[idx] = updated - } - counters = try await api.getCounters() - } catch {} + + if let idx = entries.firstIndex(where: { $0.id == entry.id }) { + entries[idx].status = newStatus + } + + Task.detached { [api] in + try? await api.markEntries(ids: [entry.id], status: newStatus) + } + + Task { + counters = try? await api.getCounters() + } } func toggleStar(_ entry: ReaderEntry) async { - do { - let result = try await api.toggleBookmark(entryId: entry.id) - if let idx = entries.firstIndex(where: { $0.id == entry.id }) { - let updated = try await api.getEntry(id: entry.id) - entries[idx] = updated - } - _ = result - } catch {} + if let idx = entries.firstIndex(where: { $0.id == entry.id }) { + entries[idx].starred.toggle() + } + + Task.detached { [api] in + _ = try? await api.toggleBookmark(entryId: entry.id) + } } func markAllRead() async { @@ -168,8 +177,13 @@ final class ReaderViewModel { default: _ = try await api.markAllRead() } + + // Update all local entries to read + for i in entries.indices { + entries[i].status = "read" + } + counters = try await api.getCounters() - await loadEntries(reset: true) } catch {} } @@ -218,7 +232,6 @@ final class ReaderViewModel { } var result: [(ReaderCategory?, [ReaderFeed])] = [] - // Uncategorized first if let uncategorized = grouped[nil], !uncategorized.isEmpty { result.append((nil, uncategorized)) } diff --git a/ios/Platform/Platform/Features/Reader/Views/ArticleWebView.swift b/ios/Platform/Platform/Features/Reader/Views/ArticleWebView.swift index 74f2e98..4e4ddad 100644 --- a/ios/Platform/Platform/Features/Reader/Views/ArticleWebView.swift +++ b/ios/Platform/Platform/Features/Reader/Views/ArticleWebView.swift @@ -1,53 +1,66 @@ import SwiftUI import WebKit -// MARK: - WebKit Pre-warmer +// MARK: - Shared Article Renderer +// +// Single WKWebView instance with shared WKProcessPool and WKWebViewConfiguration. +// Created once at app launch. Reused for every article open. +// Eliminates the ~3s WebContent process launch on first article tap. -/// Pre-warms the WebKit rendering engine on first use. -/// Call `WebKitWarmer.shared.warmUp()` early (e.g. when Reader tab loads) -/// so the first article open is instant. -final class WebKitWarmer { - static let shared = WebKitWarmer() +@MainActor +final class ArticleRenderer { + static let shared = ArticleRenderer() - private var warmedUp = false - private var warmView: WKWebView? + let webView: WKWebView - func warmUp() { - guard !warmedUp else { return } - warmedUp = true - DispatchQueue.main.async { - let config = WKWebViewConfiguration() - let wv = WKWebView(frame: CGRect(x: 0, y: 0, width: 1, height: 1), configuration: config) - wv.loadHTMLString("", baseURL: nil) - // Hold reference briefly to ensure WebKit initializes - self.warmView = wv - DispatchQueue.main.asyncAfter(deadline: .now() + 2) { - self.warmView = nil - } - } + private init() { + let pool = WKProcessPool() + let config = WKWebViewConfiguration() + config.processPool = pool + config.allowsInlineMediaPlayback = true + + webView = WKWebView(frame: .zero, configuration: config) + webView.isOpaque = false + webView.backgroundColor = .clear + webView.scrollView.isScrollEnabled = false + webView.scrollView.bounces = false + + // Pre-warm: load empty page to spin up WebContent process immediately + webView.loadHTMLString("", baseURL: nil) } } // MARK: - Article Web View +// +// UIViewRepresentable that wraps the shared WKWebView in a container UIView. +// SwiftUI owns the container's lifecycle, not the WKWebView's. +// The WKWebView survives across article opens. struct ArticleWebView: UIViewRepresentable { let html: String @Binding var contentHeight: CGFloat - func makeUIView(context: Context) -> WKWebView { - let config = WKWebViewConfiguration() - config.allowsInlineMediaPlayback = true + func makeUIView(context: Context) -> UIView { + let container = UIView() + container.backgroundColor = .clear + + let webView = ArticleRenderer.shared.webView + webView.removeFromSuperview() + webView.frame = container.bounds + webView.autoresizingMask = [.flexibleWidth, .flexibleHeight] + container.addSubview(webView) - let webView = WKWebView(frame: .zero, configuration: config) - webView.isOpaque = false - webView.backgroundColor = .clear - webView.scrollView.isScrollEnabled = false - webView.scrollView.bounces = false webView.navigationDelegate = context.coordinator - return webView + return container } - func updateUIView(_ webView: WKWebView, context: Context) { + func updateUIView(_ container: UIView, context: Context) { + let webView = ArticleRenderer.shared.webView + + // Ensure delegate points to THIS coordinator (not a stale one) + webView.navigationDelegate = context.coordinator + + // Only reload if HTML changed if context.coordinator.lastHTML != html { context.coordinator.lastHTML = html context.coordinator.heightBinding = $contentHeight @@ -78,7 +91,7 @@ struct ArticleWebView: UIViewRepresentable { } func webView(_ webView: WKWebView, didFinish navigation: WKNavigation!) { - DispatchQueue.main.asyncAfter(deadline: .now() + 0.1) { + DispatchQueue.main.asyncAfter(deadline: .now() + 0.05) { webView.evaluateJavaScript("document.body.scrollHeight") { [weak self] result, _ in if let height = result as? CGFloat, height > 0 { DispatchQueue.main.async { diff --git a/ios/Platform/Platform/Features/Reader/Views/EntryListView.swift b/ios/Platform/Platform/Features/Reader/Views/EntryListView.swift index ba3a8af..c60a532 100644 --- a/ios/Platform/Platform/Features/Reader/Views/EntryListView.swift +++ b/ios/Platform/Platform/Features/Reader/Views/EntryListView.swift @@ -114,7 +114,10 @@ struct EntryCardView: View { .frame(height: 180) .clipped() default: - EmptyView() + // Reserve space during load to prevent layout jump + Rectangle() + .fill(Color.surfaceCard) + .frame(height: 180) } } } diff --git a/ios/Platform/Platform/Features/Reader/Views/ReaderTabView.swift b/ios/Platform/Platform/Features/Reader/Views/ReaderTabView.swift index abd70a0..ca0cf40 100644 --- a/ios/Platform/Platform/Features/Reader/Views/ReaderTabView.swift +++ b/ios/Platform/Platform/Features/Reader/Views/ReaderTabView.swift @@ -10,82 +10,79 @@ struct ReaderTabView: View { var body: some View { NavigationStack { VStack(spacing: 0) { - if !vm.isLoading || !vm.entries.isEmpty { - // Sub-tab selector — only show after initial load - HStack(spacing: 0) { - ForEach(Array(subTabs.enumerated()), id: \.offset) { index, tab in - Button { - withAnimation(.easeInOut(duration: 0.2)) { - selectedSubTab = index - switch index { - case 0: vm.applyFilter(.unread) - case 1: vm.applyFilter(.starred) - case 2: vm.applyFilter(.all) - default: break - } + // Sub-tab selector — ALWAYS rendered (prevents layout jitter) + HStack(spacing: 0) { + ForEach(Array(subTabs.enumerated()), id: \.offset) { index, tab in + Button { + withAnimation(.easeInOut(duration: 0.2)) { + selectedSubTab = index + switch index { + case 0: vm.applyFilter(.unread) + case 1: vm.applyFilter(.starred) + case 2: vm.applyFilter(.all) + default: break } - } label: { - HStack(spacing: 4) { - Text(tab) - .font(.subheadline.weight(selectedSubTab == index ? .semibold : .regular)) + } + } label: { + HStack(spacing: 4) { + Text(tab) + .font(.subheadline.weight(selectedSubTab == index ? .semibold : .regular)) - if index == 0, let counters = vm.counters, counters.totalUnread > 0 { - Text("\(counters.totalUnread)") - .font(.caption2.weight(.bold)) - .foregroundStyle(.white) - .padding(.horizontal, 6) - .padding(.vertical, 2) - .background(Color.accentWarm) - .clipShape(Capsule()) - } + if index == 0, let counters = vm.counters, counters.totalUnread > 0 { + Text("\(counters.totalUnread)") + .font(.caption2.weight(.bold)) + .foregroundStyle(.white) + .padding(.horizontal, 6) + .padding(.vertical, 2) + .background(Color.accentWarm) + .clipShape(Capsule()) } - .foregroundStyle(selectedSubTab == index ? Color.accentWarm : Color.textSecondary) - .padding(.vertical, 10) - .padding(.horizontal, 16) - .background { - if selectedSubTab == index { - Capsule() - .fill(Color.accentWarm.opacity(0.12)) - } + } + .foregroundStyle(selectedSubTab == index ? Color.accentWarm : Color.textSecondary) + .padding(.vertical, 10) + .padding(.horizontal, 16) + .background { + if selectedSubTab == index { + Capsule() + .fill(Color.accentWarm.opacity(0.12)) } } } } + } + .padding(.horizontal) + .padding(.top, 8) + + // Feed filter bar — ALWAYS rendered, empty during load (stable height) + ScrollView(.horizontal, showsIndicators: false) { + HStack(spacing: 8) { + feedFilterChip("All", isSelected: isAllSelected) { + let tab = selectedSubTab + switch tab { + case 0: vm.applyFilter(.unread) + case 1: vm.applyFilter(.starred) + case 2: vm.applyFilter(.all) + default: vm.applyFilter(.unread) + } + } + + ForEach(vm.feeds) { feed in + let count = vm.counters?.count(forFeed: feed.id) ?? 0 + feedFilterChip( + feed.title, + count: selectedSubTab == 0 ? count : nil, + isSelected: vm.currentFilter == .feed(feed.id) + ) { + vm.applyFilter(.feed(feed.id)) + } + } + } .padding(.horizontal) - .padding(.top, 8) - - // Feed filter bar - if !vm.feeds.isEmpty { - ScrollView(.horizontal, showsIndicators: false) { - HStack(spacing: 8) { - feedFilterChip("All", isSelected: isAllSelected) { - let tab = selectedSubTab - switch tab { - case 0: vm.applyFilter(.unread) - case 1: vm.applyFilter(.starred) - case 2: vm.applyFilter(.all) - default: vm.applyFilter(.unread) - } - } - - ForEach(vm.feeds) { feed in - let count = vm.counters?.count(forFeed: feed.id) ?? 0 - feedFilterChip( - feed.title, - count: selectedSubTab == 0 ? count : nil, - isSelected: vm.currentFilter == .feed(feed.id) - ) { - vm.applyFilter(.feed(feed.id)) - } - } - } - .padding(.horizontal) - .padding(.vertical, 8) - } - } + .padding(.vertical, 8) } + .frame(height: 44) // Fixed height prevents layout shift - // Entry list (shows LoadingView when isLoading) + // Entry list EntryListView(vm: vm, isCardView: isCardView) } .frame(maxWidth: .infinity, maxHeight: .infinity) @@ -142,9 +139,6 @@ struct ReaderTabView: View { FeedManagementSheet(vm: vm) } } - .onAppear { - WebKitWarmer.shared.warmUp() - } } private var subTabs: [String] { ["Unread", "Starred", "All"] }