From eb8cb3854c094819e778300f103117aaf4c00e8f Mon Sep 17 00:00:00 2001
From: Jackson Harper <jacksonh@gmail.com>
Date: Fri, 17 Jun 2022 15:59:41 -0700
Subject: [PATCH] Do all scroll watching on the main window

This moves all scrolling from child divs to the main window. This
improves our keyboard handling, as focus will be given to the
body element, not the child div when navigating by keyboard
commands, so arrow keys and space bar will work after navigating
to the reader with the keyboard commands.
---
 packages/appreader/src/index.jsx              |  1 -
 .../web/components/patterns/PrimaryHeader.tsx | 36 ++++++++++---------
 .../components/templates/PrimaryLayout.tsx    | 16 +++------
 .../components/templates/article/Article.tsx  | 24 ++-----------
 .../templates/article/ArticleContainer.tsx    |  2 --
 .../templates/homeFeed/HomeFeedContainer.tsx  | 17 ++++-----
 packages/web/lib/hooks/useFetchMoreScroll.tsx | 32 ++++-------------
 packages/web/lib/hooks/useScrollWatcher.tsx   | 31 +++-------------
 .../web/pages/[username]/[slug]/index.tsx     |  2 --
 .../web/pages/app/[username]/[slug]/index.tsx |  1 -
 packages/web/pages/home.tsx                   |  7 ++--
 11 files changed, 47 insertions(+), 122 deletions(-)

diff --git a/packages/appreader/src/index.jsx b/packages/appreader/src/index.jsx
index 8dde91f96..424bb517e 100644
--- a/packages/appreader/src/index.jsx
+++ b/packages/appreader/src/index.jsx
@@ -36,7 +36,6 @@ const App = () => {
           <ArticleContainer
             article={window.omnivoreArticle}
             labels={window.omnivoreArticle.labels}
-            scrollElementRef={React.createRef()}
             isAppleAppEmbed={true}
             highlightBarDisabled={true}
             highlightsBaseURL="https://example.com"
diff --git a/packages/web/components/patterns/PrimaryHeader.tsx b/packages/web/components/patterns/PrimaryHeader.tsx
index 084b8512e..97bc38c33 100644
--- a/packages/web/components/patterns/PrimaryHeader.tsx
+++ b/packages/web/components/patterns/PrimaryHeader.tsx
@@ -5,10 +5,6 @@ import { darkenTheme, lightenTheme, updateTheme } from '../../lib/themeUpdater'
 import { AvatarDropdown } from './../elements/AvatarDropdown'
 import { ThemeId } from './../tokens/stitches.config'
 import { useCallback, useEffect, useState } from 'react'
-import {
-  ScrollOffsetChangeset,
-  useScrollWatcher,
-} from '../../lib/hooks/useScrollWatcher'
 import { useRouter } from 'next/router'
 import { useKeyboardShortcuts } from '../../lib/keyboardShortcuts/useKeyboardShortcuts'
 import { primaryCommands } from '../../lib/keyboardShortcuts/navigationShortcuts'
@@ -21,7 +17,6 @@ type HeaderProps = {
   hideHeader?: boolean
   profileImageURL?: string
   isTransparent: boolean
-  scrollElementRef?: React.RefObject<HTMLDivElement>
   toolbarControl?: JSX.Element
   alwaysDisplayToolbar?: boolean
   setShowLogoutConfirmation: (showShareModal: boolean) => void
@@ -48,19 +43,26 @@ export function PrimaryHeader(props: HeaderProps): JSX.Element {
     })
   )
 
-  const setScrollWatchedElement = useScrollWatcher(
-    (changeset: ScrollOffsetChangeset) => {
-      const isScrolledBeyondMinThreshold = changeset.current.y >= 50
-      const isScrollingDown = changeset.current.y > changeset.previous.y
+/*
+  useRegisterActions([
+    {
+      id: 'lightTheme',
+      section: 'Preferences',
+      name: 'Change theme (lighter) ',
+      shortcut: ['v', 'l'],
+      keywords: 'light theme',
+      perform: () => lightenTheme(),
     },
-    0
-  )
-
-  useEffect(() => {
-    if (props.scrollElementRef) {
-      setScrollWatchedElement(props.scrollElementRef.current)
-    }
-  }, [props.scrollElementRef, setScrollWatchedElement])
+    {
+      id: 'darkTheme',
+      section: 'Preferences',
+      name: 'Change theme (darker) ',
+      shortcut: ['v', 'd'],
+      keywords: 'dark theme',
+      perform: () => darkenTheme(),
+    },
+  ])
+  */
 
   const initAnalytics = useCallback(() => {
     setupAnalytics(props.user)
diff --git a/packages/web/components/templates/PrimaryLayout.tsx b/packages/web/components/templates/PrimaryLayout.tsx
index 973529cf3..295e7e1a8 100644
--- a/packages/web/components/templates/PrimaryLayout.tsx
+++ b/packages/web/components/templates/PrimaryLayout.tsx
@@ -21,7 +21,6 @@ type PrimaryLayoutProps = {
   pageTestId: string
   hideHeader?: boolean
   pageMetaDataProps?: PageMetaDataProps
-  scrollElementRef?: MutableRefObject<HTMLDivElement | null>
   headerToolbarControl?: JSX.Element
   alwaysDisplayToolbar?: boolean
 }
@@ -63,8 +62,8 @@ export function PrimaryLayout(props: PrimaryLayoutProps): JSX.Element {
         <PageMetaData {...props.pageMetaDataProps} />
       ) : null}
       <Box css={{
-        height: '100vh',
-        width: '100vw',
+        minHeight: '100vh',
+        minWidth: '100vw',
         bg: 'transparent',
         '@smDown': {
           bg: '$grayBase',
@@ -76,24 +75,19 @@ export function PrimaryLayout(props: PrimaryLayoutProps): JSX.Element {
           userInitials={viewerData?.me?.name.charAt(0) ?? ''}
           profileImageURL={viewerData?.me?.profile.pictureUrl}
           isTransparent={true}
-          scrollElementRef={props.scrollElementRef}
           toolbarControl={props.headerToolbarControl}
           alwaysDisplayToolbar={props.alwaysDisplayToolbar}
           setShowLogoutConfirmation={setShowLogoutConfirmation}
           setShowKeyboardCommandsModal={setShowKeyboardCommandsModal}
         />
         <Box
-          ref={props.scrollElementRef}
           css={{
-            position: 'fixed',
-            overflowY: 'auto',
-            height: '100%',
-            width: '100vw',
+            minHeight: '100%',
+            minWidth: '100vw',
             bg: '$grayBase',
           }}
         >
-                  <Box
-          ref={props.scrollElementRef}
+          <Box
           css={{
             height: '48px',
             bg: '$grayBase',
diff --git a/packages/web/components/templates/article/Article.tsx b/packages/web/components/templates/article/Article.tsx
index c826f7f03..69064446a 100644
--- a/packages/web/components/templates/article/Article.tsx
+++ b/packages/web/components/templates/article/Article.tsx
@@ -25,7 +25,6 @@ export type ArticleProps = {
   initialAnchorIndex: number
   initialReadingProgress?: number
   highlightHref: MutableRefObject<string | null>
-  scrollElementRef: MutableRefObject<HTMLDivElement | null>
   articleMutations: ArticleMutations
 }
 
@@ -89,16 +88,9 @@ export function Article(props: ArticleProps): JSX.Element {
     }
   }, [readingProgress])
 
-  const setScrollWatchedElement = useScrollWatcher(
+  useScrollWatcher(
     (changeset: ScrollOffsetChangeset) => {
-      const scrollContainer = props.scrollElementRef.current
-      if (scrollContainer) {
-        const newReadingProgress =
-          (changeset.current.y + scrollContainer.clientHeight) /
-          scrollContainer.scrollHeight
-
-        debouncedSetReadingProgress(newReadingProgress * 100)
-      } else if (window && window.document.scrollingElement) {
+      if (window && window.document.scrollingElement) {
         const newReadingProgress =
           window.scrollY / window.document.scrollingElement.scrollHeight
         const adjustedReadingProgress =
@@ -131,10 +123,6 @@ export function Article(props: ArticleProps): JSX.Element {
     []
   )
 
-  useEffect(() => {
-    setScrollWatchedElement(props.scrollElementRef.current)
-  }, [props.scrollElementRef, setScrollWatchedElement])
-
   // Scroll to initial anchor position
   useEffect(() => {
     if (typeof window === 'undefined') {
@@ -175,17 +163,11 @@ export function Article(props: ArticleProps): JSX.Element {
         }
 
         const calculatedOffset = calculateOffset(anchorElement)
-
-        if (props.scrollElementRef.current) {
-          props.scrollElementRef.current?.scroll(0, calculatedOffset - 100)
-        } else {
-          window.document.documentElement.scroll(0, calculatedOffset - 100)
-        }
+        window.document.documentElement.scroll(0, calculatedOffset - 100)
       }
     }
   }, [
     props.highlightReady,
-    props.scrollElementRef,
     props.initialAnchorIndex,
     props.initialReadingProgress,
     shouldScrollToInitialPosition,
diff --git a/packages/web/components/templates/article/ArticleContainer.tsx b/packages/web/components/templates/article/ArticleContainer.tsx
index 1811e9cf0..a79e415c6 100644
--- a/packages/web/components/templates/article/ArticleContainer.tsx
+++ b/packages/web/components/templates/article/ArticleContainer.tsx
@@ -24,7 +24,6 @@ type ArticleContainerProps = {
   article: ArticleAttributes
   labels: Label[]
   articleMutations: ArticleMutations
-  scrollElementRef: MutableRefObject<HTMLDivElement | null>
   isAppleAppEmbed: boolean
   highlightBarDisabled: boolean
   highlightsBaseURL: string
@@ -281,7 +280,6 @@ export function ArticleContainer(props: ArticleContainerProps): JSX.Element {
           articleId={props.article.id}
           content={props.article.content}
           initialAnchorIndex={props.article.readingProgressAnchorIndex}
-          scrollElementRef={props.scrollElementRef}
           articleMutations={props.articleMutations}
         />
         <Button
diff --git a/packages/web/components/templates/homeFeed/HomeFeedContainer.tsx b/packages/web/components/templates/homeFeed/HomeFeedContainer.tsx
index dc8de6f77..70274a419 100644
--- a/packages/web/components/templates/homeFeed/HomeFeedContainer.tsx
+++ b/packages/web/components/templates/homeFeed/HomeFeedContainer.tsx
@@ -31,7 +31,7 @@ import {
   createReminderMutation,
   ReminderType,
 } from '../../../lib/networking/mutations/createReminderMutation'
-import { useFetchMoreScroll } from '../../../lib/hooks/useFetchMoreScroll'
+import { useFetchMore } from '../../../lib/hooks/useFetchMoreScroll'
 import { usePersistedState } from '../../../lib/hooks/usePersistedState'
 import { showErrorToast, showSuccessToast } from '../../../lib/toastHelpers'
 import { ConfirmationModal } from '../../patterns/ConfirmationModal'
@@ -48,7 +48,6 @@ import { EditTitleModal } from './EditTitleModal'
 export type LayoutType = 'LIST_LAYOUT' | 'GRID_LAYOUT'
 
 export type HomeFeedContainerProps = {
-  scrollElementRef: React.RefObject<HTMLDivElement>
 }
 
 const timeZoneHourDiff = -new Date().getTimezoneOffset() / 60
@@ -69,7 +68,7 @@ export function HomeFeedContainer(props: HomeFeedContainerProps): JSX.Element {
   const { viewerData } = useGetViewerQuery()
   const router = useRouter()
   const defaultQuery = {
-    limit: 10,
+    limit: 5,
     sortDescending: true,
     searchQuery: undefined,
   }
@@ -175,7 +174,8 @@ export function HomeFeedContainer(props: HomeFeedContainerProps): JSX.Element {
     [libraryItems]
   )
 
-  const isVisible = function (ele: HTMLElement, container: HTMLElement) {
+  const isVisible = function (ele: HTMLElement) {
+    const container = window.document.documentElement
     const eleTop = ele.offsetTop
     const eleBottom = eleTop + ele.clientHeight
 
@@ -192,8 +192,7 @@ export function HomeFeedContainer(props: HomeFeedContainerProps): JSX.Element {
         if (target) {
           try {
             if (
-              props.scrollElementRef.current &&
-              !isVisible(target, props.scrollElementRef.current)
+              !isVisible(target)
             ) {
               target.scrollIntoView({
                 block: 'center',
@@ -424,11 +423,7 @@ export function HomeFeedContainer(props: HomeFeedContainerProps): JSX.Element {
     })
   )
 
-  const setFetchMoreRef = useFetchMoreScroll(handleFetchMore)
-
-  useEffect(() => {
-    setFetchMoreRef(props.scrollElementRef.current)
-  }, [props.scrollElementRef, setFetchMoreRef])
+  useFetchMore(handleFetchMore)
 
   return (
     <HomeFeedGrid
diff --git a/packages/web/lib/hooks/useFetchMoreScroll.tsx b/packages/web/lib/hooks/useFetchMoreScroll.tsx
index 685626e2e..f454645e0 100644
--- a/packages/web/lib/hooks/useFetchMoreScroll.tsx
+++ b/packages/web/lib/hooks/useFetchMoreScroll.tsx
@@ -1,29 +1,11 @@
-import { useCallback, useEffect, useRef, useState } from 'react'
+import { useEffect, useRef, useState } from 'react'
 
-type SetRef = (node: HTMLDivElement | null) => void
-
-export const useFetchMoreScroll = (
-  callback: () => void,
-): SetRef => {
-  const [scrollableElement, setScrollableElement] =useState<HTMLDivElement | null>(null)
-
-  useFetchMoreInternal(scrollableElement, callback)
-
-  const ref = useRef<HTMLDivElement | null>(null)
-  const setRef = useCallback((node) => {
-    setScrollableElement(node)
-    ref.current = node
-  }, [])
-
-  return setRef
-}
-
-const useFetchMoreInternal = (node: HTMLDivElement | null, callback: () => void, delay = 500): void => {
+export const useFetchMore = (callback: () => void, delay = 500): void => {
   const [first, setFirst] = useState(true)
   const throttleTimeout = useRef<NodeJS.Timeout | undefined>(undefined)
 
   useEffect(() => {
-    if (typeof window === 'undefined' || !node) {
+    if (typeof window === 'undefined') {
       return
     }
 
@@ -32,7 +14,7 @@ const useFetchMoreInternal = (node: HTMLDivElement | null, callback: () => void,
         scrollTop,
         scrollHeight,
         clientHeight
-      } = node;
+      } = window.document.documentElement;
 
       if (scrollTop + clientHeight >= scrollHeight - (scrollHeight / 3)) {
         callback()
@@ -51,10 +33,10 @@ const useFetchMoreInternal = (node: HTMLDivElement | null, callback: () => void,
       }
     }
 
-    node.addEventListener('scroll', handleScroll)
+    window.addEventListener('scroll', handleScroll)
 
     return () => {
-      node.removeEventListener('scroll', handleScroll)
+      window.removeEventListener('scroll', handleScroll)
     }
-  }, [node, callback, delay, first, setFirst])
+  }, [callback, delay, first, setFirst])
 }
diff --git a/packages/web/lib/hooks/useScrollWatcher.tsx b/packages/web/lib/hooks/useScrollWatcher.tsx
index db1c43a9e..fb65894e2 100644
--- a/packages/web/lib/hooks/useScrollWatcher.tsx
+++ b/packages/web/lib/hooks/useScrollWatcher.tsx
@@ -14,26 +14,6 @@ type Effect = (offset: ScrollOffsetChangeset) => void
 
 export function useScrollWatcher(
   effect: Effect,
-  interval: number
-): (node: HTMLDivElement | null) => void {
-  const [scrollableElement, setScrollableElement] =
-    useState<HTMLDivElement | null>(null)
-
-  useScrollWatcherInternal(effect, scrollableElement, interval)
-
-  const ref = useRef<HTMLDivElement | null>(null)
-
-  const setRef = useCallback((node) => {
-    setScrollableElement(node)
-    ref.current = node
-  }, [])
-
-  return setRef
-}
-
-function useScrollWatcherInternal(
-  effect: Effect,
-  element: HTMLDivElement | null,
   delay: number
 ): void {
   const throttleTimeout = useRef<NodeJS.Timeout | undefined>(undefined)
@@ -45,8 +25,8 @@ function useScrollWatcherInternal(
   useEffect(() => {
     const callback = () => {
       const newOffset = {
-        x: element?.scrollLeft ?? window?.scrollX ?? 0,
-        y: element?.scrollTop ?? window?.scrollY ?? 0,
+        x: window.document.documentElement.scrollLeft ?? window?.scrollX ?? 0,
+        y: window.document.documentElement.scrollTop ?? window?.scrollY ?? 0,
       }
       effect({ current: newOffset, previous: currentOffset })
       setCurrentOffset(newOffset)
@@ -59,9 +39,8 @@ function useScrollWatcherInternal(
       }
     }
 
-    (element ?? window)?.addEventListener('scroll', handleScroll)
-
+    window.addEventListener('scroll', handleScroll)
     return () =>
-      (element ?? window)?.removeEventListener('scroll', handleScroll)
-  }, [currentOffset, delay, effect, element])
+      window.removeEventListener('scroll', handleScroll)
+  }, [currentOffset, delay, effect])
 }
diff --git a/packages/web/pages/[username]/[slug]/index.tsx b/packages/web/pages/[username]/[slug]/index.tsx
index 8a8da1a92..14416db87 100644
--- a/packages/web/pages/[username]/[slug]/index.tsx
+++ b/packages/web/pages/[username]/[slug]/index.tsx
@@ -146,7 +146,6 @@ export default function Home(): JSX.Element {
   return (
     <PrimaryLayout
       pageTestId="home-page-tag"
-      scrollElementRef={scrollRef}
       headerToolbarControl={
         <ArticleActionsMenu
           article={article}
@@ -219,7 +218,6 @@ export default function Home(): JSX.Element {
             {article && viewerData?.me ? (
               <ArticleContainer
                 article={article}
-                scrollElementRef={scrollRef}
                 isAppleAppEmbed={false}
                 highlightBarDisabled={false}
                 highlightsBaseURL={`${webBaseURL}/${viewerData.me?.profile?.username}/${slug}/highlights`}
diff --git a/packages/web/pages/app/[username]/[slug]/index.tsx b/packages/web/pages/app/[username]/[slug]/index.tsx
index 2a85b4aa7..f7498acc4 100644
--- a/packages/web/pages/app/[username]/[slug]/index.tsx
+++ b/packages/web/pages/app/[username]/[slug]/index.tsx
@@ -93,7 +93,6 @@ function AppArticleEmbedContent(
         >
           <ArticleContainer
             article={articleData.article.article}
-            scrollElementRef={scrollRef}
             isAppleAppEmbed={true}
             highlightBarDisabled={props.highlightBarDisabled}
             highlightsBaseURL={`${webBaseURL}/${props.username}/${props.slug}/highlights`}
diff --git a/packages/web/pages/home.tsx b/packages/web/pages/home.tsx
index 7ddcb4c18..1e1201f72 100644
--- a/packages/web/pages/home.tsx
+++ b/packages/web/pages/home.tsx
@@ -8,19 +8,16 @@ export default function Home(): JSX.Element {
 }
 
 function LoadedContent(): JSX.Element {
-  const scrollRef = useRef<HTMLDivElement | null>(null)
-
   return (
     <PrimaryLayout
       pageMetaDataProps={{
         title: 'Home - Omnivore',
         path: '/home',
       }}
-      scrollElementRef={scrollRef}
       pageTestId="home-page-tag"
     >
-      <VStack alignment="center" distribution="center" ref={scrollRef}>
-        <HomeFeedContainer scrollElementRef={scrollRef} />
+      <VStack alignment="center" distribution="center">
+        <HomeFeedContainer />
       </VStack>
     </PrimaryLayout>
   )