From 57510d3edff57484854e0708cf7ad028b010c7ae Mon Sep 17 00:00:00 2001 From: Joel Jeremy Marquez Date: Fri, 1 Nov 2024 02:09:48 -0700 Subject: [PATCH] Rewrite useScroll to be more performant by being ref based instead of state to avoid re-renders when scrolling --- .../src/components/ScrollProvider.tsx | 162 +++++++++++++++--- .../src/components/mobile/MobileNavTabs.tsx | 23 +-- .../mobile/transactions/ListBox.jsx | 16 +- 3 files changed, 150 insertions(+), 51 deletions(-) diff --git a/packages/desktop-client/src/components/ScrollProvider.tsx b/packages/desktop-client/src/components/ScrollProvider.tsx index 6c4f0951aee..f69d21a8f07 100644 --- a/packages/desktop-client/src/components/ScrollProvider.tsx +++ b/packages/desktop-client/src/components/ScrollProvider.tsx @@ -2,17 +2,28 @@ import React, { type ReactNode, type RefObject, createContext, - useState, useContext, useEffect, useCallback, + useRef, } from 'react'; import debounce from 'debounce'; +type ScrollDirection = 'up' | 'down' | 'left' | 'right'; + +type ScrollListenerArgs = { + scrollX: number; + scrollY: number; + isScrolling: (direction: ScrollDirection) => boolean; + hasScrolledToEnd: (direction: ScrollDirection, tolerance?: number) => boolean; +}; + +type ScrollListener = (values: ScrollListenerArgs) => void; + type IScrollContext = { - scrollY: number | undefined; - hasScrolledToBottom: (tolerance?: number) => boolean; + registerListener: (listener: ScrollListener) => void; + unregisterListener: (listener: ScrollListener) => void; }; const ScrollContext = createContext(undefined); @@ -28,24 +39,100 @@ export function ScrollProvider({ isDisabled, children, }: ScrollProviderProps) { - const [scrollY, setScrollY] = useState(undefined); - const [scrollHeight, setScrollHeight] = useState( - undefined, - ); - const [clientHeight, setClientHeight] = useState( - undefined, - ); + const previousScrollX = useRef(undefined); + const scrollX = useRef(undefined); + const previousScrollY = useRef(undefined); + const scrollY = useRef(undefined); + const scrollWidth = useRef(undefined); + const scrollHeight = useRef(undefined); + const clientWidth = useRef(undefined); + const clientHeight = useRef(undefined); + const listeners = useRef([]); - const hasScrolledToBottom = useCallback( - (tolerance = 1) => { - if (scrollHeight && scrollY && clientHeight) { - return scrollHeight - scrollY <= clientHeight + tolerance; + const hasScrolledToEnd = useCallback( + (direction: ScrollDirection, tolerance = 1) => { + switch (direction) { + case 'up': + const hasScrolledToTop = () => { + if (scrollY.current) { + return scrollY.current <= tolerance; + } + return false; + }; + return hasScrolledToTop(); + case 'down': + const hasScrolledToBottom = () => { + if ( + scrollHeight.current && + scrollY.current && + clientHeight.current + ) { + return ( + scrollHeight.current - scrollY.current <= + clientHeight.current + tolerance + ); + } + return false; + }; + return hasScrolledToBottom(); + case 'left': + const hasScrollToLeft = () => { + if (scrollX.current) { + return scrollX.current <= tolerance; + } + return false; + }; + return hasScrollToLeft(); + case 'right': + const hasScrolledToRight = () => { + if (scrollWidth.current && scrollX.current && clientWidth.current) { + return ( + scrollWidth.current - scrollX.current <= + clientWidth.current + tolerance + ); + } + + return false; + }; + return hasScrolledToRight(); + default: + return false; } - return false; }, - [clientHeight, scrollHeight, scrollY], + [], ); + const isScrolling = useCallback((direction: ScrollDirection) => { + switch (direction) { + case 'up': + return ( + previousScrollY.current !== undefined && + scrollY.current !== undefined && + previousScrollY.current > scrollY.current + ); + case 'down': + return ( + previousScrollY.current !== undefined && + scrollY.current !== undefined && + previousScrollY.current < scrollY.current + ); + case 'left': + return ( + previousScrollX.current !== undefined && + scrollX.current !== undefined && + previousScrollX.current > scrollX.current + ); + case 'right': + return ( + previousScrollX.current !== undefined && + scrollX.current !== undefined && + previousScrollX.current < scrollX.current + ); + default: + return false; + } + }, []); + useEffect(() => { if (isDisabled) { return; @@ -54,9 +141,21 @@ export function ScrollProvider({ const listenToScroll = debounce((e: Event) => { const target = e.target; if (target instanceof Element) { - setScrollY(target.scrollTop || 0); - setScrollHeight(target.scrollHeight || 0); - setClientHeight(target.clientHeight || 0); + previousScrollX.current = scrollX.current; + scrollX.current = target.scrollLeft || 0; + previousScrollY.current = scrollY.current; + scrollY.current = target.scrollTop || 0; + scrollHeight.current = target.scrollHeight || 0; + clientHeight.current = target.clientHeight || 0; + + listeners.current.forEach(listener => + listener({ + scrollX: scrollX.current!, + scrollY: scrollY.current!, + isScrolling, + hasScrolledToEnd, + }), + ); } }, 10); @@ -70,19 +169,34 @@ export function ScrollProvider({ ref?.removeEventListener('scroll', listenToScroll, { capture: true, }); - }, [isDisabled, scrollableRef]); + }, [hasScrolledToEnd, isDisabled, isScrolling, scrollableRef]); + + const registerListener = useCallback((listener: ScrollListener) => { + listeners.current.push(listener); + }, []); + + const unregisterListener = useCallback((listener: ScrollListener) => { + listeners.current = listeners.current.filter(l => l !== listener); + }, []); return ( - + {children} ); } -export function useScroll(): IScrollContext { +export function useScrollEffect(listener: ScrollListener) { const context = useContext(ScrollContext); if (!context) { - throw new Error('useScroll must be used within a ScrollProvider'); + throw new Error('useScrollEffect must be used within a ScrollProvider'); } - return context; + + const { registerListener, unregisterListener } = context; + + useEffect(() => { + const _listener = listener; + registerListener(_listener); + return () => unregisterListener(_listener); + }, [listener, registerListener, unregisterListener]); } diff --git a/packages/desktop-client/src/components/mobile/MobileNavTabs.tsx b/packages/desktop-client/src/components/mobile/MobileNavTabs.tsx index 01829cc3445..a1f4c9e212e 100644 --- a/packages/desktop-client/src/components/mobile/MobileNavTabs.tsx +++ b/packages/desktop-client/src/components/mobile/MobileNavTabs.tsx @@ -1,15 +1,10 @@ // @ts-strict-ignore -import React, { - type ComponentType, - useEffect, - type CSSProperties, -} from 'react'; +import React, { type ComponentType, type CSSProperties } from 'react'; import { NavLink } from 'react-router-dom'; import { useSpring, animated, config } from 'react-spring'; import { useDrag } from '@use-gesture/react'; -import { usePrevious } from '../../hooks/usePrevious'; import { SvgAdd, SvgCog, @@ -23,7 +18,7 @@ import { SvgCalendar } from '../../icons/v2'; import { useResponsive } from '../../ResponsiveProvider'; import { theme, styles } from '../../style'; import { View } from '../common/View'; -import { useScroll } from '../ScrollProvider'; +import { useScrollEffect } from '../ScrollProvider'; const COLUMN_COUNT = 3; const PILL_HEIGHT = 15; @@ -32,7 +27,6 @@ export const MOBILE_NAV_HEIGHT = ROW_HEIGHT + PILL_HEIGHT; export function MobileNavTabs() { const { isNarrowWidth } = useResponsive(); - const { scrollY } = useScroll(); const navTabStyle = { flex: `1 1 ${100 / COLUMN_COUNT}%`, @@ -129,20 +123,13 @@ export function MobileNavTabs() { }); }; - const previousScrollY = usePrevious(scrollY); - - useEffect(() => { - if ( - scrollY && - previousScrollY && - scrollY > previousScrollY && - previousScrollY !== 0 - ) { + useScrollEffect(({ isScrolling }) => { + if (isScrolling('down')) { hide(); } else { close(); } - }, [scrollY]); + }); const bind = useDrag( ({ diff --git a/packages/desktop-client/src/components/mobile/transactions/ListBox.jsx b/packages/desktop-client/src/components/mobile/transactions/ListBox.jsx index 980fce3db6e..c1c9c476ad2 100644 --- a/packages/desktop-client/src/components/mobile/transactions/ListBox.jsx +++ b/packages/desktop-client/src/components/mobile/transactions/ListBox.jsx @@ -2,8 +2,7 @@ import React, { useRef } from 'react'; import { useListBox } from 'react-aria'; import { useListState } from 'react-stately'; -import { usePrevious } from '../../../hooks/usePrevious'; -import { useScroll } from '../../ScrollProvider'; +import { useScrollEffect } from '../../ScrollProvider'; import { ListBoxSection } from './ListBoxSection'; @@ -13,13 +12,12 @@ export function ListBox(props) { const { listBoxProps, labelProps } = useListBox(props, state, listBoxRef); const { loadMore } = props; - const { hasScrolledToBottom } = useScroll(); - const scrolledToBottom = hasScrolledToBottom(5); - const prevScrolledToBottom = usePrevious(scrolledToBottom); - - if (!prevScrolledToBottom && scrolledToBottom) { - loadMore?.(); - } + useScrollEffect(({ hasScrolledToEnd }) => { + const scrolledToBottom = hasScrolledToEnd('down', 5); + if (scrolledToBottom) { + loadMore?.(); + } + }); return ( <>