diff --git a/.changeset/fix-trustlab-list-filter-ux.md b/.changeset/fix-trustlab-list-filter-ux.md new file mode 100644 index 000000000..e53a25041 --- /dev/null +++ b/.changeset/fix-trustlab-list-filter-ux.md @@ -0,0 +1,9 @@ +--- +"trustlab": patch +--- + +Improve the filterable list views (opportunities, reports, toolkits, playbooks): + +- Keep the current results in place while a filter/search/page change is fetching (no flash back to the unfiltered list), and show a progress indicator for user-driven fetches only — not on first load, tab focus, or reconnect. +- Restore the filter controls (dropdowns/chips, sort, search) from bookmarked/shared URLs, and stop dropping other active filters on the first interaction. +- Centralise list data-fetching in a shared `useListData` hook. diff --git a/apps/trustlab/package.json b/apps/trustlab/package.json index 290ba7ce7..4220697bc 100644 --- a/apps/trustlab/package.json +++ b/apps/trustlab/package.json @@ -1,6 +1,6 @@ { "name": "trustlab", - "version": "0.0.23", + "version": "0.0.24", "private": true, "scripts": { "dev": "next dev", diff --git a/apps/trustlab/src/components/Filters/Filters.js b/apps/trustlab/src/components/Filters/Filters.js index 7f0afed8d..0dd75eafa 100644 --- a/apps/trustlab/src/components/Filters/Filters.js +++ b/apps/trustlab/src/components/Filters/Filters.js @@ -1,22 +1,23 @@ import { Box, - Typography, Button, - Stack, Chip, - SvgIcon, - InputBase, InputAdornment, + InputBase, + LinearProgress, + ListItemText, Menu, MenuItem, - ListItemText, + Stack, + SvgIcon, + Typography, } from "@mui/material"; import React, { - useState, - useMemo, useCallback, - useRef, useEffect, + useMemo, + useRef, + useState, } from "react"; import FilterDropdown from "./FilterDropdown"; @@ -34,14 +35,15 @@ const defaultIcons = { report: DocumentIcon, }; -const generateYearOptions = () => { +// Computed once at module load (year list is bounded 2020..current year), so we +// don't re-run `new Date()` each time the filters config changes. +const YEAR_OPTIONS = (() => { const currentYear = new Date().getFullYear(); const len = currentYear - 2020 + 1; return Array.from({ length: len }, (_, i) => currentYear - i); -}; +})(); -// Generate month options -const generateMonthOptions = () => [ +const MONTH_OPTIONS = [ { label: "January", value: 1 }, { label: "February", value: 2 }, { label: "March", value: 3 }, @@ -160,7 +162,13 @@ const Filters = React.forwardRef(function Filters( { filterByLabel, filters = [], + // Current control state from the parent's URL-synced params: filter + // selections keyed by type, plus `sort` and `search`. Filters is controlled + // by this and holds no selection state of its own. + selectedValues = {}, + defaultSort, clearFiltersLabel, + isBusy = false, onApply, onClear, sx, @@ -173,14 +181,24 @@ const Filters = React.forwardRef(function Filters( onSortChange, // Atomic clear (preferred over onApply/onClear when search/sort are active) onClearAll, + showProgress = true, + // progress will take .5 height e.g. if showProgress = false, spacing should be 2.5 + spacing = { xs: 2 }, }, ref, ) { - const [selectedValues, setSelectedValues] = useState({}); - const [sortValue, setSortValue] = useState(null); - const [searchValue, setSearchValue] = useState(""); + // Sort is controlled and the search box keeps a local draft for immediate + // typing while syncing from the current controlled state. + const sortValue = selectedValues.sort ?? null; + const hasActiveSort = Boolean(sortValue) && sortValue !== defaultSort; + const controlledSearchValue = selectedValues.search ?? ""; + const [searchValue, setSearchValue] = useState(controlledSearchValue); const searchTimerRef = useRef(null); + useEffect(() => { + setSearchValue(controlledSearchValue); + }, [controlledSearchValue]); + // Cleanup debounce timer on unmount useEffect(() => { return () => { @@ -200,9 +218,9 @@ const Filters = React.forwardRef(function Filters( let resolvedOptions = options; if (!resolvedOptions?.length) { if (type === "year") { - resolvedOptions = generateYearOptions(); + resolvedOptions = YEAR_OPTIONS; } else if (type === "month") { - resolvedOptions = generateMonthOptions(); + resolvedOptions = MONTH_OPTIONS; } } @@ -227,6 +245,27 @@ const Filters = React.forwardRef(function Filters( }); }, [filters]); + // Reconcile the incoming selection (from the URL/params, where values are + // strings) against each filter's options so types match for selection + // highlighting and chip labels (e.g. year/month options are numbers). + const normalizedSelected = useMemo(() => { + const result = {}; + processedFilters.forEach((filter) => { + const incoming = selectedValues?.[filter.type]; + if (incoming == null) { + return; + } + const values = Array.isArray(incoming) ? incoming : [incoming]; + result[filter.type] = values.map((value) => { + const option = filter.options.find( + (opt) => String(filter.getOptionValue(opt)) === String(value), + ); + return option ? filter.getOptionValue(option) : value; + }); + }); + return result; + }, [selectedValues, processedFilters]); + const getChipLabel = useCallback( (filterType, value) => { const filter = processedFilters.find((f) => f.type === filterType); @@ -255,26 +294,20 @@ const Filters = React.forwardRef(function Filters( const handleFilterChange = useCallback( (filterType, values) => { - const newSelectedValues = { - ...selectedValues, - [filterType]: values, - }; - setSelectedValues(newSelectedValues); - - if (onApply) { - onApply(newSelectedValues); - } + // Controlled: emit the full selection and let the parent update + // params/URL, which flows back down as `selectedValues`. + onApply?.({ ...normalizedSelected, [filterType]: values }); }, - [selectedValues, onApply], + [normalizedSelected, onApply], ); const handleChipDelete = useCallback( (filterType, value) => { - const currentValues = selectedValues[filterType] || []; + const currentValues = normalizedSelected[filterType] || []; const newValues = currentValues.filter((v) => v !== value); handleFilterChange(filterType, newValues); }, - [selectedValues, handleFilterChange], + [normalizedSelected, handleFilterChange], ); const handleSearchChange = useCallback( @@ -293,15 +326,13 @@ const Filters = React.forwardRef(function Filters( const handleSortChange = useCallback( (value) => { - setSortValue(value); + // Controlled: let the parent update params/URL, which flows back as sortValue. onSortChange?.(value); }, [onSortChange], ); const clearAll = useCallback(() => { - setSelectedValues({}); - setSortValue(null); setSearchValue(""); if (searchTimerRef.current) { clearTimeout(searchTimerRef.current); @@ -318,16 +349,16 @@ const Filters = React.forwardRef(function Filters( const anySelected = useMemo(() => { return ( - Object.values(selectedValues).some((values) => values?.length > 0) || + Object.values(normalizedSelected).some((values) => values?.length > 0) || !!searchValue || - !!sortValue + hasActiveSort ); - }, [selectedValues, searchValue, sortValue]); + }, [normalizedSelected, searchValue, hasActiveSort]); // Collect all chips from all filter types const allChips = useMemo(() => { const chips = []; - Object.entries(selectedValues).forEach(([filterType, values]) => { + Object.entries(normalizedSelected).forEach(([filterType, values]) => { (values || []).forEach((value) => { chips.push({ filterType, @@ -337,7 +368,7 @@ const Filters = React.forwardRef(function Filters( }); }); return chips; - }, [selectedValues, getChipLabel]); + }, [normalizedSelected, getChipLabel]); // Gate on the callback props so a stale CMS label can't accidentally re-show // a control whose feature flag has been disabled @@ -345,157 +376,164 @@ const Filters = React.forwardRef(function Filters( const showSort = Boolean(onSortChange) && sortOptions.length > 0; return ( - - {/* Filter By label sits above the controls row */} - {filterByLabel && ( - - {filterByLabel} - - )} - - {/* Controls row — flat flex row that wraps. + + + {/* Filter By label sits above the controls row */} + {filterByLabel && ( + + {filterByLabel} + + )} + + {/* Controls row — flat flex row that wraps. xs: search claims its own line (width 100%), filters + sort wrap below. sm+: search is capped at 50%, everything stays on one line. */} - - {showSearch && ( - - - - - - - } - sx={{ - width: { xs: "100%", sm: "auto" }, - flex: { sm: 1 }, - maxWidth: { sm: "50%" }, - minWidth: 180, - border: "1px solid #C9CACB", - borderRadius: "10px", - px: 1.5, - py: 0.5, - fontSize: "14px", - backgroundColor: "#fff", - }} - /> - )} - {processedFilters.map((filter) => ( - handleFilterChange(filter.type, values)} - getOptionValue={filter.getOptionValue} - getOptionLabel={filter.getOptionLabel} - startIcon={ - - } - size="small" - /> - ))} - {/* ml:auto keeps sort at the far right of whichever line it lands on */} - {showSort && ( - - - - )} - - - {/* Row 3: Chips + Actions */} - - - {allChips.map((chip) => ( - handleChipDelete(chip.filterType, chip.value)} + {showSearch && ( + + + + + + + } sx={{ - px: 2, + width: { xs: "100%", sm: "auto" }, + flex: { sm: 1 }, + maxWidth: { sm: "50%" }, + minWidth: 180, + border: "1px solid #C9CACB", borderRadius: "10px", - p: 0.5, + px: 1.5, + py: 0.5, + fontSize: "14px", + backgroundColor: "#fff", }} /> - ))} - {!!anySelected && ( - - )} + ))} + {!!anySelected && ( + + )} + + + + {showProgress ? ( + + {isBusy ? : null} - - + ) : null} + ); }); diff --git a/apps/trustlab/src/components/Filters/Filters.snap.js b/apps/trustlab/src/components/Filters/Filters.snap.js index fd8e2e9d1..e532ed946 100644 --- a/apps/trustlab/src/components/Filters/Filters.snap.js +++ b/apps/trustlab/src/components/Filters/Filters.snap.js @@ -3,102 +3,109 @@ exports[`ReportFilters renders unchanged 1`] = `
-
- Filter By -
- - - + + -
-
+ + `; diff --git a/apps/trustlab/src/components/OpportunitiesList/OpportunitiesList.js b/apps/trustlab/src/components/OpportunitiesList/OpportunitiesList.js index a8a03eca9..9be0943ef 100644 --- a/apps/trustlab/src/components/OpportunitiesList/OpportunitiesList.js +++ b/apps/trustlab/src/components/OpportunitiesList/OpportunitiesList.js @@ -96,7 +96,11 @@ const OpportunitiesList = forwardRef(function OpportunitiesList(props, ref) { setParams(newParams); }, [router.isReady]); - const { items = [], pagination = p } = useOpportunities( + const { + items = [], + pagination = p, + isBusy, + } = useOpportunities( page, params, initialItems, @@ -223,12 +227,14 @@ const OpportunitiesList = forwardRef(function OpportunitiesList(props, ref) { } router.push(urlPath, undefined, { shallow: true, scroll: false }); } + const showTitle = title || description; + const showFilterBar = hasFilters || hasSearch || hasSortBy; return ( - {title || description ? ( + {showTitle ? (
{title} {description && ( @@ -252,13 +258,17 @@ const OpportunitiesList = forwardRef(function OpportunitiesList(props, ref) {
) : null} - {hasFilters || hasSearch || hasSortBy ? ( -
+ {showFilterBar ? ( + // pb is 2 instead of 2.5 because of progress bar taking .5 i.e. 4px +
diff --git a/apps/trustlab/src/components/OpportunitiesList/useOpportunities.js b/apps/trustlab/src/components/OpportunitiesList/useOpportunities.js index b0089bd29..b4421b72a 100644 --- a/apps/trustlab/src/components/OpportunitiesList/useOpportunities.js +++ b/apps/trustlab/src/components/OpportunitiesList/useOpportunities.js @@ -1,9 +1,6 @@ -import useSWR from "swr"; - +import useListData from "@/trustlab/hooks/useListData"; import { setSearchParam } from "@/trustlab/utils/queryParams"; -const fetcher = (url) => fetch(url).then((res) => res.json()); - // Allowlist of params forwarded to the API. Intentionally excludes `page` // (set separately) and keeps arbitrary URL params from being proxied through. // Keep in sync with the /api/v1/opportunities handler. @@ -34,14 +31,17 @@ function useOpportunities( setSearchParam(searchParams, key, params?.[key]); }); - const { data } = useSWR( - skip ? null : `${apiEndpoint}?${searchParams.toString()}`, - fetcher, - { fallbackData: { docs: initialItems, page, totalPages: initialCount } }, + const { data, isBusy } = useListData( + `${apiEndpoint}?${searchParams.toString()}`, + { + fallbackData: { docs: initialItems, page, totalPages: initialCount }, + enabled: !skip, + }, ); return { items: skip ? initialItems : (data?.docs ?? initialItems), + isBusy, pagination: { page: data?.page ?? page, count: data?.totalPages ?? initialCount, diff --git a/apps/trustlab/src/components/PlaybooksList/PlaybooksList.js b/apps/trustlab/src/components/PlaybooksList/PlaybooksList.js index 2a21daf5c..b4301285f 100644 --- a/apps/trustlab/src/components/PlaybooksList/PlaybooksList.js +++ b/apps/trustlab/src/components/PlaybooksList/PlaybooksList.js @@ -57,13 +57,11 @@ const PlaybooksList = forwardRef(function PlaybooksList(props, ref) { })); }, [router.isReady]); - const { playbooks = [], pagination = p } = usePlaybooks( - page, - params, - initialPlaybooks, - p?.count, - !hasPagination, - ); + const { + playbooks = [], + pagination = p, + isBusy, + } = usePlaybooks(page, params, initialPlaybooks, p?.count, !hasPagination); const handlePageChange = (value) => { setPage(value); @@ -107,11 +105,12 @@ const PlaybooksList = forwardRef(function PlaybooksList(props, ref) { ref={ref} > {hasFilters ? ( -
+
{ - const query = new URLSearchParams(); - Object.entries(params || {}).forEach(([key, value]) => { - setSearchParam(query, key, value); - }); - return query.toString(); -}; - -const fetcher = (url) => fetch(url).then((res) => res.json()); +import useListData from "@/trustlab/hooks/useListData"; +import { buildQueryString } from "@/trustlab/utils/queryParams"; const usePlaybooks = ( page, @@ -20,23 +9,20 @@ const usePlaybooks = ( showAllPosts, ) => { const queryString = buildQueryString({ ...params, page }); - const { data, isLoading } = useSWR( - `/api/v1/playbooks?${queryString}`, - fetcher, - ); + const { data, isBusy } = useListData(`/api/v1/playbooks?${queryString}`); if (!data) { return { playbooks: initialPlaybooks || [], pagination: { count: initialCount, page }, - isLoading: true, + isBusy, }; } return { playbooks: !showAllPosts ? data?.playbooks || [] : initialPlaybooks || [], pagination: data?.pagination, - isLoading, + isBusy, }; }; diff --git a/apps/trustlab/src/components/ReportsList/ReportsList.js b/apps/trustlab/src/components/ReportsList/ReportsList.js index b024db538..c39ffccef 100644 --- a/apps/trustlab/src/components/ReportsList/ReportsList.js +++ b/apps/trustlab/src/components/ReportsList/ReportsList.js @@ -44,17 +44,27 @@ const ReportsList = forwardRef(function ReportsList(props, ref) { defaultSort, } = props; + const router = useRouter(); + const { query } = router; + const { page: initialPage } = query; + const [page, setPage] = useState(p?.page); - const [params, setParams] = useState({ + const [params, setParams] = useState(() => ({ reportsType, limit: reportsPerPage, ...(defaultSort ? { sort: defaultSort } : {}), - }); + // Restore filters from the URL at mount (mirrors the router.isReady effect + // below) so client-side nav to a filtered URL renders without a flash. + ...parseQueryParams({ + year: query.year, + month: query.month, + report: query.report, + sort: query.sort, + search: query.search, + }), + })); const listRef = useRef(null); useImperativeHandle(ref, () => listRef.current); - const router = useRouter(); - const { query } = router; - const { page: initialPage } = query; useEffect(() => { if (initialPage) { @@ -65,7 +75,11 @@ const ReportsList = forwardRef(function ReportsList(props, ref) { } }, [initialPage]); - const { reports = [], pagination = p } = useReports( + const { + reports = [], + pagination = p, + isBusy, + } = useReports( page, params, initialReports, @@ -222,10 +236,13 @@ const ReportsList = forwardRef(function ReportsList(props, ref) { return ( {showFiltersBar ? ( -
+
fetch(url).then((res) => res.json()); - // Allowlist of params forwarded to the API. Intentionally excludes `page` // (set separately) and keeps arbitrary URL params from being proxied through. // Keep in sync with the /api/v1/reports handler. @@ -25,16 +22,17 @@ function useReports(page, params, initialReports, initialCount, skip) { setSearchParam(searchParams, key, params?.[key]); }); - const { data } = useSWR( - skip ? null : `/api/v1/reports?${searchParams.toString()}`, - fetcher, + const { data, isBusy } = useListData( + `/api/v1/reports?${searchParams.toString()}`, { fallbackData: { reports: initialReports, page, totalPages: initialCount }, + enabled: !skip, }, ); return { reports: skip ? initialReports : (data?.reports ?? initialReports), + isBusy, pagination: { page: data?.pagination?.page ?? page, count: data?.pagination?.count ?? initialCount, diff --git a/apps/trustlab/src/components/ToolkitList/ToolkitList.js b/apps/trustlab/src/components/ToolkitList/ToolkitList.js index ec14cf0ef..f01c8b765 100644 --- a/apps/trustlab/src/components/ToolkitList/ToolkitList.js +++ b/apps/trustlab/src/components/ToolkitList/ToolkitList.js @@ -60,13 +60,11 @@ const ToolkitList = forwardRef(function ToolkitList(props, ref) { })); }, [router.isReady]); - const { toolkits = [], pagination = p } = useToolkits( - page, - params, - initialToolkits, - p?.count, - !hasPagination, - ); + const { + toolkits = [], + pagination = p, + isBusy, + } = useToolkits(page, params, initialToolkits, p?.count, !hasPagination); const handlePageChange = (value) => { setPage(value); @@ -104,11 +102,12 @@ const ToolkitList = forwardRef(function ToolkitList(props, ref) { return ( {hasFilters && ( -
+
{ - const query = new URLSearchParams(); - Object.entries(params || {}).forEach(([key, value]) => { - setSearchParam(query, key, value); - }); - return query.toString(); -}; - -const fetcher = (url) => fetch(url).then((res) => res.json()); +import useListData from "@/trustlab/hooks/useListData"; +import { buildQueryString } from "@/trustlab/utils/queryParams"; const useToolkits = (page, params, initialToolkits, _, showAll) => { const queryString = buildQueryString({ ...params, page }); - const { data, isLoading } = useSWR( - `/api/v1/toolkits?${queryString}`, - fetcher, - ); + const { data, isBusy } = useListData(`/api/v1/toolkits?${queryString}`, { + enabled: !showAll, + }); if (!data?.toolkits || showAll) { return { toolkits: initialToolkits || [], pagination: { count: 1, page: 1 }, - isLoading: true, + isBusy, }; } return { - toolkits: !showAll ? data?.toolkits || [] : initialToolkits || [], + toolkits: data?.toolkits || [], pagination: data?.pagination, - isLoading, + isBusy, }; }; diff --git a/apps/trustlab/src/hooks/useListData.js b/apps/trustlab/src/hooks/useListData.js new file mode 100644 index 000000000..9137fbe11 --- /dev/null +++ b/apps/trustlab/src/hooks/useListData.js @@ -0,0 +1,40 @@ +import { useEffect, useRef } from "react"; +import useSWR from "swr"; + +const fetcher = (url) => fetch(url).then((res) => res.json()); + +// Sensible defaults for the filterable list views (opportunities, reports, +// toolkits, playbooks) — callers may override any of them. By default: +// - keepPreviousData: hold the current results in place during a refetch so +// the list never flashes back to the unfiltered set. +// - no focus/reconnect revalidation. +// Mount revalidation stays ON so ISR-stale pages refresh on load. `isBusy` +// reflects only fetches for a NEW key (the user changed the query); revalidating +// the current key (mount/background freshness) stays silent — SWR's isLoading +// can't make this distinction because it tracks cache presence, not whether the +// fetched key differs from the one on screen (and on mount we show fallback). +// Extra options (e.g. fallbackData) pass through to useSWR; `enabled` mirrors +// each list's skip flag. +export default function useListData(url, { enabled = true, ...options } = {}) { + const key = enabled ? url : null; + const { data, isValidating } = useSWR(key, fetcher, { + keepPreviousData: true, + revalidateOnFocus: false, + revalidateOnReconnect: false, + ...options, + }); + + // Track the key whose data is currently on screen; we're "busy" only while + // fetching a different key, not while revalidating the current one. + const shownKey = useRef(key); + useEffect(() => { + if (!isValidating) { + shownKey.current = key; + } + }, [isValidating, key]); + + return { + data, + isBusy: Boolean(key) && isValidating && shownKey.current !== key, + }; +} diff --git a/apps/trustlab/src/utils/queryParams.js b/apps/trustlab/src/utils/queryParams.js index 482d408be..e2b11ca83 100644 --- a/apps/trustlab/src/utils/queryParams.js +++ b/apps/trustlab/src/utils/queryParams.js @@ -43,3 +43,13 @@ export function setSearchParam(searchParams, key, value) { searchParams.delete(key); normalizeQueryList(value).forEach((v) => searchParams.append(key, v)); } + +// Serialize a params object into a query string, applying setSearchParam per +// key (repeated params for arrays, trimmed, blanks dropped). +export function buildQueryString(params) { + const searchParams = new URLSearchParams(); + Object.entries(params || {}).forEach(([key, value]) => { + setSearchParam(searchParams, key, value); + }); + return searchParams.toString(); +}