From 0adb2461e9bdd21c56412a2b9d3695fad6e06902 Mon Sep 17 00:00:00 2001 From: "Marc M." <146180665+grafakus@users.noreply.github.com> Date: Fri, 5 Dec 2025 16:48:34 +0100 Subject: [PATCH] Dashboards: Improve custom variable editor and undo/redo (#114559) --- .../dashboards-edit-custom-variables.spec.ts | 6 +- .../src/selectors/pages.ts | 3 + .../components/VariableValuesPreview.tsx | 2 +- .../CustomVariableEditor/ModalEditor.tsx | 92 +++++++++++++++---- .../editors/CustomVariableEditor/PaneItem.tsx | 2 +- .../CustomVariableEditor/ValuesBuilder.tsx | 52 ----------- .../CustomVariableEditor/ValuesPreview.tsx | 13 --- public/locales/en-US/grafana.json | 4 +- 8 files changed, 85 insertions(+), 89 deletions(-) delete mode 100644 public/app/features/dashboard-scene/settings/variables/editors/CustomVariableEditor/ValuesBuilder.tsx delete mode 100644 public/app/features/dashboard-scene/settings/variables/editors/CustomVariableEditor/ValuesPreview.tsx diff --git a/e2e-playwright/dashboard-new-layouts/dashboards-edit-custom-variables.spec.ts b/e2e-playwright/dashboard-new-layouts/dashboards-edit-custom-variables.spec.ts index 4715dfc7128..e11f2dd099a 100644 --- a/e2e-playwright/dashboard-new-layouts/dashboards-edit-custom-variables.spec.ts +++ b/e2e-playwright/dashboard-new-layouts/dashboards-edit-custom-variables.spec.ts @@ -84,9 +84,9 @@ test.describe( refetchItems(dashboardPage, selectors); }; - const closeModal = async (dashboardPage: DashboardPage, selectors: E2ESelectorGroups) => { + const applyAndcloseModal = async (dashboardPage: DashboardPage, selectors: E2ESelectorGroups) => { await dashboardPage - .getByGrafanaSelector(selectors.pages.Dashboard.Settings.Variables.Edit.CustomVariable.closeButton) + .getByGrafanaSelector(selectors.pages.Dashboard.Settings.Variables.Edit.CustomVariable.applyButton) .click(); }; @@ -149,7 +149,7 @@ test.describe( await removeItem(dashboardPage, selectors, 2); await checkRows(3); await checkPreview(dashboardPage, selectors, ['first value', 'second label', 'fourth value']); - await closeModal(dashboardPage, selectors); + await applyAndcloseModal(dashboardPage, selectors); // assert variable is visible and has the correct values const variableLabel = dashboardPage.getByGrafanaSelector( diff --git a/packages/grafana-e2e-selectors/src/selectors/pages.ts b/packages/grafana-e2e-selectors/src/selectors/pages.ts index 47a5573b00d..1fa640a2563 100644 --- a/packages/grafana-e2e-selectors/src/selectors/pages.ts +++ b/packages/grafana-e2e-selectors/src/selectors/pages.ts @@ -567,6 +567,9 @@ export const versionedPages = { closeButton: { [MIN_GRAFANA_VERSION]: 'data-testid custom-variable-close-button', }, + applyButton: { + [MIN_GRAFANA_VERSION]: 'data-testid custom-variable-apply-button', + }, }, IntervalVariable: { intervalsValueInput: { diff --git a/public/app/features/dashboard-scene/settings/variables/components/VariableValuesPreview.tsx b/public/app/features/dashboard-scene/settings/variables/components/VariableValuesPreview.tsx index 73d57bd811a..ac59419cda7 100644 --- a/public/app/features/dashboard-scene/settings/variables/components/VariableValuesPreview.tsx +++ b/public/app/features/dashboard-scene/settings/variables/components/VariableValuesPreview.tsx @@ -37,7 +37,7 @@ export const VariableValuesPreview = ({ options }: VariableValuesPreviewProps) = {previewOptions.map((o, index) => ( -
{o.label}
+
{o.label || String(o.value)}
 ))} diff --git a/public/app/features/dashboard-scene/settings/variables/editors/CustomVariableEditor/ModalEditor.tsx b/public/app/features/dashboard-scene/settings/variables/editors/CustomVariableEditor/ModalEditor.tsx index 3e8a8aa57b1..aed926a6809 100644 --- a/public/app/features/dashboard-scene/settings/variables/editors/CustomVariableEditor/ModalEditor.tsx +++ b/public/app/features/dashboard-scene/settings/variables/editors/CustomVariableEditor/ModalEditor.tsx @@ -1,47 +1,103 @@ -import { useCallback, useRef } from 'react'; +import { useRef, useState } from 'react'; +import { lastValueFrom } from 'rxjs'; import { selectors } from '@grafana/e2e-selectors'; import { t, Trans } from '@grafana/i18n'; -import { CustomVariable } from '@grafana/scenes'; +import { CustomVariable, VariableValueOption, VariableValueSingle } from '@grafana/scenes'; import { Button, Modal, Stack } from '@grafana/ui'; -import { VariableStaticOptionsFormRef } from '../../components/VariableStaticOptionsForm'; +import { dashboardEditActions } from '../../../../edit-pane/shared'; +import { VariableStaticOptionsForm, VariableStaticOptionsFormRef } from '../../components/VariableStaticOptionsForm'; import { VariableStaticOptionsFormAddButton } from '../../components/VariableStaticOptionsFormAddButton'; - -import { ValuesBuilder } from './ValuesBuilder'; -import { ValuesPreview } from './ValuesPreview'; +import { VariableValuesPreview } from '../../components/VariableValuesPreview'; interface ModalEditorProps { variable: CustomVariable; - isOpen: boolean; onClose: () => void; } -export function ModalEditor({ variable, isOpen, onClose }: ModalEditorProps) { - const formRef = useRef(null); - - const handleOnAdd = useCallback(() => formRef.current?.addItem(), []); +export function ModalEditor(props: ModalEditorProps) { + const { formRef, onCloseModal, options, onChangeOptions, onAddNewOption, onSaveOptions } = useModalEditor(props); return ( - - + + - }> + }> + ); } + +function useModalEditor({ variable, onClose }: ModalEditorProps) { + const { query } = variable.state; + const [options, setOptions] = useState(() => transformQueryToOptions(variable, query)); + const initialQueryRef = useRef(query); + const formRef = useRef(null); + + return { + formRef, + onCloseModal: onClose, + options, + onChangeOptions: setOptions, + onAddNewOption() { + formRef.current?.addItem(); + }, + onSaveOptions() { + dashboardEditActions.edit({ + source: variable, + description: t('dashboard.edit-pane.variable.custom-options.change-value', 'Change variable value'), + perform: () => { + variable.setState({ query: transformOptionsToQuery(options) }); + lastValueFrom(variable.validateAndUpdate!()); + }, + undo: () => { + variable.setState({ query: initialQueryRef.current }); + lastValueFrom(variable.validateAndUpdate!()); + }, + }); + + onClose(); + }, + }; +} + +const transformQueryToOptions = (variable: ModalEditorProps['variable'], query: string) => + variable.transformCsvStringToOptions(query, false).map(({ label, value }) => ({ + value, + label: value === label ? '' : label, + })); + +const formatOption = (option: VariableValueOption) => { + if (!option.label || option.label === option.value) { + return escapeEntities(option.value); + } + return `${escapeEntities(option.label)} : ${escapeEntities(String(option.value))}`; +}; + +const escapeEntities = (text: VariableValueSingle) => String(text).trim().replaceAll(',', '\\,'); + +const transformOptionsToQuery = (options: VariableValueOption[]) => options.map(formatOption).join(', '); diff --git a/public/app/features/dashboard-scene/settings/variables/editors/CustomVariableEditor/PaneItem.tsx b/public/app/features/dashboard-scene/settings/variables/editors/CustomVariableEditor/PaneItem.tsx index e453fc6b8b8..d1dab1e554f 100644 --- a/public/app/features/dashboard-scene/settings/variables/editors/CustomVariableEditor/PaneItem.tsx +++ b/public/app/features/dashboard-scene/settings/variables/editors/CustomVariableEditor/PaneItem.tsx @@ -31,7 +31,7 @@ export function PaneItem({ variable }: PaneItemProps) { Open variable editor - setIsOpen(false)} /> + {isOpen && setIsOpen(false)} />} ); } diff --git a/public/app/features/dashboard-scene/settings/variables/editors/CustomVariableEditor/ValuesBuilder.tsx b/public/app/features/dashboard-scene/settings/variables/editors/CustomVariableEditor/ValuesBuilder.tsx deleted file mode 100644 index e2eceea5fd3..00000000000 --- a/public/app/features/dashboard-scene/settings/variables/editors/CustomVariableEditor/ValuesBuilder.tsx +++ /dev/null @@ -1,52 +0,0 @@ -import { forwardRef, useCallback } from 'react'; -import { lastValueFrom } from 'rxjs'; - -import { CustomVariable, VariableValueOption, VariableValueSingle } from '@grafana/scenes'; - -import { VariableStaticOptionsForm, VariableStaticOptionsFormRef } from '../../components/VariableStaticOptionsForm'; - -interface ValuesBuilderProps { - variable: CustomVariable; -} - -export const ValuesBuilder = forwardRef(function ( - { variable }: ValuesBuilderProps, - ref -) { - const { query } = variable.useState(); - - const options = variable.transformCsvStringToOptions(query, false).map(({ label, value }) => ({ - value, - label: value === label ? '' : label, - })); - - const escapeEntities = useCallback((text: VariableValueSingle) => String(text).trim().replaceAll(',', '\\,'), []); - - const formatOption = useCallback( - (option: VariableValueOption) => { - if (!option.label || option.label === option.value) { - return escapeEntities(option.value); - } - - return `${escapeEntities(option.label)} : ${escapeEntities(String(option.value))}`; - }, - [escapeEntities] - ); - - const generateQuery = useCallback( - (options: VariableValueOption[]) => options.map(formatOption).join(', '), - [formatOption] - ); - - const handleOptionsChange = useCallback( - async (options: VariableValueOption[]) => { - variable.setState({ query: generateQuery(options) }); - await lastValueFrom(variable.validateAndUpdate!()); - }, - [variable, generateQuery] - ); - - return ; -}); - -ValuesBuilder.displayName = 'ValuesBuilder'; diff --git a/public/app/features/dashboard-scene/settings/variables/editors/CustomVariableEditor/ValuesPreview.tsx b/public/app/features/dashboard-scene/settings/variables/editors/CustomVariableEditor/ValuesPreview.tsx deleted file mode 100644 index 49a3e8dd55b..00000000000 --- a/public/app/features/dashboard-scene/settings/variables/editors/CustomVariableEditor/ValuesPreview.tsx +++ /dev/null @@ -1,13 +0,0 @@ -import { CustomVariable } from '@grafana/scenes'; - -import { VariableValuesPreview } from '../../components/VariableValuesPreview'; -import { hasVariableOptions } from '../../utils'; - -export function ValuesPreview({ variable }: { variable: CustomVariable }) { - // Workaround to toggle a component refresh when values change so that the preview is updated - variable.useState(); - - const isHasVariableOptions = hasVariableOptions(variable); - - return isHasVariableOptions ? : null; -} diff --git a/public/locales/en-US/grafana.json b/public/locales/en-US/grafana.json index 6ee3b9e50e6..cc056e38526 100644 --- a/public/locales/en-US/grafana.json +++ b/public/locales/en-US/grafana.json @@ -4811,7 +4811,9 @@ }, "variable": { "custom-options": { - "close": "Close", + "apply": "Apply", + "change-value": "Change variable value", + "discard": "Discard", "modal-title": "Custom Variable", "values": "Values separated by comma" },