From e10ef2241d951b1d9a399ed0ee09a336234df4bb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Torkel=20=C3=96degaard?= <torkel@grafana.com>
Date: Fri, 7 Apr 2023 08:31:37 +0200
Subject: [PATCH] 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>
---
.../FieldsByFrameRefIdMatcher.test.tsx | 54 +++++++
.../MatchersUI/FieldsByFrameRefIdMatcher.tsx | 153 ++++++++++++------
.../TransformationFilter.tsx | 19 +--
.../geomap/editor/FrameSelectionEditor.tsx | 79 +--------
4 files changed, 179 insertions(+), 126 deletions(-)
create mode 100644 packages/grafana-ui/src/components/MatchersUI/FieldsByFrameRefIdMatcher.test.tsx
diff --git a/packages/grafana-ui/src/components/MatchersUI/FieldsByFrameRefIdMatcher.test.tsx b/packages/grafana-ui/src/components/MatchersUI/FieldsByFrameRefIdMatcher.test.tsx
new file mode 100644
index 00000000000..c8ab4606ea7
--- /dev/null
+++ b/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');
+ });
+});
diff --git a/packages/grafana-ui/src/components/MatchersUI/FieldsByFrameRefIdMatcher.tsx b/packages/grafana-ui/src/components/MatchersUI/FieldsByFrameRefIdMatcher.tsx
index e452fcde5bb..65617bfacb4 100644
--- a/packages/grafana-ui/src/components/MatchersUI/FieldsByFrameRefIdMatcher.tsx
+++ 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 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;
+};
- const onChange = useCallback(
- (selection: SelectableValue<string>) => {
- if (!selection.value || !referenceIDs.has(selection.value)) {
- return;
- }
- return onChangeFromProps(selection.value);
+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]
);
- const selectedOption = selectOptions.find((v) => v.value === options);
- return <Select value={selectedOption} options={selectOptions} onChange={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}
+ />
+ );
+}
-FieldsByFrameRefIdMatcher.displayName = 'FieldsByFrameRefIdMatcher';
+function getListOfQueryRefIds(data: DataFrame[]): Array<SelectableValue<string>> {
+ const queries = new Map<string, DataFrame[]>();
+
+ for (const frame of data) {
+ const refId = frame.refId ?? '';
+ const frames = queries.get(refId) ?? [];
+
+ 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]);
-};
diff --git a/public/app/features/dashboard/components/TransformationsEditor/TransformationFilter.tsx b/public/app/features/dashboard/components/TransformationsEditor/TransformationFilter.tsx
index ebbccbab512..7de44c7d3a6 100644
--- a/public/app/features/dashboard/components/TransformationsEditor/TransformationFilter.tsx
+++ b/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>
);
};
diff --git a/public/app/plugins/panel/geomap/editor/FrameSelectionEditor.tsx b/public/app/plugins/panel/geomap/editor/FrameSelectionEditor.tsx
index 98bcac91b8b..26bf727d8b9 100644
--- a/public/app/plugins/panel/geomap/editor/FrameSelectionEditor.tsx
+++ b/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" />
);
};