From 32d3e895b3c956003c2bebdcbda3a96418a5f022 Mon Sep 17 00:00:00 2001 From: Josh Hunt Date: Tue, 2 May 2023 10:17:25 +0000 Subject: [PATCH] NestedFolders: Indicate when folders have mixed-selection children (#67337) * Show indeterminate checkbox for folders with partially selected children * When selecting an item, check ancestors to see if all their children are now selected * reword comment * fix test * fix lint * Check all descendants for mixed state * Use indeterminate checkbox * fix test description * make header checkbox select/unselect automatically * mixed header checkbox: * fix tests * add tests --- .../BrowseDashboardsPage.test.tsx | 5 +- .../components/BrowseView.test.tsx | 20 ++++ .../components/BrowseView.tsx | 77 +++++++++++- .../components/CheckboxCell.tsx | 29 +++++ .../components/CheckboxHeaderCell.tsx | 17 +++ .../components/DashboardsTree.test.tsx | 18 ++- .../components/DashboardsTree.tsx | 87 ++++++-------- .../features/browse-dashboards/state/hooks.ts | 4 + .../browse-dashboards/state/reducers.test.ts | 113 +++++++++++++++++- .../browse-dashboards/state/reducers.ts | 45 ++++--- .../app/features/browse-dashboards/types.ts | 20 ++++ 11 files changed, 344 insertions(+), 91 deletions(-) create mode 100644 public/app/features/browse-dashboards/components/CheckboxCell.tsx create mode 100644 public/app/features/browse-dashboards/components/CheckboxHeaderCell.tsx diff --git a/public/app/features/browse-dashboards/BrowseDashboardsPage.test.tsx b/public/app/features/browse-dashboards/BrowseDashboardsPage.test.tsx index 27c5724157b..3cd8b448e51 100644 --- a/public/app/features/browse-dashboards/BrowseDashboardsPage.test.tsx +++ b/public/app/features/browse-dashboards/BrowseDashboardsPage.test.tsx @@ -75,9 +75,10 @@ describe('browse-dashboards BrowseDashboardsPage', () => { it('displays the filters and hides the actions initially', async () => { render(); + await screen.findByPlaceholderText('Search for dashboards and folders'); - expect(await screen.findByText('Sort')).toBeInTheDocument(); - expect(await screen.findByText('Filter by tag')).toBeInTheDocument(); + expect(screen.queryByText('Sort')).toBeInTheDocument(); + expect(screen.queryByText('Filter by tag')).toBeInTheDocument(); expect(screen.queryByRole('button', { name: 'Move' })).not.toBeInTheDocument(); expect(screen.queryByRole('button', { name: 'Delete' })).not.toBeInTheDocument(); diff --git a/public/app/features/browse-dashboards/components/BrowseView.test.tsx b/public/app/features/browse-dashboards/components/BrowseView.test.tsx index 24d262105ca..3b176523902 100644 --- a/public/app/features/browse-dashboards/components/BrowseView.test.tsx +++ b/public/app/features/browse-dashboards/components/BrowseView.test.tsx @@ -116,6 +116,26 @@ describe('browse-dashboards BrowseView', () => { const grandparentCheckbox = screen.queryByTestId(selectors.pages.BrowseDashbards.table.checkbox(folderA.item.uid)); expect(grandparentCheckbox).not.toBeChecked(); }); + + it('shows indeterminate checkboxes when a descendant is selected', async () => { + render(); + await screen.findByText(folderA.item.title); + + await expandFolder(folderA.item.uid); + await expandFolder(folderA_folderB.item.uid); + + await clickCheckbox(folderA_folderB_dashbdB.item.uid); + + const parentCheckbox = screen.queryByTestId( + selectors.pages.BrowseDashbards.table.checkbox(folderA_folderB.item.uid) + ); + expect(parentCheckbox).not.toBeChecked(); + expect(parentCheckbox).toBePartiallyChecked(); + + const grandparentCheckbox = screen.queryByTestId(selectors.pages.BrowseDashbards.table.checkbox(folderA.item.uid)); + expect(grandparentCheckbox).not.toBeChecked(); + expect(grandparentCheckbox).toBePartiallyChecked(); + }); }); async function expandFolder(uid: string) { diff --git a/public/app/features/browse-dashboards/components/BrowseView.tsx b/public/app/features/browse-dashboards/components/BrowseView.tsx index d3f9b2e927d..021e02e5e83 100644 --- a/public/app/features/browse-dashboards/components/BrowseView.tsx +++ b/public/app/features/browse-dashboards/components/BrowseView.tsx @@ -9,8 +9,10 @@ import { fetchChildren, setFolderOpenState, setItemSelectionState, + useChildrenByParentUIDState, setAllSelection, } from '../state'; +import { DashboardTreeSelection, SelectionState } from '../types'; import { DashboardsTree } from './DashboardsTree'; @@ -25,10 +27,7 @@ export function BrowseView({ folderUID, width, height, canSelect }: BrowseViewPr const dispatch = useDispatch(); const flatTree = useFlatTreeState(folderUID); const selectedItems = useCheckboxSelectionState(); - - useEffect(() => { - dispatch(fetchChildren(folderUID)); - }, [dispatch, folderUID]); + const childrenByParentUID = useChildrenByParentUIDState(); const handleFolderClick = useCallback( (clickedFolderUID: string, isOpen: boolean) => { @@ -41,6 +40,10 @@ export function BrowseView({ folderUID, width, height, canSelect }: BrowseViewPr [dispatch] ); + useEffect(() => { + dispatch(fetchChildren(folderUID)); + }, [handleFolderClick, dispatch, folderUID]); + const handleItemSelectionChange = useCallback( (item: DashboardViewItem, isSelected: boolean) => { dispatch(setItemSelectionState({ item, isSelected })); @@ -48,16 +51,80 @@ export function BrowseView({ folderUID, width, height, canSelect }: BrowseViewPr [dispatch] ); + const isSelected = useCallback( + (item: DashboardViewItem | '$all'): SelectionState => { + if (item === '$all') { + // We keep the boolean $all state up to date in redux, so we can short-circut + // the logic if we know this has been selected + if (selectedItems.$all) { + return SelectionState.Selected; + } + + // Otherwise, if we have any selected items, then it should be in 'mixed' state + for (const selection of Object.values(selectedItems)) { + if (typeof selection === 'boolean') { + continue; + } + + for (const uid in selection) { + const isSelected = selection[uid]; + if (isSelected) { + return SelectionState.Mixed; + } + } + } + + // Otherwise otherwise, nothing is selected and header should be unselected + return SelectionState.Unselected; + } + + const isSelected = selectedItems[item.kind][item.uid]; + if (isSelected) { + return SelectionState.Selected; + } + + // Because if _all_ children, then the parent is selected (and bailed in the previous check), + // this .some check will only return true if the children are partially selected + const isMixed = hasSelectedDescendants(item, childrenByParentUID, selectedItems); + if (isMixed) { + return SelectionState.Mixed; + } + + return SelectionState.Unselected; + }, + [selectedItems, childrenByParentUID] + ); + return ( dispatch(setAllSelection({ isSelected: newState }))} onItemSelectionChange={handleItemSelectionChange} /> ); } + +function hasSelectedDescendants( + item: DashboardViewItem, + childrenByParentUID: Record, + selectedItems: DashboardTreeSelection +): boolean { + const children = childrenByParentUID[item.uid]; + if (!children) { + return false; + } + + return children.some((v) => { + const thisIsSelected = selectedItems[v.kind][v.uid]; + if (thisIsSelected) { + return thisIsSelected; + } + + return hasSelectedDescendants(v, childrenByParentUID, selectedItems); + }); +} diff --git a/public/app/features/browse-dashboards/components/CheckboxCell.tsx b/public/app/features/browse-dashboards/components/CheckboxCell.tsx new file mode 100644 index 00000000000..0ab480efb58 --- /dev/null +++ b/public/app/features/browse-dashboards/components/CheckboxCell.tsx @@ -0,0 +1,29 @@ +import React from 'react'; + +import { selectors } from '@grafana/e2e-selectors'; +import { Checkbox } from '@grafana/ui'; + +import { DashboardsTreeCellProps, SelectionState } from '../types'; + +export default function CheckboxCell({ + row: { original: row }, + isSelected, + onItemSelectionChange, +}: DashboardsTreeCellProps) { + const item = row.item; + + if (item.kind === 'ui-empty-folder' || !isSelected) { + return null; + } + + const state = isSelected(item); + + return ( + onItemSelectionChange?.(item, ev.currentTarget.checked)} + /> + ); +} diff --git a/public/app/features/browse-dashboards/components/CheckboxHeaderCell.tsx b/public/app/features/browse-dashboards/components/CheckboxHeaderCell.tsx new file mode 100644 index 00000000000..910a18103b3 --- /dev/null +++ b/public/app/features/browse-dashboards/components/CheckboxHeaderCell.tsx @@ -0,0 +1,17 @@ +import React from 'react'; + +import { Checkbox } from '@grafana/ui'; + +import { DashboardTreeHeaderProps, SelectionState } from '../types'; + +export default function CheckboxHeaderCell({ isSelected, onAllSelectionChange }: DashboardTreeHeaderProps) { + const state = isSelected?.('$all') ?? SelectionState.Unselected; + + return ( + onAllSelectionChange?.(ev.currentTarget.checked)} + /> + ); +} diff --git a/public/app/features/browse-dashboards/components/DashboardsTree.test.tsx b/public/app/features/browse-dashboards/components/DashboardsTree.test.tsx index 0f3192636e6..f070bcd26b6 100644 --- a/public/app/features/browse-dashboards/components/DashboardsTree.test.tsx +++ b/public/app/features/browse-dashboards/components/DashboardsTree.test.tsx @@ -7,6 +7,7 @@ import { assertIsDefined } from 'test/helpers/asserts'; import { selectors } from '@grafana/e2e-selectors'; import { wellFormedDashboard, wellFormedEmptyFolder, wellFormedFolder } from '../fixtures/dashboardsTreeItem.fixture'; +import { SelectionState } from '../types'; import { DashboardsTree } from './DashboardsTree'; @@ -22,19 +23,14 @@ describe('browse-dashboards DashboardsTree', () => { const emptyFolderIndicator = wellFormedEmptyFolder(); const dashboard = wellFormedDashboard(2); const noop = () => {}; - const selectedItems = { - $all: false, - folder: {}, - dashboard: {}, - panel: {}, - }; + const isSelected = () => SelectionState.Unselected; it('renders a dashboard item', () => { render( { { { { SelectionState; onFolderClick: (uid: string, newOpenState: boolean) => void; onAllSelectionChange: (newState: boolean) => void; onItemSelectionChange: (item: DashboardViewItem, newState: boolean) => void; canSelect: boolean; } -type DashboardsTreeColumn = Column; -type DashboardTreeHeaderProps = HeaderProps & { - // Note: userProps for cell renderers (e.g. second argument in `cell.render('Cell', foo)` ) - // aren't typed, so we must be careful when accessing this - selectedItems?: DashboardsTreeProps['selectedItems']; -}; -type DashboardsTreeCellProps = CellProps & { - selectedItems?: DashboardsTreeProps['selectedItems']; -}; - const HEADER_HEIGHT = 35; const ROW_HEIGHT = 35; @@ -43,7 +41,7 @@ export function DashboardsTree({ items, width, height, - selectedItems, + isSelected, onFolderClick, onAllSelectionChange, onItemSelectionChange, @@ -52,32 +50,12 @@ export function DashboardsTree({ const styles = useStyles2(getStyles); const tableColumns = useMemo(() => { - const checkboxColumn: DashboardsTreeColumn | null = canSelect - ? { - id: 'checkbox', - width: 0, - Header: ({ selectedItems }: DashboardTreeHeaderProps) => { - const isAllSelected = selectedItems?.$all ?? false; - return onAllSelectionChange(ev.currentTarget.checked)} />; - }, - Cell: ({ row: { original: row }, selectedItems }: DashboardsTreeCellProps) => { - const item = row.item; - if (item.kind === 'ui-empty-folder' || !selectedItems) { - return <>; - } - - const isSelected = selectedItems?.[item.kind][item.uid] ?? false; - - return ( - onItemSelectionChange(item, ev.currentTarget.checked)} - /> - ); - }, - } - : null; + const checkboxColumn: DashboardsTreeColumn = { + id: 'checkbox', + width: 0, + Header: CheckboxHeaderCell, + Cell: CheckboxCell, + }; const nameColumn: DashboardsTreeColumn = { id: 'name', @@ -102,17 +80,20 @@ export function DashboardsTree({ const columns = [canSelect && checkboxColumn, nameColumn, typeColumn, tagsColumns].filter(isTruthy); return columns; - }, [onItemSelectionChange, onAllSelectionChange, onFolderClick, canSelect]); + }, [onFolderClick, canSelect]); const table = useTable({ columns: tableColumns, data: items }, useCustomFlexLayout); const { getTableProps, getTableBodyProps, headerGroups } = table; - const virtualData = useMemo(() => { - return { + const virtualData = useMemo( + () => ({ table, - selectedItems, - }; - }, [table, selectedItems]); + isSelected, + onAllSelectionChange, + onItemSelectionChange, + }), + [table, isSelected, onAllSelectionChange, onItemSelectionChange] + ); return (
@@ -128,7 +109,7 @@ export function DashboardsTree({ return (
- {column.render('Header', { selectedItems })} + {column.render('Header', { isSelected, onAllSelectionChange })}
); })} @@ -156,13 +137,15 @@ interface VirtualListRowProps { style: React.CSSProperties; data: { table: TableInstance; - selectedItems: Record>; + isSelected: DashboardsTreeCellProps['isSelected']; + onAllSelectionChange: DashboardsTreeCellProps['onAllSelectionChange']; + onItemSelectionChange: DashboardsTreeCellProps['onItemSelectionChange']; }; } function VirtualListRow({ index, style, data }: VirtualListRowProps) { const styles = useStyles2(getStyles); - const { table, selectedItems } = data; + const { table, isSelected, onItemSelectionChange } = data; const { rows, prepareRow } = table; const row = rows[index]; @@ -179,7 +162,7 @@ function VirtualListRow({ index, style, data }: VirtualListRowProps) { return (
- {cell.render('Cell', { selectedItems })} + {cell.render('Cell', { isSelected, onItemSelectionChange })}
); })} diff --git a/public/app/features/browse-dashboards/state/hooks.ts b/public/app/features/browse-dashboards/state/hooks.ts index 3bd672b0946..097ef3ebb7e 100644 --- a/public/app/features/browse-dashboards/state/hooks.ts +++ b/public/app/features/browse-dashboards/state/hooks.ts @@ -44,6 +44,10 @@ export function useCheckboxSelectionState() { return useSelector((wholeState: StoreState) => wholeState.browseDashboards.selectedItems); } +export function useChildrenByParentUIDState() { + return useSelector((wholeState: StoreState) => wholeState.browseDashboards.childrenByParentUID); +} + export function useActionSelectionState() { return useSelector((state) => selectedItemsForActionsSelector(state)); } diff --git a/public/app/features/browse-dashboards/state/reducers.test.ts b/public/app/features/browse-dashboards/state/reducers.test.ts index 32e5b3bbb99..1bf0ce0d41a 100644 --- a/public/app/features/browse-dashboards/state/reducers.test.ts +++ b/public/app/features/browse-dashboards/state/reducers.test.ts @@ -115,8 +115,10 @@ describe('browse-dashboards reducers', () => { describe('setItemSelectionState', () => { it('marks items as selected', () => { + const folder = wellFormedFolder(1).item; + const dashboard = wellFormedDashboard(2).item; const state = createInitialState(); - const dashboard = wellFormedDashboard().item; + state.rootItems = [folder, dashboard]; setItemSelectionState(state, { type: 'setItemSelectionState', payload: { item: dashboard, isSelected: true } }); @@ -133,11 +135,13 @@ describe('browse-dashboards reducers', () => { it('marks descendants as selected when the parent folder is selected', () => { const state = createInitialState(); - const parentFolder = wellFormedFolder(1).item; - const childDashboard = wellFormedDashboard(2, {}, { parentUID: parentFolder.uid }).item; - const childFolder = wellFormedFolder(3, {}, { parentUID: parentFolder.uid }).item; - const grandchildDashboard = wellFormedDashboard(4, {}, { parentUID: childFolder.uid }).item; + const rootDashboard = wellFormedDashboard(1).item; + const parentFolder = wellFormedFolder(2).item; + const childDashboard = wellFormedDashboard(3, {}, { parentUID: parentFolder.uid }).item; + const childFolder = wellFormedFolder(4, {}, { parentUID: parentFolder.uid }).item; + const grandchildDashboard = wellFormedDashboard(5, {}, { parentUID: childFolder.uid }).item; + state.rootItems = [parentFolder, rootDashboard]; state.childrenByParentUID[parentFolder.uid] = [childDashboard, childFolder]; state.childrenByParentUID[childFolder.uid] = [grandchildDashboard]; @@ -196,6 +200,105 @@ describe('browse-dashboards reducers', () => { panel: {}, }); }); + + it('selects ancestors when all their children are now selected', () => { + const state = createInitialState(); + + const rootDashboard = wellFormedDashboard(1).item; + const parentFolder = wellFormedFolder(2).item; + const childDashboard = wellFormedDashboard(3, {}, { parentUID: parentFolder.uid }).item; + const childFolder = wellFormedFolder(4, {}, { parentUID: parentFolder.uid }).item; + const grandchildDashboard = wellFormedDashboard(5, {}, { parentUID: childFolder.uid }).item; + + state.rootItems = [parentFolder, rootDashboard]; + state.childrenByParentUID[parentFolder.uid] = [childDashboard, childFolder]; + state.childrenByParentUID[childFolder.uid] = [grandchildDashboard]; + + // Selected the deepest grandchild dashboard + setItemSelectionState(state, { + type: 'setItemSelectionState', + payload: { item: grandchildDashboard, isSelected: true }, + }); + + expect(state.selectedItems).toEqual({ + $all: false, + dashboard: { + [grandchildDashboard.uid]: true, + }, + folder: { + [parentFolder.uid]: false, + [childFolder.uid]: true, // is selected because all it's children (grandchildDashboard) is selected + }, + panel: {}, + }); + + setItemSelectionState(state, { + type: 'setItemSelectionState', + payload: { item: childDashboard, isSelected: true }, + }); + + expect(state.selectedItems).toEqual({ + $all: false, + dashboard: { + [childDashboard.uid]: true, + [grandchildDashboard.uid]: true, + }, + folder: { + [parentFolder.uid]: true, // is now selected because we also selected its other child + [childFolder.uid]: true, + }, + panel: {}, + }); + }); + + it('selects the $all header checkbox when all descendants are now selected', () => { + const state = createInitialState(); + + const rootDashboard = wellFormedDashboard(1).item; + const rootFolder = wellFormedFolder(2).item; + const childDashboardA = wellFormedDashboard(3, {}, { parentUID: rootFolder.uid }).item; + const childDashboardB = wellFormedDashboard(4, {}, { parentUID: rootFolder.uid }).item; + + state.rootItems = [rootFolder, rootDashboard]; + state.childrenByParentUID[rootFolder.uid] = [childDashboardA, childDashboardB]; + + state.selectedItems.dashboard = { [rootDashboard.uid]: true, [childDashboardA.uid]: true }; + + // Selected the deepest grandchild dashboard + setItemSelectionState(state, { + type: 'setItemSelectionState', + payload: { item: childDashboardB, isSelected: true }, + }); + + expect(state.selectedItems.$all).toBeTruthy(); + }); + + it('unselects the $all header checkbox a descendant is unselected', () => { + const state = createInitialState(); + + const rootDashboard = wellFormedDashboard(1).item; + const rootFolder = wellFormedFolder(2).item; + const childDashboardA = wellFormedDashboard(3, {}, { parentUID: rootFolder.uid }).item; + const childDashboardB = wellFormedDashboard(4, {}, { parentUID: rootFolder.uid }).item; + + state.rootItems = [rootFolder, rootDashboard]; + state.childrenByParentUID[rootFolder.uid] = [childDashboardA, childDashboardB]; + + state.selectedItems.dashboard = { + [rootDashboard.uid]: true, + [childDashboardA.uid]: true, + [childDashboardB.uid]: true, + }; + state.selectedItems.folder = { [rootFolder.uid]: true }; + + // Selected the deepest grandchild dashboard + setItemSelectionState(state, { + type: 'setItemSelectionState', + payload: { item: childDashboardB, isSelected: false }, + }); + + expect(state.selectedItems.$all).toBeFalsy(); + }); }); describe('setAllSelection', () => { diff --git a/public/app/features/browse-dashboards/state/reducers.ts b/public/app/features/browse-dashboards/state/reducers.ts index ac79052360c..86414f423ec 100644 --- a/public/app/features/browse-dashboards/state/reducers.ts +++ b/public/app/features/browse-dashboards/state/reducers.ts @@ -45,6 +45,8 @@ export function setItemSelectionState( ) { const { item, isSelected } = action.payload; + // Selecting a folder selects all children, and unselecting a folder deselects all children + // so propagate the new selection state to all descendants function markChildren(kind: DashboardViewItemKind, uid: string) { state.selectedItems[kind][uid] = isSelected; @@ -60,26 +62,37 @@ export function setItemSelectionState( markChildren(item.kind, item.uid); - // If we're unselecting an item, unselect all ancestors (parent, grandparent, etc) also - // so we can later show a UI-only 'mixed' checkbox - if (!isSelected) { - let nextParentUID = item.parentUID; + // If all children of a folder are selected, then the folder is also selected. + // If *any* child of a folder is unselelected, then the folder is alo unselected. + // Reconcile all ancestors to make sure they're in the correct state. + let nextParentUID = item.parentUID; - // this is like a recursive climb up the parents of the tree while we have a - // parentUID (we've hit a root dashboard/folder) - while (nextParentUID) { - const parent = findItem(state.rootItems, state.childrenByParentUID, nextParentUID); + while (nextParentUID) { + const parent = findItem(state.rootItems, state.childrenByParentUID, nextParentUID); - // This case should not happen, but a find can theortically return undefined, and it - // helps limit infinite loops - if (!parent) { - break; - } - - state.selectedItems[parent.kind][parent.uid] = false; - nextParentUID = parent.parentUID; + // This case should not happen, but a find can theortically return undefined, and it + // helps limit infinite loops + if (!parent) { + break; } + + if (isSelected) { + // If we're selecting an item, check all ancestors and see if all their children are + // now selected and update them appropriately + const children = state.childrenByParentUID[parent.uid]; + + const allChildrenSelected = children?.every((v) => state.selectedItems[v.kind][v.uid]) ?? false; + state.selectedItems[parent.kind][parent.uid] = allChildrenSelected; + } else { + // A folder cannot be selected if any of it's children are unselected + state.selectedItems[parent.kind][parent.uid] = false; + } + + nextParentUID = parent.parentUID; } + + // Check to see if we should mark the header checkbox selected if all root items are selected + state.selectedItems.$all = state.rootItems.every((v) => state.selectedItems[v.kind][v.uid]) ?? false; } export function setAllSelection(state: BrowseDashboardsState, action: PayloadAction<{ isSelected: boolean }>) { diff --git a/public/app/features/browse-dashboards/types.ts b/public/app/features/browse-dashboards/types.ts index 7ace5982127..aa97eb5d737 100644 --- a/public/app/features/browse-dashboards/types.ts +++ b/public/app/features/browse-dashboards/types.ts @@ -1,3 +1,5 @@ +import { CellProps, Column, HeaderProps } from 'react-table'; + import { DashboardViewItem as DashboardViewItem, DashboardViewItemKind } from 'app/features/search/types'; export type DashboardTreeSelection = Record> & { @@ -27,3 +29,21 @@ export interface DashboardsTreeItem SelectionState; + onAllSelectionChange?: (newState: boolean) => void; + onItemSelectionChange?: (item: DashboardViewItem, newState: boolean) => void; +} + +export type DashboardsTreeColumn = Column; +export type DashboardsTreeCellProps = CellProps & RendererUserProps; +export type DashboardTreeHeaderProps = HeaderProps & RendererUserProps; + +export enum SelectionState { + Unselected, + Selected, + Mixed, +}