Merge pull request #821 from omnivore-app/fix/document-scrolling

Do all scroll watching on the main window

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"

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)

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',

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,

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

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'
@@ -47,10 +47,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
 
 const SAVED_SEARCHES: Record<string, string> = {
@@ -65,11 +61,11 @@ const SAVED_SEARCHES: Record<string, string> = {
   Newsletters: `in:inbox label:Newsletter`,
 }
 
-export function HomeFeedContainer(props: HomeFeedContainerProps): JSX.Element {
+export function HomeFeedContainer(): JSX.Element {
   const { viewerData } = useGetViewerQuery()
   const router = useRouter()
   const defaultQuery = {
-    limit: 10,
+    limit: 5,
     sortDescending: true,
     searchQuery: undefined,
   }
@@ -175,7 +171,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 +189,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 +420,7 @@ export function HomeFeedContainer(props: HomeFeedContainerProps): JSX.Element {
     })
   )
 
-  const setFetchMoreRef = useFetchMoreScroll(handleFetchMore)
-
-  useEffect(() => {
-    setFetchMoreRef(props.scrollElementRef.current)
-  }, [props.scrollElementRef, setFetchMoreRef])
+  useFetchMore(handleFetchMore)
 
   return (
     <HomeFeedGrid

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])
 }

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])
 }

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`}

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`}

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>
   )