From 29ed310af9901ed0833b98bc64adaf8c74df3b50 Mon Sep 17 00:00:00 2001 From: Ashley Harrison Date: Wed, 12 Jul 2023 10:37:08 +0100 Subject: [PATCH] Nested folder picker: Move options into overlay (#71042) * move options into overlay * add some unit tests * Update public/app/features/browse-dashboards/components/BrowseActions/MoveModal.tsx Co-authored-by: Tobias Skarhed <1438972+tskarhed@users.noreply.github.com> * minor refactoring based on review comments * just pass in uid --------- Co-authored-by: Tobias Skarhed <1438972+tskarhed@users.noreply.github.com> --- .../NestedFolderPicker/NestedFolderList.tsx | 21 ++- .../NestedFolderPicker.test.tsx | 127 ++++++++++++++++ .../NestedFolderPicker/NestedFolderPicker.tsx | 141 +++++++++++++----- .../components/NestedFolderPicker/types.ts | 4 +- .../components/BrowseActions/MoveModal.tsx | 4 +- .../DashboardSettings/GeneralSettings.tsx | 4 +- 6 files changed, 246 insertions(+), 55 deletions(-) create mode 100644 public/app/core/components/NestedFolderPicker/NestedFolderPicker.test.tsx diff --git a/public/app/core/components/NestedFolderPicker/NestedFolderList.tsx b/public/app/core/components/NestedFolderPicker/NestedFolderList.tsx index 839c9f04231..7c8dedc630f 100644 --- a/public/app/core/components/NestedFolderPicker/NestedFolderList.tsx +++ b/public/app/core/components/NestedFolderPicker/NestedFolderList.tsx @@ -13,7 +13,6 @@ import { DashboardViewItem } from 'app/features/search/types'; import { FolderUID } from './types'; const ROW_HEIGHT = 40; -const LIST_HEIGHT = ROW_HEIGHT * 6.5; // show 6 and a bit rows const CHEVRON_SIZE = 'md'; interface NestedFolderListProps { @@ -40,8 +39,13 @@ export function NestedFolderList({ return (
-
Name
- + {Row}
@@ -112,16 +116,15 @@ function Row({ index, style: virtualStyles, data }: RowProps) { onKeyDown={handleKeyDown} /> +
- - {foldersAreOpenable ? ( ) : ( @@ -146,12 +149,12 @@ const getStyles = (theme: GrafanaTheme2) => { position: 'relative', alignItems: 'center', flexGrow: 1, + gap: theme.spacing(0.5), paddingLeft: theme.spacing(1), }); return { table: css({ - border: `solid 1px ${theme.components.input.borderColor}`, background: theme.components.input.background, }), @@ -172,7 +175,9 @@ const getStyles = (theme: GrafanaTheme2) => { display: 'flex', position: 'relative', alignItems: 'center', - borderTop: `solid 1px ${theme.components.input.borderColor}`, + [':not(:first-child)']: { + borderTop: `solid 1px ${theme.colors.border.weak}`, + }, }), radio: css({ diff --git a/public/app/core/components/NestedFolderPicker/NestedFolderPicker.test.tsx b/public/app/core/components/NestedFolderPicker/NestedFolderPicker.test.tsx new file mode 100644 index 00000000000..a9a0cba087c --- /dev/null +++ b/public/app/core/components/NestedFolderPicker/NestedFolderPicker.test.tsx @@ -0,0 +1,127 @@ +import 'whatwg-fetch'; // fetch polyfill +import { render as rtlRender, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { rest } from 'msw'; +import { SetupServer, setupServer } from 'msw/node'; +import React from 'react'; +import { TestProvider } from 'test/helpers/TestProvider'; + +import { backendSrv } from 'app/core/services/backend_srv'; + +import { wellFormedTree } from '../../../features/browse-dashboards/fixtures/dashboardsTreeItem.fixture'; + +import { NestedFolderPicker } from './NestedFolderPicker'; + +const [mockTree, { folderA, folderB, folderC, folderA_folderA, folderA_folderB }] = wellFormedTree(); + +jest.mock('@grafana/runtime', () => ({ + ...jest.requireActual('@grafana/runtime'), + getBackendSrv: () => backendSrv, +})); + +jest.mock('app/features/browse-dashboards/api/services', () => { + const orig = jest.requireActual('app/features/browse-dashboards/api/services'); + + return { + ...orig, + listFolders(parentUID?: string) { + const childrenForUID = mockTree + .filter((v) => v.item.kind === 'folder' && v.item.parentUID === parentUID) + .map((v) => v.item); + + return Promise.resolve(childrenForUID); + }, + }; +}); + +function render(...[ui, options]: Parameters) { + rtlRender({ui}, options); +} + +describe('NestedFolderPicker', () => { + const mockOnChange = jest.fn(); + let server: SetupServer; + + beforeAll(() => { + server = setupServer( + rest.get('/api/folders/:uid', (_, res, ctx) => { + return res( + ctx.status(200), + ctx.json({ + title: folderA.item.title, + uid: folderA.item.uid, + }) + ); + }) + ); + server.listen(); + }); + + afterAll(() => { + server.close(); + }); + + afterEach(() => { + server.resetHandlers(); + }); + + it('renders a button with the correct label when no folder is selected', async () => { + render(); + expect(await screen.findByRole('button', { name: 'Select folder' })).toBeInTheDocument(); + }); + + it('renders a button with the folder name instead when a folder is selected', async () => { + render(); + expect(await screen.findByRole('button', { name: folderA.item.title })).toBeInTheDocument(); + expect(screen.queryByRole('button', { name: 'Select folder' })).not.toBeInTheDocument(); + }); + + it('clicking the button opens the folder picker', async () => { + render(); + const button = await screen.findByRole('button', { name: 'Select folder' }); + + await userEvent.click(button); + + // Select folder button is no longer visible + expect(screen.queryByRole('button', { name: 'Select folder' })).not.toBeInTheDocument(); + + // Search input and folder tree are visible + expect(screen.getByPlaceholderText('Search folder')).toBeInTheDocument(); + expect(screen.getByLabelText('Dashboards')).toBeInTheDocument(); + expect(screen.getByLabelText(folderA.item.title)).toBeInTheDocument(); + expect(screen.getByLabelText(folderB.item.title)).toBeInTheDocument(); + expect(screen.getByLabelText(folderC.item.title)).toBeInTheDocument(); + }); + + it('can select a folder from the picker', async () => { + render(); + const button = await screen.findByRole('button', { name: 'Select folder' }); + + await userEvent.click(button); + + await userEvent.click(screen.getByLabelText(folderA.item.title)); + expect(mockOnChange).toHaveBeenCalledWith({ + uid: folderA.item.uid, + title: folderA.item.title, + }); + }); + + it('can expand and collapse a folder to show its children', async () => { + render(); + const button = await screen.findByRole('button', { name: 'Select folder' }); + + await userEvent.click(button); + + // Expand Folder A + await userEvent.click(screen.getByRole('button', { name: `Expand folder ${folderA.item.title}` })); + + // Folder A's children are visible + expect(screen.getByLabelText(folderA_folderA.item.title)).toBeInTheDocument(); + expect(screen.getByLabelText(folderA_folderB.item.title)).toBeInTheDocument(); + + // Collapse Folder A + await userEvent.click(screen.getByRole('button', { name: `Collapse folder ${folderA.item.title}` })); + expect(screen.queryByLabelText(folderA_folderA.item.title)).not.toBeInTheDocument(); + expect(screen.queryByLabelText(folderA_folderB.item.title)).not.toBeInTheDocument(); + }); +}); diff --git a/public/app/core/components/NestedFolderPicker/NestedFolderPicker.tsx b/public/app/core/components/NestedFolderPicker/NestedFolderPicker.tsx index dcc8c217626..e468cb8ad68 100644 --- a/public/app/core/components/NestedFolderPicker/NestedFolderPicker.tsx +++ b/public/app/core/components/NestedFolderPicker/NestedFolderPicker.tsx @@ -1,10 +1,12 @@ import { css } from '@emotion/css'; import React, { useCallback, useMemo, useState } from 'react'; +import Skeleton from 'react-loading-skeleton'; +import { usePopperTooltip } from 'react-popper-tooltip'; import { useAsync } from 'react-use'; import { GrafanaTheme2 } from '@grafana/data'; -import { Stack } from '@grafana/experimental'; -import { Alert, FilterInput, LoadingBar, useStyles2 } from '@grafana/ui'; +import { Alert, Button, FilterInput, LoadingBar, useStyles2 } from '@grafana/ui'; +import { skipToken, useGetFolderQuery } from 'app/features/browse-dashboards/api/browseDashboardsAPI'; import { listFolders, PAGE_SIZE } from 'app/features/browse-dashboards/api/services'; import { createFlatTree } from 'app/features/browse-dashboards/state'; import { DashboardViewItemCollection } from 'app/features/browse-dashboards/types'; @@ -20,19 +22,21 @@ async function fetchRootFolders() { } interface NestedFolderPickerProps { - value?: FolderUID | undefined; + value?: FolderUID; // TODO: think properly (and pragmatically) about how to communicate moving to general folder, // vs removing selection (if possible?) - onChange?: (folderUID: FolderChange) => void; + onChange?: (folder: FolderChange) => void; } export function NestedFolderPicker({ value, onChange }: NestedFolderPickerProps) { const styles = useStyles2(getStyles); const [search, setSearch] = useState(''); + const [overlayOpen, setOverlayOpen] = useState(false); const [folderOpenState, setFolderOpenState] = useState>({}); const [childrenForUID, setChildrenForUID] = useState>({}); const rootFoldersState = useAsync(fetchRootFolders); + const selectedFolder = useGetFolderQuery(value || skipToken); const searchState = useAsync(async () => { if (!search) { @@ -120,68 +124,125 @@ export function NestedFolderPicker({ value, onChange }: NestedFolderPickerProps) const handleSelectionChange = useCallback( (event: React.FormEvent, item: DashboardViewItem) => { if (onChange) { - onChange({ title: item.title, uid: item.uid }); + onChange({ + uid: item.uid, + title: item.title, + }); } + setOverlayOpen(false); }, [onChange] ); + const { getTooltipProps, setTooltipRef, setTriggerRef, visible, triggerRef } = usePopperTooltip({ + visible: overlayOpen, + placement: 'bottom', + interactive: true, + offset: [0, 0], + trigger: 'click', + onVisibleChange: (value: boolean) => { + // Clear search state when closing the overlay + if (!value) { + setSearch(''); + } + setOverlayOpen(value); + }, + }); + const isLoading = rootFoldersState.loading || searchState.loading; const error = rootFoldersState.error || searchState.error; const tree = flatTree; - return ( -
- - setSearch(val)} - /> + let label = selectedFolder.data?.title; + if (value === '') { + label = 'Dashboards'; + } - {error && ( - + if (!visible) { + return ( + + ); + } + + return ( + <> + setSearch(val)} + /> +
+ {error ? ( + {error.message || error.toString?.() || 'Unknown error'} + ) : ( +
+ {isLoading && ( +
+ +
+ )} + + +
)} - -
- {isLoading && ( -
- -
- )} - - -
- -
+
+ ); } const getStyles = (theme: GrafanaTheme2) => { return { - tableWrapper: css({ - position: 'relative', - zIndex: 1, - background: 'palegoldenrod', + button: css({ + maxWidth: '100%', + }), + error: css({ + marginBottom: 0, + }), + tableWrapper: css({ + boxShadow: theme.shadows.z3, + position: 'relative', + zIndex: theme.zIndex.portal, }), - loader: css({ position: 'absolute', top: 0, left: 0, right: 0, - zIndex: 2, + zIndex: theme.zIndex.portal + 1, overflow: 'hidden', // loading bar overflows its container, so we need to clip it }), + search: css({ + input: { + cursor: 'default', + }, + }), }; }; diff --git a/public/app/core/components/NestedFolderPicker/types.ts b/public/app/core/components/NestedFolderPicker/types.ts index 5eb76761b8e..78e48ce7a1c 100644 --- a/public/app/core/components/NestedFolderPicker/types.ts +++ b/public/app/core/components/NestedFolderPicker/types.ts @@ -1,4 +1,2 @@ -export const ROOT_FOLDER: unique symbol = Symbol('Root folder'); - -export type FolderUID = string | typeof ROOT_FOLDER; +export type FolderUID = string; export type FolderChange = { title: string; uid: FolderUID }; diff --git a/public/app/features/browse-dashboards/components/BrowseActions/MoveModal.tsx b/public/app/features/browse-dashboards/components/BrowseActions/MoveModal.tsx index cf6674bd49c..f1d5cbbbcfd 100644 --- a/public/app/features/browse-dashboards/components/BrowseActions/MoveModal.tsx +++ b/public/app/features/browse-dashboards/components/BrowseActions/MoveModal.tsx @@ -5,7 +5,7 @@ import { config } from '@grafana/runtime'; import { Alert, Button, Field, Modal } from '@grafana/ui'; import { P } from '@grafana/ui/src/unstable'; import { NestedFolderPicker } from 'app/core/components/NestedFolderPicker/NestedFolderPicker'; -import { FolderChange, ROOT_FOLDER } from 'app/core/components/NestedFolderPicker/types'; +import { FolderChange } from 'app/core/components/NestedFolderPicker/types'; import { FolderPicker } from 'app/core/components/Select/FolderPicker'; import { t, Trans } from 'app/core/internationalization'; @@ -26,7 +26,7 @@ export const MoveModal = ({ onConfirm, onDismiss, selectedItems, ...props }: Pro const selectedFolders = Object.keys(selectedItems.folder).filter((uid) => selectedItems.folder[uid]); const handleFolderChange = (newFolder: FolderChange) => { - setMoveTarget(newFolder.uid === ROOT_FOLDER ? '' : newFolder.uid); + setMoveTarget(newFolder.uid); }; const onMove = async () => { diff --git a/public/app/features/dashboard/components/DashboardSettings/GeneralSettings.tsx b/public/app/features/dashboard/components/DashboardSettings/GeneralSettings.tsx index d5396248772..c5a6a46bc82 100644 --- a/public/app/features/dashboard/components/DashboardSettings/GeneralSettings.tsx +++ b/public/app/features/dashboard/components/DashboardSettings/GeneralSettings.tsx @@ -5,7 +5,7 @@ import { TimeZone } from '@grafana/data'; import { config } from '@grafana/runtime'; import { CollapsableSection, Field, Input, RadioButtonGroup, TagsInput } from '@grafana/ui'; import { NestedFolderPicker } from 'app/core/components/NestedFolderPicker/NestedFolderPicker'; -import { FolderChange, ROOT_FOLDER } from 'app/core/components/NestedFolderPicker/types'; +import { FolderChange } from 'app/core/components/NestedFolderPicker/types'; import { Page } from 'app/core/components/Page/Page'; import { FolderPicker } from 'app/core/components/Select/FolderPicker'; import { updateTimeZoneDashboard, updateWeekStartDashboard } from 'app/features/dashboard/state/actions'; @@ -32,7 +32,7 @@ export function GeneralSettingsUnconnected({ const [renderCounter, setRenderCounter] = useState(0); const onFolderChange = (newFolder: FolderChange) => { - dashboard.meta.folderUid = newFolder.uid === ROOT_FOLDER ? '' : newFolder.uid; + dashboard.meta.folderUid = newFolder.uid; dashboard.meta.folderTitle = newFolder.title; dashboard.meta.hasUnsavedFolderChange = true; setRenderCounter(renderCounter + 1);