From 6cff4a903643e23f5e4bcb92dbed8aa5311fd53f Mon Sep 17 00:00:00 2001 From: Yusuf Suleman Date: Sat, 4 Apr 2026 00:06:24 -0500 Subject: [PATCH] =?UTF-8?q?fix:=20scroll=20mark-as-read=20=E2=80=94=20move?= =?UTF-8?q?=20all=20tracking=20to=20per-row=20GeometryReader?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ROOT CAUSE (confirmed by instrumentation): 1. viewportHeight was 0 — background GeometryReader onAppear fired before ScrollView layout, never updated. Visibility ratio was always 0.00, so wasVisible was never populated. 2. cumulativeDown was 0 — PreferenceKey + onPreferenceChange on the zero-height anchor never delivered scroll offset updates. 3. Both tracking mechanisms were dead. Only per-row onChange fired. FIX: Removed dead PreferenceKey scroll tracker and dead viewport background GeometryReader. All tracking now lives in the per-row GeometryReader onChange(of: frame.minY), which the logs confirmed fires reliably: - Scroll direction: computed from delta between current and previous minY for each entry (stored in lastKnownMinY dictionary) - Cumulative scroll: accumulated from positive deltas (>2pt filter) - Activation: requires cumulative downward scroll > threshold - Visibility: computed using UIScreen.main.bounds.height (reliable, doesn't depend on layout timing) - Mark condition: trackingActive + moving down + unread + was visible + maxY < 0 (fully above viewport) Navigation protection preserved: onAppear resets trackingActive, cumulativeDown, and lastKnownMinY. Removed debug instrumentation. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../Features/Reader/Views/EntryListView.swift | 146 ++++++------------ 1 file changed, 44 insertions(+), 102 deletions(-) diff --git a/ios/Platform/Platform/Features/Reader/Views/EntryListView.swift b/ios/Platform/Platform/Features/Reader/Views/EntryListView.swift index 8367970..5525e24 100644 --- a/ios/Platform/Platform/Features/Reader/Views/EntryListView.swift +++ b/ios/Platform/Platform/Features/Reader/Views/EntryListView.swift @@ -1,28 +1,22 @@ import SwiftUI -// MARK: - Scroll Offset Preference Key - -private struct ScrollOffsetKey: PreferenceKey { - static var defaultValue: CGFloat = 0 - static func reduce(value: inout CGFloat, nextValue: () -> CGFloat) { - value = nextValue() - } -} - -// MARK: - Entry List View - struct EntryListView: View { @Bindable var vm: ReaderViewModel var isCardView: Bool = true - // Scroll-based read tracking - @State private var previousOffset: CGFloat = 0 - @State private var isScrollingDown = false + // Scroll-based read tracking — all driven from per-row GeometryReader + @State private var lastKnownMinY: [Int: CGFloat] = [:] // entry.id → last minY @State private var trackingActive = false @State private var cumulativeDown: CGFloat = 0 @State private var markedByScroll: Set = [] - @State private var wasVisible: Set = [] // entries that were >=50% visible - @State private var viewportHeight: CGFloat = 800 // measured from geometry + @State private var wasVisible: Set = [] + + // Viewport height — measured once from screen bounds. + // Safe because this view only appears on iPhone in portrait/landscape. + private var viewportHeight: CGFloat { UIScreen.main.bounds.height } + + // Dynamic threshold: max(100pt, 20% of viewport) + private var activationThreshold: CGFloat { max(100, viewportHeight * 0.2) } var body: some View { if vm.isLoading && vm.entries.isEmpty { @@ -39,15 +33,6 @@ struct EntryListView: View { .frame(maxWidth: .infinity, maxHeight: .infinity) } else { ScrollView { - // Invisible anchor to measure scroll offset - GeometryReader { geo in - Color.clear.preference( - key: ScrollOffsetKey.self, - value: geo.frame(in: .named("readerScroll")).minY - ) - } - .frame(height: 0) - if isCardView { cardLayout } else { @@ -55,21 +40,11 @@ struct EntryListView: View { } } .coordinateSpace(name: "readerScroll") - .background( - // Measure viewport height once - GeometryReader { geo in - Color.clear.onAppear { viewportHeight = geo.size.height } - } - ) - .onPreferenceChange(ScrollOffsetKey.self) { value in - handleScrollOffset(-value) - } .onAppear { - // Reset tracking on every appear (including back from article). - // Requires genuine downward scroll before marking resumes. + // Reset on every appear (including back from article) trackingActive = false cumulativeDown = 0 - isScrollingDown = false + lastKnownMinY.removeAll() } .refreshable { await vm.refresh() @@ -80,95 +55,62 @@ struct EntryListView: View { } } - // MARK: - Scroll Direction + Activation - - /// Dynamic threshold: max(100pt, 20% of viewport). - /// Taller screens require more scroll before tracking activates. - private var activationThreshold: CGFloat { - max(100, viewportHeight * 0.2) - } - - private func handleScrollOffset(_ newOffset: CGFloat) { - let delta = newOffset - previousOffset - previousOffset = newOffset - - // Ignore micro deltas (<2pt) — layout noise, rubber-banding - guard abs(delta) > 2 else { return } - - if delta > 0 { - // Scrolling down - isScrollingDown = true - cumulativeDown += delta - if cumulativeDown > activationThreshold { - trackingActive = true - } - } else { - // Scrolling up — pause marking, don't reset cumulative - isScrollingDown = false - } - } - - // MARK: - Entry Row with Scroll-Read Tracking + // MARK: - Scroll-Read Tracking (per-row GeometryReader) private func scrollTracked(_ entry: ReaderEntry, content: some View) -> some View { - let debugThis = vm.entries.prefix(3).contains(where: { $0.id == entry.id }) - return content + content .background( GeometryReader { geo in let frame = geo.frame(in: .named("readerScroll")) Color.clear - .onChange(of: frame.minY) { _, _ in + .onChange(of: frame.minY) { _, newMinY in + let entryId = entry.id let entryHeight = frame.height guard entryHeight > 0 else { return } + + // --- Scroll direction + activation --- + if let prev = lastKnownMinY[entryId] { + let delta = prev - newMinY // positive = scrolling down + if delta > 2 { + cumulativeDown += delta + if cumulativeDown > activationThreshold { + trackingActive = true + } + } + // Don't reset trackingActive on upward scroll — + // just don't mark anything. Keeps cumulative progress. + } + lastKnownMinY[entryId] = newMinY + + // --- Visibility tracking --- let visibleTop = max(frame.minY, 0) let visibleBottom = min(frame.maxY, viewportHeight) let visibleHeight = max(visibleBottom - visibleTop, 0) let visibleRatio = visibleHeight / entryHeight - if visibleRatio >= 0.5 { - wasVisible.insert(entry.id) - } - - if debugThis { - print("[SCROLL-VIS] id=\(entry.id) minY=\(Int(frame.minY)) maxY=\(Int(frame.maxY)) height=\(Int(entryHeight)) visH=\(Int(visibleHeight)) ratio=\(String(format:"%.2f", visibleRatio)) vpH=\(Int(viewportHeight)) wasVis=\(wasVisible.contains(entry.id))") - } - } - .onChange(of: frame.maxY) { _, newMaxY in - if debugThis { - print("[SCROLL-MRK] id=\(entry.id) maxY=\(Int(newMaxY)) active=\(trackingActive) down=\(isScrollingDown) read=\(entry.isRead) marked=\(markedByScroll.contains(entry.id)) wasVis=\(wasVisible.contains(entry.id)) cumDown=\(Int(cumulativeDown)) thresh=\(Int(activationThreshold))") + wasVisible.insert(entryId) } + // --- Mark-as-read --- + // Current scroll direction for THIS entry is "down" + // if its minY decreased since last check + let isMovingUp = (lastKnownMinY[entryId] ?? 0) <= newMinY guard trackingActive, - isScrollingDown, + !isMovingUp, !entry.isRead, - !markedByScroll.contains(entry.id), - wasVisible.contains(entry.id), - newMaxY < 0 else { - if debugThis && newMaxY < 0 { - // Log WHY it failed when entry IS above viewport - var reasons: [String] = [] - if !trackingActive { reasons.append("tracking-inactive") } - if !isScrollingDown { reasons.append("not-scrolling-down") } - if entry.isRead { reasons.append("already-read") } - if markedByScroll.contains(entry.id) { reasons.append("already-marked") } - if !wasVisible.contains(entry.id) { reasons.append("never-visible") } - print("[SCROLL-FAIL] id=\(entry.id) maxY=\(Int(newMaxY)) reasons=\(reasons.joined(separator: ","))") - } - return - } + !markedByScroll.contains(entryId), + wasVisible.contains(entryId), + frame.maxY < 0 else { return } - print("[SCROLL-READ] ✅ id=\(entry.id) MARKED AS READ") - markedByScroll.insert(entry.id) + markedByScroll.insert(entryId) - // Local-first: update immediately - if let idx = vm.entries.firstIndex(where: { $0.id == entry.id }) { + if let idx = vm.entries.firstIndex(where: { $0.id == entryId }) { vm.entries[idx].status = "read" } - // API sync in background Task { let api = ReaderAPI() - try? await api.markEntries(ids: [entry.id], status: "read") + try? await api.markEntries(ids: [entryId], status: "read") vm.counters = try? await api.getCounters() } }