Dashboards: Improve custom variable editor and undo/redo (#114559)

2025-12-05 16:48:34 +01:00
parent 7cd10aa49e
commit 0adb2461e9
8 changed files with 85 additions and 89 deletions
@@ -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(
@@ -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: {
@@ -37,7 +37,7 @@ export const VariableValuesPreview = ({ options }: VariableValuesPreviewProps) =
        {previewOptions.map((o, index) => (
          <InlineFieldRow key={`${o.value}-${index}`} className={styles.optionContainer}>
            <InlineLabel data-testid={selectors.pages.Dashboard.Settings.Variables.Edit.General.previewOfValuesOption}>
-              <div className={styles.label}>{o.label}</div>
+              <div className={styles.label}>{o.label || String(o.value)}</div>
            </InlineLabel>
          </InlineFieldRow>
        ))}
@@ -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<VariableStaticOptionsFormRef | null>(null);
-
-  const handleOnAdd = useCallback(() => formRef.current?.addItem(), []);
+export function ModalEditor(props: ModalEditorProps) {
+  const { formRef, onCloseModal, options, onChangeOptions, onAddNewOption, onSaveOptions } = useModalEditor(props);

  return (
    <Modal
      title={t('dashboard.edit-pane.variable.custom-options.modal-title', 'Custom Variable')}
-      isOpen={isOpen}
-      onDismiss={onClose}
+      isOpen={true}
+      onDismiss={onCloseModal}
+      closeOnBackdropClick={false}
+      closeOnEscape={false}
    >
      <Stack direction="column" gap={2}>
-        <ValuesBuilder variable={variable} ref={formRef} />
-        <ValuesPreview variable={variable} />
+        <VariableStaticOptionsForm options={options} onChange={onChangeOptions} ref={formRef} isInModal />
+        <VariableValuesPreview options={options} />
      </Stack>
-      <Modal.ButtonRow leftItems={<VariableStaticOptionsFormAddButton onAdd={handleOnAdd} />}>
+      <Modal.ButtonRow leftItems={<VariableStaticOptionsFormAddButton onAdd={onAddNewOption} />}>
        <Button
          variant="secondary"
          fill="outline"
-          onClick={onClose}
+          onClick={onCloseModal}
          data-testid={selectors.pages.Dashboard.Settings.Variables.Edit.CustomVariable.closeButton}
        >
-          <Trans i18nKey="dashboard.edit-pane.variable.custom-options.close">Close</Trans>
+          <Trans i18nKey="dashboard.edit-pane.variable.custom-options.discard">Discard</Trans>
+        </Button>
+        <Button
+          variant="primary"
+          onClick={onSaveOptions}
+          data-testid={selectors.pages.Dashboard.Settings.Variables.Edit.CustomVariable.applyButton}
+        >
+          <Trans i18nKey="dashboard.edit-pane.variable.custom-options.apply">Apply</Trans>
        </Button>
      </Modal.ButtonRow>
    </Modal>
  );
 }
+
+function useModalEditor({ variable, onClose }: ModalEditorProps) {
+  const { query } = variable.state;
+  const [options, setOptions] = useState(() => transformQueryToOptions(variable, query));
+  const initialQueryRef = useRef(query);
+  const formRef = useRef<VariableStaticOptionsFormRef | null>(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(', ');
@@ -31,7 +31,7 @@ export function PaneItem({ variable }: PaneItemProps) {
          <Trans i18nKey="dashboard.edit-pane.variable.open-editor">Open variable editor</Trans>
        </Button>
      </Box>
-      <ModalEditor variable={variable} isOpen={isOpen} onClose={() => setIsOpen(false)} />
+      {isOpen && <ModalEditor variable={variable} onClose={() => setIsOpen(false)} />}
    </>
  );
 }
@@ -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<VariableStaticOptionsFormRef, ValuesBuilderProps>(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 <VariableStaticOptionsForm options={options} onChange={handleOptionsChange} ref={ref} isInModal />;
-});
-
-ValuesBuilder.displayName = 'ValuesBuilder';
@@ -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 ? <VariableValuesPreview options={variable.getOptionsForSelect(false)} /> : null;
-}
@@ -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"
        },