Transformations: Improve UX and fix refId issues (#65982)

* Transformations: Improve UX and fix refId issues

* Show query names and frame names in description

* move to main grafan UI component

* Added unit test

* Fix lint error

---------

Co-authored-by: Ryan McKinley <ryantxu@gmail.com>

packages/grafana-ui/src/components/MatchersUI/FieldsByFrameRefIdMatcher.test.tsx

@@ -0,0 +1,54 @@
+import { fireEvent, render, screen } from '@testing-library/react';
+import React from 'react';
+
+import { toDataFrame, FieldType } from '@grafana/data';
+
+import { RefIDPicker, Props } from './FieldsByFrameRefIdMatcher';
+
+beforeEach(() => {
+  jest.clearAllMocks();
+});
+
+const frame1 = toDataFrame({
+  refId: 'A',
+  name: 'Series A',
+  fields: [],
+});
+
+const frame2 = toDataFrame({
+  refId: 'A',
+  fields: [{ name: 'Value', type: FieldType.number, values: [10, 200], config: { displayName: 'Second series' } }],
+});
+
+const frame3 = toDataFrame({
+  refId: 'B',
+  name: 'Series B',
+  fields: [],
+});
+
+const mockOnChange = jest.fn();
+
+const props: Props = {
+  data: [frame1, frame2, frame3],
+  onChange: mockOnChange,
+};
+
+const setup = (testProps?: Partial<Props>) => {
+  const editorProps = { ...props, ...testProps };
+  return render(<RefIDPicker {...editorProps} />);
+};
+
+describe('RefIDPicker', () => {
+  it('Should be able to select frame', async () => {
+    setup();
+
+    const select = await screen.findByRole('combobox');
+    fireEvent.keyDown(select, { keyCode: 40 });
+
+    const selectOptions = screen.getAllByLabelText('Select option');
+
+    expect(selectOptions).toHaveLength(2);
+    expect(selectOptions[0]).toHaveTextContent('Query: AFrames (2): Series A, Second series');
+    expect(selectOptions[1]).toHaveTextContent('Query: BFrames (1): Series B');
+  });
+});

packages/grafana-ui/src/components/MatchersUI/FieldsByFrameRefIdMatcher.tsx

@@ -1,35 +1,117 @@
-import React, { memo, useMemo, useCallback } from 'react';
+import React, { useMemo, useState, useCallback } from 'react';
 
-import { FieldMatcherID, fieldMatchers, SelectableValue, DataFrame } from '@grafana/data';
+import { DataFrame, getFrameDisplayName, FieldMatcherID, fieldMatchers, SelectableValue } from '@grafana/data';
 
 import { Select } from '../Select/Select';
 
-import { MatcherUIProps, FieldMatcherUIRegistryItem } from './types';
+import { FieldMatcherUIRegistryItem, MatcherUIProps } from './types';
 
-/**
- * UI to configure "fields by frame refId"-matcher.
- * @public
- */
-export const FieldsByFrameRefIdMatcher = memo<MatcherUIProps<string>>((props) => {
-  const { data, options, onChange: onChangeFromProps } = props;
-  const referenceIDs = useFrameRefIds(data);
-  const selectOptions = useSelectOptions(referenceIDs);
-
-  const onChange = useCallback(
-    (selection: SelectableValue<string>) => {
-      if (!selection.value || !referenceIDs.has(selection.value)) {
-        return;
-      }
-      return onChangeFromProps(selection.value);
+const recoverRefIdMissing = (
+  newRefIds: SelectableValue[],
+  oldRefIds: SelectableValue[],
+  previousValue: string | undefined
+): SelectableValue | undefined => {
+  if (!previousValue) {
+    return;
+  }
+  // Previously selected value is missing from the new list.
+  // Find the value that is in the new list but isn't in the old list
+  let changedTo = newRefIds.find((refId) => {
+    return !oldRefIds.some((refId2) => {
+      return refId === refId2;
+    });
+  });
+  if (changedTo) {
+    // Found the new value, we assume the old value changed to this one, so we'll use it
+    return changedTo;
+  }
+  return;
+};
+
+export interface Props {
+  value?: string; // refID
+  data: DataFrame[];
+  onChange: (value: string) => void;
+  placeholder?: string;
+}
+
+// Not exported globally... but used in grafana core
+export function RefIDPicker({ value, data, onChange, placeholder }: Props) {
+  const listOfRefIds = useMemo(() => getListOfQueryRefIds(data), [data]);
+
+  const [priorSelectionState, updatePriorSelectionState] = useState<{
+    refIds: SelectableValue[];
+    value: string | undefined;
+  }>({
+    refIds: [],
+    value: undefined,
+  });
+
+  const currentValue = useMemo(() => {
+    return (
+      listOfRefIds.find((refId) => refId.value === value) ??
+      recoverRefIdMissing(listOfRefIds, priorSelectionState.refIds, priorSelectionState.value)
+    );
+  }, [value, listOfRefIds, priorSelectionState]);
+
+  const onFilterChange = useCallback(
+    (v: SelectableValue<string>) => {
+      onChange(v.value!);
     },
-    [referenceIDs, onChangeFromProps]
+    [onChange]
+  );
+
+  if (listOfRefIds !== priorSelectionState.refIds || currentValue?.value !== priorSelectionState.value) {
+    updatePriorSelectionState({
+      refIds: listOfRefIds,
+      value: currentValue?.value,
+    });
+  }
+  return (
+    <Select
+      options={listOfRefIds}
+      onChange={onFilterChange}
+      isClearable={true}
+      placeholder={placeholder ?? 'Select query refId'}
+      value={currentValue}
+    />
   );
+}
+
+function getListOfQueryRefIds(data: DataFrame[]): Array<SelectableValue<string>> {
+  const queries = new Map<string, DataFrame[]>();
 
-  const selectedOption = selectOptions.find((v) => v.value === options);
-  return <Select value={selectedOption} options={selectOptions} onChange={onChange} />;
-});
+  for (const frame of data) {
+    const refId = frame.refId ?? '';
+    const frames = queries.get(refId) ?? [];
 
-FieldsByFrameRefIdMatcher.displayName = 'FieldsByFrameRefIdMatcher';
+    if (frames.length === 0) {
+      queries.set(refId, frames);
+    }
+
+    frames.push(frame);
+  }
+
+  const values: Array<SelectableValue<string>> = [];
+
+  for (const [refId, frames] of queries.entries()) {
+    values.push({
+      value: refId,
+      label: `Query: ${refId ?? '(missing refId)'}`,
+      description: getFramesDescription(frames),
+    });
+  }
+
+  return values;
+}
+
+function getFramesDescription(frames: DataFrame[]): string {
+  return `Frames (${frames.length}):
+    ${frames
+      .slice(0, Math.min(3, frames.length))
+      .map((x) => getFrameDisplayName(x))
+      .join(', ')} ${frames.length > 3 ? '...' : ''}`;
+}
 
 /**
  * Registry item for UI to configure "fields by frame refId"-matcher.
@@ -37,32 +119,11 @@ FieldsByFrameRefIdMatcher.displayName = 'FieldsByFrameRefIdMatcher';
  */
 export const fieldsByFrameRefIdItem: FieldMatcherUIRegistryItem<string> = {
   id: FieldMatcherID.byFrameRefID,
-  component: FieldsByFrameRefIdMatcher,
+  component: (props: MatcherUIProps<string>) => {
+    return <RefIDPicker value={props.options} data={props.data} onChange={props.onChange} />;
+  },
   matcher: fieldMatchers.get(FieldMatcherID.byFrameRefID),
   name: 'Fields returned by query',
   description: 'Set properties for fields from a specific query',
   optionsToLabel: (options) => options,
 };
-
-const useFrameRefIds = (data: DataFrame[]): Set<string> => {
-  return useMemo(() => {
-    const refIds: Set<string> = new Set();
-
-    for (const frame of data) {
-      if (frame.refId) {
-        refIds.add(frame.refId);
-      }
-    }
-
-    return refIds;
-  }, [data]);
-};
-
-const useSelectOptions = (displayNames: Set<string>): Array<SelectableValue<string>> => {
-  return useMemo(() => {
-    return Array.from(displayNames).map((n) => ({
-      value: n,
-      label: n,
-    }));
-  }, [displayNames]);
-};

public/app/features/dashboard/components/TransformationsEditor/TransformationFilter.tsx

@@ -8,7 +8,7 @@ import {
   StandardEditorContext,
   StandardEditorsRegistryItem,
 } from '@grafana/data';
-import { useStyles2 } from '@grafana/ui';
+import { Field, useStyles2 } from '@grafana/ui';
 import { FrameSelectionEditor } from 'app/plugins/panel/geomap/editor/FrameSelectionEditor';
 
 interface TransformationFilterProps {
@@ -27,14 +27,15 @@ export const TransformationFilter = ({ index, data, config, onChange }: Transfor
 
   return (
     <div className={styles.wrapper}>
-      <h5>Apply tranformation to</h5>
-      <FrameSelectionEditor
-        value={config.filter!}
-        context={context}
-        // eslint-disable-next-line
-        item={{} as StandardEditorsRegistryItem}
-        onChange={(filter) => onChange(index, { ...config, filter })}
-      />
+      <Field label="Apply tranformation to">
+        <FrameSelectionEditor
+          value={config.filter!}
+          context={context}
+          // eslint-disable-next-line
+          item={{} as StandardEditorsRegistryItem}
+          onChange={(filter) => onChange(index, { ...config, filter })}
+        />
+      </Field>
     </div>
   );
 };

public/app/plugins/panel/geomap/editor/FrameSelectionEditor.tsx

@@ -1,69 +1,18 @@
-import React, { useCallback, useMemo, useState } from 'react';
+import React, { useCallback } from 'react';
 
-import {
-  FrameMatcherID,
-  getFieldDisplayName,
-  MatcherConfig,
-  SelectableValue,
-  StandardEditorProps,
-} from '@grafana/data';
-import { Select } from '@grafana/ui';
-
-const recoverRefIdMissing = (
-  newRefIds: SelectableValue[],
-  oldRefIds: SelectableValue[],
-  previousValue: string | undefined
-): SelectableValue | undefined => {
-  if (!previousValue) {
-    return;
-  }
-  // Previously selected value is missing from the new list.
-  // Find the value that is in the new list but isn't in the old list
-  let changedTo = newRefIds.find((refId) => {
-    return !oldRefIds.some((refId2) => {
-      return refId === refId2;
-    });
-  });
-  if (changedTo) {
-    // Found the new value, we assume the old value changed to this one, so we'll use it
-    return changedTo;
-  }
-  return;
-};
+import { FrameMatcherID, MatcherConfig, StandardEditorProps } from '@grafana/data';
+import { RefIDPicker } from '@grafana/ui/src/components/MatchersUI/FieldsByFrameRefIdMatcher';
 
 type Props = StandardEditorProps<MatcherConfig>;
 
-export const FrameSelectionEditor = ({ value, context, onChange, item }: Props) => {
-  const listOfRefId = useMemo(() => {
-    return context.data.map((f) => ({
-      value: f.refId,
-      label: `Query: ${f.refId} (size: ${f.length})`,
-      description: f.fields.map((f) => getFieldDisplayName(f)).join(', '),
-    }));
-  }, [context.data]);
-
-  const [priorSelectionState, updatePriorSelectionState] = useState<{
-    refIds: SelectableValue[];
-    value: string | undefined;
-  }>({
-    refIds: [],
-    value: undefined,
-  });
-
-  const currentValue = useMemo(() => {
-    return (
-      listOfRefId.find((refId) => refId.value === value?.options) ??
-      recoverRefIdMissing(listOfRefId, priorSelectionState.refIds, priorSelectionState.value)
-    );
-  }, [value, listOfRefId, priorSelectionState]);
-
+export const FrameSelectionEditor = ({ value, context, onChange }: Props) => {
   const onFilterChange = useCallback(
-    (v: SelectableValue<string>) => {
+    (v: string) => {
       onChange(
-        v?.value
+        v?.length
           ? {
               id: FrameMatcherID.byRefId,
-              options: v.value,
+              options: v,
             }
           : undefined
       );
@@ -71,19 +20,7 @@ export const FrameSelectionEditor = ({ value, context, onChange, item }: Props)
     [onChange]
   );
 
-  if (listOfRefId !== priorSelectionState.refIds || currentValue?.value !== priorSelectionState.value) {
-    updatePriorSelectionState({
-      refIds: listOfRefId,
-      value: currentValue?.value,
-    });
-  }
   return (
-    <Select
-      options={listOfRefId}
-      onChange={onFilterChange}
-      isClearable={true}
-      placeholder="Change filter"
-      value={currentValue}
-    />
+    <RefIDPicker value={value.options} onChange={onFilterChange} data={context.data} placeholder="Change filter" />
   );
 };