From 862bcebfb4a496023f173818fd2ea09932b27ee6 Mon Sep 17 00:00:00 2001 From: Carson Full Date: Thu, 10 Oct 2024 09:26:43 -0500 Subject: [PATCH 1/8] Create our own useSet hook --- src/hooks/index.ts | 1 + src/hooks/useSet.ts | 96 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+) create mode 100644 src/hooks/useSet.ts diff --git a/src/hooks/index.ts b/src/hooks/index.ts index bbf27fceaf..451a36b5bc 100644 --- a/src/hooks/index.ts +++ b/src/hooks/index.ts @@ -1,4 +1,5 @@ export * from './useFirstMountState'; +export * from './useSet'; export * from './useUserAgent'; export * from './useQueryParams'; export * from './useIsomorphicEffect'; diff --git a/src/hooks/useSet.ts b/src/hooks/useSet.ts new file mode 100644 index 0000000000..8351059a28 --- /dev/null +++ b/src/hooks/useSet.ts @@ -0,0 +1,96 @@ +/* eslint-disable react-hooks/exhaustive-deps */ +import { useLatest } from 'ahooks'; +import { useMemo, useState } from 'react'; +import { Merge } from 'type-fest'; + +export type SetHook = Merge, SetModifiers>; +export interface SetModifiers { + /** + * Add item to the set. + * No change if the item already exists, which the return value conveys. + */ + readonly add: (item: T) => boolean & + // Not really, just so this can be typed as Set + Set; + /** + * Alias of {@link delete}. + */ + readonly remove: (item: T) => boolean; + /** + * Remove an item from the set. + * No change if the item already exists, which the return value conveys. + */ + readonly delete: (item: T) => boolean; + /** + * Toggle adding/removing an item from the set. + * Optionally, explicitly state if the value should be added/removed with the `next` arg. + * No change if the item already exists, which the return value conveys. + */ + readonly toggle: (item: T, next?: boolean) => boolean; + /** + * Replace the entries with the ones given here. + * This always produces an identity change, even if the entry values are the same. + */ + readonly set: (items: Iterable) => void; + /** + * Remove all the entries. + * No change if the set is already empty. + */ + readonly clear: () => void; + /** + * Reset the entries to the ones given in the hook creation. + * This always produces an identity change, even if the entry values are the same. + */ + readonly reset: () => void; +} + +/** + * Provides a Set in React state. + * + * Each change produces a new Set. + * However, the modifier functions (their identities) don't change + * and will always reference the current entries when needed. + * + * We do differ with `add()` return value, though. + * It returns whether an entry was added instead of returning `this`. + */ +export function useSet(initialValue?: Iterable): SetHook { + const getInitValue = useLatest(() => new Set(initialValue)); + const [current, set] = useState(getInitValue.current); + const ref = useLatest(current); + + const modifiers = useMemo((): SetModifiers => { + const toggle = (item: T, next?: boolean) => { + // Keeping this found check independent of the setter scope below. + // React handles when it should be called and it could be not synchronous. + const found = ref.current.has(item); + + set((prev) => { + // Check again here, because maybe prev has changed, and we don't want + // to change identity redundantly. + const currentlyIn = prev.has(item); + next = next ?? !currentlyIn; + if ((next && currentlyIn) || (!next && !currentlyIn)) { + return prev; + } + + const temp = new Set(prev); + temp[next ? 'add' : 'delete'](item); + return temp; + }); + + return found; + }; + return { + toggle, + add: (item) => toggle(item, true) as any, + remove: (item) => toggle(item, false), + delete: (item) => toggle(item, false), + set: (items) => set(new Set(items)), + reset: () => set(getInitValue.current), + clear: () => set((prev) => (prev.size === 0 ? prev : new Set())), + }; + }, []); + + return useMemo(() => Object.assign(current, modifiers), [current]); +} From 1c640786a7cc907865493277802edd579a60eeb1 Mon Sep 17 00:00:00 2001 From: Carson Full Date: Thu, 10 Oct 2024 09:26:00 -0500 Subject: [PATCH 2/8] Update comments to use our useSet --- src/components/Comments/CommentsContext.tsx | 23 ++++----------------- 1 file changed, 4 insertions(+), 19 deletions(-) diff --git a/src/components/Comments/CommentsContext.tsx b/src/components/Comments/CommentsContext.tsx index 44a679353d..8228b1d88c 100644 --- a/src/components/Comments/CommentsContext.tsx +++ b/src/components/Comments/CommentsContext.tsx @@ -1,4 +1,4 @@ -import { useLocalStorageState, useSet } from 'ahooks'; +import { useLocalStorageState } from 'ahooks'; import { noop } from 'lodash'; import { createContext, @@ -9,16 +9,12 @@ import { useState, } from 'react'; import { ChildrenProp } from '~/common'; - -type ExpandedThreads = ReturnType>[1] & { - has: (threadId: string) => boolean; - toggle: (threadId: string, next?: boolean) => void; -}; +import { SetHook, useSet } from '~/hooks'; const initialCommentsBarContext = { toggleCommentsBar: noop as (state?: boolean) => void, isCommentsBarOpen: false, - expandedThreads: {} as unknown as ExpandedThreads, + expandedThreads: {} as unknown as SetHook, resourceId: undefined as string | undefined, setResourceId: noop as (resourceId: string | undefined) => void, }; @@ -31,18 +27,7 @@ export const CommentsProvider = ({ children }: ChildrenProp) => { const [resourceId, setResourceId] = useState(undefined); - const [currentExpandedThreads, setExpandedThreads] = useSet(); - const expandedThreads = useMemo( - (): ExpandedThreads => ({ - ...setExpandedThreads, - has: currentExpandedThreads.has.bind(currentExpandedThreads), - toggle: (threadId: string, next?: boolean) => { - next = next ?? !currentExpandedThreads.has(threadId); - setExpandedThreads[next ? 'add' : 'remove'](threadId); - }, - }), - [currentExpandedThreads, setExpandedThreads] - ); + const expandedThreads = useSet(); const toggleCommentsBar = useCallback( (state?: boolean) => { From 4f754abb75c269ff8c1ef183c706b5dc146aa4ba Mon Sep 17 00:00:00 2001 From: Carson Full Date: Thu, 10 Oct 2024 10:57:40 -0500 Subject: [PATCH 3/8] Allow multiple rows to be expanded --- .../ProgressReportsExpandedGrid.tsx | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/scenes/Dashboard/ProgressReportsWidget/ProgressReportsExpandedGrid.tsx b/src/scenes/Dashboard/ProgressReportsWidget/ProgressReportsExpandedGrid.tsx index 6eb9b0f2c8..5467b691f5 100644 --- a/src/scenes/Dashboard/ProgressReportsWidget/ProgressReportsExpandedGrid.tsx +++ b/src/scenes/Dashboard/ProgressReportsWidget/ProgressReportsExpandedGrid.tsx @@ -9,7 +9,7 @@ import { useGridApiRef, } from '@mui/x-data-grid-pro'; import { entries } from '@seedcompany/common'; -import { useState } from 'react'; +import { useMemo } from 'react'; import { extendSx } from '~/common'; import { getInitialVisibility, @@ -19,6 +19,7 @@ import { Toolbar, useFilterToggle, } from '~/components/Grid'; +import { useSet } from '~/hooks'; import { ExpansionMarker, ProgressReportsColumnMap, @@ -107,7 +108,7 @@ export const ProgressReportsExpandedGrid = ( ) => { const apiRef = useGridApiRef(); - const [selected, setSelected] = useState([]); + const selected = useSet(); return ( setSelected(selected.length > 0 ? [] : [id])} - rowSelectionModel={selected} + onRowClick={({ id }) => selected.toggle(id)} + rowSelectionModel={useMemo(() => [...selected], [selected])} getRowHeight={(params) => apiRef.current.isRowSelected(params.id) ? 'auto' : COLLAPSED_ROW_HEIGHT } From 2217aedb036a84e274302632b7494557addc18e4 Mon Sep 17 00:00:00 2001 From: Carson Full Date: Sun, 13 Oct 2024 10:50:53 -0500 Subject: [PATCH 4/8] Split expansion state to be independent of selected --- .../ProgressReportsWidget/ExpansionCell.tsx | 14 ++---- .../ProgressReportsExpandedGrid.tsx | 47 ++++++++++--------- .../ProgressReportsWidget/expansionState.tsx | 9 ++++ 3 files changed, 36 insertions(+), 34 deletions(-) create mode 100644 src/scenes/Dashboard/ProgressReportsWidget/expansionState.tsx diff --git a/src/scenes/Dashboard/ProgressReportsWidget/ExpansionCell.tsx b/src/scenes/Dashboard/ProgressReportsWidget/ExpansionCell.tsx index 946841f412..d37b85f735 100644 --- a/src/scenes/Dashboard/ProgressReportsWidget/ExpansionCell.tsx +++ b/src/scenes/Dashboard/ProgressReportsWidget/ExpansionCell.tsx @@ -1,23 +1,15 @@ import { Box } from '@mui/material'; -import { - GridState, - GridRenderCellParams as RenderCellParams, - useGridSelector, -} from '@mui/x-data-grid'; +import { GridRenderCellParams as RenderCellParams } from '@mui/x-data-grid'; import { ChildrenProp, extendSx, StyleProps } from '~/common'; +import { useExpanded } from './expansionState'; export const ExpansionCell = ({ id, - api, sx, className, children, }: Pick & StyleProps & ChildrenProp) => { - const selectedRows = useGridSelector( - { current: api }, - (state: GridState) => state.rowSelection - ); - const isExpanded = selectedRows.includes(id); + const isExpanded = useExpanded().has(id); return ( { const apiRef = useGridApiRef(); - const selected = useSet(); + const expanded = useSet(); return ( - selected.toggle(id)} - rowSelectionModel={useMemo(() => [...selected], [selected])} - getRowHeight={(params) => - apiRef.current.isRowSelected(params.id) ? 'auto' : COLLAPSED_ROW_HEIGHT - } - sx={[ - { - // Don't want 'auto' to shrink below this when the cell is empty - '.MuiDataGrid-cell': { - minHeight: COLLAPSED_ROW_HEIGHT, + + expanded.toggle(id)} + getRowHeight={(params) => + expanded.has(params.id) ? 'auto' : COLLAPSED_ROW_HEIGHT + } + sx={[ + { + // Don't want 'auto' to shrink below this when the cell is empty + '.MuiDataGrid-cell': { + minHeight: COLLAPSED_ROW_HEIGHT, + }, }, - }, - ...extendSx(props.sx), - ]} - /> + ...extendSx(props.sx), + ]} + /> + ); }; diff --git a/src/scenes/Dashboard/ProgressReportsWidget/expansionState.tsx b/src/scenes/Dashboard/ProgressReportsWidget/expansionState.tsx new file mode 100644 index 0000000000..7356fa14ed --- /dev/null +++ b/src/scenes/Dashboard/ProgressReportsWidget/expansionState.tsx @@ -0,0 +1,9 @@ +import { GridRowId } from '@mui/x-data-grid-pro'; +import { createContext, useContext } from 'react'; +import { SetHook } from '~/hooks'; + +export const ExpansionContext = createContext>( + new Set() as any +); + +export const useExpanded = () => useContext(ExpansionContext); From 6f5c3332eea0c94bfde6ae2be9fe7925b4643620 Mon Sep 17 00:00:00 2001 From: Carson Full Date: Sun, 13 Oct 2024 10:51:41 -0500 Subject: [PATCH 5/8] Fix Quarter Select comparison of current value to available options --- .../Dashboard/ProgressReportsWidget/ProgressReportsWidget.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/scenes/Dashboard/ProgressReportsWidget/ProgressReportsWidget.tsx b/src/scenes/Dashboard/ProgressReportsWidget/ProgressReportsWidget.tsx index b03593a101..a8cf5f076a 100644 --- a/src/scenes/Dashboard/ProgressReportsWidget/ProgressReportsWidget.tsx +++ b/src/scenes/Dashboard/ProgressReportsWidget/ProgressReportsWidget.tsx @@ -40,6 +40,7 @@ export const ProgressReportsWidget = ({ disablePortal options={quarter.available} getOptionLabel={(q) => `Q${q.fiscalQuarter} FY${q.fiscalYear}`} + isOptionEqualToValue={(a, b) => +a === +b} value={quarter.current} onChange={(_, q) => quarter.set(q)} disableClearable From 366ec9582e600388057c33bc8bd0c6b5a72e42d5 Mon Sep 17 00:00:00 2001 From: Carson Full Date: Sun, 13 Oct 2024 10:57:29 -0500 Subject: [PATCH 6/8] Add expand/collapse all buttons --- .../ProgressReportsExpandedGrid.tsx | 8 ++++- .../ProgressReportsWidget/expansionState.tsx | 33 ++++++++++++++++++- 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/src/scenes/Dashboard/ProgressReportsWidget/ProgressReportsExpandedGrid.tsx b/src/scenes/Dashboard/ProgressReportsWidget/ProgressReportsExpandedGrid.tsx index 994877fb8d..5490138778 100644 --- a/src/scenes/Dashboard/ProgressReportsWidget/ProgressReportsExpandedGrid.tsx +++ b/src/scenes/Dashboard/ProgressReportsWidget/ProgressReportsExpandedGrid.tsx @@ -19,7 +19,11 @@ import { useFilterToggle, } from '~/components/Grid'; import { useSet } from '~/hooks'; -import { ExpansionContext } from './expansionState'; +import { + CollapseAllButton, + ExpandAllButton, + ExpansionContext, +} from './expansionState'; import { ExpansionMarker, ProgressReportsColumnMap, @@ -96,6 +100,8 @@ const ProgressReportsToolbar = () => ( Pinned + + ); diff --git a/src/scenes/Dashboard/ProgressReportsWidget/expansionState.tsx b/src/scenes/Dashboard/ProgressReportsWidget/expansionState.tsx index 7356fa14ed..21fa5734be 100644 --- a/src/scenes/Dashboard/ProgressReportsWidget/expansionState.tsx +++ b/src/scenes/Dashboard/ProgressReportsWidget/expansionState.tsx @@ -1,4 +1,9 @@ -import { GridRowId } from '@mui/x-data-grid-pro'; +import { + UnfoldLess as CollapseIcon, + UnfoldMore as ExpandIcon, +} from '@mui/icons-material'; +import { IconButton, Tooltip } from '@mui/material'; +import { GridRowId, useGridApiContext } from '@mui/x-data-grid-pro'; import { createContext, useContext } from 'react'; import { SetHook } from '~/hooks'; @@ -7,3 +12,29 @@ export const ExpansionContext = createContext>( ); export const useExpanded = () => useContext(ExpansionContext); + +export const CollapseAllButton = () => { + const expanded = useExpanded(); + return ( + + + + + + ); +}; + +export const ExpandAllButton = () => { + const apiRef = useGridApiContext(); + const expanded = useExpanded(); + const expandAll = () => { + expanded.set(apiRef.current.state.rows.dataRowIds); + }; + return ( + + + + + + ); +}; From ec00a4b3af031f143cefe3380fee44d521f4b70c Mon Sep 17 00:00:00 2001 From: Carson Full Date: Wed, 9 Oct 2024 21:40:47 -0500 Subject: [PATCH 7/8] Avoid toggling expansion when the user intends to drag instead of click --- .../ProgressReportsExpandedGrid.tsx | 18 +++++-- .../ProgressReportsWidget/expansionState.tsx | 49 +++++++++++++++++-- 2 files changed, 60 insertions(+), 7 deletions(-) diff --git a/src/scenes/Dashboard/ProgressReportsWidget/ProgressReportsExpandedGrid.tsx b/src/scenes/Dashboard/ProgressReportsWidget/ProgressReportsExpandedGrid.tsx index 5490138778..ff1f0244fa 100644 --- a/src/scenes/Dashboard/ProgressReportsWidget/ProgressReportsExpandedGrid.tsx +++ b/src/scenes/Dashboard/ProgressReportsWidget/ProgressReportsExpandedGrid.tsx @@ -3,12 +3,12 @@ import { DataGridProProps as DataGridProps, GridColDef, GridRenderCellParams, - GridRowId, GridToolbarColumnsButton, GridToolbarFilterButton, useGridApiRef, } from '@mui/x-data-grid-pro'; import { entries } from '@seedcompany/common'; +import { useMemo } from 'react'; import { extendSx } from '~/common'; import { getInitialVisibility, @@ -18,11 +18,11 @@ import { Toolbar, useFilterToggle, } from '~/components/Grid'; -import { useSet } from '~/hooks'; import { CollapseAllButton, ExpandAllButton, ExpansionContext, + useExpandedSetup, } from './expansionState'; import { ExpansionMarker, @@ -114,7 +114,16 @@ export const ProgressReportsExpandedGrid = ( ) => { const apiRef = useGridApiRef(); - const expanded = useSet(); + const { expanded, onMouseDown, onRowClick } = useExpandedSetup(); + + const slotProps = useMemo( + (): DataGridProps['slotProps'] => ({ + row: { + onMouseDown, + }, + }), + [onMouseDown] + ); return ( @@ -125,7 +134,8 @@ export const ProgressReportsExpandedGrid = ( apiRef={apiRef} columns={columns} initialState={initialState} - onRowClick={({ id }) => expanded.toggle(id)} + slotProps={slotProps} + onRowClick={onRowClick} getRowHeight={(params) => expanded.has(params.id) ? 'auto' : COLLAPSED_ROW_HEIGHT } diff --git a/src/scenes/Dashboard/ProgressReportsWidget/expansionState.tsx b/src/scenes/Dashboard/ProgressReportsWidget/expansionState.tsx index 21fa5734be..d6bec9ea9f 100644 --- a/src/scenes/Dashboard/ProgressReportsWidget/expansionState.tsx +++ b/src/scenes/Dashboard/ProgressReportsWidget/expansionState.tsx @@ -3,9 +3,19 @@ import { UnfoldMore as ExpandIcon, } from '@mui/icons-material'; import { IconButton, Tooltip } from '@mui/material'; -import { GridRowId, useGridApiContext } from '@mui/x-data-grid-pro'; -import { createContext, useContext } from 'react'; -import { SetHook } from '~/hooks'; +import { + DataGridProProps as GridProps, + GridRowId, + useGridApiContext, +} from '@mui/x-data-grid-pro'; +import { + createContext, + MouseEvent, + useCallback, + useContext, + useRef, +} from 'react'; +import { SetHook, useSet } from '~/hooks'; export const ExpansionContext = createContext>( new Set() as any @@ -38,3 +48,36 @@ export const ExpandAllButton = () => { ); }; + +// if moving more than this many pixels, will consider the click event +// to be movement rather than a click. +// i.e. to select the text within. +const MOVEMENT_THRESHOLD = 10; + +export const useExpandedSetup = () => { + const expanded = useSet(); + + const lastDownPos = useRef({ x: 0, y: 0 }); + const onMouseDown = useCallback((e: MouseEvent) => { + lastDownPos.current = { x: e.clientX, y: e.clientY }; + }, []); + + const onRowClick = useCallback( + ({ id }, event) => { + const now = { x: event.clientX, y: event.clientY }; + const prev = lastDownPos.current; + if ( + Math.abs(now.x - prev.x) > MOVEMENT_THRESHOLD || + Math.abs(now.y - prev.y) > MOVEMENT_THRESHOLD + ) { + // This click (up) event appears to be a drag, not a click. + return; + } + expanded.toggle(id); + }, + // eslint-disable-next-line react-hooks/exhaustive-deps + [] + ); + + return { expanded, onMouseDown, onRowClick }; +}; From 41b127d17e27a6ea623400a5de4fd73c3cbe2cd5 Mon Sep 17 00:00:00 2001 From: Carson Full Date: Thu, 10 Oct 2024 10:37:33 -0500 Subject: [PATCH 8/8] Avoid collapsing on double click (need to delay collapse) --- .../ProgressReportsWidget/expansionState.tsx | 71 ++++++++++++++++--- 1 file changed, 61 insertions(+), 10 deletions(-) diff --git a/src/scenes/Dashboard/ProgressReportsWidget/expansionState.tsx b/src/scenes/Dashboard/ProgressReportsWidget/expansionState.tsx index d6bec9ea9f..11528f05a8 100644 --- a/src/scenes/Dashboard/ProgressReportsWidget/expansionState.tsx +++ b/src/scenes/Dashboard/ProgressReportsWidget/expansionState.tsx @@ -54,29 +54,80 @@ export const ExpandAllButton = () => { // i.e. to select the text within. const MOVEMENT_THRESHOLD = 10; +interface Position { + x: number; + y: number; +} +const isPosClose = (a: Position, b: Position) => + Math.abs(a.x - b.x) < MOVEMENT_THRESHOLD && + Math.abs(a.y - b.y) < MOVEMENT_THRESHOLD; +const eventPos = (e: MouseEvent) => ({ x: e.clientX, y: e.clientY }); + export const useExpandedSetup = () => { const expanded = useSet(); - const lastDownPos = useRef({ x: 0, y: 0 }); + const { current: lastDownPositions } = useRef(new Set()); + const { current: collapseTimers } = useRef(new Map()); + const onMouseDown = useCallback((e: MouseEvent) => { - lastDownPos.current = { x: e.clientX, y: e.clientY }; + if (!(e.target instanceof HTMLElement)) { + return; + } + const id = e.target.closest('[role="row"]')?.getAttribute('data-id'); + if (!id) { + return; + } + const pos = eventPos(e); + + const hasPrev = [...lastDownPositions].some((prev) => + isPosClose(prev, pos) + ); + if (hasPrev) { + const prevTimer = collapseTimers.get(id); + if (prevTimer) { + // console.log('cancelling collapse', id); + clearTimeout(prevTimer); + collapseTimers.delete(id); + } + } + // console.log('marking down pos'); + lastDownPositions.add(pos); + setTimeout(() => { + // console.log('removing down pos'); + lastDownPositions.delete(pos); + }, 500); + // eslint-disable-next-line react-hooks/exhaustive-deps }, []); const onRowClick = useCallback( ({ id }, event) => { - const now = { x: event.clientX, y: event.clientY }; - const prev = lastDownPos.current; - if ( - Math.abs(now.x - prev.x) > MOVEMENT_THRESHOLD || - Math.abs(now.y - prev.y) > MOVEMENT_THRESHOLD - ) { + const now = eventPos(event); + const prev = [...lastDownPositions].at(-1); + if (!prev || !isPosClose(prev, now)) { + // console.log('up, but dragging'); // This click (up) event appears to be a drag, not a click. return; } - expanded.toggle(id); + if (event.detail > 1) { + // console.log('double click', event.detail); + const timer = collapseTimers.get(id); + clearTimeout(timer); + collapseTimers.delete(id); + return; + } + // console.log('single click'); + if (!expanded.has(id)) { + expanded.add(id); + } else if (!collapseTimers.has(id)) { + const timer = window.setTimeout(() => { + collapseTimers.delete(id); + expanded.remove(id); + }, 500); + collapseTimers.set(id, timer); + } }, // eslint-disable-next-line react-hooks/exhaustive-deps - [] + [expanded] ); return { expanded, onMouseDown, onRowClick };