From 1c4c328bbd6bdb09fec20eb3733e650b4e7da346 Mon Sep 17 00:00:00 2001 From: Abbey Campbell Date: Wed, 26 Aug 2026 15:11:39 -0700 Subject: [PATCH] add shared hook for resetting state when a key changes --- .../MyObservationsGroupedByIconicTaxaView.tsx | 26 +++---- .../hooks/useIconicTaxaSectionObservations.ts | 39 ++++------- src/sharedHooks/index.ts | 1 + src/sharedHooks/useStateResetOn.ts | 30 ++++++++ .../unit/sharedHooks/useStateResetOn.test.js | 68 +++++++++++++++++++ 5 files changed, 122 insertions(+), 42 deletions(-) create mode 100644 src/sharedHooks/useStateResetOn.ts create mode 100644 tests/unit/sharedHooks/useStateResetOn.test.js diff --git a/src/components/MyObservations/MyObservationsGroupedByIconicTaxaView.tsx b/src/components/MyObservations/MyObservationsGroupedByIconicTaxaView.tsx index 5f2481762..ae6ca35f2 100644 --- a/src/components/MyObservations/MyObservationsGroupedByIconicTaxaView.tsx +++ b/src/components/MyObservations/MyObservationsGroupedByIconicTaxaView.tsx @@ -13,8 +13,7 @@ import React, { useCallback, useEffect, useMemo, useRef, useState, } from "react"; import { ICONIC_TAXA_GROUP, iconicTaxaGroupIcon } from "sharedHelpers/iconicTaxaGroupOrder"; -import type { OBSERVATIONS_SORT } from "sharedHelpers/observationsSort"; -import { useCurrentUser, useTranslation } from "sharedHooks"; +import { useCurrentUser, useStateResetOn, useTranslation } from "sharedHooks"; import type { IconicTaxaHeader, @@ -56,13 +55,6 @@ interface Props { listHeaderContent?: React.ReactElement | null; } -// Which categories the user has collapsed, tracked alongside the sort they were collapsed -// under so changing sort reopens everything without an effect -interface CollapseState { - sortBy: OBSERVATIONS_SORT; - categories: Set; -} - const NONE_COLLAPSED: Set = new Set( ); // How many tiles ahead of the last loaded one to start fetching. @@ -87,13 +79,11 @@ const MyObservationsGroupedByIconicTaxaView = ( { } = useIconicTaxaObservationCounts( ); const unsyncedByCategory = useUnsyncedObservationIdsByIconicTaxon( ); - const [collapseState, setCollapseState] = useState( { - sortBy: observationsSort, - categories: NONE_COLLAPSED, - } ); - const collapsedCategories = collapseState.sortBy === observationsSort - ? collapseState.categories - : NONE_COLLAPSED; + // Changing sort reopens every section, since the list they were collapsed against is gone + const [collapsedCategories, setCollapsedCategories] = useStateResetOn( + observationsSort, + NONE_COLLAPSED, + ); const { sections, @@ -166,7 +156,7 @@ const MyObservationsGroupedByIconicTaxaView = ( { } else { categories.delete( category ); } - setCollapseState( { sortBy: observationsSort, categories } ); + setCollapsedCategories( categories ); if ( !isCollapsing ) return; const headerRow = rows.findIndex( @@ -177,7 +167,7 @@ const MyObservationsGroupedByIconicTaxaView = ( { } advanceFrontier( ); - }, [advanceFrontier, collapsedCategories, observationsSort, rows] ); + }, [advanceFrontier, collapsedCategories, rows, setCollapsedCategories] ); useEffect( ( ) => { const index = pinHeaderRowRef.current; diff --git a/src/components/MyObservations/hooks/useIconicTaxaSectionObservations.ts b/src/components/MyObservations/hooks/useIconicTaxaSectionObservations.ts index 01f4a0422..be7ce321d 100644 --- a/src/components/MyObservations/hooks/useIconicTaxaSectionObservations.ts +++ b/src/components/MyObservations/hooks/useIconicTaxaSectionObservations.ts @@ -3,14 +3,14 @@ import { searchObservations } from "api/observations"; import { getJWT } from "components/LoginSignUp/AuthenticationService"; import type { IconicTaxaSectionState } from "components/MyObservations/helpers/iconicTaxaSections"; import { RealmContext } from "providers/contexts"; -import { useCallback, useMemo, useState } from "react"; +import { useCallback, useMemo } from "react"; import Observation from "realmModels/Observation"; import type { ICONIC_TAXA_GROUP, IconicTaxaGroupCount } from "sharedHelpers/iconicTaxaGroupOrder"; import { log } from "sharedHelpers/logger"; import { handleRetryDelay, reactQueryRetry } from "sharedHelpers/logging"; import type { OBSERVATIONS_SORT } from "sharedHelpers/observationsSort"; import { observationSortToApiParams } from "sharedHelpers/observationsSort"; -import { useCurrentUser } from "sharedHooks"; +import { useCurrentUser, useStateResetOn } from "sharedHooks"; const { useRealm } = RealmContext; @@ -33,12 +33,7 @@ interface IconicTaxonPage { // so this doubles as the activation frontier. type PagesByCategory = Partial>; -// Pages are tracked alongside the sort they were fetched under, so changing sort discards them -// on the next render -interface PageState { - sortKey: string; - pages: PagesByCategory; -} +const NOTHING_REQUESTED: PagesByCategory = {}; interface Params { collapsedCategories: Set; @@ -84,27 +79,23 @@ const useIconicTaxaSectionObservations = ( { const sortParams = useMemo( ( ) => observationSortToApiParams( sortBy ), [sortBy] ); const sortKey = `${sortParams.order_by}-${sortParams.order}`; - const [pageState, setPageState] = useState( { sortKey, pages: {} } ); + // Changing sort invalidates every page, so this starts over rather than re-requesting them + // all under the new order + const [requestedPages, setPages] = useStateResetOn( sortKey, NOTHING_REQUESTED ); + // Before the user has asked for anything, the most-observed category is treated as already + // requested so it starts loading on this render; everything below it waits for them to + // scroll or collapse their way down. Seeded rather than written back, so requestedPages + // stays honest about what the user has actually asked for. Categories the server has nothing + // for are skipped: their header still renders and will show any locally-saved observations, + // but there's nothing to request. const pagesByCategory = useMemo( ( ) => { - const pages = pageState.sortKey === sortKey - ? pageState.pages - : {}; - if ( Object.keys( pages ).length > 0 ) return pages; - // Nothing requested yet, either because the view just opened or because the sort changed. - // Seed the most-observed category so it starts loading on this render; everything below it - // waits for the user to scroll or collapse their way down. Categories the server has - // nothing for are skipped: their header still renders and will show any locally-saved - // observations, but there's nothing to request. + if ( Object.keys( requestedPages ).length > 0 ) return requestedPages; const first = orderedCounts.find( ( { count } ) => count > 0 ); return first ? { [first.category]: 1 } - : pages; - }, [orderedCounts, pageState, sortKey] ); - - const setPages = useCallback( ( pages: PagesByCategory ) => { - setPageState( { sortKey, pages } ); - }, [sortKey] ); + : requestedPages; + }, [orderedCounts, requestedPages] ); const descriptors = useMemo( ( ) => orderedCounts.flatMap( ( { category } ) => { const highestPage = pagesByCategory[category] ?? 0; diff --git a/src/sharedHooks/index.ts b/src/sharedHooks/index.ts index 404ba7518..881f36f5e 100644 --- a/src/sharedHooks/index.ts +++ b/src/sharedHooks/index.ts @@ -25,6 +25,7 @@ export { default as useQuery } from "./useQuery"; export { default as useRemoteObservation } from "./useRemoteObservation"; export { default as useScrollToOffset } from "./useScrollToOffset"; export { default as useShare } from "./useShare"; +export { default as useStateResetOn } from "./useStateResetOn"; export { default as useStoredLayout } from "./useStoredLayout"; export { default as useSuggestions } from "./useSuggestions/useSuggestions"; export { default as useTaxon } from "./useTaxon"; diff --git a/src/sharedHooks/useStateResetOn.ts b/src/sharedHooks/useStateResetOn.ts new file mode 100644 index 000000000..7302e4d37 --- /dev/null +++ b/src/sharedHooks/useStateResetOn.ts @@ -0,0 +1,30 @@ +import { useCallback, useState } from "react"; + +// State that goes back to its initial value whenever `key` changes, without an effect and +// without the extra render an effect would cost — the reset happens during the render where +// the new key first appears. +// +// `initial` must be a stable reference since it gets returned after a key change. A fresh object +// every render would defeat memoization in everything downstream. +// +// The value is stored against the key it was written under, so returning to an earlier key +// restores what was set then rather than the initial value. Only the most recent write is +// kept, so this is a one-slot memory, not a history. +const useStateResetOn = ( + key: unknown, + initial: T, +): [T, ( next: T ) => void] => { + const [state, setState] = useState<{ key: unknown; value: T }>( { key, value: initial } ); + + const value = state.key === key + ? state.value + : initial; + + const setValue = useCallback( ( next: T ) => { + setState( { key, value: next } ); + }, [key] ); + + return [value, setValue]; +}; + +export default useStateResetOn; diff --git a/tests/unit/sharedHooks/useStateResetOn.test.js b/tests/unit/sharedHooks/useStateResetOn.test.js new file mode 100644 index 000000000..b60b5b4e6 --- /dev/null +++ b/tests/unit/sharedHooks/useStateResetOn.test.js @@ -0,0 +1,68 @@ +import { act, renderHook } from "@testing-library/react-native"; +import useStateResetOn from "sharedHooks/useStateResetOn"; + +const INITIAL = { open: false }; + +const renderStateHook = ( ) => renderHook( + ( { key } ) => useStateResetOn( key, INITIAL ), + { initialProps: { key: "a" } }, +); + +describe( "useStateResetOn", ( ) => { + it( "keeps what was set while the key is unchanged", ( ) => { + const { rerender, result } = renderStateHook( ); + + act( ( ) => result.current[1]( { open: true } ) ); + rerender( { key: "a" } ); + + expect( result.current[0] ).toEqual( { open: true } ); + } ); + + it( "goes back to the initial value when the key changes", ( ) => { + const { rerender, result } = renderStateHook( ); + act( ( ) => result.current[1]( { open: true } ) ); + + rerender( { key: "b" } ); + + expect( result.current[0] ).toBe( INITIAL ); + } ); + + it( "resets during the render the new key arrives in, not a render later", ( ) => { + const values = []; + const { rerender } = renderHook( + ( { key } ) => { + const [value] = useStateResetOn( key, INITIAL ); + values.push( value ); + return null; + }, + { initialProps: { key: "a" } }, + ); + + rerender( { key: "b" } ); + + // an effect-based reset would show the stale value once before correcting it + expect( values.every( value => value === INITIAL ) ).toBe( true ); + } ); + + it( "restores what was set under a key when that key comes back", ( ) => { + const { rerender, result } = renderStateHook( ); + act( ( ) => result.current[1]( { open: true } ) ); + + rerender( { key: "b" } ); + expect( result.current[0] ).toBe( INITIAL ); + + rerender( { key: "a" } ); + expect( result.current[0] ).toEqual( { open: true } ); + } ); + + it( "keeps only the most recent write, so it is one slot rather than a history", ( ) => { + const { rerender, result } = renderStateHook( ); + act( ( ) => result.current[1]( { open: true } ) ); + + rerender( { key: "b" } ); + act( ( ) => result.current[1]( { open: false } ) ); + rerender( { key: "a" } ); + + expect( result.current[0] ).toBe( INITIAL ); + } ); +} );