remove ScrollArea as it causes performance issues on chrome (#1087)

Athou · Jun 24, 2023 · bdcfbc2 · bdcfbc2
1 parent 53b06f4
commit bdcfbc2
Show file tree

Hide file tree

Showing 6 changed files with 26 additions and 57 deletions.
diff --git a/commafeed-client/src/app/constants.ts b/commafeed-client/src/app/constants.ts
@@ -94,7 +94,6 @@ export const Constants = {
         isBottomVisible: (div: HTMLElement) => div.getBoundingClientRect().bottom <= window.innerHeight,
     },
     dom: {
-        mainScrollAreaId: "main-scroll-area-id",
         entryId: (entry: Entry) => `entry-id-${entry.id}`,
     },
     browserExtensionUrl: "https://github.com/Athou/commafeed-browser-extension",

diff --git a/commafeed-client/src/app/slices/entries.ts b/commafeed-client/src/app/slices/entries.ts
@@ -204,18 +204,14 @@ export const selectEntry = createAsyncThunk<
     }
 })
 const scrollToEntry = (entryElement: HTMLElement, scrollSpeed: number | undefined, onScrollEnded: () => void) => {
-    const scrollArea = document.getElementById(Constants.dom.mainScrollAreaId)
-    if (scrollArea) {
-        scrollToWithCallback({
-            element: scrollArea,
-            options: {
-                // add a small gap between the top of the content and the top of the page
-                top: entryElement.offsetTop - 3,
-                behavior: scrollSpeed && scrollSpeed > 0 ? "smooth" : "auto",
-            },
-            onScrollEnded,
-        })
-    }
+    scrollToWithCallback({
+        options: {
+            // add a small gap between the top of the content and the top of the page
+            top: entryElement.offsetTop - Constants.layout.headerHeight - 3,
+            behavior: scrollSpeed && scrollSpeed > 0 ? "smooth" : "auto",
+        },
+        onScrollEnded,
+    })
 }
 
 export const selectPreviousEntry = createAsyncThunk<

diff --git a/commafeed-client/src/app/utils.ts b/commafeed-client/src/app/utils.ts
@@ -26,30 +26,22 @@ export const calculatePlaceholderSize = ({ width, height, maxWidth }: { width?:
     return { width: placeholderWidth, height: placeholderHeight }
 }
 
-export const scrollToWithCallback = ({
-    element,
-    options,
-    onScrollEnded,
-}: {
-    element: HTMLElement
-    options: ScrollToOptions
-    onScrollEnded: () => void
-}) => {
+export const scrollToWithCallback = ({ options, onScrollEnded }: { options: ScrollToOptions; onScrollEnded: () => void }) => {
     const offset = (options.top ?? 0).toFixed()
 
     const onScroll = () => {
-        if (element.offsetTop.toFixed() === offset) {
-            element.removeEventListener("scroll", onScroll)
+        if (window.scrollY.toFixed() === offset) {
+            window.removeEventListener("scroll", onScroll)
             onScrollEnded()
         }
     }
 
-    element.addEventListener("scroll", onScroll)
+    window.addEventListener("scroll", onScroll)
 
     // scrollTo does not trigger if there's nothing to do, trigger it manually
     onScroll()
 
-    element.scrollTo(options)
+    window.scrollTo(options)
 }
 
 export const truncate = (str: string, n: number) => (str.length > n ? `${str.slice(0, n - 1)}\u2026` : str)
diff --git a/commafeed-client/src/components/content/FeedEntries.tsx b/commafeed-client/src/components/content/FeedEntries.tsx
@@ -74,8 +74,6 @@ export function FeedEntries() {
     const swipedRight = (entry: ExpendableEntry) => dispatch(markEntry({ entry, read: !entry.read }))
 
     useEffect(() => {
-        const scrollArea = document.getElementById(Constants.dom.mainScrollAreaId)
-
         const listener = () => {
             if (viewMode !== "expanded") return
             if (scrollingToEntry) return
@@ -100,8 +98,8 @@ export function FeedEntries() {
             }
         }
         const throttledListener = throttle(100, listener)
-        scrollArea?.addEventListener("scroll", throttledListener)
-        return () => scrollArea?.removeEventListener("scroll", throttledListener)
+        window.addEventListener("scroll", throttledListener)
+        return () => window.removeEventListener("scroll", throttledListener)
     }, [dispatch, entries, viewMode, scrollMarks, scrollingToEntry])
 
     useMousetrap("r", () => dispatch(reloadEntries()))
@@ -154,9 +152,8 @@ export function FeedEntries() {
                         })
                     )
                 } else {
-                    const scrollArea = document.getElementById(Constants.dom.mainScrollAreaId)
-                    scrollArea?.scrollTo({
-                        top: scrollArea.scrollTop + scrollArea.clientHeight * 0.8,
+                    window.scrollTo({
+                        top: window.scrollY + document.documentElement.clientHeight * 0.8,
                         behavior: "smooth",
                     })
                 }
@@ -193,9 +190,8 @@ export function FeedEntries() {
                         })
                     )
                 } else {
-                    const scrollArea = document.getElementById(Constants.dom.mainScrollAreaId)
-                    scrollArea?.scrollTo({
-                        top: scrollArea.scrollTop - scrollArea.clientHeight * 0.8,
+                    window.scrollTo({
+                        top: window.scrollY - document.documentElement.clientHeight * 0.8,
                         behavior: "smooth",
                     })
                 }
@@ -267,8 +263,6 @@ export function FeedEntries() {
             loadMore={() => dispatch(loadMoreEntries())}
             hasMore={hasMore}
             loader={<Loader key={0} />}
-            useWindow={false}
-            getScrollParent={() => document.getElementById(Constants.dom.mainScrollAreaId)}
         >
             {entries.map(entry => (
                 <div

diff --git a/commafeed-client/src/components/content/FeedEntryContextMenu.tsx b/commafeed-client/src/components/content/FeedEntryContextMenu.tsx
@@ -1,6 +1,5 @@
 import { Trans } from "@lingui/macro"
 import { createStyles, Group } from "@mantine/core"
-import { Constants } from "app/constants"
 import { markEntriesUpToEntry, markEntry, starEntry } from "app/slices/entries"
 import { redirectToFeed } from "app/slices/redirect"
 import { useAppDispatch, useAppSelector } from "app/store"
@@ -117,13 +116,11 @@ export function useFeedEntryContextMenu(entry: Entry) {
 
     // close context menu on scroll
     useEffect(() => {
-        const scrollArea = document.getElementById(Constants.dom.mainScrollAreaId)
-
         const listener = () => contextMenu.hideAll()
         const throttledListener = throttle(100, listener)
 
-        scrollArea?.addEventListener("scroll", throttledListener)
-        return () => scrollArea?.removeEventListener("scroll", throttledListener)
+        window.addEventListener("scroll", throttledListener)
+        return () => window.removeEventListener("scroll", throttledListener)
     }, [contextMenu])
 
     return { onContextMenu }

diff --git a/commafeed-client/src/pages/app/Layout.tsx b/commafeed-client/src/pages/app/Layout.tsx
@@ -13,7 +13,6 @@ import {
     Title,
     useMantineTheme,
 } from "@mantine/core"
-import { useViewportSize } from "@mantine/hooks"
 import { Constants } from "app/constants"
 import { redirectToAdd, redirectToRootCategory } from "app/slices/redirect"
 import { reloadTree, setMobileMenuOpen, setSidebarWidth } from "app/slices/tree"
@@ -91,7 +90,6 @@ function LogoAndTitle() {
 export default function Layout(props: LayoutProps) {
     const { classes } = useStyles(props)
     const theme = useMantineTheme()
-    const viewport = useViewportSize()
     const { loading } = useAppLoading()
     const mobile = useMobile()
     const mobileMenuOpen = useAppSelector(state => state.tree.mobileMenuOpen)
@@ -197,18 +195,11 @@ export default function Layout(props: LayoutProps) {
                 </Header>
             }
         >
-            <ScrollArea
-                sx={{ height: viewport.height - Constants.layout.headerHeight }}
-                viewportRef={ref => {
-                    if (ref) ref.id = Constants.dom.mainScrollAreaId
-                }}
-            >
-                <Box id="content" className={classes.mainContent}>
-                    <Suspense fallback={<Loader />}>
-                        <Outlet />
-                    </Suspense>
-                </Box>
-            </ScrollArea>
+            <Box id="content" className={classes.mainContent}>
+                <Suspense fallback={<Loader />}>
+                    <Outlet />
+                </Suspense>
+            </Box>
         </AppShell>
     )
 }