NestedFolderPicker: separate toggle to force enable picker without nestedFolders (#80461)

* separate nestedFolderPickerOverride toggle to force enable it without nestedFolders

* let's call it newFolderPicker

* update unit tests and keyboard handling

* reduce spacing when no folder open chevron

---------

Co-authored-by: Josh Hunt <joshhunt@users.noreply.github.com>
This commit is contained in:
Ashley Harrison 2024-01-15 11:43:19 +00:00 committed by GitHub
parent d91d4e87b9
commit ec53487c99
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 119 additions and 48 deletions

View File

@ -172,6 +172,7 @@ Experimental features might be changed or removed without prior notice.
| `pluginsSkipHostEnvVars` | Disables passing host environment variable to plugin processes |
| `tableSharedCrosshair` | Enables shared crosshair in table panel |
| `enablePluginsTracingByDefault` | Enable plugin tracing for all external plugins |
| `newFolderPicker` | Enables the nested folder picker without having nested folders enabled |
## Development feature toggles

View File

@ -174,4 +174,5 @@ export interface FeatureToggles {
enablePluginsTracingByDefault?: boolean;
cloudRBACRoles?: boolean;
alertingQueryOptimization?: boolean;
newFolderPicker?: boolean;
}

View File

@ -1323,5 +1323,13 @@ var (
AllowSelfServe: false,
Created: time.Date(2024, time.January, 10, 12, 0, 0, 0, time.UTC),
},
{
Name: "newFolderPicker",
Description: "Enables the nested folder picker without having nested folders enabled",
Stage: FeatureStageExperimental,
Owner: grafanaFrontendPlatformSquad,
FrontendOnly: true,
Created: time.Date(2024, time.January, 12, 12, 0, 0, 0, time.UTC),
},
}
)

View File

@ -155,3 +155,4 @@ alertingPreviewUpgrade,experimental,@grafana/alerting-squad,2024-01-03,false,fal
enablePluginsTracingByDefault,experimental,@grafana/plugins-platform-backend,2024-01-09,false,false,true,false
cloudRBACRoles,experimental,@grafana/identity-access-team,2024-01-10,false,false,true,false
alertingQueryOptimization,GA,@grafana/alerting-squad,2024-01-10,false,false,false,false
newFolderPicker,experimental,@grafana/grafana-frontend-platform,2024-01-12,false,false,false,true

1 Name Stage Owner Created requiresDevMode RequiresLicense RequiresRestart FrontendOnly
155 enablePluginsTracingByDefault experimental @grafana/plugins-platform-backend 2024-01-09 false false true false
156 cloudRBACRoles experimental @grafana/identity-access-team 2024-01-10 false false true false
157 alertingQueryOptimization GA @grafana/alerting-squad 2024-01-10 false false false false
158 newFolderPicker experimental @grafana/grafana-frontend-platform 2024-01-12 false false false true

View File

@ -630,4 +630,8 @@ const (
// FlagAlertingQueryOptimization
// Optimizes eligible queries in order to reduce load on datasources
FlagAlertingQueryOptimization = "alertingQueryOptimization"
// FlagNewFolderPicker
// Enables the nested folder picker without having nested folders enabled
FlagNewFolderPicker = "newFolderPicker"
)

View File

@ -6,7 +6,6 @@ import InfiniteLoader from 'react-window-infinite-loader';
import { GrafanaTheme2 } from '@grafana/data';
import { IconButton, useStyles2 } from '@grafana/ui';
import { getSvgSize } from '@grafana/ui/src/components/Icon/utils';
import { Text } from '@grafana/ui/src/components/Text/Text';
import { Indent } from 'app/core/components/Indent/Indent';
import { Trans } from 'app/core/internationalization';
@ -191,6 +190,7 @@ function Row({ index, style: virtualStyles, data }: RowProps) {
>
<div className={styles.rowBody}>
<Indent level={level} spacing={2} />
{foldersAreOpenable ? (
<IconButton
size={CHEVRON_SIZE}
@ -237,9 +237,8 @@ const getStyles = (theme: GrafanaTheme2) => {
width: '100%',
}),
// Should be the same size as the <IconButton /> for proper alignment
folderButtonSpacer: css({
paddingLeft: `calc(${getSvgSize(CHEVRON_SIZE)}px + ${theme.spacing(0.5)})`,
paddingLeft: theme.spacing(0.5),
}),
row: css({

View File

@ -6,6 +6,7 @@ import { SetupServer, setupServer } from 'msw/node';
import React from 'react';
import { TestProvider } from 'test/helpers/TestProvider';
import { config } from '@grafana/runtime';
import { backendSrv } from 'app/core/services/backend_srv';
import { wellFormedTree } from '../../../features/browse-dashboards/fixtures/dashboardsTreeItem.fixture';
@ -122,60 +123,111 @@ describe('NestedFolderPicker', () => {
expect(mockOnChange).toHaveBeenCalledWith(folderA.item.uid, folderA.item.title);
});
it('can expand and collapse a folder to show its children', async () => {
render(<NestedFolderPicker onChange={mockOnChange} />);
describe('when nestedFolders is enabled', () => {
let originalToggles = { ...config.featureToggles };
// Open the picker and wait for children to load
const button = await screen.findByRole('button', { name: 'Select folder' });
await userEvent.click(button);
await screen.findByLabelText(folderA.item.title);
beforeAll(() => {
config.featureToggles.nestedFolders = true;
});
// Expand Folder A
// Note: we need to use mouseDown here because userEvent's click event doesn't get prevented correctly
fireEvent.mouseDown(screen.getByRole('button', { name: `Expand folder ${folderA.item.title}` }));
afterAll(() => {
config.featureToggles = originalToggles;
});
// Folder A's children are visible
expect(await screen.findByLabelText(folderA_folderA.item.title)).toBeInTheDocument();
expect(await screen.findByLabelText(folderA_folderB.item.title)).toBeInTheDocument();
it('can expand and collapse a folder to show its children', async () => {
render(<NestedFolderPicker onChange={mockOnChange} />);
// Collapse Folder A
// Note: we need to use mouseDown here because userEvent's click event doesn't get prevented correctly
fireEvent.mouseDown(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();
// Open the picker and wait for children to load
const button = await screen.findByRole('button', { name: 'Select folder' });
await userEvent.click(button);
await screen.findByLabelText(folderA.item.title);
// Expand Folder A again
// Note: we need to use mouseDown here because userEvent's click event doesn't get prevented correctly
fireEvent.mouseDown(screen.getByRole('button', { name: `Expand folder ${folderA.item.title}` }));
// Expand Folder A
// Note: we need to use mouseDown here because userEvent's click event doesn't get prevented correctly
fireEvent.mouseDown(screen.getByRole('button', { name: `Expand folder ${folderA.item.title}` }));
// Select the first child
await userEvent.click(screen.getByLabelText(folderA_folderA.item.title));
expect(mockOnChange).toHaveBeenCalledWith(folderA_folderA.item.uid, folderA_folderA.item.title);
// Folder A's children are visible
expect(await screen.findByLabelText(folderA_folderA.item.title)).toBeInTheDocument();
expect(await screen.findByLabelText(folderA_folderB.item.title)).toBeInTheDocument();
// Collapse Folder A
// Note: we need to use mouseDown here because userEvent's click event doesn't get prevented correctly
fireEvent.mouseDown(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();
// Expand Folder A again
// Note: we need to use mouseDown here because userEvent's click event doesn't get prevented correctly
fireEvent.mouseDown(screen.getByRole('button', { name: `Expand folder ${folderA.item.title}` }));
// Select the first child
await userEvent.click(screen.getByLabelText(folderA_folderA.item.title));
expect(mockOnChange).toHaveBeenCalledWith(folderA_folderA.item.uid, folderA_folderA.item.title);
});
it('can expand and collapse a folder to show its children with the keyboard', async () => {
render(<NestedFolderPicker onChange={mockOnChange} />);
const button = await screen.findByRole('button', { name: 'Select folder' });
await userEvent.click(button);
// Expand Folder A
await userEvent.keyboard('{ArrowDown}{ArrowDown}{ArrowRight}');
// 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.keyboard('{ArrowLeft}');
expect(screen.queryByLabelText(folderA_folderA.item.title)).not.toBeInTheDocument();
expect(screen.queryByLabelText(folderA_folderB.item.title)).not.toBeInTheDocument();
// Expand Folder A again
await userEvent.keyboard('{ArrowRight}');
// Select the first child
await userEvent.keyboard('{ArrowDown}{Enter}');
expect(mockOnChange).toHaveBeenCalledWith(folderA_folderA.item.uid, folderA_folderA.item.title);
});
});
it('can expand and collapse a folder to show its children with the keyboard', async () => {
render(<NestedFolderPicker onChange={mockOnChange} />);
const button = await screen.findByRole('button', { name: 'Select folder' });
describe('when nestedFolders is disabled', () => {
let originalToggles = { ...config.featureToggles };
await userEvent.click(button);
beforeAll(() => {
config.featureToggles.nestedFolders = false;
});
// Expand Folder A
await userEvent.keyboard('{ArrowDown}{ArrowDown}{ArrowRight}');
afterAll(() => {
config.featureToggles = originalToggles;
});
// Folder A's children are visible
expect(screen.getByLabelText(folderA_folderA.item.title)).toBeInTheDocument();
expect(screen.getByLabelText(folderA_folderB.item.title)).toBeInTheDocument();
it('does not show an expand button', async () => {
render(<NestedFolderPicker onChange={mockOnChange} />);
// Collapse Folder A
await userEvent.keyboard('{ArrowLeft}');
expect(screen.queryByLabelText(folderA_folderA.item.title)).not.toBeInTheDocument();
expect(screen.queryByLabelText(folderA_folderB.item.title)).not.toBeInTheDocument();
// Open the picker and wait for children to load
const button = await screen.findByRole('button', { name: 'Select folder' });
await userEvent.click(button);
await screen.findByLabelText(folderA.item.title);
// Expand Folder A again
await userEvent.keyboard('{ArrowRight}');
// There should be no expand button
// Note: we need to use mouseDown here because userEvent's click event doesn't get prevented correctly
expect(screen.queryByRole('button', { name: `Expand folder ${folderA.item.title}` })).not.toBeInTheDocument();
});
// Select the first child
await userEvent.keyboard('{ArrowDown}{Enter}');
expect(mockOnChange).toHaveBeenCalledWith(folderA_folderA.item.uid, folderA_folderA.item.title);
it('does not expand a folder with the keyboard', async () => {
render(<NestedFolderPicker onChange={mockOnChange} />);
const button = await screen.findByRole('button', { name: 'Select folder' });
await userEvent.click(button);
// try to expand Folder A
await userEvent.keyboard('{ArrowDown}{ArrowDown}{ArrowRight}');
// Folder A's children are not visible
expect(screen.queryByLabelText(folderA_folderA.item.title)).not.toBeInTheDocument();
expect(screen.queryByLabelText(folderA_folderB.item.title)).not.toBeInTheDocument();
});
});
});

View File

@ -59,6 +59,7 @@ export function NestedFolderPicker({
const selectedFolder = useGetFolderQuery(value || skipToken);
const rootStatus = useBrowseLoadingStatus(undefined);
const nestedFoldersEnabled = Boolean(config.featureToggles.nestedFolders);
const [search, setSearch] = useState('');
const [autoFocusButton, setAutoFocusButton] = useState(false);
@ -310,7 +311,7 @@ export function NestedFolderPicker({
onFolderExpand={handleFolderExpand}
onFolderSelect={handleFolderSelect}
idPrefix={overlayId}
foldersAreOpenable={!(search && searchState.value)}
foldersAreOpenable={nestedFoldersEnabled && !(search && searchState.value)}
isItemLoaded={isItemLoaded}
requestLoadMore={handleLoadMore}
/>

View File

@ -1,5 +1,6 @@
import React, { useCallback, useEffect, useState } from 'react';
import { config } from '@grafana/runtime';
import { DashboardsTreeItem } from 'app/features/browse-dashboards/types';
import { DashboardViewItem } from 'app/features/search/types';
@ -25,6 +26,7 @@ export function useTreeInteractions({
visible,
}: TreeInteractionProps) {
const [focusedItemIndex, setFocusedItemIndex] = useState(-1);
const nestedFoldersEnabled = Boolean(config.featureToggles.nestedFolders);
useEffect(() => {
if (visible) {
@ -44,7 +46,7 @@ export function useTreeInteractions({
const handleKeyDown = useCallback(
(ev: React.KeyboardEvent<HTMLInputElement>) => {
const foldersAreOpenable = !search;
const foldersAreOpenable = nestedFoldersEnabled && !search;
switch (ev.key) {
// Expand/collapse folder on right/left arrow keys
case 'ArrowRight':
@ -84,7 +86,7 @@ export function useTreeInteractions({
break;
}
},
[focusedItemIndex, handleCloseOverlay, handleFolderExpand, handleFolderSelect, search, tree]
[focusedItemIndex, handleCloseOverlay, handleFolderExpand, handleFolderSelect, nestedFoldersEnabled, search, tree]
);
return {

View File

@ -28,7 +28,9 @@ interface FolderPickerProps extends NestedFolderPickerProps {
// Temporary wrapper component to switch between the NestedFolderPicker and the old flat
// FolderPicker depending on feature flags
export function FolderPicker(props: FolderPickerProps) {
const nestedEnabled = config.featureToggles.nestedFolders && config.featureToggles.nestedFolderPicker;
const nestedEnabled =
config.featureToggles.newFolderPicker ||
(config.featureToggles.nestedFolders && config.featureToggles.nestedFolderPicker);
const { initialTitle, dashboardId, enableCreateNew, ...newFolderPickerProps } = props;
return nestedEnabled ? <NestedFolderPicker {...newFolderPickerProps} /> : <OldFolderPickerWrapper {...props} />;