diff --git a/packages/desktop-client/e2e/settings.mobile.test.js-snapshots/Mobile-Settings-checks-that-settings-page-can-be-opened-1-chromium-linux.png b/packages/desktop-client/e2e/settings.mobile.test.js-snapshots/Mobile-Settings-checks-that-settings-page-can-be-opened-1-chromium-linux.png index 5f2fd61f65a..b97876c75fb 100644 Binary files a/packages/desktop-client/e2e/settings.mobile.test.js-snapshots/Mobile-Settings-checks-that-settings-page-can-be-opened-1-chromium-linux.png and b/packages/desktop-client/e2e/settings.mobile.test.js-snapshots/Mobile-Settings-checks-that-settings-page-can-be-opened-1-chromium-linux.png differ diff --git a/packages/desktop-client/e2e/settings.mobile.test.js-snapshots/Mobile-Settings-checks-that-settings-page-can-be-opened-2-chromium-linux.png b/packages/desktop-client/e2e/settings.mobile.test.js-snapshots/Mobile-Settings-checks-that-settings-page-can-be-opened-2-chromium-linux.png index cd4126a2cbd..1ddcee49cc5 100644 Binary files a/packages/desktop-client/e2e/settings.mobile.test.js-snapshots/Mobile-Settings-checks-that-settings-page-can-be-opened-2-chromium-linux.png and b/packages/desktop-client/e2e/settings.mobile.test.js-snapshots/Mobile-Settings-checks-that-settings-page-can-be-opened-2-chromium-linux.png differ diff --git a/packages/desktop-client/e2e/settings.mobile.test.js-snapshots/Mobile-Settings-checks-that-settings-page-can-be-opened-3-chromium-linux.png b/packages/desktop-client/e2e/settings.mobile.test.js-snapshots/Mobile-Settings-checks-that-settings-page-can-be-opened-3-chromium-linux.png index 002a17b784e..e3b70167236 100644 Binary files a/packages/desktop-client/e2e/settings.mobile.test.js-snapshots/Mobile-Settings-checks-that-settings-page-can-be-opened-3-chromium-linux.png and b/packages/desktop-client/e2e/settings.mobile.test.js-snapshots/Mobile-Settings-checks-that-settings-page-can-be-opened-3-chromium-linux.png differ diff --git a/packages/desktop-client/src/components/App.tsx b/packages/desktop-client/src/components/App.tsx index 902dc657053..4c5895c52eb 100644 --- a/packages/desktop-client/src/components/App.tsx +++ b/packages/desktop-client/src/components/App.tsx @@ -40,7 +40,6 @@ import { FinancesApp } from './FinancesApp'; import { ManagementApp } from './manager/ManagementApp'; import { Modals } from './Modals'; import { ResponsiveProvider } from './responsive/ResponsiveProvider'; -import { ScrollProvider } from './ScrollProvider'; import { SidebarProvider } from './sidebar/SidebarProvider'; import { UpdateNotification } from './UpdateNotification'; @@ -180,36 +179,34 @@ export function App() { - + - - - {process.env.REACT_APP_REVIEW_ID && - !Platform.isPlaywright && } - - - - - - + + {process.env.REACT_APP_REVIEW_ID && + !Platform.isPlaywright && } + + + + + - + diff --git a/packages/desktop-client/src/components/FinancesApp.tsx b/packages/desktop-client/src/components/FinancesApp.tsx index 5ce026b8a38..6983b2708be 100644 --- a/packages/desktop-client/src/components/FinancesApp.tsx +++ b/packages/desktop-client/src/components/FinancesApp.tsx @@ -1,5 +1,5 @@ // @ts-strict-ignore -import React, { type ReactElement, useEffect } from 'react'; +import React, { type ReactElement, useEffect, useRef } from 'react'; import { useTranslation } from 'react-i18next'; import { useDispatch, useSelector } from 'react-redux'; import { @@ -34,6 +34,7 @@ import { Reports } from './reports'; import { LoadingIndicator } from './reports/LoadingIndicator'; import { NarrowAlternate, WideComponent } from './responsive'; import { useResponsive } from './responsive/ResponsiveProvider'; +import { ScrollProvider } from './ScrollProvider'; import { Settings } from './settings'; import { FloatableSidebar } from './sidebar'; import { Titlebar } from './Titlebar'; @@ -156,6 +157,8 @@ export function FinancesApp() { run(); }, [lastUsedVersion, setLastUsedVersion]); + const scrollableRef = useRef(null); + return ( @@ -179,113 +182,119 @@ export function FinancesApp() { width: '100%', }} > - - - - - - - 0 ? ( - + > + + + + + + 0 ? ( + + ) : ( + // If there are no accounts, we want to redirect the user to + // the All Accounts screen which will prompt them to add an account + + ) ) : ( - // If there are no accounts, we want to redirect the user to - // the All Accounts screen which will prompt them to add an account - + ) - ) : ( - - ) - } - /> - - } /> + } + /> + + } /> + + } + /> + + + + + } + /> + + } /> + } /> + } /> + + + + + } + /> + + } + /> + + } + /> + + + + + } + /> + + + + + } + /> + + {/* redirect all other traffic to the budget page */} + } /> + + - } - /> - - - - - } - /> - - } /> - } /> - } /> - - - - - } - /> - - } - /> - - } - /> - - - - - } - /> - - - - - } - /> - - {/* redirect all other traffic to the budget page */} - } /> + + } /> + } /> + } /> + } /> + - - - - } /> - } /> - } /> - } /> - - + diff --git a/packages/desktop-client/src/components/ScrollProvider.tsx b/packages/desktop-client/src/components/ScrollProvider.tsx index 21bb683b156..1921681e2af 100644 --- a/packages/desktop-client/src/components/ScrollProvider.tsx +++ b/packages/desktop-client/src/components/ScrollProvider.tsx @@ -1,61 +1,204 @@ -// @ts-strict-ignore 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 = (args: ScrollListenerArgs) => void; +type UnregisterScrollListener = () => void; +type RegisterScrollListener = ( + listener: ScrollListener, +) => UnregisterScrollListener; + type IScrollContext = { - scrollY: number | undefined; - hasScrolledToBottom: (tolerance?: number) => boolean; + registerScrollListener: RegisterScrollListener; }; const ScrollContext = createContext(undefined); -type ScrollProviderProps = { +type ScrollProviderProps = { + scrollableRef: RefObject; + isDisabled?: boolean; + delayMs?: number; children?: ReactNode; }; -export function ScrollProvider({ children }: ScrollProviderProps) { - const [scrollY, setScrollY] = useState(undefined); - const [scrollHeight, setScrollHeight] = useState(undefined); - const [clientHeight, setClientHeight] = useState(undefined); +export function ScrollProvider({ + scrollableRef, + isDisabled, + delayMs = 10, + children, +}: ScrollProviderProps) { + 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 hasScrolledToEnd = useCallback( + (direction: ScrollDirection, tolerance = 1) => { + const isAtStart = (currentCoordinate?: number) => + currentCoordinate !== undefined && currentCoordinate <= tolerance; - const hasScrolledToBottom = useCallback( - (tolerance = 1) => scrollHeight - scrollY <= clientHeight + tolerance, - [clientHeight, scrollHeight, scrollY], + const isAtEnd = ( + totalSize?: number, + currentCoordinate?: number, + viewportSize?: number, + ) => + totalSize !== undefined && + currentCoordinate !== undefined && + viewportSize !== undefined && + totalSize - currentCoordinate <= viewportSize + tolerance; + + switch (direction) { + case 'up': { + return isAtStart(scrollY.current); + } + case 'down': { + return isAtEnd( + scrollHeight.current, + scrollY.current, + clientHeight.current, + ); + } + case 'left': { + return isAtStart(scrollX.current); + } + case 'right': { + return isAtEnd( + scrollWidth.current, + scrollX.current, + clientWidth.current, + ); + } + default: + return false; + } + }, + [], ); + 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(() => { - const listenToScroll = debounce(e => { + if (isDisabled) { + return; + } + + const listenToScroll = debounce((e: Event) => { const target = e.target; - setScrollY(target?.scrollTop || 0); - setScrollHeight(target?.scrollHeight || 0); - setClientHeight(target?.clientHeight || 0); - }, 10); + if (target instanceof Element) { + previousScrollX.current = scrollX.current; + scrollX.current = target.scrollLeft; + scrollHeight.current = target.scrollHeight; + + previousScrollY.current = scrollY.current; + scrollY.current = target.scrollTop; + clientHeight.current = target.clientHeight; - window.addEventListener('scroll', listenToScroll, { + const currentScrollX = scrollX.current; + const currentScrollY = scrollY.current; + + if (currentScrollX !== undefined && currentScrollY !== undefined) { + listeners.current.forEach(listener => + listener({ + scrollX: currentScrollX, + scrollY: currentScrollY, + isScrolling, + hasScrolledToEnd, + }), + ); + } + } + }, delayMs); + + const ref = scrollableRef.current; + + ref?.addEventListener('scroll', listenToScroll, { capture: true, passive: true, }); return () => - window.removeEventListener('scroll', listenToScroll, { + ref?.removeEventListener('scroll', listenToScroll, { capture: true, }); - }, []); + }, [delayMs, hasScrolledToEnd, isDisabled, isScrolling, scrollableRef]); + + const registerScrollListener: RegisterScrollListener = useCallback( + listener => { + listeners.current.push(listener); + + return () => { + listeners.current = listeners.current.filter(l => l !== listener); + }; + }, + [], + ); return ( - + {children} ); } -export function useScroll(): IScrollContext { - return useContext(ScrollContext); +export function useScrollListener(listener: ScrollListener) { + const context = useContext(ScrollContext); + if (!context) { + throw new Error('useScrollListener must be used within a ScrollProvider'); + } + + const { registerScrollListener } = context; + + useEffect(() => { + return registerScrollListener(listener); + }, [listener, registerScrollListener]); } diff --git a/packages/desktop-client/src/components/mobile/MobileNavTabs.tsx b/packages/desktop-client/src/components/mobile/MobileNavTabs.tsx index 220812f73e6..4544517d3da 100644 --- a/packages/desktop-client/src/components/mobile/MobileNavTabs.tsx +++ b/packages/desktop-client/src/components/mobile/MobileNavTabs.tsx @@ -1,7 +1,7 @@ // @ts-strict-ignore import React, { + useCallback, type ComponentType, - useEffect, type CSSProperties, } from 'react'; import { NavLink } from 'react-router-dom'; @@ -9,7 +9,6 @@ import { useSpring, animated, config } from 'react-spring'; import { useDrag } from '@use-gesture/react'; -import { usePrevious } from '../../hooks/usePrevious'; import { SvgAdd, SvgCog, @@ -23,16 +22,20 @@ import { SvgCalendar } from '../../icons/v2'; import { theme, styles } from '../../style'; import { View } from '../common/View'; import { useResponsive } from '../responsive/ResponsiveProvider'; -import { useScroll } from '../ScrollProvider'; +import { useScrollListener } from '../ScrollProvider'; const COLUMN_COUNT = 3; const PILL_HEIGHT = 15; const ROW_HEIGHT = 70; +const TOTAL_HEIGHT = ROW_HEIGHT * COLUMN_COUNT; +const OPEN_FULL_Y = 1; +const OPEN_DEFAULT_Y = TOTAL_HEIGHT - ROW_HEIGHT; +const HIDDEN_Y = TOTAL_HEIGHT; + 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}%`, @@ -96,53 +99,50 @@ export function MobileNavTabs() {
)); - const totalHeight = ROW_HEIGHT * COLUMN_COUNT; - const openY = 0; - const closeY = totalHeight - ROW_HEIGHT; - const hiddenY = totalHeight; - - const [{ y }, api] = useSpring(() => ({ y: totalHeight })); - - const open = ({ canceled }) => { - // when cancel is true, it means that the user passed the upwards threshold - // so we change the spring config to create a nice wobbly effect - api.start({ - y: openY, - immediate: false, - config: canceled ? config.wobbly : config.stiff, - }); - }; + const [{ y }, api] = useSpring(() => ({ y: OPEN_DEFAULT_Y })); - const close = (velocity = 0) => { - api.start({ - y: closeY, - immediate: false, - config: { ...config.stiff, velocity }, - }); - }; + const openFull = useCallback( + ({ canceled }) => { + // when cancel is true, it means that the user passed the upwards threshold + // so we change the spring config to create a nice wobbly effect + api.start({ + y: OPEN_FULL_Y, + immediate: false, + config: canceled ? config.wobbly : config.stiff, + }); + }, + [api, OPEN_FULL_Y], + ); - const hide = (velocity = 0) => { - api.start({ - y: hiddenY, - immediate: false, - config: { ...config.stiff, velocity }, - }); - }; + const openDefault = useCallback( + (velocity = 0) => { + api.start({ + y: OPEN_DEFAULT_Y, + immediate: false, + config: { ...config.stiff, velocity }, + }); + }, + [api, OPEN_DEFAULT_Y], + ); - const previousScrollY = usePrevious(scrollY); + const hide = useCallback( + (velocity = 0) => { + api.start({ + y: HIDDEN_Y, + immediate: false, + config: { ...config.stiff, velocity }, + }); + }, + [api, HIDDEN_Y], + ); - useEffect(() => { - if ( - scrollY && - previousScrollY && - scrollY > previousScrollY && - previousScrollY !== 0 - ) { + useScrollListener(({ isScrolling }) => { + if (isScrolling('down')) { hide(); - } else { - close(); + } else if (isScrolling('up')) { + openDefault(); } - }, [scrollY]); + }); const bind = useDrag( ({ @@ -163,9 +163,9 @@ export function MobileNavTabs() { // the threshold for it to close, or if we reset it to its open position if (last) { if (oy > ROW_HEIGHT * 0.5 || (vy > 0.5 && dy > 0)) { - close(vy); + openDefault(vy); } else { - open({ canceled }); + openFull({ canceled }); } } else { // when the user keeps dragging, we just move the sheet according to @@ -176,7 +176,7 @@ export function MobileNavTabs() { { from: () => [0, y.get()], filterTaps: true, - bounds: { top: -totalHeight, bottom: totalHeight - ROW_HEIGHT }, + bounds: { top: -TOTAL_HEIGHT, bottom: TOTAL_HEIGHT - ROW_HEIGHT }, axis: 'y', rubberband: true, }, @@ -192,7 +192,7 @@ export function MobileNavTabs() { backgroundColor: theme.mobileNavBackground, borderTop: `1px solid ${theme.menuBorder}`, ...styles.shadow, - height: totalHeight + PILL_HEIGHT, + height: TOTAL_HEIGHT + PILL_HEIGHT, width: '100%', position: 'fixed', zIndex: 100, @@ -216,7 +216,7 @@ export function MobileNavTabs() { style={{ flexDirection: 'row', flexWrap: 'wrap', - height: totalHeight, + height: TOTAL_HEIGHT, width: '100%', }} > diff --git a/packages/desktop-client/src/components/mobile/transactions/ListBox.jsx b/packages/desktop-client/src/components/mobile/transactions/ListBox.jsx index 980fce3db6e..f06a730f5e8 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 { useScrollListener } 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?.(); - } + useScrollListener(({ hasScrolledToEnd }) => { + const scrolledToBottom = hasScrolledToEnd('down', 5); + if (scrolledToBottom) { + loadMore?.(); + } + }); return ( <> diff --git a/upcoming-release-notes/3731.md b/upcoming-release-notes/3731.md new file mode 100644 index 00000000000..03e59a6d5e2 --- /dev/null +++ b/upcoming-release-notes/3731.md @@ -0,0 +1,6 @@ +--- +category: Bugfix +authors: [joel-jeremy] +--- + +Fix mobile navigation tabs scrolling when scrolling anywhere in the app e.g. scrolling through category/group notes in mobile budget view.