From 18dd5aa44d92ab210918e3ad54ad65e61966fd25 Mon Sep 17 00:00:00 2001 From: Yusuf Suleman Date: Fri, 3 Apr 2026 21:28:24 -0500 Subject: [PATCH] fix: mark-read on first open + eliminate long-article scroll freeze MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Bug 1: First open didn't mark as read ROOT CAUSE: Race condition. markAsRead set local status="read", then getEntry returned the server's status="unread" (API sync hadn't completed yet) and overwrote the local mutation. FIX: - markAsRead runs FIRST, before getEntry (was concurrent before) - After getEntry, merge server response but PRESERVE local status/starred (which may differ from server due to race) - currentEntry syncs from vm.entries after markAsRead, ensuring the toolbar reflects the correct state ## Bug 2: Long articles freeze before scrollable ROOT CAUSE: WKWebView.scrollView.isScrollEnabled = false, embedded inside SwiftUI ScrollView with .frame(height: webViewHeight). For a 15000px article, WebKit had to render the entire document, JavaScript measured document.body.scrollHeight, SwiftUI relaid out the 15000px frame — all blocking before scroll became responsive. FIX: - WKWebView now handles its own scrolling (isScrollEnabled = true) - Removed SwiftUI ScrollView wrapper around article - Removed contentHeight binding and height measurement JavaScript - Removed the Coordinator's didFinish height evaluation - Article header (title, feed, time) moved into the HTML document so it scrolls naturally with the content - WKWebView fills available space, scrolls natively via WebKit's compositor thread — immediate scroll response Both fixes preserve the shared WKWebView architecture. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../Features/Reader/Views/ArticleView.swift | 196 +++++++++--------- .../Reader/Views/ArticleWebView.swift | 36 +--- 2 files changed, 107 insertions(+), 125 deletions(-) diff --git a/ios/Platform/Platform/Features/Reader/Views/ArticleView.swift b/ios/Platform/Platform/Features/Reader/Views/ArticleView.swift index 01c9ff3..a259310 100644 --- a/ios/Platform/Platform/Features/Reader/Views/ArticleView.swift +++ b/ios/Platform/Platform/Features/Reader/Views/ArticleView.swift @@ -6,7 +6,6 @@ struct ArticleView: View { @State private var currentEntry: ReaderEntry @State private var isFetchingFull = false @State private var savedToBrain = false - @State private var webViewHeight: CGFloat = 400 @State private var isContentReady = false @State private var articleContent = "" @@ -17,95 +16,47 @@ struct ArticleView: View { } var body: some View { - ScrollView { - VStack(alignment: .leading, spacing: 12) { - // Article header - VStack(alignment: .leading, spacing: 8) { - Text(currentEntry.displayTitle) - .font(.title2.weight(.bold)) - .foregroundStyle(Color.textPrimary) - - HStack(spacing: 8) { - Text(currentEntry.feedName) - .font(.subheadline.weight(.medium)) - .foregroundStyle(Color.accentWarm) - - if let author = currentEntry.author, !author.isEmpty { - Text("\u{2022} \(author)") - .font(.subheadline) - .foregroundStyle(Color.textSecondary) - } - } - - HStack(spacing: 12) { - if !currentEntry.timeAgo.isEmpty { - Label(currentEntry.timeAgo, systemImage: "clock") - .font(.caption) - .foregroundStyle(Color.textTertiary) - } - Label(currentEntry.readingTimeText, systemImage: "book") - .font(.caption) - .foregroundStyle(Color.textTertiary) - } + Group { + if !isContentReady { + VStack { + Spacer() + ProgressView() + .controlSize(.regular) + Text("Loading article...") + .font(.caption) + .foregroundStyle(Color.textTertiary) + .padding(.top, 8) + Spacer() } - .padding(.horizontal, 16) - .padding(.top, 8) - - Divider() - .padding(.horizontal, 16) - - // Article body - if !isContentReady { - HStack { - ProgressView() - .controlSize(.small) - Text("Loading article...") - .font(.caption) - .foregroundStyle(Color.textTertiary) + .frame(maxWidth: .infinity, maxHeight: .infinity) + } else if articleContent.isEmpty { + VStack(spacing: 12) { + Spacer() + Image(systemName: "doc.text") + .font(.system(size: 32)) + .foregroundStyle(Color.textTertiary) + Text("No content available") + .font(.subheadline) + .foregroundStyle(Color.textSecondary) + Button("Fetch Full Article") { + Task { await fetchFull() } } - .frame(maxWidth: .infinity) - .padding(.vertical, 40) - } else if articleContent.isEmpty { - if isFetchingFull { - HStack { - ProgressView() - .controlSize(.small) - Text("Fetching article...") - .font(.caption) - .foregroundStyle(Color.textTertiary) - } - .frame(maxWidth: .infinity) - .padding(.vertical, 40) - } else { - VStack(spacing: 12) { - Image(systemName: "doc.text") - .font(.system(size: 32)) - .foregroundStyle(Color.textTertiary) - Text("No content available") - .font(.subheadline) - .foregroundStyle(Color.textSecondary) - Button("Fetch Full Article") { - Task { await fetchFull() } - } - .font(.subheadline.weight(.medium)) - .foregroundStyle(Color.accentWarm) - } - .frame(maxWidth: .infinity) - .padding(.vertical, 40) - } - } else { - ArticleWebView(html: wrapHTML(articleContent), contentHeight: $webViewHeight) - .frame(height: webViewHeight) + .font(.subheadline.weight(.medium)) + .foregroundStyle(Color.accentWarm) + Spacer() } - - Spacer(minLength: 80) + .frame(maxWidth: .infinity, maxHeight: .infinity) + } else { + // WKWebView handles all scrolling — header is in the HTML + ArticleWebView(html: buildArticleHTML()) + .ignoresSafeArea(edges: .bottom) } } + .frame(maxWidth: .infinity, maxHeight: .infinity) .background(Color.canvas) .navigationBarTitleDisplayMode(.inline) .toolbar { ToolbarItemGroup(placement: .topBarTrailing) { - // Star toggle Button { Task { await toggleStar() } } label: { @@ -113,7 +64,6 @@ struct ArticleView: View { .foregroundStyle(currentEntry.starred ? .orange : Color.textTertiary) } - // Read/unread toggle Button { Task { await toggleRead() } } label: { @@ -121,7 +71,6 @@ struct ArticleView: View { .foregroundStyle(Color.textTertiary) } - // More actions Menu { if currentEntry.url != nil { Button { @@ -154,19 +103,27 @@ struct ArticleView: View { } } .task { - // Auto-mark as read (fire-and-forget) - Task { await vm.markAsRead(entry) } + // 1. Mark as read IMMEDIATELY — before any network call + await vm.markAsRead(entry) - // Fetch full entry content on demand + // 2. Sync local state with the read mutation + if let updated = vm.entries.first(where: { $0.id == entry.id }) { + currentEntry = updated + } + + // 3. Fetch full content — do NOT overwrite status/starred from server do { - let fullEntry = try await ReaderAPI().getEntry(id: currentEntry.id) - currentEntry = fullEntry + let fullEntry = try await ReaderAPI().getEntry(id: entry.id) articleContent = fullEntry.articleHTML - if let idx = vm.entries.firstIndex(where: { $0.id == fullEntry.id }) { - vm.entries[idx] = fullEntry + + // Preserve local status/starred (may differ from server due to race) + var merged = fullEntry + if let local = vm.entries.first(where: { $0.id == entry.id }) { + merged.status = local.status + merged.starred = local.starred } + currentEntry = merged } catch { - // Fall back to whatever content we already have articleContent = currentEntry.articleHTML } isContentReady = true @@ -191,11 +148,14 @@ struct ArticleView: View { isFetchingFull = true do { let updated = try await ReaderAPI().fetchFullContent(entryId: currentEntry.id) - currentEntry = updated articleContent = updated.articleHTML - if let idx = vm.entries.firstIndex(where: { $0.id == updated.id }) { - vm.entries[idx] = updated + // Preserve local status + var merged = updated + if let local = vm.entries.first(where: { $0.id == updated.id }) { + merged.status = local.status + merged.starred = local.starred } + currentEntry = merged } catch {} isFetchingFull = false } @@ -207,8 +167,25 @@ struct ArticleView: View { } } - private func wrapHTML(_ body: String) -> String { - """ + // MARK: - HTML Builder (header + body in one document) + + private func buildArticleHTML() -> String { + let title = currentEntry.displayTitle + .replacingOccurrences(of: "<", with: "<") + .replacingOccurrences(of: ">", with: ">") + let feed = currentEntry.feedName + .replacingOccurrences(of: "<", with: "<") + let author = currentEntry.author ?? "" + let time = currentEntry.timeAgo + let reading = currentEntry.readingTimeText + + var metaParts = [feed] + if !author.isEmpty { metaParts.append("by \(author)") } + if !time.isEmpty { metaParts.append(time) } + metaParts.append("\(reading) read") + let meta = metaParts.joined(separator: " · ") + + return """ @@ -221,7 +198,7 @@ struct ArticleView: View { font-size: 17px; line-height: 1.6; color: #1f1f1f; - padding: 0 16px 40px; + padding: 0 16px 60px; margin: 0; background: transparent; -webkit-text-size-adjust: 100%; @@ -232,6 +209,23 @@ struct ArticleView: View { pre, code { background: #1e1c1a !important; } blockquote { border-left-color: #c79e40; color: #9a9590; } td, th { border-color: #333; } + .article-meta { color: #8a8580; } + } + .article-header { + padding: 16px 0 12px; + border-bottom: 1px solid rgba(128,128,128,0.2); + margin-bottom: 16px; + } + .article-title { + font-size: 24px; + font-weight: 700; + line-height: 1.25; + margin: 0 0 8px; + } + .article-meta { + font-size: 13px; + color: #8B6914; + line-height: 1.4; } img { max-width: 100%; @@ -279,7 +273,13 @@ struct ArticleView: View { } - \(body) + +
+

\(title)

+
\(meta)
+
+ \(articleContent) + """ } diff --git a/ios/Platform/Platform/Features/Reader/Views/ArticleWebView.swift b/ios/Platform/Platform/Features/Reader/Views/ArticleWebView.swift index eab0de6..338b2b1 100644 --- a/ios/Platform/Platform/Features/Reader/Views/ArticleWebView.swift +++ b/ios/Platform/Platform/Features/Reader/Views/ArticleWebView.swift @@ -3,9 +3,9 @@ import WebKit // 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. +// Single WKWebView instance with shared WKWebViewConfiguration. +// Created once at app launch. Reused for every article. +// WKWebView handles its own scrolling — no height measurement needed. @MainActor final class ArticleRenderer { @@ -20,23 +20,22 @@ final class ArticleRenderer { webView = WKWebView(frame: .zero, configuration: config) webView.isOpaque = false webView.backgroundColor = .clear - webView.scrollView.isScrollEnabled = false - webView.scrollView.bounces = false + // WKWebView handles its own scrolling — no SwiftUI ScrollView wrapper + webView.scrollView.isScrollEnabled = true - // Pre-warm: load empty page to spin up WebContent process immediately + // Pre-warm: spin up WebContent process 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. +// Wraps the shared WKWebView in a container UIView. +// SwiftUI owns the container lifecycle, not the WKWebView's. +// No height binding — WKWebView scrolls natively. struct ArticleWebView: UIViewRepresentable { let html: String - @Binding var contentHeight: CGFloat func makeUIView(context: Context) -> UIView { let container = UIView() @@ -54,14 +53,10 @@ struct ArticleWebView: UIViewRepresentable { 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 webView.loadHTMLString(html, baseURL: nil) } } @@ -72,7 +67,6 @@ struct ArticleWebView: UIViewRepresentable { class Coordinator: NSObject, WKNavigationDelegate { var lastHTML: String? - var heightBinding: Binding? func webView( _ webView: WKWebView, @@ -87,17 +81,5 @@ struct ArticleWebView: UIViewRepresentable { } decisionHandler(.allow) } - - func webView(_ webView: WKWebView, didFinish navigation: WKNavigation!) { - 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 { - self?.heightBinding?.wrappedValue = height - } - } - } - } - } } }