From 1c3981e5ab2fb73e7a9dff724b3fa44e7419c60e Mon Sep 17 00:00:00 2001 From: Ashley Harrison Date: Tue, 16 Jan 2024 11:39:28 +0000 Subject: [PATCH] [v10.2.x] NestedFolderPicker: separate toggle to force enable picker without (#80550) * 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 (cherry picked from commit ec53487c995777b314f566f5a1054e3f8e29ec05) * add config import to NestedFolderPicker --- .../feature-toggles/index.md | 1 + .../src/types/featureToggles.gen.ts | 1 + pkg/services/featuremgmt/registry.go | 8 ++ pkg/services/featuremgmt/toggles_gen.csv | 1 + pkg/services/featuremgmt/toggles_gen.go | 4 + .../NestedFolderPicker/NestedFolderList.tsx | 5 +- .../NestedFolderPicker.test.tsx | 134 ++++++++++++------ .../NestedFolderPicker/NestedFolderPicker.tsx | 4 +- .../components/NestedFolderPicker/hooks.ts | 6 +- .../core/components/Select/FolderPicker.tsx | 4 +- 10 files changed, 120 insertions(+), 48 deletions(-) diff --git a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md index 32c5fe089b5..159f22cad03 100644 --- a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md +++ b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md @@ -167,6 +167,7 @@ Experimental features might be changed or removed without prior notice. | `pluginsSkipHostEnvVars` | Disables passing host environment variable to plugin processes | | `regressionTransformation` | Enables regression analysis transformation | | `displayAnonymousStats` | Enables anonymous stats to be shown in the UI for Grafana | +| `newFolderPicker` | Enables the nested folder picker without having nested folders enabled | ## Development feature toggles diff --git a/packages/grafana-data/src/types/featureToggles.gen.ts b/packages/grafana-data/src/types/featureToggles.gen.ts index 89de259631d..464b8d90feb 100644 --- a/packages/grafana-data/src/types/featureToggles.gen.ts +++ b/packages/grafana-data/src/types/featureToggles.gen.ts @@ -168,4 +168,5 @@ export interface FeatureToggles { pluginsSkipHostEnvVars?: boolean; regressionTransformation?: boolean; displayAnonymousStats?: boolean; + newFolderPicker?: boolean; } diff --git a/pkg/services/featuremgmt/registry.go b/pkg/services/featuremgmt/registry.go index 0330391a3da..8dc91176966 100644 --- a/pkg/services/featuremgmt/registry.go +++ b/pkg/services/featuremgmt/registry.go @@ -1258,6 +1258,14 @@ var ( Owner: identityAccessTeam, Created: time.Date(2023, time.November, 29, 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), + }, } ) diff --git a/pkg/services/featuremgmt/toggles_gen.csv b/pkg/services/featuremgmt/toggles_gen.csv index 69ea390b652..65d3abee1d3 100644 --- a/pkg/services/featuremgmt/toggles_gen.csv +++ b/pkg/services/featuremgmt/toggles_gen.csv @@ -149,3 +149,4 @@ logRowsPopoverMenu,experimental,@grafana/observability-logs,2023-11-16,false,fal pluginsSkipHostEnvVars,experimental,@grafana/plugins-platform-backend,2023-11-15,false,false,false,false regressionTransformation,experimental,@grafana/grafana-bi-squad,2023-11-24,false,false,false,true displayAnonymousStats,experimental,@grafana/identity-access-team,2023-11-29,false,false,false,true +newFolderPicker,experimental,@grafana/grafana-frontend-platform,2024-01-12,false,false,false,true diff --git a/pkg/services/featuremgmt/toggles_gen.go b/pkg/services/featuremgmt/toggles_gen.go index 3e5ce38bd38..2f4f6841a8a 100644 --- a/pkg/services/featuremgmt/toggles_gen.go +++ b/pkg/services/featuremgmt/toggles_gen.go @@ -606,4 +606,8 @@ const ( // FlagDisplayAnonymousStats // Enables anonymous stats to be shown in the UI for Grafana FlagDisplayAnonymousStats = "displayAnonymousStats" + + // FlagNewFolderPicker + // Enables the nested folder picker without having nested folders enabled + FlagNewFolderPicker = "newFolderPicker" ) diff --git a/public/app/core/components/NestedFolderPicker/NestedFolderList.tsx b/public/app/core/components/NestedFolderPicker/NestedFolderList.tsx index f0874362c93..7de1643f74d 100644 --- a/public/app/core/components/NestedFolderPicker/NestedFolderList.tsx +++ b/public/app/core/components/NestedFolderPicker/NestedFolderList.tsx @@ -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) { >
+ {foldersAreOpenable ? ( { width: '100%', }), - // Should be the same size as the for proper alignment folderButtonSpacer: css({ - paddingLeft: `calc(${getSvgSize(CHEVRON_SIZE)}px + ${theme.spacing(0.5)})`, + paddingLeft: theme.spacing(0.5), }), row: css({ diff --git a/public/app/core/components/NestedFolderPicker/NestedFolderPicker.test.tsx b/public/app/core/components/NestedFolderPicker/NestedFolderPicker.test.tsx index fc71b260d5c..3c24d81440b 100644 --- a/public/app/core/components/NestedFolderPicker/NestedFolderPicker.test.tsx +++ b/public/app/core/components/NestedFolderPicker/NestedFolderPicker.test.tsx @@ -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(); + 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(); - // 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(); + 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(); - 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(); - // 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(); + 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(); + }); }); }); diff --git a/public/app/core/components/NestedFolderPicker/NestedFolderPicker.tsx b/public/app/core/components/NestedFolderPicker/NestedFolderPicker.tsx index baff647e306..fd92432c1da 100644 --- a/public/app/core/components/NestedFolderPicker/NestedFolderPicker.tsx +++ b/public/app/core/components/NestedFolderPicker/NestedFolderPicker.tsx @@ -4,6 +4,7 @@ import { usePopperTooltip } from 'react-popper-tooltip'; import { useAsync } from 'react-use'; import { GrafanaTheme2 } from '@grafana/data'; +import { config } from '@grafana/runtime'; import { Alert, Icon, Input, LoadingBar, useStyles2 } from '@grafana/ui'; import { t } from 'app/core/internationalization'; import { skipToken, useGetFolderQuery } from 'app/features/browse-dashboards/api/browseDashboardsAPI'; @@ -58,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); @@ -290,7 +292,7 @@ export function NestedFolderPicker({ onFolderExpand={handleFolderExpand} onFolderSelect={handleFolderSelect} idPrefix={overlayId} - foldersAreOpenable={!(search && searchState.value)} + foldersAreOpenable={nestedFoldersEnabled && !(search && searchState.value)} isItemLoaded={isItemLoaded} requestLoadMore={handleLoadMore} /> diff --git a/public/app/core/components/NestedFolderPicker/hooks.ts b/public/app/core/components/NestedFolderPicker/hooks.ts index 29a76e8c28f..393d35320e5 100644 --- a/public/app/core/components/NestedFolderPicker/hooks.ts +++ b/public/app/core/components/NestedFolderPicker/hooks.ts @@ -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) => { - 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 { diff --git a/public/app/core/components/Select/FolderPicker.tsx b/public/app/core/components/Select/FolderPicker.tsx index 71cfb8262ff..1611b961366 100644 --- a/public/app/core/components/Select/FolderPicker.tsx +++ b/public/app/core/components/Select/FolderPicker.tsx @@ -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 ? : ;