From a5d94d1aee6a64bb39cf193a1372216678d9cd87 Mon Sep 17 00:00:00 2001 From: Ilyas Hallak Date: Sun, 12 Oct 2025 20:56:59 +0200 Subject: [PATCH] feat: Implement performant scroll progress tracking with PreferenceKey Replace onScrollGeometryChange/onScrollPhaseChange with ContentHeightPreferenceKey approach for improved scroll performance and accurate read progress tracking. Changes: - Add ScrollOffsetPreferenceKey and ContentHeightPreferenceKey for scroll tracking - Track content end position dynamically as WebView loads - Calculate progress from scroll offset relative to total scrollable content - Implement 3% threshold for progress updates to reduce API calls - Add progress locking at 100% to prevent pixel-variation regressions - Guarantee 100% update when reaching end of content - Apply to both BookmarkDetailLegacyView and BookmarkDetailView2 Technical approach: - Place invisible marker at end of content to measure position in scrollView - Update initialContentEndPosition as content grows during WebView loading - Progress = (initialPosition - currentPosition) / (initialPosition - containerHeight) - Lock progress once 100% reached to avoid 100% -> 99% fluctuations --- .../BookmarkDetailLegacyView.swift | 77 ++++++++++++++----- .../BookmarkDetail/BookmarkDetailView2.swift | 67 +++++++++++----- 2 files changed, 102 insertions(+), 42 deletions(-) diff --git a/readeck/UI/BookmarkDetail/BookmarkDetailLegacyView.swift b/readeck/UI/BookmarkDetail/BookmarkDetailLegacyView.swift index 92736b3..674cabd 100644 --- a/readeck/UI/BookmarkDetail/BookmarkDetailLegacyView.swift +++ b/readeck/UI/BookmarkDetail/BookmarkDetailLegacyView.swift @@ -25,7 +25,8 @@ struct BookmarkDetailLegacyView: View { @State private var viewModel: BookmarkDetailViewModel @State private var webViewHeight: CGFloat = 300 - @State private var contentHeight: CGFloat = 0 + @State private var contentEndPosition: CGFloat = 0 + @State private var initialContentEndPosition: CGFloat = 0 @State private var showingFontSettings = false @State private var showingLabelsSheet = false @State private var readingProgress: Double = 0.0 @@ -121,42 +122,76 @@ struct BookmarkDetailLegacyView: View { } .frame(maxWidth: .infinity) } - } - .background( - GeometryReader { contentGeo in - Color.clear.preference( - key: ContentHeightPreferenceKey.self, - value: contentGeo.size.height + + // Invisible marker to measure total content height - placed AFTER all content + Color.clear + .frame(height: 1) + .background( + GeometryReader { endGeo in + Color.clear.preference( + key: ContentHeightPreferenceKey.self, + value: endGeo.frame(in: .named("scrollView")).maxY + ) + } ) - } - ) + } } .coordinateSpace(name: "scrollView") .clipped() .ignoresSafeArea(edges: .top) .scrollPosition($scrollPosition) - .onPreferenceChange(ContentHeightPreferenceKey.self) { height in - contentHeight = height - } - .onPreferenceChange(ScrollOffsetPreferenceKey.self) { offset in - // Calculate progress from scroll offset - let scrollOffset = -offset.y // Negative because scroll goes down + .onPreferenceChange(ContentHeightPreferenceKey.self) { endPosition in + contentEndPosition = endPosition + let containerHeight = geometry.size.height - let maxOffset = contentHeight - containerHeight - guard maxOffset > 0 else { return } + // Update initial position if content grows (WebView still loading) or first time + // We always take the maximum position seen (when scrolled to top, this is total content height) + if endPosition > initialContentEndPosition && endPosition > containerHeight * 1.2 { + initialContentEndPosition = endPosition + print("📏 Content end position updated: \(Int(endPosition)) (container: \(Int(containerHeight)))") + } - let rawProgress = scrollOffset / maxOffset - let progress = min(max(rawProgress, 0), 1) + // Calculate progress from how much the end marker has moved up + guard initialContentEndPosition > 0 else { + print("⏳ Waiting for content to load... current: \(Int(endPosition)), container: \(Int(containerHeight))") + return + } - // Only update if change >= 3% + let totalScrollableDistance = initialContentEndPosition - containerHeight + + guard totalScrollableDistance > 0 else { + print("⚠️ Content not scrollable: initial=\(initialContentEndPosition), container=\(containerHeight)") + return + } + + // How far has the marker moved from its initial position? + let scrolled = initialContentEndPosition - endPosition + let rawProgress = scrolled / totalScrollableDistance + var progress = min(max(rawProgress, 0), 1) + + // Lock progress at 100% once reached (don't go back to 99% due to pixel variations) + if lastSentProgress >= 0.995 { + progress = max(progress, 1.0) + } + + print("📊 Progress: \(Int(progress * 100))% | scrolled: \(Int(scrolled)) / \(Int(totalScrollableDistance)) | endPos: \(Int(endPosition))") + + // Check if we should update: threshold OR reaching 100% for first time let threshold: Double = 0.03 - if abs(progress - lastSentProgress) >= threshold { + let reachedEnd = progress >= 1.0 && lastSentProgress < 1.0 + let shouldUpdate = abs(progress - lastSentProgress) >= threshold || reachedEnd + + if shouldUpdate { + print("✅ Updating progress: \(Int(lastSentProgress * 100))% → \(Int(progress * 100))%\(reachedEnd ? " [END]" : "")") lastSentProgress = progress readingProgress = progress viewModel.debouncedUpdateReadProgress(id: bookmarkId, progress: progress, anchor: nil) } } + .onPreferenceChange(ScrollOffsetPreferenceKey.self) { _ in + // Not needed anymore, we track via ContentHeightPreferenceKey + } } } .frame(maxWidth: .infinity) diff --git a/readeck/UI/BookmarkDetail/BookmarkDetailView2.swift b/readeck/UI/BookmarkDetail/BookmarkDetailView2.swift index 19b4483..6fcb057 100644 --- a/readeck/UI/BookmarkDetail/BookmarkDetailView2.swift +++ b/readeck/UI/BookmarkDetail/BookmarkDetailView2.swift @@ -10,7 +10,8 @@ struct BookmarkDetailView2: View { @State private var viewModel: BookmarkDetailViewModel @State private var webViewHeight: CGFloat = 300 - @State private var contentHeight: CGFloat = 0 + @State private var contentEndPosition: CGFloat = 0 + @State private var initialContentEndPosition: CGFloat = 0 @State private var showingFontSettings = false @State private var showingLabelsSheet = false @State private var readingProgress: Double = 0.0 @@ -120,42 +121,66 @@ struct BookmarkDetailView2: View { } .frame(maxWidth: .infinity) } - } - .background( - GeometryReader { contentGeo in - Color.clear.preference( - key: ContentHeightPreferenceKey.self, - value: contentGeo.size.height + + // Invisible marker to measure total content height - placed AFTER all content + Color.clear + .frame(height: 1) + .background( + GeometryReader { endGeo in + Color.clear.preference( + key: ContentHeightPreferenceKey.self, + value: endGeo.frame(in: .named("scrollView")).maxY + ) + } ) - } - ) + } } .coordinateSpace(name: "scrollView") .clipped() .ignoresSafeArea(edges: .top) .scrollPosition($scrollPosition) - .onPreferenceChange(ContentHeightPreferenceKey.self) { height in - contentHeight = height - } - .onPreferenceChange(ScrollOffsetPreferenceKey.self) { offset in - // Calculate progress from scroll offset - let scrollOffset = -offset.y + .onPreferenceChange(ContentHeightPreferenceKey.self) { endPosition in + contentEndPosition = endPosition + let containerHeight = geometry.size.height - let maxOffset = contentHeight - containerHeight - guard maxOffset > 0 else { return } + // Update initial position if content grows (WebView still loading) or first time + // We always take the maximum position seen (when scrolled to top, this is total content height) + if endPosition > initialContentEndPosition && endPosition > containerHeight * 1.2 { + initialContentEndPosition = endPosition + } - let rawProgress = scrollOffset / maxOffset - let progress = min(max(rawProgress, 0), 1) + // Calculate progress from how much the end marker has moved up + guard initialContentEndPosition > 0 else { return } - // Only update if change >= 3% + let totalScrollableDistance = initialContentEndPosition - containerHeight + + guard totalScrollableDistance > 0 else { return } + + // How far has the marker moved from its initial position? + let scrolled = initialContentEndPosition - endPosition + let rawProgress = scrolled / totalScrollableDistance + var progress = min(max(rawProgress, 0), 1) + + // Lock progress at 100% once reached (don't go back to 99% due to pixel variations) + if lastSentProgress >= 0.995 { + progress = max(progress, 1.0) + } + + // Check if we should update: threshold OR reaching 100% for first time let threshold: Double = 0.03 - if abs(progress - lastSentProgress) >= threshold { + let reachedEnd = progress >= 1.0 && lastSentProgress < 1.0 + let shouldUpdate = abs(progress - lastSentProgress) >= threshold || reachedEnd + + if shouldUpdate { lastSentProgress = progress readingProgress = progress viewModel.debouncedUpdateReadProgress(id: bookmarkId, progress: progress, anchor: nil) } } + .onPreferenceChange(ScrollOffsetPreferenceKey.self) { _ in + // Not needed anymore, we track via ContentHeightPreferenceKey + } } }