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.
2022-06-17 15:59:41 -07:00
parent 65b8893fc0
commit eb8cb3854c
11 changed files with 47 additions and 122 deletions
--- 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"
--- 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)
--- 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',
--- 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,
--- 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
--- 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
--- 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])
 }
--- 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])
 }
--- 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`}
--- 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`}
--- 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>
  )