fix: mark-read on first open + eliminate long-article scroll freeze
## 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) <noreply@anthropic.com>
This commit is contained in:
Yusuf Suleman
@@ -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 """
|
||||
<!DOCTYPE html>
|
||||
<html>
|
||||
<head>
|
||||
@@ -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 {
|
||||
}
|
||||
</style>
|
||||
</head>
|
||||
<body>\(body)</body>
|
||||
<body>
|
||||
<div class="article-header">
|
||||
<h1 class="article-title">\(title)</h1>
|
||||
<div class="article-meta">\(meta)</div>
|
||||
</div>
|
||||
\(articleContent)
|
||||
</body>
|
||||
</html>
|
||||
"""
|
||||
}
|
||||
|
||||
@@ -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("<html><body></body></html>", 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<CGFloat>?
|
||||
|
||||
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
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user