From b033fe8d738c7d17020d1b5688aa149196ad2ea2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Jamr=C3=B3z?= Date: Wed, 22 Mar 2023 20:20:45 +0100 Subject: [PATCH] Glue: Split correlations editor into 3 steps (#64818) * Create simple Wizard for Correlations editor * Allow using custom navigation in the wizard * Update types * Add more info * Add comments * Update comments * Remove main info box to avoid having too many info boxes * Fix CorrelationsPage.test.tsx * Add Wizard test * Simplify Correlations wizard * Make expected typing error more explicit * Don't use meaningless defaults --- .../correlations/CorrelationsPage.test.tsx | 48 +++++--- .../correlations/CorrelationsPage.tsx | 1 - .../correlations/Forms/AddCorrelationForm.tsx | 106 ++++-------------- .../ConfigureCorrelationBasicInfoForm.tsx | 57 ++++++++++ .../Forms/ConfigureCorrelationSourceForm.tsx | 79 +++++++++++++ .../Forms/ConfigureCorrelationTargetForm.tsx | 54 +++++++++ .../Forms/CorrelationDetailsFormPart.tsx | 87 -------------- .../Forms/CorrelationFormNavigation.tsx | 36 ++++++ .../Forms/EditCorrelationForm.tsx | 35 +++--- .../correlations/Forms/QueryEditorField.tsx | 19 +++- .../Forms/correlationsFormContext.tsx | 28 +++++ .../app/features/correlations/Forms/utils.ts | 11 ++ .../components/Wizard/Wizard.test.tsx | 37 ++++++ .../correlations/components/Wizard/Wizard.tsx | 18 +++ .../components/Wizard/WizardContent.tsx | 31 +++++ .../correlations/components/Wizard/index.ts | 2 + .../correlations/components/Wizard/types.ts | 30 +++++ .../components/Wizard/wizardContext.tsx | 54 +++++++++ 18 files changed, 518 insertions(+), 215 deletions(-) create mode 100644 public/app/features/correlations/Forms/ConfigureCorrelationBasicInfoForm.tsx create mode 100644 public/app/features/correlations/Forms/ConfigureCorrelationSourceForm.tsx create mode 100644 public/app/features/correlations/Forms/ConfigureCorrelationTargetForm.tsx delete mode 100644 public/app/features/correlations/Forms/CorrelationDetailsFormPart.tsx create mode 100644 public/app/features/correlations/Forms/CorrelationFormNavigation.tsx create mode 100644 public/app/features/correlations/Forms/correlationsFormContext.tsx create mode 100644 public/app/features/correlations/Forms/utils.ts create mode 100644 public/app/features/correlations/components/Wizard/Wizard.test.tsx create mode 100644 public/app/features/correlations/components/Wizard/Wizard.tsx create mode 100644 public/app/features/correlations/components/Wizard/WizardContent.tsx create mode 100644 public/app/features/correlations/components/Wizard/index.ts create mode 100644 public/app/features/correlations/components/Wizard/types.ts create mode 100644 public/app/features/correlations/components/Wizard/wizardContext.tsx diff --git a/public/app/features/correlations/CorrelationsPage.test.tsx b/public/app/features/correlations/CorrelationsPage.test.tsx index 4d8f484441d..f143a2bed43 100644 --- a/public/app/features/correlations/CorrelationsPage.test.tsx +++ b/public/app/features/correlations/CorrelationsPage.test.tsx @@ -271,12 +271,12 @@ describe('CorrelationsPage', () => { mocks.reportInteraction.mockClear(); }); - it('shows CTA', async () => { + it('shows the first page of the wizard', async () => { const CTAButton = await screen.findByRole('button', { name: /add correlation/i }); expect(CTAButton).toBeInTheDocument(); // insert form should not be present - expect(screen.queryByRole('button', { name: /add$/i })).not.toBeInTheDocument(); + expect(screen.queryByRole('button', { name: /next$/i })).not.toBeInTheDocument(); // "add new" button is the button on the top of the page, not visible when the CTA is rendered expect(screen.queryByRole('button', { name: /add new$/i })).not.toBeInTheDocument(); @@ -286,8 +286,8 @@ describe('CorrelationsPage', () => { await userEvent.click(CTAButton); - // form's submit button - expect(await screen.findByRole('button', { name: /add$/i })).toBeInTheDocument(); + // form's next button + expect(await screen.findByRole('button', { name: /next$/i })).toBeInTheDocument(); }); it('correctly adds first correlation', async () => { @@ -299,22 +299,27 @@ describe('CorrelationsPage', () => { await userEvent.click(CTAButton); + // step 1: label and description await userEvent.clear(screen.getByRole('textbox', { name: /label/i })); await userEvent.type(screen.getByRole('textbox', { name: /label/i }), 'A Label'); await userEvent.clear(screen.getByRole('textbox', { name: /description/i })); await userEvent.type(screen.getByRole('textbox', { name: /description/i }), 'A Description'); + await userEvent.click(await screen.findByRole('button', { name: /next$/i })); - // set source datasource picker value - fireEvent.keyDown(screen.getByLabelText(/^source$/i), { keyCode: 40 }); - await userEvent.click(screen.getByText('loki')); - + // step 2: // set target datasource picker value - fireEvent.keyDown(screen.getByLabelText(/^target$/i), { keyCode: 40 }); + fireEvent.keyDown(screen.getByLabelText(/^target/i), { keyCode: 40 }); await userEvent.click(screen.getByText('prometheus')); + await userEvent.click(await screen.findByRole('button', { name: /next$/i })); - await userEvent.clear(screen.getByRole('textbox', { name: /target field/i })); - await userEvent.type(screen.getByRole('textbox', { name: /target field/i }), 'Line'); + // step 3: + // set source datasource picker value + fireEvent.keyDown(screen.getByLabelText(/^source/i), { keyCode: 40 }); + await userEvent.click(screen.getByText('loki')); + await userEvent.click(await screen.findByRole('button', { name: /add$/i })); + await userEvent.clear(screen.getByRole('textbox', { name: /results field/i })); + await userEvent.type(screen.getByRole('textbox', { name: /results field/i }), 'Line'); await userEvent.click(await screen.findByRole('button', { name: /add$/i })); expect(mocks.reportInteraction).toHaveBeenLastCalledWith('grafana_correlations_added'); @@ -432,21 +437,26 @@ describe('CorrelationsPage', () => { expect(addNewButton).toBeInTheDocument(); await userEvent.click(addNewButton); + // step 1: await userEvent.clear(screen.getByRole('textbox', { name: /label/i })); await userEvent.type(screen.getByRole('textbox', { name: /label/i }), 'A Label'); await userEvent.clear(screen.getByRole('textbox', { name: /description/i })); await userEvent.type(screen.getByRole('textbox', { name: /description/i }), 'A Description'); + await userEvent.click(await screen.findByRole('button', { name: /next$/i })); + // step 2: + // set target datasource picker value + fireEvent.keyDown(screen.getByLabelText(/^target/i), { keyCode: 40 }); + await userEvent.click(screen.getByText('elastic')); + await userEvent.click(await screen.findByRole('button', { name: /next$/i })); + + // step 3: // set source datasource picker value - fireEvent.keyDown(screen.getByLabelText(/^source$/i), { keyCode: 40 }); + fireEvent.keyDown(screen.getByLabelText(/^source/i), { keyCode: 40 }); await userEvent.click(within(screen.getByLabelText('Select options menu')).getByText('prometheus')); - // set target datasource picker value - fireEvent.keyDown(screen.getByLabelText(/^target$/i), { keyCode: 40 }); - await userEvent.click(screen.getByText('elastic')); - - await userEvent.clear(screen.getByRole('textbox', { name: /target field/i })); - await userEvent.type(screen.getByRole('textbox', { name: /target field/i }), 'Line'); + await userEvent.clear(screen.getByRole('textbox', { name: /results field/i })); + await userEvent.type(screen.getByRole('textbox', { name: /results field/i }), 'Line'); await userEvent.click(screen.getByRole('button', { name: /add$/i })); @@ -506,6 +516,8 @@ describe('CorrelationsPage', () => { expect(screen.queryByRole('cell', { name: /edited label$/i })).not.toBeInTheDocument(); + await userEvent.click(screen.getByRole('button', { name: /next$/i })); + await userEvent.click(screen.getByRole('button', { name: /next$/i })); await userEvent.click(screen.getByRole('button', { name: /save$/i })); expect(await screen.findByRole('cell', { name: /edited label$/i })).toBeInTheDocument(); diff --git a/public/app/features/correlations/CorrelationsPage.tsx b/public/app/features/correlations/CorrelationsPage.tsx index 303d0827862..9f1904f07ae 100644 --- a/public/app/features/correlations/CorrelationsPage.tsx +++ b/public/app/features/correlations/CorrelationsPage.tsx @@ -145,7 +145,6 @@ export default function CorrelationsPage() {
-

Correlations

Define how data living in different data sources relates to each other.

{canWriteCorrelations && data?.length !== 0 && data !== undefined && !isAdding && ( diff --git a/public/app/features/correlations/Forms/AddCorrelationForm.tsx b/public/app/features/correlations/Forms/AddCorrelationForm.tsx index 3be1bae60ed..21664725530 100644 --- a/public/app/features/correlations/Forms/AddCorrelationForm.tsx +++ b/public/app/features/correlations/Forms/AddCorrelationForm.tsx @@ -1,16 +1,18 @@ import { css } from '@emotion/css'; import React, { useEffect } from 'react'; -import { Controller, FormProvider, useForm } from 'react-hook-form'; -import { DataSourceInstanceSettings, GrafanaTheme2 } from '@grafana/data'; -import { DataSourcePicker } from '@grafana/runtime'; -import { Button, Field, HorizontalGroup, PanelContainer, useStyles2 } from '@grafana/ui'; +import { GrafanaTheme2 } from '@grafana/data'; +import { PanelContainer, useStyles2 } from '@grafana/ui'; import { CloseButton } from 'app/core/components/CloseButton/CloseButton'; -import { getDatasourceSrv } from 'app/features/plugins/datasource_srv'; +import { Wizard } from '../components/Wizard'; import { useCorrelations } from '../useCorrelations'; -import { CorrelationDetailsFormPart } from './CorrelationDetailsFormPart'; +import { ConfigureCorrelationBasicInfoForm } from './ConfigureCorrelationBasicInfoForm'; +import { ConfigureCorrelationSourceForm } from './ConfigureCorrelationSourceForm'; +import { ConfigureCorrelationTargetForm } from './ConfigureCorrelationTargetForm'; +import { CorrelationFormNavigation } from './CorrelationFormNavigation'; +import { CorrelationsFormContextProvider } from './correlationsFormContext'; import { FormDTO } from './types'; const getStyles = (theme: GrafanaTheme2) => ({ @@ -19,20 +21,8 @@ const getStyles = (theme: GrafanaTheme2) => ({ padding: ${theme.spacing(1)}; margin-bottom: ${theme.spacing(2)}; `, - linksToContainer: css` - flex-grow: 1; - /* This is the width of the textarea minus the sum of the label&description fields, - * so that this element takes exactly the remaining space and the inputs will be - * nicely aligned with the textarea - **/ - max-width: ${theme.spacing(80 - 64)}; - margin-top: ${theme.spacing(3)}; - text-align: right; - padding-right: ${theme.spacing(1)}; - `, - // we can't use HorizontalGroup because it wraps elements in divs and sets margins on them - horizontalGroup: css` - display: flex; + infoBox: css` + margin-top: 20px; // give space for close button `, }); @@ -41,8 +31,6 @@ interface Props { onCreated: () => void; } -const withDsUID = (fn: Function) => (ds: DataSourceInstanceSettings) => fn(ds.uid); - export const AddCorrelationForm = ({ onClose, onCreated }: Props) => { const styles = useStyles2(getStyles); @@ -56,75 +44,19 @@ export const AddCorrelationForm = ({ onClose, onCreated }: Props) => { } }, [error, loading, value, onCreated]); - const methods = useForm({ defaultValues: { config: { type: 'query', target: {} } } }); + const defaultValues: Partial = { config: { type: 'query', target: {}, field: '' } }; return ( - -
-
- - !getDatasourceSrv().getInstanceSettings(uid)?.readOnly || - "Source can't be a read-only data source.", - }, - }} - render={({ field: { onChange, value } }) => ( - - - - )} - /> -
Links to
- ( - - - - )} - /> -
- - - - - - - -
+ + + defaultValues={defaultValues} + pages={[ConfigureCorrelationBasicInfoForm, ConfigureCorrelationTargetForm, ConfigureCorrelationSourceForm]} + navigation={CorrelationFormNavigation} + onSubmit={execute} + /> + ); }; diff --git a/public/app/features/correlations/Forms/ConfigureCorrelationBasicInfoForm.tsx b/public/app/features/correlations/Forms/ConfigureCorrelationBasicInfoForm.tsx new file mode 100644 index 00000000000..00a120c5817 --- /dev/null +++ b/public/app/features/correlations/Forms/ConfigureCorrelationBasicInfoForm.tsx @@ -0,0 +1,57 @@ +import { css, cx } from '@emotion/css'; +import React from 'react'; +import { useFormContext } from 'react-hook-form'; + +import { GrafanaTheme2 } from '@grafana/data'; +import { Field, FieldSet, Input, TextArea, useStyles2 } from '@grafana/ui'; + +import { useCorrelationsFormContext } from './correlationsFormContext'; +import { FormDTO } from './types'; +import { getInputId } from './utils'; + +const getStyles = (theme: GrafanaTheme2) => ({ + label: css` + max-width: ${theme.spacing(80)}; + `, + description: css` + max-width: ${theme.spacing(80)}; + `, +}); + +export const ConfigureCorrelationBasicInfoForm = () => { + const { register, formState } = useFormContext(); + const styles = useStyles2(getStyles); + const { correlation, readOnly } = useCorrelationsFormContext(); + + return ( + <> +
+

The name of the correlation is used as the label of the link.

+ + + + + + +