Suggestions: Add intermediate state to avoid unexpected saved states (#115709)

* Suggestions: Add intermediate state to avoid unexpected saved states

* cleanup

* update and add e2es to confirm behavior

* fix some of the change dispatch

* codeowners

* fix js error that this exposed

* Apply suggestion from @fastfrwrd
This commit is contained in:
Paul Marbach
2025-12-31 10:56:47 -05:00
committed by GitHub
parent cc6a75d021
commit e3bc61e7d2
12 changed files with 323 additions and 81 deletions
+1 -1
View File
@@ -519,7 +519,7 @@ i18next.config.ts @grafana/grafana-frontend-platform
/e2e-playwright/various-suite/solo-route.spec.ts @grafana/dashboards-squad
/e2e-playwright/various-suite/trace-view-scrolling.spec.ts @grafana/observability-traces-and-profiling
/e2e-playwright/various-suite/verify-i18n.spec.ts @grafana/grafana-frontend-platform
/e2e-playwright/various-suite/visualization-suggestions.spec.ts @grafana/dataviz-squad
/e2e-playwright/various-suite/visualization-suggestions*.spec.ts @grafana/dataviz-squad
/e2e-playwright/various-suite/perf-test.spec.ts @grafana/grafana-frontend-platform
# Packages
@@ -1,4 +1,4 @@
import { BootData } from '@grafana/data';
import { BootData, PanelPluginMeta } from '@grafana/data';
import { test, expect } from '@grafana/plugin-e2e';
test.describe(
@@ -22,7 +22,7 @@ test.describe(
await dashboardPage.addPanel();
// Get panel types from window object
const panelTypes = await page.evaluate(() => {
const panelTypes: PanelPluginMeta[] = await page.evaluate(() => {
// @grafana/plugin-e2e doesn't export the full bootdata config
// eslint-disable-next-line @typescript-eslint/consistent-type-assertions
const win = window as typeof window & { grafanaBootData: BootData };
@@ -0,0 +1,178 @@
import { test, expect } from '@grafana/plugin-e2e';
test.use({
featureToggles: {
newVizSuggestions: true,
externalVizSuggestions: false,
},
viewport: {
width: 800,
height: 1500,
},
});
test.describe(
'Visualization suggestions v2',
{
tag: ['@various', '@suggestions'],
},
() => {
test('Should be shown and clickable', async ({ selectors, gotoPanelEditPage }) => {
// Open dashboard with edit panel
const panelEditPage = await gotoPanelEditPage({
dashboard: {
uid: 'aBXrJ0R7z',
},
id: '9',
});
await expect(
panelEditPage.getByGrafanaSelector(selectors.components.Panels.Panel.content).locator('.uplot'),
'time series to be rendered inside panel'
).toBeVisible();
// Try visualization suggestions
await panelEditPage.getByGrafanaSelector(selectors.components.PanelEditor.toggleVizPicker).click();
await panelEditPage.getByGrafanaSelector(selectors.components.Tab.title('Suggestions')).click();
// Verify we see suggestions
await expect(
panelEditPage.getByGrafanaSelector(selectors.components.VisualizationPreview.card('Line chart')),
'line chart suggestion to be rendered'
).toBeVisible();
// TODO: in this part of the test, we will change the query and the transforms and observe suggestions being updated.
// Select a visualization and verify table header is visible from preview
await panelEditPage.getByGrafanaSelector(selectors.components.VisualizationPreview.card('Table')).click();
await expect(
panelEditPage
.getByGrafanaSelector(selectors.components.Panels.Panel.content)
.getByRole('grid')
.getByRole('row')
.first(),
'table to be rendered inside panel'
).toBeVisible();
await expect(
panelEditPage.getByGrafanaSelector(selectors.components.NavToolbar.editDashboard.discardChangesButton),
'discard changes button disabled since panel has not yet changed'
).toBeDisabled();
// apply the suggestion and verify panel options are visible
await panelEditPage.getByGrafanaSelector(selectors.components.VisualizationPreview.confirm('Table')).click();
await expect(
panelEditPage
.getByGrafanaSelector(selectors.components.Panels.Panel.content)
.getByRole('grid')
.getByRole('row')
.first(),
'table to be rendered inside panel'
).toBeVisible();
await expect(
panelEditPage.getByGrafanaSelector(selectors.components.PanelEditor.OptionsPane.header),
'options pane to be rendered'
).toBeVisible();
await expect(
panelEditPage.getByGrafanaSelector(selectors.components.NavToolbar.editDashboard.discardChangesButton),
'discard changes button enabled now that panel is dirty'
).toBeEnabled();
});
test('should not apply suggestion if you navigate toggle the viz picker back off', async ({
selectors,
gotoPanelEditPage,
}) => {
// Open dashboard with edit panel
const panelEditPage = await gotoPanelEditPage({
dashboard: {
uid: 'aBXrJ0R7z',
},
id: '9',
});
await expect(
panelEditPage.getByGrafanaSelector(selectors.components.Panels.Panel.content).locator('.uplot'),
'time series to be rendered inside panel;'
).toBeVisible();
// Try visualization suggestions
await panelEditPage.getByGrafanaSelector(selectors.components.PanelEditor.toggleVizPicker).click();
await panelEditPage.getByGrafanaSelector(selectors.components.Tab.title('Suggestions')).click();
// Verify we see suggestions
await expect(
panelEditPage.getByGrafanaSelector(selectors.components.VisualizationPreview.card('Line chart')),
'line chart suggestion to be rendered'
).toBeVisible();
// Select a visualization
await panelEditPage.getByGrafanaSelector(selectors.components.VisualizationPreview.card('Table')).click();
await expect(
panelEditPage
.getByGrafanaSelector(selectors.components.Panels.Panel.content)
.getByRole('grid')
.getByRole('row')
.first(),
'table to be rendered inside panel'
).toBeVisible();
await expect(
panelEditPage.getByGrafanaSelector(selectors.components.NavToolbar.editDashboard.discardChangesButton)
).toBeDisabled();
// Verify that toggling the viz picker back cancels the suggestion, restores the line chart, shows panel options
await panelEditPage.getByGrafanaSelector(selectors.components.PanelEditor.toggleVizPicker).click();
await expect(
panelEditPage.getByGrafanaSelector(selectors.components.Panels.Panel.content).locator('.uplot'),
'time series to be rendered inside panel'
).toBeVisible();
await expect(
panelEditPage.getByGrafanaSelector(selectors.components.PanelEditor.OptionsPane.header),
'options pane to be rendered'
).toBeVisible();
await expect(
panelEditPage.getByGrafanaSelector(selectors.components.NavToolbar.editDashboard.discardChangesButton),
'discard changes button is still disabled since no changes were applied'
).toBeDisabled();
});
test('should not apply suggestion if you navigate back to the dashboard', async ({
page,
selectors,
gotoPanelEditPage,
}) => {
// Open dashboard with edit panel
const panelEditPage = await gotoPanelEditPage({
dashboard: {
uid: 'aBXrJ0R7z',
},
id: '9',
});
// Try visualization suggestions
await panelEditPage.getByGrafanaSelector(selectors.components.PanelEditor.toggleVizPicker).click();
await panelEditPage.getByGrafanaSelector(selectors.components.Tab.title('Suggestions')).click();
// Verify we see suggestions
await expect(
panelEditPage.getByGrafanaSelector(selectors.components.VisualizationPreview.card('Line chart')),
'line chart suggestion to be rendered'
).toBeVisible();
// Select a visualization
await panelEditPage.getByGrafanaSelector(selectors.components.VisualizationPreview.card('Table')).click();
await expect(page.getByRole('grid').getByRole('row').first(), 'table row to be rendered').toBeVisible();
await expect(
panelEditPage.getByGrafanaSelector(selectors.components.NavToolbar.editDashboard.discardChangesButton)
).toBeDisabled();
// Verify that navigating back to the dashboard cancels the suggestion and restores the line chart.
await panelEditPage
.getByGrafanaSelector(selectors.components.NavToolbar.editDashboard.backToDashboardButton)
.click();
await expect(
page.locator('[data-viz-panel-key="panel-9"]').locator('.uplot'),
'time series to be rendered inside the panel'
).toBeVisible();
});
}
);
@@ -3,7 +3,7 @@ import { test, expect } from '@grafana/plugin-e2e';
test.describe(
'Visualization suggestions',
{
tag: ['@various'],
tag: ['@various', '@suggestions'],
},
() => {
test('Should be shown and clickable', async ({ page, selectors, gotoPanelEditPage }) => {
@@ -1293,6 +1293,9 @@ export const versionedComponents = {
card: {
[MIN_GRAFANA_VERSION]: (name: string) => `data-testid suggestion-${name}`,
},
confirm: {
'12.4.0': (name: string) => `data-testid suggestion-${name} confirm button`,
},
},
ColorSwatch: {
name: {
@@ -192,7 +192,7 @@ export const preparePlotConfigBuilder: UPlotConfigPrepFn<UPlotConfigOptions> = (
});
const xField = frame.fields[0];
const xAxisHidden = xField.config.custom.axisPlacement === AxisPlacement.Hidden;
const xAxisHidden = xField.config.custom?.axisPlacement === AxisPlacement.Hidden;
builder.addAxis({
show: !xAxisHidden,
@@ -51,6 +51,7 @@ export interface PanelEditorState extends SceneObjectState {
panelRef: SceneObjectRef<VizPanel>;
showLibraryPanelSaveModal?: boolean;
showLibraryPanelUnlinkModal?: boolean;
editPreview?: VizPanel;
tableView?: VizPanel;
pluginLoadErrror?: string;
/**
@@ -150,6 +151,9 @@ export class PanelEditor extends SceneObjectBase<PanelEditorState> {
const changedState = layoutItem.state;
const originalState = this._layoutItemState!;
this.setState({ editPreview: undefined });
this.state.optionsPane?.setState({ editPreviewRef: undefined });
// Temp fix for old edit mode
if (this._layoutItem instanceof DashboardGridItem && !config.featureToggles.dashboardNewLayouts) {
this._layoutItem.handleEditChange();
@@ -256,16 +260,40 @@ export class PanelEditor extends SceneObjectBase<PanelEditorState> {
);
// Setup options pane
const optionsPane = new PanelOptionsPane({
panelRef: this.state.panelRef,
editPreviewRef: this.state.editPreview?.getRef(),
searchQuery: '',
listMode: OptionFilter.All,
isVizPickerOpen: isUnconfigured,
isNewPanel: this.state.isNewPanel,
});
this.setState({
optionsPane: new PanelOptionsPane({
panelRef: this.state.panelRef,
searchQuery: '',
listMode: OptionFilter.All,
isVizPickerOpen: isUnconfigured,
isNewPanel: this.state.isNewPanel,
}),
optionsPane,
isInitializing: false,
});
this._subs.add(
optionsPane.subscribeToState((newState, oldState) => {
if (newState.isVizPickerOpen !== oldState.isVizPickerOpen) {
const panel = this.state.panelRef.resolve();
let editPreview: VizPanel | undefined;
if (newState.isVizPickerOpen) {
// we just "pick" timeseries, viz type will likely be overridden by Suggestions.
const editPreviewBuilder = PanelBuilders.timeseries()
.setTitle(panel.state.title)
.setDescription(panel.state.description);
if (panel.state.$data) {
editPreviewBuilder.setData(new DataProviderSharer({ source: panel.state.$data.getRef() }));
}
editPreview = editPreviewBuilder.build();
}
this.setState({ editPreview });
optionsPane.setState({ editPreviewRef: editPreview?.getRef() });
}
})
);
} else {
// plugin changed after first time initialization
// Just update data pane
@@ -81,7 +81,7 @@ export function PanelEditorRenderer({ model }: SceneComponentProps<PanelEditor>)
function VizAndDataPane({ model }: SceneComponentProps<PanelEditor>) {
const dashboard = getDashboardSceneFor(model);
const { dataPane, showLibraryPanelSaveModal, showLibraryPanelUnlinkModal, tableView } = model.useState();
const { dataPane, showLibraryPanelSaveModal, showLibraryPanelUnlinkModal, tableView, editPreview } = model.useState();
const panel = model.getPanel();
const libraryPanel = getLibraryPanelBehavior(panel);
const { controls } = dashboard.useState();
@@ -113,7 +113,7 @@ function VizAndDataPane({ model }: SceneComponentProps<PanelEditor>) {
)}
<div {...containerProps}>
<div {...primaryProps} className={cx(primaryProps.className, isScrollingLayout && styles.fixedSizeViz)}>
<VizWrapper panel={panel} tableView={tableView} />
<VizWrapper panel={editPreview ?? panel} tableView={tableView} />
</div>
{showLibraryPanelSaveModal && libraryPanel && (
<SaveLibraryVizPanelModal
@@ -27,7 +27,7 @@ describe('PanelOptionsPane', () => {
expect(panel.state.pluginId).toBe('timeseries');
optionsPane.onChangePanelPlugin({ pluginId: 'table' });
optionsPane.onChangePanel({ pluginId: 'table' });
expect(optionsPane['_cachedPluginOptions']['timeseries']?.options).toBe(panel.state.options);
expect(optionsPane['_cachedPluginOptions']['timeseries']?.fieldConfig).toBe(panel.state.fieldConfig);
@@ -52,7 +52,7 @@ describe('PanelOptionsPane', () => {
panel.setState({ $data: undefined });
panel.activate();
optionsPane.onChangePanelPlugin({
optionsPane.onChangePanel({
pluginId: 'table',
options: { showHeader: false },
fieldConfig: {
@@ -114,7 +114,7 @@ describe('PanelOptionsPane', () => {
expect(panel.state.fieldConfig.overrides[1].properties).toHaveLength(1);
expect(panel.state.fieldConfig.defaults.custom).toHaveProperty('axisBorderShow');
optionsPane.onChangePanelPlugin({ pluginId: 'table' });
optionsPane.onChangePanel({ pluginId: 'table' });
expect(mockFn).toHaveBeenCalled();
expect(mockFn.mock.calls[0][2].defaults.color?.mode).toBe('palette-classic');
@@ -146,8 +146,8 @@ describe('PanelOptionsPane', () => {
const mockOnFieldConfigChange = jest.fn();
panel.onFieldConfigChange = mockOnFieldConfigChange;
// Call onChangePanelPlugin with fieldConfig that has overrides
optionsPane.onChangePanelPlugin({
// Call onChangePanel with fieldConfig that has overrides
optionsPane.onChangePanel({
pluginId: 'table',
fieldConfig: {
defaults: { unit: 'percent' },
@@ -178,7 +178,7 @@ describe('PanelOptionsPane', () => {
panel.onFieldConfigChange = mockOnFieldConfigChange;
// Call without fieldConfig
optionsPane.onChangePanelPlugin({
optionsPane.onChangePanel({
pluginId: 'table',
options: { showHeader: false },
});
@@ -41,6 +41,7 @@ export interface PanelOptionsPaneState extends SceneObjectState {
panelRef: SceneObjectRef<VizPanel>;
isNewPanel?: boolean;
hasPickedViz?: boolean;
editPreviewRef?: SceneObjectRef<VizPanel>;
}
interface PluginOptionsCache {
@@ -63,8 +64,7 @@ export class PanelOptionsPane extends SceneObjectBase<PanelOptionsPaneState> {
});
};
onChangePanelPlugin = (options: VizTypeChangeDetails) => {
const panel = this.state.panelRef.resolve();
onChangePanel = (options: VizTypeChangeDetails, panel = this.state.panelRef.resolve()) => {
const { options: prevOptions, fieldConfig: prevFieldConfig, pluginId: prevPluginId } = panel.state;
const pluginId = options.pluginId;
@@ -137,8 +137,10 @@ export class PanelOptionsPane extends SceneObjectBase<PanelOptionsPaneState> {
}
function PanelOptionsPaneComponent({ model }: SceneComponentProps<PanelOptionsPane>) {
const { isVizPickerOpen, searchQuery, listMode, panelRef, isNewPanel, hasPickedViz } = model.useState();
const { isVizPickerOpen, searchQuery, listMode, panelRef, isNewPanel, hasPickedViz, editPreviewRef } =
model.useState();
const panel = panelRef.resolve();
const editPreview = editPreviewRef?.resolve() ?? panel; // if something goes wrong, at least update the panel.
const { pluginId } = panel.useState();
const { data } = sceneGraph.getData(panel).useState();
const styles = useStyles2(getStyles);
@@ -229,7 +231,8 @@ function PanelOptionsPaneComponent({ model }: SceneComponentProps<PanelOptionsPa
{isVizPickerOpen && (
<PanelVizTypePicker
panel={panel}
onChange={model.onChangePanelPlugin}
editPreview={editPreview}
onChange={model.onChangePanel}
onClose={model.onToggleVizPicker}
data={data}
showBackButton={config.featureToggles.newVizSuggestions ? hasPickedViz || !isNewPanel : true}
@@ -23,7 +23,8 @@ export interface Props {
data?: PanelData;
showBackButton?: boolean;
panel: VizPanel;
onChange: (options: VizTypeChangeDetails) => void;
editPreview: VizPanel;
onChange: (options: VizTypeChangeDetails, panel?: VizPanel) => void;
onClose: () => void;
}
@@ -41,7 +42,7 @@ const getTabs = (): Array<{ label: string; value: VisualizationSelectPaneTab }>
: [allVisualizationsTab, suggestionsTab];
};
export function PanelVizTypePicker({ panel, data, onChange, onClose, showBackButton }: Props) {
export function PanelVizTypePicker({ panel, editPreview, data, onChange, onClose, showBackButton }: Props) {
const styles = useStyles2(getStyles);
const panelModel = useMemo(() => new PanelModelCompatibilityWrapper(panel), [panel]);
const filterId = useId();
@@ -97,49 +98,55 @@ export function PanelVizTypePicker({ panel, data, onChange, onClose, showBackBut
</TabsBar>
<ScrollContainer>
<TabContent className={styles.tabContent}>
{listMode === VisualizationSelectPaneTab.Suggestions && (
<VisualizationSuggestions onChange={onChange} panel={panelModel} data={data} />
)}
{listMode === VisualizationSelectPaneTab.Visualizations && (
<Stack gap={1} direction="column">
<Field
tabIndex={0}
className={styles.searchField}
noMargin
htmlFor={filterId}
aria-label={t('dashboard-scene.panel-viz-type-picker.placeholder-search-for', 'Search for...')}
>
<Stack direction="row" gap={1}>
{showBackButton && (
<Button
aria-label={t('dashboard-scene.panel-viz-type-picker.title-close', 'Close')}
fill="text"
variant="secondary"
icon="arrow-left"
data-testid={selectors.components.PanelEditor.toggleVizPicker}
onClick={onClose}
>
<Trans i18nKey="dashboard-scene.panel-viz-type-picker.button.close">Back</Trans>
</Button>
)}
<FilterInput
id={filterId}
className={styles.filter}
value={searchQuery}
onChange={setSearchQuery}
placeholder={t('dashboard-scene.panel-viz-type-picker.placeholder-search-for', 'Search for...')}
/>
</Stack>
</Field>
<Stack gap={1} direction="column">
<Field
tabIndex={0}
className={styles.searchField}
noMargin
htmlFor={filterId}
aria-label={t('dashboard-scene.panel-viz-type-picker.placeholder-search-for', 'Search for...')}
>
<Stack direction="row" gap={1}>
{showBackButton && (
<Button
aria-label={t('dashboard-scene.panel-viz-type-picker.title-close', 'Close')}
fill="text"
variant="secondary"
icon="arrow-left"
data-testid={selectors.components.PanelEditor.toggleVizPicker}
onClick={onClose}
>
<Trans i18nKey="dashboard-scene.panel-viz-type-picker.button.close">Back</Trans>
</Button>
)}
<FilterInput
id={filterId}
className={styles.filter}
value={searchQuery}
onChange={setSearchQuery}
placeholder={t('dashboard-scene.panel-viz-type-picker.placeholder-search-for', 'Search for...')}
/>
</Stack>
</Field>
{listMode === VisualizationSelectPaneTab.Suggestions && (
<VisualizationSuggestions
onChange={onChange}
panel={panelModel}
editPreview={editPreview}
data={data}
searchQuery={searchQuery}
/>
)}
{listMode === VisualizationSelectPaneTab.Visualizations && (
<VizTypePicker
pluginId={panel.state.pluginId}
searchQuery={searchQuery}
trackSearch={trackSearch}
onChange={onChange}
/>
</Stack>
)}
)}
</Stack>
</TabContent>
</ScrollContainer>
</div>
@@ -155,7 +162,7 @@ const getStyles = (theme: GrafanaTheme2) => ({
gap: theme.spacing(2),
}),
searchField: css({
marginTop: theme.spacing(0.5), // input glow with the boundary without this
margin: theme.spacing(0.5, 0, 1, 0), // input glow with the boundary without this
}),
tabs: css({
width: '100%',
@@ -9,8 +9,10 @@ import {
PanelPluginMeta,
PanelPluginVisualizationSuggestion,
} from '@grafana/data';
import { selectors } from '@grafana/e2e-selectors';
import { Trans, t } from '@grafana/i18n';
import { config } from '@grafana/runtime';
import { VizPanel } from '@grafana/scenes';
import { Alert, Button, Icon, Spinner, Text, useStyles2 } from '@grafana/ui';
import { UNCONFIGURED_PANEL_PLUGIN_ID } from 'app/features/dashboard-scene/scene/UnconfiguredPanel';
@@ -23,25 +25,47 @@ import { VisualizationSuggestionCard } from './VisualizationSuggestionCard';
import { VizTypeChangeDetails } from './types';
export interface Props {
onChange: (options: VizTypeChangeDetails) => void;
onChange: (options: VizTypeChangeDetails, panel?: VizPanel) => void;
editPreview?: VizPanel;
data?: PanelData;
panel?: PanelModel;
searchQuery?: string;
}
const useSuggestions = (data: PanelData | undefined) => {
const useSuggestions = (data: PanelData | undefined, searchQuery: string | undefined) => {
const [hasFetched, setHasFetched] = useState(false);
const { value, loading, error, retry } = useAsyncRetry(async () => {
await new Promise((resolve) => setTimeout(resolve, hasFetched ? 75 : 0));
setHasFetched(true);
return await getAllSuggestions(data);
}, [hasFetched, data]);
return { value, loading, error, retry };
const filteredValue = useMemo(() => {
if (!value || !searchQuery) {
return value;
}
const lowerCaseQuery = searchQuery.toLowerCase();
const filteredSuggestions = value.suggestions.filter(
(suggestion) =>
suggestion.name.toLowerCase().includes(lowerCaseQuery) ||
suggestion.pluginId.toLowerCase().includes(lowerCaseQuery) ||
suggestion.description?.toLowerCase().includes(lowerCaseQuery)
);
return {
...value,
suggestions: filteredSuggestions,
};
}, [value, searchQuery]);
return { value: filteredValue, loading, error, retry };
};
export function VisualizationSuggestions({ onChange, data, panel }: Props) {
export function VisualizationSuggestions({ onChange, editPreview, data, panel, searchQuery }: Props) {
const styles = useStyles2(getStyles);
const { value: result, loading, error, retry } = useSuggestions(data);
const { value: result, loading, error, retry } = useSuggestions(data, searchQuery);
const suggestions = result?.suggestions;
const hasLoadingErrors = result?.hasErrors ?? false;
@@ -73,18 +97,21 @@ export function VisualizationSuggestions({ onChange, data, panel }: Props) {
const applySuggestion = useCallback(
(suggestion: PanelPluginVisualizationSuggestion, isPreview?: boolean) => {
onChange({
pluginId: suggestion.pluginId,
options: suggestion.options,
fieldConfig: suggestion.fieldConfig,
withModKey: isPreview,
});
onChange(
{
pluginId: suggestion.pluginId,
options: suggestion.options,
fieldConfig: suggestion.fieldConfig,
withModKey: isPreview,
},
isPreview ? editPreview : undefined
);
if (isPreview) {
setSuggestionHash(suggestion.hash);
}
},
[onChange]
[onChange, editPreview]
);
useEffect(() => {
@@ -185,17 +212,13 @@ export function VisualizationSuggestions({ onChange, data, panel }: Props) {
variant="primary"
size={'md'}
className={styles.applySuggestionButton}
data-testid={selectors.components.VisualizationPreview.confirm(suggestion.name)}
aria-label={t(
'panel.visualization-suggestions.apply-suggestion-aria-label',
'Apply {{suggestionName}} visualization',
{ suggestionName: suggestion.name }
)}
onClick={() =>
onChange({
pluginId: suggestion.pluginId,
withModKey: false,
})
}
onClick={() => applySuggestion(suggestion, false)}
>
{t('panel.visualization-suggestions.use-this-suggestion', 'Use this suggestion')}
</Button>