fix: scroll mark-as-read — move all tracking to per-row GeometryReader
All checks were successful
Security Checks / dependency-audit (push) Successful in 14s
Security Checks / secret-scanning (push) Successful in 4s
Security Checks / dockerfile-lint (push) Successful in 4s

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) <noreply@anthropic.com>
This commit is contained in:
Yusuf Suleman
2026-04-04 00:06:24 -05:00
parent 93bdffaae5
commit 6cff4a9036

View File

@@ -1,28 +1,22 @@
import SwiftUI 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 { struct EntryListView: View {
@Bindable var vm: ReaderViewModel @Bindable var vm: ReaderViewModel
var isCardView: Bool = true var isCardView: Bool = true
// Scroll-based read tracking // Scroll-based read tracking all driven from per-row GeometryReader
@State private var previousOffset: CGFloat = 0 @State private var lastKnownMinY: [Int: CGFloat] = [:] // entry.id last minY
@State private var isScrollingDown = false
@State private var trackingActive = false @State private var trackingActive = false
@State private var cumulativeDown: CGFloat = 0 @State private var cumulativeDown: CGFloat = 0
@State private var markedByScroll: Set<Int> = [] @State private var markedByScroll: Set<Int> = []
@State private var wasVisible: Set<Int> = [] // entries that were >=50% visible @State private var wasVisible: Set<Int> = []
@State private var viewportHeight: CGFloat = 800 // measured from geometry
// 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 { var body: some View {
if vm.isLoading && vm.entries.isEmpty { if vm.isLoading && vm.entries.isEmpty {
@@ -39,15 +33,6 @@ struct EntryListView: View {
.frame(maxWidth: .infinity, maxHeight: .infinity) .frame(maxWidth: .infinity, maxHeight: .infinity)
} else { } else {
ScrollView { 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 { if isCardView {
cardLayout cardLayout
} else { } else {
@@ -55,21 +40,11 @@ struct EntryListView: View {
} }
} }
.coordinateSpace(name: "readerScroll") .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 { .onAppear {
// Reset tracking on every appear (including back from article). // Reset on every appear (including back from article)
// Requires genuine downward scroll before marking resumes.
trackingActive = false trackingActive = false
cumulativeDown = 0 cumulativeDown = 0
isScrollingDown = false lastKnownMinY.removeAll()
} }
.refreshable { .refreshable {
await vm.refresh() await vm.refresh()
@@ -80,95 +55,62 @@ struct EntryListView: View {
} }
} }
// MARK: - Scroll Direction + Activation // MARK: - Scroll-Read Tracking (per-row GeometryReader)
/// 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
private func scrollTracked(_ entry: ReaderEntry, content: some View) -> some View { private func scrollTracked(_ entry: ReaderEntry, content: some View) -> some View {
let debugThis = vm.entries.prefix(3).contains(where: { $0.id == entry.id }) content
return content
.background( .background(
GeometryReader { geo in GeometryReader { geo in
let frame = geo.frame(in: .named("readerScroll")) let frame = geo.frame(in: .named("readerScroll"))
Color.clear Color.clear
.onChange(of: frame.minY) { _, _ in .onChange(of: frame.minY) { _, newMinY in
let entryId = entry.id
let entryHeight = frame.height let entryHeight = frame.height
guard entryHeight > 0 else { return } 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 visibleTop = max(frame.minY, 0)
let visibleBottom = min(frame.maxY, viewportHeight) let visibleBottom = min(frame.maxY, viewportHeight)
let visibleHeight = max(visibleBottom - visibleTop, 0) let visibleHeight = max(visibleBottom - visibleTop, 0)
let visibleRatio = visibleHeight / entryHeight let visibleRatio = visibleHeight / entryHeight
if visibleRatio >= 0.5 { if visibleRatio >= 0.5 {
wasVisible.insert(entry.id) wasVisible.insert(entryId)
}
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))")
} }
// --- 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, guard trackingActive,
isScrollingDown, !isMovingUp,
!entry.isRead, !entry.isRead,
!markedByScroll.contains(entry.id), !markedByScroll.contains(entryId),
wasVisible.contains(entry.id), wasVisible.contains(entryId),
newMaxY < 0 else { frame.maxY < 0 else { return }
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
}
print("[SCROLL-READ] ✅ id=\(entry.id) MARKED AS READ") markedByScroll.insert(entryId)
markedByScroll.insert(entry.id)
// Local-first: update immediately if let idx = vm.entries.firstIndex(where: { $0.id == entryId }) {
if let idx = vm.entries.firstIndex(where: { $0.id == entry.id }) {
vm.entries[idx].status = "read" vm.entries[idx].status = "read"
} }
// API sync in background
Task { Task {
let api = ReaderAPI() 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() vm.counters = try? await api.getCounters()
} }
} }