NestedFolders: API improvements for NestedFolderPicker (#72093)

* NestedFolders: Prepare nested folder picker for more usage

* fix betterer results

* Update usage of NestedFolderPicker

* fix tests

* fix betterer
This commit is contained in:
Josh Hunt 2023-07-21 14:40:39 +00:00 committed by GitHub
parent cfa1a2c55f
commit a5016c9e88
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 50 additions and 53 deletions

View File

@ -15,8 +15,6 @@ import { DashboardsTreeItem } from 'app/features/browse-dashboards/types';
import { DashboardViewItem } from 'app/features/search/types'; import { DashboardViewItem } from 'app/features/search/types';
import { useSelector } from 'app/types'; import { useSelector } from 'app/types';
import { FolderUID } from './types';
const ROW_HEIGHT = 40; const ROW_HEIGHT = 40;
const CHEVRON_SIZE = 'md'; const CHEVRON_SIZE = 'md';
@ -27,7 +25,7 @@ interface NestedFolderListProps {
focusedItemIndex: number; focusedItemIndex: number;
foldersAreOpenable: boolean; foldersAreOpenable: boolean;
idPrefix: string; idPrefix: string;
selectedFolder: FolderUID | undefined; selectedFolder: string | undefined;
onFolderExpand: (uid: string, newOpenState: boolean) => void; onFolderExpand: (uid: string, newOpenState: boolean) => void;
onFolderSelect: (item: DashboardViewItem) => void; onFolderSelect: (item: DashboardViewItem) => void;
isItemLoaded: (itemIndex: number) => boolean; isItemLoaded: (itemIndex: number) => boolean;

View File

@ -109,10 +109,7 @@ describe('NestedFolderPicker', () => {
await screen.findByLabelText(folderA.item.title); await screen.findByLabelText(folderA.item.title);
await userEvent.click(screen.getByLabelText(folderA.item.title)); await userEvent.click(screen.getByLabelText(folderA.item.title));
expect(mockOnChange).toHaveBeenCalledWith({ expect(mockOnChange).toHaveBeenCalledWith(folderA.item.uid, folderA.item.title);
uid: folderA.item.uid,
title: folderA.item.title,
});
}); });
it('can select a folder from the picker with the keyboard', async () => { it('can select a folder from the picker with the keyboard', async () => {
@ -122,10 +119,7 @@ describe('NestedFolderPicker', () => {
await userEvent.click(button); await userEvent.click(button);
await userEvent.keyboard('{ArrowDown}{ArrowDown}{Enter}'); await userEvent.keyboard('{ArrowDown}{ArrowDown}{Enter}');
expect(mockOnChange).toHaveBeenCalledWith({ expect(mockOnChange).toHaveBeenCalledWith(folderA.item.uid, folderA.item.title);
uid: folderA.item.uid,
title: folderA.item.title,
});
}); });
it('can expand and collapse a folder to show its children', async () => { it('can expand and collapse a folder to show its children', async () => {
@ -156,10 +150,7 @@ describe('NestedFolderPicker', () => {
// Select the first child // Select the first child
await userEvent.click(screen.getByLabelText(folderA_folderA.item.title)); await userEvent.click(screen.getByLabelText(folderA_folderA.item.title));
expect(mockOnChange).toHaveBeenCalledWith({ expect(mockOnChange).toHaveBeenCalledWith(folderA_folderA.item.uid, folderA_folderA.item.title);
uid: folderA_folderA.item.uid,
title: folderA_folderA.item.title,
});
}); });
it('can expand and collapse a folder to show its children with the keyboard', async () => { it('can expand and collapse a folder to show its children with the keyboard', async () => {
@ -185,9 +176,6 @@ describe('NestedFolderPicker', () => {
// Select the first child // Select the first child
await userEvent.keyboard('{ArrowDown}{Enter}'); await userEvent.keyboard('{ArrowDown}{Enter}');
expect(mockOnChange).toHaveBeenCalledWith({ expect(mockOnChange).toHaveBeenCalledWith(folderA_folderA.item.uid, folderA_folderA.item.title);
uid: folderA_folderA.item.uid,
title: folderA_folderA.item.title,
});
}); });
}); });

View File

@ -26,19 +26,24 @@ import { useDispatch, useSelector } from 'app/types/store';
import { getDOMId, NestedFolderList } from './NestedFolderList'; import { getDOMId, NestedFolderList } from './NestedFolderList';
import Trigger from './Trigger'; import Trigger from './Trigger';
import { useTreeInteractions } from './hooks'; import { useTreeInteractions } from './hooks';
import { FolderChange, FolderUID } from './types';
interface NestedFolderPickerProps { export interface NestedFolderPickerProps {
value?: FolderUID; /* Folder UID to show as selected */
// TODO: think properly (and pragmatically) about how to communicate moving to general folder, value?: string;
// vs removing selection (if possible?)
onChange?: (folder: FolderChange) => void; /* Whether to show the root 'Dashboards' (formally General) folder as selectable */
showRootFolder?: boolean;
/* Folder UIDs to exclude from the picker, to prevent invalid operations */
excludeUIDs?: string[]; excludeUIDs?: string[];
/* Callback for when the user selects a folder */
onChange?: (folderUID: string, folderName: string) => void;
} }
const EXCLUDED_KINDS = ['empty-folder' as const, 'dashboard' as const]; const EXCLUDED_KINDS = ['empty-folder' as const, 'dashboard' as const];
export function NestedFolderPicker({ value, onChange, excludeUIDs = [] }: NestedFolderPickerProps) { export function NestedFolderPicker({ value, showRootFolder = true, excludeUIDs, onChange }: NestedFolderPickerProps) {
const styles = useStyles2(getStyles); const styles = useStyles2(getStyles);
const dispatch = useDispatch(); const dispatch = useDispatch();
const selectedFolder = useGetFolderQuery(value || skipToken); const selectedFolder = useGetFolderQuery(value || skipToken);
@ -101,10 +106,7 @@ export function NestedFolderPicker({ value, onChange, excludeUIDs = [] }: Nested
const handleFolderSelect = useCallback( const handleFolderSelect = useCallback(
(item: DashboardViewItem) => { (item: DashboardViewItem) => {
if (onChange) { if (onChange) {
onChange({ onChange(item.uid, item.title);
uid: item.uid,
title: item.title,
});
} }
setOverlayOpen(false); setOverlayOpen(false);
}, },
@ -150,20 +152,22 @@ export function NestedFolderPicker({ value, onChange, excludeUIDs = [] }: Nested
excludeUIDs excludeUIDs
); );
// Increase the level of each item to 'make way' for the fake root Dashboards item if (showRootFolder) {
for (const item of flatTree) { // Increase the level of each item to 'make way' for the fake root Dashboards item
item.level += 1; for (const item of flatTree) {
} item.level += 1;
}
flatTree.unshift({ flatTree.unshift({
isOpen: true, isOpen: true,
level: 0, level: 0,
item: { item: {
kind: 'folder', kind: 'folder',
title: 'Dashboards', title: 'Dashboards',
uid: '', uid: '',
}, },
}); });
}
// If the root collection hasn't loaded yet, create loading placeholders // If the root collection hasn't loaded yet, create loading placeholders
if (!rootCollection) { if (!rootCollection) {
@ -171,7 +175,7 @@ export function NestedFolderPicker({ value, onChange, excludeUIDs = [] }: Nested
} }
return flatTree; return flatTree;
}, [search, searchState.value, rootCollection, childrenCollections, folderOpenState, excludeUIDs]); }, [search, searchState.value, rootCollection, childrenCollections, folderOpenState, excludeUIDs, showRootFolder]);
const isItemLoaded = useCallback( const isItemLoaded = useCallback(
(itemIndex: number) => { (itemIndex: number) => {

View File

@ -1,2 +0,0 @@
export type FolderUID = string;
export type FolderChange = { title: string; uid: FolderUID };

View File

@ -5,7 +5,6 @@ import { config } from '@grafana/runtime';
import { Alert, Button, Field, Modal } from '@grafana/ui'; import { Alert, Button, Field, Modal } from '@grafana/ui';
import { Text } from '@grafana/ui/src/unstable'; import { Text } from '@grafana/ui/src/unstable';
import { NestedFolderPicker } from 'app/core/components/NestedFolderPicker/NestedFolderPicker'; import { NestedFolderPicker } from 'app/core/components/NestedFolderPicker/NestedFolderPicker';
import { FolderChange } from 'app/core/components/NestedFolderPicker/types';
import { OldFolderPicker } from 'app/core/components/Select/OldFolderPicker'; import { OldFolderPicker } from 'app/core/components/Select/OldFolderPicker';
import { t, Trans } from 'app/core/internationalization'; import { t, Trans } from 'app/core/internationalization';
@ -25,7 +24,7 @@ export const MoveModal = ({ onConfirm, onDismiss, selectedItems, ...props }: Pro
const [isMoving, setIsMoving] = useState(false); const [isMoving, setIsMoving] = useState(false);
const selectedFolders = Object.keys(selectedItems.folder).filter((uid) => selectedItems.folder[uid]); const selectedFolders = Object.keys(selectedItems.folder).filter((uid) => selectedItems.folder[uid]);
const handleFolderChange = (newFolder: FolderChange) => { const handleFolderChange = (newFolder: { uid: string; title: string }) => {
setMoveTarget(newFolder.uid); setMoveTarget(newFolder.uid);
}; };
@ -61,7 +60,7 @@ export const MoveModal = ({ onConfirm, onDismiss, selectedItems, ...props }: Pro
<Field label={t('browse-dashboards.action.move-modal-field-label', 'Folder name')}> <Field label={t('browse-dashboards.action.move-modal-field-label', 'Folder name')}>
{config.featureToggles.nestedFolderPicker ? ( {config.featureToggles.nestedFolderPicker ? (
<NestedFolderPicker value={moveTarget} onChange={handleFolderChange} excludeUIDs={selectedFolders} /> <NestedFolderPicker value={moveTarget} excludeUIDs={selectedFolders} onChange={setMoveTarget} />
) : ( ) : (
<OldFolderPicker allowEmpty onChange={handleFolderChange} /> <OldFolderPicker allowEmpty onChange={handleFolderChange} />
)} )}

View File

@ -5,7 +5,6 @@ import { TimeZone } from '@grafana/data';
import { config } from '@grafana/runtime'; import { config } from '@grafana/runtime';
import { CollapsableSection, Field, Input, RadioButtonGroup, TagsInput } from '@grafana/ui'; import { CollapsableSection, Field, Input, RadioButtonGroup, TagsInput } from '@grafana/ui';
import { NestedFolderPicker } from 'app/core/components/NestedFolderPicker/NestedFolderPicker'; import { NestedFolderPicker } from 'app/core/components/NestedFolderPicker/NestedFolderPicker';
import { FolderChange } from 'app/core/components/NestedFolderPicker/types';
import { Page } from 'app/core/components/Page/Page'; import { Page } from 'app/core/components/Page/Page';
import { OldFolderPicker } from 'app/core/components/Select/OldFolderPicker'; import { OldFolderPicker } from 'app/core/components/Select/OldFolderPicker';
import { updateTimeZoneDashboard, updateWeekStartDashboard } from 'app/features/dashboard/state/actions'; import { updateTimeZoneDashboard, updateWeekStartDashboard } from 'app/features/dashboard/state/actions';
@ -31,13 +30,20 @@ export function GeneralSettingsUnconnected({
}: Props): JSX.Element { }: Props): JSX.Element {
const [renderCounter, setRenderCounter] = useState(0); const [renderCounter, setRenderCounter] = useState(0);
const onFolderChange = (newFolder: FolderChange) => { const onFolderChange = (newFolder: { uid: string; title: string }) => {
dashboard.meta.folderUid = newFolder.uid; dashboard.meta.folderUid = newFolder.uid;
dashboard.meta.folderTitle = newFolder.title; dashboard.meta.folderTitle = newFolder.title;
dashboard.meta.hasUnsavedFolderChange = true; dashboard.meta.hasUnsavedFolderChange = true;
setRenderCounter(renderCounter + 1); setRenderCounter(renderCounter + 1);
}; };
const onNestedFolderChange = (newUID: string, newTitle: string) => {
dashboard.meta.folderUid = newUID;
dashboard.meta.folderTitle = newTitle;
dashboard.meta.hasUnsavedFolderChange = true;
setRenderCounter(renderCounter + 1);
};
const onBlur = (event: React.FocusEvent<HTMLInputElement>) => { const onBlur = (event: React.FocusEvent<HTMLInputElement>) => {
if (event.currentTarget.name === 'title' || event.currentTarget.name === 'description') { if (event.currentTarget.name === 'title' || event.currentTarget.name === 'description') {
dashboard[event.currentTarget.name] = event.currentTarget.value; dashboard[event.currentTarget.name] = event.currentTarget.value;
@ -110,7 +116,7 @@ export function GeneralSettingsUnconnected({
<Field label="Folder"> <Field label="Folder">
{config.featureToggles.nestedFolderPicker ? ( {config.featureToggles.nestedFolderPicker ? (
<NestedFolderPicker value={dashboard.meta.folderUid} onChange={onFolderChange} /> <NestedFolderPicker value={dashboard.meta.folderUid} onChange={onNestedFolderChange} />
) : ( ) : (
<OldFolderPicker <OldFolderPicker
inputId="dashboard-folder-input" inputId="dashboard-folder-input"

View File

@ -112,7 +112,11 @@ export const SaveDashboardAsForm = ({
<InputControl <InputControl
render={({ field: { ref, ...field } }) => render={({ field: { ref, ...field } }) =>
config.featureToggles.nestedFolderPicker ? ( config.featureToggles.nestedFolderPicker ? (
<NestedFolderPicker {...field} value={field.value?.uid} /> <NestedFolderPicker
{...field}
onChange={(uid: string, title: string) => field.onChange({ uid, title })}
value={field.value?.uid}
/>
) : ( ) : (
<OldFolderPicker <OldFolderPicker
{...field} {...field}