From 6f8c1525da689564d3ff5e7a46ead500892beb3d Mon Sep 17 00:00:00 2001 From: Tom Ratcliffe Date: Wed, 19 Nov 2025 11:05:11 +0000 Subject: [PATCH] Folders: Improve wording for actions and move/delete (#114090) --- .../components/BrowseActions/DeleteModal.tsx | 31 ++++++++-- .../BrowseActions/MoveModal.test.tsx | 2 +- .../components/BrowseActions/MoveModal.tsx | 29 +++++++++- .../components/FolderActionsButton.test.tsx | 58 +++++++++---------- .../components/FolderActionsButton.tsx | 4 +- public/locales/en-US/grafana.json | 6 +- 6 files changed, 88 insertions(+), 42 deletions(-) diff --git a/public/app/features/browse-dashboards/components/BrowseActions/DeleteModal.tsx b/public/app/features/browse-dashboards/components/BrowseActions/DeleteModal.tsx index be238108e7f..9434e634661 100644 --- a/public/app/features/browse-dashboards/components/BrowseActions/DeleteModal.tsx +++ b/public/app/features/browse-dashboards/components/BrowseActions/DeleteModal.tsx @@ -3,7 +3,7 @@ import { useState } from 'react'; import { Trans, t } from '@grafana/i18n'; import { config, reportInteraction } from '@grafana/runtime'; import { Alert, ConfirmModal, Text, Space } from '@grafana/ui'; -import { useGetAffectedItems } from 'app/api/clients/folder/v1beta1/hooks'; +import { useGetAffectedItems, useGetFolderQueryFacade } from 'app/api/clients/folder/v1beta1/hooks'; import { DashboardTreeSelection } from '../../types'; @@ -21,6 +21,16 @@ export const DeleteModal = ({ onConfirm, onDismiss, selectedItems, ...props }: P const deleteIsInvalid = Boolean(data && (data.alertrules || data.library_elements)); const [isDeleting, setIsDeleting] = useState(false); + const selectedFolders = Object.keys(selectedItems.folder || {}).filter((uid) => selectedItems.folder[uid]); + const selectedDashboards = Object.keys(selectedItems.dashboard || {}).filter((uid) => selectedItems.dashboard[uid]); + const selectedPanels = Object.keys(selectedItems.panel || {}).filter((uid) => selectedItems.panel[uid]); + const { data: folderData } = useGetFolderQueryFacade(selectedFolders.length === 1 ? selectedFolders[0] : undefined); + + // If we are only moving one folder, we can show a different message + // (we might be in the "Folder actions" version of the modal) + const onlyOneFolderSelected = + selectedFolders.length === 1 && selectedDashboards.length === 0 && selectedPanels.length === 0; + const onDelete = async () => { reportInteraction('grafana_manage_dashboards_delete_clicked', { item_counts: { @@ -58,9 +68,22 @@ export const DeleteModal = ({ onConfirm, onDismiss, selectedItems, ...props }: P )} - - This action will delete the following content: - + {onlyOneFolderSelected ? ( + + This action will delete the folder " + + {'{{ folderName }}'} + + " and the following content: + + ) : ( + + This action will delete the following content: + + )} 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 1f7b53b8511..c877808daa5 100644 --- a/public/app/features/browse-dashboards/components/BrowseActions/MoveModal.test.tsx +++ b/public/app/features/browse-dashboards/components/BrowseActions/MoveModal.test.tsx @@ -86,7 +86,7 @@ describe('browse-dashboards MoveModal', () => { it('displays summary of affected items', async () => { render(); - expect(await screen.findByText(/This action will move the following content/i)).toBeInTheDocument(); + expect(await screen.findByText(/This action will move the folder/i)).toBeInTheDocument(); expect(await screen.findByText(/5 item/)).toBeInTheDocument(); expect(screen.getByText(/2 folder/)).toBeInTheDocument(); diff --git a/public/app/features/browse-dashboards/components/BrowseActions/MoveModal.tsx b/public/app/features/browse-dashboards/components/BrowseActions/MoveModal.tsx index dc7c576134d..abf3a3636ae 100644 --- a/public/app/features/browse-dashboards/components/BrowseActions/MoveModal.tsx +++ b/public/app/features/browse-dashboards/components/BrowseActions/MoveModal.tsx @@ -2,6 +2,7 @@ import { useState } from 'react'; import { Trans, t } from '@grafana/i18n'; import { Alert, Button, Field, Modal, Text, Space, Box } from '@grafana/ui'; +import { useGetFolderQueryFacade } from 'app/api/clients/folder/v1beta1/hooks'; import { MoveActionAvailableTargetWarning } from 'app/features/provisioning/components/Shared/MoveActionAvailableTargetWarning'; import { ProvisioningAwareFolderPicker } from 'app/features/provisioning/components/Shared/ProvisioningAwareFolderPicker'; @@ -19,7 +20,15 @@ export interface Props { export const MoveModal = ({ onConfirm, onDismiss, selectedItems, ...props }: Props) => { const [moveTarget, setMoveTarget] = useState(); 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 selectedDashboards = Object.keys(selectedItems.dashboard || {}).filter((uid) => selectedItems.dashboard[uid]); + const selectedPanels = Object.keys(selectedItems.panel || {}).filter((uid) => selectedItems.panel[uid]); + const { data: folderData } = useGetFolderQueryFacade(selectedFolders.length === 1 ? selectedFolders[0] : undefined); + + // If we are only moving one folder, we can show a different message + // (we might be in the "Folder actions" version of the modal) + const onlyOneFolderSelected = + selectedFolders.length === 1 && selectedDashboards.length === 0 && selectedPanels.length === 0; const onMove = async () => { if (moveTarget !== undefined) { @@ -47,9 +56,23 @@ export const MoveModal = ({ onConfirm, onDismiss, selectedItems, ...props }: Pro - This action will move the following content: + {onlyOneFolderSelected ? ( + + This action will move the folder " + + {'{{ folderName }}'} + + " and the following content: + + ) : ( + + This action will move the following content: + + )} - diff --git a/public/app/features/browse-dashboards/components/FolderActionsButton.test.tsx b/public/app/features/browse-dashboards/components/FolderActionsButton.test.tsx index c253fb0d186..10f787ae26b 100644 --- a/public/app/features/browse-dashboards/components/FolderActionsButton.test.tsx +++ b/public/app/features/browse-dashboards/components/FolderActionsButton.test.tsx @@ -1,6 +1,4 @@ -import { render as rtlRender, screen } from '@testing-library/react'; -import userEvent from '@testing-library/user-event'; -import { TestProvider } from 'test/helpers/TestProvider'; +import { render, screen, userEvent } from 'test/test-utils'; import { appEvents } from 'app/core/app_events'; import { ManagerKind } from 'app/features/apiserver/types'; @@ -13,15 +11,15 @@ import { DeleteModal } from './BrowseActions/DeleteModal'; import { MoveModal } from './BrowseActions/MoveModal'; import { FolderActionsButton } from './FolderActionsButton'; -function render(...[ui, options]: Parameters) { - rtlRender({ui}, options); -} - // Mock out the Permissions component for now jest.mock('app/core/components/AccessControl/Permissions', () => ({ Permissions: () =>
Hello!
, })); +const managePermissionsLabel = /Manage permissions/i; +const moveMenuItemLabel = /Move this folder/i; +const deleteMenuItemLabel = /Delete this folder/i; + describe('browse-dashboards FolderActionsButton', () => { const mockFolder = mockFolderDTO(); const mockPermissions = { @@ -63,12 +61,12 @@ describe('browse-dashboards FolderActionsButton', () => { }); it('renders all the options if the user has full permissions', async () => { - render(); + const { user } = render(); - await userEvent.click(screen.getByRole('button', { name: 'Folder actions' })); - expect(screen.getByRole('menuitem', { name: 'Manage permissions' })).toBeInTheDocument(); - expect(screen.getByRole('menuitem', { name: 'Move' })).toBeInTheDocument(); - expect(screen.getByRole('menuitem', { name: 'Delete' })).toBeInTheDocument(); + await user.click(screen.getByRole('button', { name: 'Folder actions' })); + expect(screen.getByRole('menuitem', { name: managePermissionsLabel })).toBeInTheDocument(); + expect(screen.getByRole('menuitem', { name: moveMenuItemLabel })).toBeInTheDocument(); + expect(screen.getByRole('menuitem', { name: deleteMenuItemLabel })).toBeInTheDocument(); }); it('does not render the "Manage permissions" option if the user does not have permission to view permissions', async () => { @@ -81,9 +79,9 @@ describe('browse-dashboards FolderActionsButton', () => { render(); await userEvent.click(screen.getByRole('button', { name: 'Folder actions' })); - expect(screen.queryByRole('menuitem', { name: 'Manage permissions' })).not.toBeInTheDocument(); - expect(screen.getByRole('menuitem', { name: 'Move' })).toBeInTheDocument(); - expect(screen.getByRole('menuitem', { name: 'Delete' })).toBeInTheDocument(); + expect(screen.queryByRole('menuitem', { name: managePermissionsLabel })).not.toBeInTheDocument(); + expect(screen.getByRole('menuitem', { name: moveMenuItemLabel })).toBeInTheDocument(); + expect(screen.getByRole('menuitem', { name: deleteMenuItemLabel })).toBeInTheDocument(); }); it('does not render the "Move" option if the user does not have permission to edit', async () => { @@ -96,9 +94,9 @@ describe('browse-dashboards FolderActionsButton', () => { render(); await userEvent.click(screen.getByRole('button', { name: 'Folder actions' })); - expect(screen.getByRole('menuitem', { name: 'Manage permissions' })).toBeInTheDocument(); - expect(screen.queryByRole('menuitem', { name: 'Move' })).not.toBeInTheDocument(); - expect(screen.getByRole('menuitem', { name: 'Delete' })).toBeInTheDocument(); + expect(screen.getByRole('menuitem', { name: managePermissionsLabel })).toBeInTheDocument(); + expect(screen.queryByRole('menuitem', { name: moveMenuItemLabel })).not.toBeInTheDocument(); + expect(screen.getByRole('menuitem', { name: deleteMenuItemLabel })).toBeInTheDocument(); }); it('does not render the "Delete" option if the user does not have permission to delete', async () => { @@ -111,16 +109,16 @@ describe('browse-dashboards FolderActionsButton', () => { render(); await userEvent.click(screen.getByRole('button', { name: 'Folder actions' })); - expect(screen.getByRole('menuitem', { name: 'Manage permissions' })).toBeInTheDocument(); - expect(screen.getByRole('menuitem', { name: 'Move' })).toBeInTheDocument(); - expect(screen.queryByRole('menuitem', { name: 'Delete' })).not.toBeInTheDocument(); + expect(screen.getByRole('menuitem', { name: managePermissionsLabel })).toBeInTheDocument(); + expect(screen.getByRole('menuitem', { name: moveMenuItemLabel })).toBeInTheDocument(); + expect(screen.queryByRole('menuitem', { name: deleteMenuItemLabel })).not.toBeInTheDocument(); }); it('clicking the "Manage permissions" option opens the permissions drawer', async () => { render(); await userEvent.click(screen.getByRole('button', { name: 'Folder actions' })); - await userEvent.click(screen.getByRole('menuitem', { name: 'Manage permissions' })); + await userEvent.click(screen.getByRole('menuitem', { name: managePermissionsLabel })); expect(screen.getByRole('dialog', { name: 'Drawer title Manage permissions' })).toBeInTheDocument(); }); @@ -129,7 +127,7 @@ describe('browse-dashboards FolderActionsButton', () => { render(); await userEvent.click(screen.getByRole('button', { name: 'Folder actions' })); - await userEvent.click(screen.getByRole('menuitem', { name: 'Move' })); + await userEvent.click(screen.getByRole('menuitem', { name: moveMenuItemLabel })); expect(appEvents.publish).toHaveBeenCalledWith( new ShowModalReactEvent( expect.objectContaining({ @@ -144,7 +142,7 @@ describe('browse-dashboards FolderActionsButton', () => { render(); await userEvent.click(screen.getByRole('button', { name: 'Folder actions' })); - await userEvent.click(screen.getByRole('menuitem', { name: 'Delete' })); + await userEvent.click(screen.getByRole('menuitem', { name: deleteMenuItemLabel })); expect(appEvents.publish).toHaveBeenCalledWith( new ShowModalReactEvent( expect.objectContaining({ @@ -165,8 +163,8 @@ describe('browse-dashboards FolderActionsButton', () => { render(); await userEvent.click(screen.getByRole('button', { name: 'Folder actions' })); - expect(screen.queryByRole('menuitem', { name: 'Manage permissions' })).not.toBeInTheDocument(); - expect(screen.getByRole('menuitem', { name: 'Delete' })).toBeInTheDocument(); + expect(screen.queryByRole('menuitem', { name: managePermissionsLabel })).not.toBeInTheDocument(); + expect(screen.getByRole('menuitem', { name: deleteMenuItemLabel })).toBeInTheDocument(); }); it('does not render the "Move" option if folder is provisioned and is root repo folder', async () => { @@ -179,8 +177,8 @@ describe('browse-dashboards FolderActionsButton', () => { render(); await userEvent.click(screen.getByRole('button', { name: 'Folder actions' })); - expect(screen.queryByRole('menuitem', { name: 'Move' })).not.toBeInTheDocument(); - expect(screen.getByRole('menuitem', { name: 'Delete' })).toBeInTheDocument(); + expect(screen.queryByRole('menuitem', { name: moveMenuItemLabel })).not.toBeInTheDocument(); + expect(screen.getByRole('menuitem', { name: deleteMenuItemLabel })).toBeInTheDocument(); }); it('does render the "Move" option if folder is provisioned and is NOT root repo folder', async () => { @@ -193,7 +191,7 @@ describe('browse-dashboards FolderActionsButton', () => { render(); await userEvent.click(screen.getByRole('button', { name: 'Folder actions' })); - expect(screen.getByRole('menuitem', { name: 'Move' })).toBeInTheDocument(); - expect(screen.getByRole('menuitem', { name: 'Delete' })).toBeInTheDocument(); + expect(screen.getByRole('menuitem', { name: moveMenuItemLabel })).toBeInTheDocument(); + expect(screen.getByRole('menuitem', { name: deleteMenuItemLabel })).toBeInTheDocument(); }); }); diff --git a/public/app/features/browse-dashboards/components/FolderActionsButton.tsx b/public/app/features/browse-dashboards/components/FolderActionsButton.tsx index 664bc53c8c0..7c3070c4184 100644 --- a/public/app/features/browse-dashboards/components/FolderActionsButton.tsx +++ b/public/app/features/browse-dashboards/components/FolderActionsButton.tsx @@ -126,8 +126,8 @@ export function FolderActionsButton({ folder, repoType, isReadOnlyRepo }: Props) }; const managePermissionsLabel = t('browse-dashboards.folder-actions-button.manage-permissions', 'Manage permissions'); - const moveLabel = t('browse-dashboards.folder-actions-button.move', 'Move'); - const deleteLabel = t('browse-dashboards.folder-actions-button.delete', 'Delete'); + const moveLabel = t('browse-dashboards.folder-actions-button.move', 'Move this folder'); + const deleteLabel = t('browse-dashboards.folder-actions-button.delete', 'Delete this folder'); const menu = ( diff --git a/public/locales/en-US/grafana.json b/public/locales/en-US/grafana.json index 248c53b3b2b..4492a771832 100644 --- a/public/locales/en-US/grafana.json +++ b/public/locales/en-US/grafana.json @@ -3541,6 +3541,7 @@ "delete-modal-invalid-title": "Cannot delete folder", "delete-modal-restore-dashboards-text": "This action will delete the selected folders immediately. Deleted dashboards will be kept in the history for up to 12 months and can be restored by your organization administrator during that time. The history is limited to 1000 dashboards — older ones may be removed sooner if the limit is reached. Folders cannot be restored.", "delete-modal-text": "This action will delete the following content:", + "delete-modal-text-one-folder": "This action will delete the folder \" <1>{{ folderName }} \" and the following content:", "delete-modal-title": "Delete", "delete-provisioned-folder": "Delete provisioned folder", "deleting": "Deleting...", @@ -3549,6 +3550,7 @@ "move-modal-alert": "Moving this item may change its permissions.", "move-modal-field-label": "Folder name", "move-modal-text": "This action will move the following content:", + "move-modal-text-one-folder": "This action will move the folder \" <1>{{ folderName }} \" and the following content:", "move-modal-title": "Move", "move-provisioned-folder": "Move provisioned folder", "moving": "Moving...", @@ -3636,11 +3638,11 @@ "title-folder": "This folder doesn't have any dashboards yet" }, "folder-actions-button": { - "delete": "Delete", + "delete": "Delete this folder", "delete-folder-error": "Error deleting folder. Please try again later.", "folder-actions": "Folder actions", "manage-permissions": "Manage permissions", - "move": "Move" + "move": "Move this folder" }, "folder-picker": { "accessible-label": "Select folder: {{ label }} currently selected",