From 334ecd1be79a2cd51af4bc24a7822bf187893470 Mon Sep 17 00:00:00 2001 From: Josh Hunt Date: Thu, 27 Apr 2023 10:29:26 +0100 Subject: [PATCH] NestedFolders: Select all for Browse and Search (#67227) * NestedFolders: Select all for BrowseView and SearchView * show fake indeterminate state for SearchView * fix types * Select search results as additional pages are loaded * fix (de)select all between browse and search * tests * fix test * fix test --- .../api/browseDashboardsAPI.ts | 2 +- .../BrowseActions/DeleteModal.test.tsx | 1 + .../BrowseActions/MoveModal.test.tsx | 1 + .../components/BrowseView.tsx | 2 + .../components/DashboardsTree.test.tsx | 5 ++ .../components/DashboardsTree.tsx | 19 +++-- .../components/SearchView.tsx | 20 +++-- .../features/browse-dashboards/state/hooks.ts | 4 +- .../browse-dashboards/state/reducers.test.ts | 80 ++++++++++++++++++- .../browse-dashboards/state/reducers.ts | 39 +++++++++ .../features/browse-dashboards/state/slice.ts | 3 +- .../app/features/browse-dashboards/types.ts | 4 +- .../components/SearchResultsTable.test.tsx | 14 ++-- .../page/components/SearchResultsTable.tsx | 24 +++++- 14 files changed, 196 insertions(+), 22 deletions(-) diff --git a/public/app/features/browse-dashboards/api/browseDashboardsAPI.ts b/public/app/features/browse-dashboards/api/browseDashboardsAPI.ts index a3e0ca101a7..3802bdf9326 100644 --- a/public/app/features/browse-dashboards/api/browseDashboardsAPI.ts +++ b/public/app/features/browse-dashboards/api/browseDashboardsAPI.ts @@ -93,5 +93,5 @@ export const browseDashboardsAPI = createApi({ }), }); -export const { useGetFolderQuery, useGetAffectedItemsQuery } = browseDashboardsAPI; +export const { useGetFolderQuery, useLazyGetFolderQuery, useGetAffectedItemsQuery } = browseDashboardsAPI; export { skipToken } from '@reduxjs/toolkit/query/react'; diff --git a/public/app/features/browse-dashboards/components/BrowseActions/DeleteModal.test.tsx b/public/app/features/browse-dashboards/components/BrowseActions/DeleteModal.test.tsx index aec7aef75b3..d5fb11eec3d 100644 --- a/public/app/features/browse-dashboards/components/BrowseActions/DeleteModal.test.tsx +++ b/public/app/features/browse-dashboards/components/BrowseActions/DeleteModal.test.tsx @@ -18,6 +18,7 @@ describe('browse-dashboards DeleteModal', () => { onConfirm: mockOnConfirm, onDismiss: mockOnDismiss, selectedItems: { + $all: false, folder: {}, dashboard: {}, panel: {}, diff --git a/public/app/features/browse-dashboards/components/BrowseActions/MoveModal.test.tsx b/public/app/features/browse-dashboards/components/BrowseActions/MoveModal.test.tsx index a5ae8362f95..5ac7b077728 100644 --- a/public/app/features/browse-dashboards/components/BrowseActions/MoveModal.test.tsx +++ b/public/app/features/browse-dashboards/components/BrowseActions/MoveModal.test.tsx @@ -28,6 +28,7 @@ describe('browse-dashboards MoveModal', () => { onConfirm: mockOnConfirm, onDismiss: mockOnDismiss, selectedItems: { + $all: false, folder: {}, dashboard: {}, panel: {}, diff --git a/public/app/features/browse-dashboards/components/BrowseView.tsx b/public/app/features/browse-dashboards/components/BrowseView.tsx index 8245f789213..e67c2fbc4e3 100644 --- a/public/app/features/browse-dashboards/components/BrowseView.tsx +++ b/public/app/features/browse-dashboards/components/BrowseView.tsx @@ -9,6 +9,7 @@ import { fetchChildren, setFolderOpenState, setItemSelectionState, + setAllSelection, } from '../state'; import { DashboardsTree } from './DashboardsTree'; @@ -53,6 +54,7 @@ export function BrowseView({ folderUID, width, height }: BrowseViewProps) { height={height} selectedItems={selectedItems} onFolderClick={handleFolderClick} + onAllSelectionChange={(newState) => dispatch(setAllSelection({ isSelected: newState }))} onItemSelectionChange={handleItemSelectionChange} /> ); diff --git a/public/app/features/browse-dashboards/components/DashboardsTree.test.tsx b/public/app/features/browse-dashboards/components/DashboardsTree.test.tsx index 42847fc3c2a..65b498d29e0 100644 --- a/public/app/features/browse-dashboards/components/DashboardsTree.test.tsx +++ b/public/app/features/browse-dashboards/components/DashboardsTree.test.tsx @@ -21,6 +21,7 @@ describe('browse-dashboards DashboardsTree', () => { const dashboard = wellFormedDashboard(2); const noop = () => {}; const selectedItems = { + $all: false, folder: {}, dashboard: {}, panel: {}, @@ -35,6 +36,7 @@ describe('browse-dashboards DashboardsTree', () => { height={HEIGHT} onFolderClick={noop} onItemSelectionChange={noop} + onAllSelectionChange={noop} /> ); expect(screen.queryByText(dashboard.item.title)).toBeInTheDocument(); @@ -51,6 +53,7 @@ describe('browse-dashboards DashboardsTree', () => { height={HEIGHT} onFolderClick={noop} onItemSelectionChange={noop} + onAllSelectionChange={noop} /> ); expect(screen.queryByText(folder.item.title)).toBeInTheDocument(); @@ -67,6 +70,7 @@ describe('browse-dashboards DashboardsTree', () => { height={HEIGHT} onFolderClick={handler} onItemSelectionChange={noop} + onAllSelectionChange={noop} /> ); const folderButton = screen.getByLabelText('Collapse folder'); @@ -84,6 +88,7 @@ describe('browse-dashboards DashboardsTree', () => { height={HEIGHT} onFolderClick={noop} onItemSelectionChange={noop} + onAllSelectionChange={noop} /> ); expect(screen.queryByText('Empty folder')).toBeInTheDocument(); diff --git a/public/app/features/browse-dashboards/components/DashboardsTree.tsx b/public/app/features/browse-dashboards/components/DashboardsTree.tsx index 11cf3529c80..10f2dee9f3a 100644 --- a/public/app/features/browse-dashboards/components/DashboardsTree.tsx +++ b/public/app/features/browse-dashboards/components/DashboardsTree.tsx @@ -1,6 +1,6 @@ import { css, cx } from '@emotion/css'; import React, { useMemo } from 'react'; -import { CellProps, Column, TableInstance, useTable } from 'react-table'; +import { CellProps, Column, HeaderProps, TableInstance, useTable } from 'react-table'; import { FixedSizeList as List } from 'react-window'; import { GrafanaTheme2 } from '@grafana/data'; @@ -21,15 +21,19 @@ interface DashboardsTreeProps { height: number; selectedItems: DashboardTreeSelection; onFolderClick: (uid: string, newOpenState: boolean) => void; + onAllSelectionChange: (newState: boolean) => void; onItemSelectionChange: (item: DashboardViewItem, newState: boolean) => void; } type DashboardsTreeColumn = Column; -type DashboardsTreeCellProps = CellProps & { +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; @@ -40,6 +44,7 @@ export function DashboardsTree({ height, selectedItems, onFolderClick, + onAllSelectionChange, onItemSelectionChange, }: DashboardsTreeProps) { const styles = useStyles2(getStyles); @@ -48,7 +53,10 @@ export function DashboardsTree({ const checkboxColumn: DashboardsTreeColumn = { id: 'checkbox', width: 0, - Header: () => , + 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) { @@ -56,6 +64,7 @@ export function DashboardsTree({ } const isSelected = selectedItems?.[item.kind][item.uid] ?? false; + return ( - {column.render('Header')} + {column.render('Header', { selectedItems })} ); })} diff --git a/public/app/features/browse-dashboards/components/SearchView.tsx b/public/app/features/browse-dashboards/components/SearchView.tsx index 0af421731f4..92e985f4d0f 100644 --- a/public/app/features/browse-dashboards/components/SearchView.tsx +++ b/public/app/features/browse-dashboards/components/SearchView.tsx @@ -7,7 +7,7 @@ import { useSearchStateManager } from 'app/features/search/state/SearchStateMana import { DashboardViewItemKind } from 'app/features/search/types'; import { useDispatch, useSelector } from 'app/types'; -import { setItemSelectionState } from '../state'; +import { setAllSelection, setItemSelectionState, useHasSelection } from '../state'; interface SearchViewProps { height: number; @@ -17,6 +17,7 @@ interface SearchViewProps { export function SearchView({ width, height }: SearchViewProps) { const dispatch = useDispatch(); const selectedItems = useSelector((wholeState) => wholeState.browseDashboards.selectedItems); + const hasSelection = useHasSelection(); const { keyboardEvents } = useKeyNavigationListener(); const [searchState, stateManager] = useSearchStateManager(); @@ -25,18 +26,27 @@ export function SearchView({ width, height }: SearchViewProps) { const selectionChecker = useCallback( (kind: string | undefined, uid: string): boolean => { - if (!kind || kind === '*') { + if (!kind) { + return false; + } + + // Currently, this indicates _some_ items are selected, not nessicarily all are + // selected. + if (kind === '*' && uid === '*') { + return hasSelection; + } else if (kind === '*') { + // Unsure how this case can happen return false; } return selectedItems[assertDashboardViewItemKind(kind)][uid] ?? false; }, - [selectedItems] + [selectedItems, hasSelection] ); const clearSelection = useCallback(() => { - console.log('TODO: clearSelection'); - }, []); + dispatch(setAllSelection({ isSelected: false })); + }, [dispatch]); const handleItemSelectionChange = useCallback( (kind: string, uid: string) => { diff --git a/public/app/features/browse-dashboards/state/hooks.ts b/public/app/features/browse-dashboards/state/hooks.ts index dd6a78319bc..3bd672b0946 100644 --- a/public/app/features/browse-dashboards/state/hooks.ts +++ b/public/app/features/browse-dashboards/state/hooks.ts @@ -106,10 +106,10 @@ function createFlatTree( function getSelectedItemsForActions( selectedItemsState: DashboardTreeSelection, childrenByParentUID: Record -): Omit { +): Omit { // Take a copy of the selected items to work with // We don't care about panels here, only dashboards and folders can be moved or deleted - const result: Omit = { + const result = { dashboard: { ...selectedItemsState.dashboard }, folder: { ...selectedItemsState.folder }, }; diff --git a/public/app/features/browse-dashboards/state/reducers.test.ts b/public/app/features/browse-dashboards/state/reducers.test.ts index 9c9cbde83c3..32e5b3bbb99 100644 --- a/public/app/features/browse-dashboards/state/reducers.test.ts +++ b/public/app/features/browse-dashboards/state/reducers.test.ts @@ -1,7 +1,12 @@ import { wellFormedDashboard, wellFormedFolder } from '../fixtures/dashboardsTreeItem.fixture'; import { BrowseDashboardsState } from '../types'; -import { extraReducerFetchChildrenFulfilled, setFolderOpenState, setItemSelectionState } from './reducers'; +import { + extraReducerFetchChildrenFulfilled, + setAllSelection, + setFolderOpenState, + setItemSelectionState, +} from './reducers'; function createInitialState(): BrowseDashboardsState { return { @@ -9,6 +14,7 @@ function createInitialState(): BrowseDashboardsState { childrenByParentUID: {}, openFolders: {}, selectedItems: { + $all: false, dashboard: {}, folder: {}, panel: {}, @@ -84,6 +90,7 @@ describe('browse-dashboards reducers', () => { extraReducerFetchChildrenFulfilled(state, action); expect(state.selectedItems).toEqual({ + $all: false, dashboard: { [childDashboard.uid]: true, }, @@ -114,6 +121,7 @@ describe('browse-dashboards reducers', () => { setItemSelectionState(state, { type: 'setItemSelectionState', payload: { item: dashboard, isSelected: true } }); expect(state.selectedItems).toEqual({ + $all: false, dashboard: { [dashboard.uid]: true, }, @@ -139,6 +147,7 @@ describe('browse-dashboards reducers', () => { }); expect(state.selectedItems).toEqual({ + $all: false, dashboard: { [childDashboard.uid]: true, [grandchildDashboard.uid]: true, @@ -175,6 +184,7 @@ describe('browse-dashboards reducers', () => { }); expect(state.selectedItems).toEqual({ + $all: false, dashboard: { [childDashboard.uid]: true, [grandchildDashboard.uid]: false, @@ -187,4 +197,72 @@ describe('browse-dashboards reducers', () => { }); }); }); + + describe('setAllSelection', () => { + it('selects all loaded items', () => { + const state = createInitialState(); + + let seed = 1; + const topLevelDashboard = wellFormedDashboard(seed++).item; + const topLevelFolder = wellFormedFolder(seed++).item; + const childDashboard = wellFormedDashboard(seed++, {}, { parentUID: topLevelFolder.uid }).item; + const childFolder = wellFormedFolder(seed++, {}, { parentUID: topLevelFolder.uid }).item; + const grandchildDashboard = wellFormedDashboard(seed++, {}, { parentUID: childFolder.uid }).item; + + state.rootItems = [topLevelFolder, topLevelDashboard]; + state.childrenByParentUID[topLevelFolder.uid] = [childDashboard, childFolder]; + state.childrenByParentUID[childFolder.uid] = [grandchildDashboard]; + + state.selectedItems.folder[childFolder.uid] = false; + state.selectedItems.dashboard[grandchildDashboard.uid] = true; + + setAllSelection(state, { type: 'setAllSelection', payload: { isSelected: true } }); + + expect(state.selectedItems).toEqual({ + $all: true, + dashboard: { + [topLevelDashboard.uid]: true, + [childDashboard.uid]: true, + [grandchildDashboard.uid]: true, + }, + folder: { + [topLevelFolder.uid]: true, + [childFolder.uid]: true, + }, + panel: {}, + }); + }); + + it('deselects all items', () => { + const state = createInitialState(); + + let seed = 1; + const topLevelDashboard = wellFormedDashboard(seed++).item; + const topLevelFolder = wellFormedFolder(seed++).item; + const childDashboard = wellFormedDashboard(seed++, {}, { parentUID: topLevelFolder.uid }).item; + const childFolder = wellFormedFolder(seed++, {}, { parentUID: topLevelFolder.uid }).item; + const grandchildDashboard = wellFormedDashboard(seed++, {}, { parentUID: childFolder.uid }).item; + + state.rootItems = [topLevelFolder, topLevelDashboard]; + state.childrenByParentUID[topLevelFolder.uid] = [childDashboard, childFolder]; + state.childrenByParentUID[childFolder.uid] = [grandchildDashboard]; + + state.selectedItems.folder[childFolder.uid] = false; + state.selectedItems.dashboard[grandchildDashboard.uid] = true; + + setAllSelection(state, { type: 'setAllSelection', payload: { isSelected: false } }); + + // Deselecting only sets selection = false for things already selected + expect(state.selectedItems).toEqual({ + $all: false, + dashboard: { + [grandchildDashboard.uid]: false, + }, + folder: { + [childFolder.uid]: false, + }, + panel: {}, + }); + }); + }); }); diff --git a/public/app/features/browse-dashboards/state/reducers.ts b/public/app/features/browse-dashboards/state/reducers.ts index 5d6f6674163..ac79052360c 100644 --- a/public/app/features/browse-dashboards/state/reducers.ts +++ b/public/app/features/browse-dashboards/state/reducers.ts @@ -82,6 +82,45 @@ export function setItemSelectionState( } } +export function setAllSelection(state: BrowseDashboardsState, action: PayloadAction<{ isSelected: boolean }>) { + const { isSelected } = action.payload; + + state.selectedItems.$all = isSelected; + + // Search works a bit differently so the state here does different things... + // In search: + // - When "Selecting all", it sends individual state updates with setItemSelectionState. + // - When "Deselecting all", it uses this setAllSelection. Search results aren't stored in + // redux, so we just need to iterate over the selected items to flip them to false + + if (isSelected) { + for (const folderUID in state.childrenByParentUID) { + const children = state.childrenByParentUID[folderUID] ?? []; + + for (const child of children) { + state.selectedItems[child.kind][child.uid] = isSelected; + } + } + + for (const child of state.rootItems) { + state.selectedItems[child.kind][child.uid] = isSelected; + } + } else { + // if deselecting only need to loop over what we've already selected + for (const kind in state.selectedItems) { + if (!(kind === 'dashboard' || kind === 'panel' || kind === 'folder')) { + continue; + } + + const selection = state.selectedItems[kind]; + + for (const uid in selection) { + selection[uid] = isSelected; + } + } + } +} + function findItem( rootItems: DashboardViewItem[], childrenByUID: Record, diff --git a/public/app/features/browse-dashboards/state/slice.ts b/public/app/features/browse-dashboards/state/slice.ts index 564f1fc4db7..39db1edbdb9 100644 --- a/public/app/features/browse-dashboards/state/slice.ts +++ b/public/app/features/browse-dashboards/state/slice.ts @@ -15,6 +15,7 @@ const initialState: BrowseDashboardsState = { dashboard: {}, folder: {}, panel: {}, + $all: false, }, }; @@ -30,7 +31,7 @@ const browseDashboardsSlice = createSlice({ export const browseDashboardsReducer = browseDashboardsSlice.reducer; -export const { setFolderOpenState, setItemSelectionState } = browseDashboardsSlice.actions; +export const { setFolderOpenState, setItemSelectionState, setAllSelection } = browseDashboardsSlice.actions; export default { browseDashboards: browseDashboardsReducer, diff --git a/public/app/features/browse-dashboards/types.ts b/public/app/features/browse-dashboards/types.ts index e3b08f17d6c..7ace5982127 100644 --- a/public/app/features/browse-dashboards/types.ts +++ b/public/app/features/browse-dashboards/types.ts @@ -1,6 +1,8 @@ import { DashboardViewItem as DashboardViewItem, DashboardViewItemKind } from 'app/features/search/types'; -export type DashboardTreeSelection = Record>; +export type DashboardTreeSelection = Record> & { + $all: boolean; +}; export interface BrowseDashboardsState { rootItems: DashboardViewItem[]; diff --git a/public/app/features/search/page/components/SearchResultsTable.test.tsx b/public/app/features/search/page/components/SearchResultsTable.test.tsx index fafa6abdf96..979865b9f6a 100644 --- a/public/app/features/search/page/components/SearchResultsTable.test.tsx +++ b/public/app/features/search/page/components/SearchResultsTable.test.tsx @@ -53,7 +53,7 @@ describe('SearchResultsTable', () => { jest.spyOn(getGrafanaSearcher(), 'search').mockResolvedValue(mockSearchResult); }); - it('shows the table with the correct accessible label', () => { + it('shows the table with the correct accessible label', async () => { render( { width={1000} /> ); - expect(screen.getByRole('table', { name: 'Search results table' })).toBeInTheDocument(); + const table = await screen.findByRole('table', { name: 'Search results table' }); + expect(table).toBeInTheDocument(); }); it('has the correct row headers', async () => { @@ -82,12 +83,13 @@ describe('SearchResultsTable', () => { width={1000} /> ); + await screen.findByRole('table'); expect(screen.getByRole('columnheader', { name: 'Name' })).toBeInTheDocument(); expect(screen.getByRole('columnheader', { name: 'Type' })).toBeInTheDocument(); expect(screen.getByRole('columnheader', { name: 'Tags' })).toBeInTheDocument(); }); - it('displays the data correctly in the table', () => { + it('displays the data correctly in the table', async () => { render( { width={1000} /> ); + await screen.findByRole('table'); const rows = screen.getAllByRole('row'); @@ -134,7 +137,7 @@ describe('SearchResultsTable', () => { jest.spyOn(getGrafanaSearcher(), 'search').mockResolvedValue(mockEmptySearchResult); }); - it('shows a "No data" message', () => { + it('shows a "No data" message', async () => { render( { width={1000} /> ); + const noData = await screen.findByText('No data'); + expect(noData).toBeInTheDocument(); expect(screen.queryByRole('table', { name: 'Search results table' })).not.toBeInTheDocument(); - expect(screen.getByText('No data')).toBeInTheDocument(); }); }); }); diff --git a/public/app/features/search/page/components/SearchResultsTable.tsx b/public/app/features/search/page/components/SearchResultsTable.tsx index 1801a8ac5e1..6ab728be20c 100644 --- a/public/app/features/search/page/components/SearchResultsTable.tsx +++ b/public/app/features/search/page/components/SearchResultsTable.tsx @@ -103,6 +103,28 @@ export const SearchResultsTable = React.memo( const { getTableProps, getTableBodyProps, headerGroups, rows, prepareRow } = useTable(options, useAbsoluteLayout); + const handleLoadMore = useCallback( + async (startIndex: number, endIndex: number) => { + await response.loadMoreItems(startIndex, endIndex); + + // After we load more items, select them if the "select all" checkbox + // is selected + const isAllSelected = selection?.('*', '*'); + if (!selectionToggle || !selection || !isAllSelected) { + return; + } + + for (let index = startIndex; index < response.view.length; index++) { + const item = response.view.get(index); + const itemIsSelected = selection(item.kind, item.uid); + if (!itemIsSelected) { + selectionToggle(item.kind, item.uid); + } + } + }, + [response, selection, selectionToggle] + ); + const RenderRow = useCallback( ({ index: rowIndex, style }: { index: number; style: CSSProperties }) => { const row = rows[rowIndex]; @@ -164,7 +186,7 @@ export const SearchResultsTable = React.memo( ref={infiniteLoaderRef} isItemLoaded={response.isItemLoaded} itemCount={rows.length} - loadMoreItems={response.loadMoreItems} + loadMoreItems={handleLoadMore} > {({ onItemsRendered, ref }) => (