From 3d47dddbd2e552877f465c4eb1e470d57042ec5f Mon Sep 17 00:00:00 2001 From: Gigi Date: Thu, 23 Oct 2025 00:02:26 +0200 Subject: [PATCH] refactor(reading): simplify back to basics, remove complex timing logic Removed: - isTrackingEnabled state and delays - Complex composite keys - Verbose debug logging - isTrackingEnabledRef checks Back to simple: - isTextContent = basic check (loading, content exists, not video) - Restore once per articleIdentifier - Save on unmount - Suppression during restore window Much simpler, closer to original working version. --- src/components/ContentPanel.tsx | 71 ++++----------------------------- 1 file changed, 8 insertions(+), 63 deletions(-) diff --git a/src/components/ContentPanel.tsx b/src/components/ContentPanel.tsx index 4916a188..2679fd06 100644 --- a/src/components/ContentPanel.tsx +++ b/src/components/ContentPanel.tsx @@ -150,15 +150,8 @@ const ContentPanel: React.FC = ({ // Get event store for reading position service const eventStore = Hooks.useEventStore() - // Reading position tracking - only for text content that's loaded and long enough - // Wait for content to load, check it's not a video, and verify it's long enough to track - const isTextContent = useMemo(() => { - if (loading) return false - if (!markdown && !html) return false - if (selectedUrl?.includes('youtube') || selectedUrl?.includes('vimeo')) return false - if (!shouldTrackReadingProgress(html, markdown)) return false - return true - }, [loading, markdown, html, selectedUrl]) + // Reading position tracking - only for text content, not videos + const isTextContent = !loading && !!(markdown || html) && !selectedUrl?.includes('youtube') && !selectedUrl?.includes('vimeo') // Generate article identifier for saving/loading position const articleIdentifier = useMemo(() => { @@ -205,28 +198,8 @@ const ContentPanel: React.FC = ({ } }, [activeAccount, relayPool, eventStore, articleIdentifier, settings?.syncReadingPosition, html, markdown]) - // Delay enabling position tracking to ensure content is stable - const [isTrackingEnabled, setIsTrackingEnabled] = useState(false) - - // Reset tracking when article changes - useEffect(() => { - setIsTrackingEnabled(false) - }, [selectedUrl]) - - // Enable tracking after content is stable - useEffect(() => { - if (isTextContent && !isTrackingEnabled) { - // Wait 500ms after content loads before enabling tracking - const timer = setTimeout(() => { - console.log('[reading-position] ✅ Enabling tracking after stability delay') - setIsTrackingEnabled(true) - }, 500) - return () => clearTimeout(timer) - } - }, [isTextContent, isTrackingEnabled]) - const { progressPercentage, saveNow, suppressSavesFor } = useReadingPosition({ - enabled: isTrackingEnabled, + enabled: isTextContent, syncEnabled: settings?.syncReadingPosition !== false, onSave: handleSavePosition, onReadingComplete: () => { @@ -252,45 +225,24 @@ const ContentPanel: React.FC = ({ suppressSavesForRef.current = suppressSavesFor }, [suppressSavesFor]) - // Track if we've successfully started restore for this article + tracking state - // Use a composite key to ensure we only restore once per article when tracking is enabled - const restoreKey = `${articleIdentifier}-${isTrackingEnabled}` + // Track if we've already attempted restore for this article const hasAttemptedRestoreRef = useRef(null) useEffect(() => { - console.log('[reading-position] 🔍 Restore effect running:', { - isTextContent, - isTrackingEnabled, - hasAccount: !!activeAccount, - hasRelayPool: !!relayPool, - hasEventStore: !!eventStore, - articleIdentifier, - restoreKey, - hasAttempted: hasAttemptedRestoreRef.current - }) - if (!isTextContent || !activeAccount || !relayPool || !eventStore || !articleIdentifier) { - console.log('[reading-position] ⏭️ Restore skipped: missing dependencies or not text content') return } if (settings?.syncReadingPosition === false) { - console.log('[reading-position] ⏭️ Restore skipped: sync disabled in settings') - return - } - if (!isTrackingEnabled) { - console.log('[reading-position] ⏭️ Restore skipped: tracking not yet enabled (waiting for content stability)') return } - // Only attempt restore once per article (after tracking is enabled) - if (hasAttemptedRestoreRef.current === restoreKey) { - console.log('[reading-position] ⏭️ Restore skipped: already attempted for this article') + // Only attempt restore once per article + if (hasAttemptedRestoreRef.current === articleIdentifier) { return } console.log('[reading-position] 🔄 Initiating restore for article:', articleIdentifier) - // Mark as attempted using composite key - hasAttemptedRestoreRef.current = restoreKey + hasAttemptedRestoreRef.current = articleIdentifier // Suppress saves during restore window (700ms collection + 500ms render + 500ms buffer = 1700ms) if (suppressSavesForRef.current) { @@ -370,20 +322,13 @@ const ContentPanel: React.FC = ({ // Save position before unmounting or changing article const saveNowRef = useRef(saveNow) - const isTrackingEnabledRef = useRef(isTrackingEnabled) - useEffect(() => { saveNowRef.current = saveNow }, [saveNow]) - - useEffect(() => { - isTrackingEnabledRef.current = isTrackingEnabled - }, [isTrackingEnabled]) useEffect(() => { return () => { - // Only save on unmount if tracking was actually enabled - if (saveNowRef.current && isTrackingEnabledRef.current) { + if (saveNowRef.current) { saveNowRef.current() } }