From 5f339433976704aea2991053255a63aceb3f13f1 Mon Sep 17 00:00:00 2001 From: Ashley Harrison Date: Thu, 6 Jun 2024 17:33:31 +0100 Subject: [PATCH] Select: Revert "preserving custom value" changes (#88856) * revert Select changes as we can handle it outside of the base select component * update scenes * update scenes properly * revert changes to azure-monitor e2e tests --- e2e/cloud-plugins-suite/azure-monitor.spec.ts | 8 ++--- package.json | 2 +- .../components/PromQueryBuilder.test.tsx | 12 +++++-- .../src/components/Select/SelectBase.test.tsx | 33 ------------------- .../src/components/Select/SelectBase.tsx | 31 ----------------- ...CreatableSelectPersistedBehaviour.test.tsx | 3 +- .../jaeger/components/SearchForm.test.tsx | 1 - yarn.lock | 10 +++--- 8 files changed, 20 insertions(+), 80 deletions(-) diff --git a/e2e/cloud-plugins-suite/azure-monitor.spec.ts b/e2e/cloud-plugins-suite/azure-monitor.spec.ts index 1c7e03d3fa5..64ab9069c91 100644 --- a/e2e/cloud-plugins-suite/azure-monitor.spec.ts +++ b/e2e/cloud-plugins-suite/azure-monitor.spec.ts @@ -194,7 +194,7 @@ describe('Azure monitor datasource', () => { dataSourceName, visitDashboardAtStart: false, queriesForm: () => { - e2eSelectors.queryEditor.header.select().find('input').type('{selectall}Logs{enter}'); + e2eSelectors.queryEditor.header.select().find('input').type('Logs{enter}'); e2eSelectors.queryEditor.resourcePicker.select.button().click(); e2eSelectors.queryEditor.resourcePicker.search .input() @@ -214,7 +214,7 @@ describe('Azure monitor datasource', () => { dataSourceName, visitDashboardAtStart: false, queriesForm: () => { - e2eSelectors.queryEditor.header.select().find('input').type('{selectall}Azure Resource Graph{enter}'); + e2eSelectors.queryEditor.header.select().find('input').type('Azure Resource Graph{enter}'); cy.wait(1000); // Need to wait for code editor to completely load e2eSelectors.queryEditor.argsQueryEditor.subscriptions .input() @@ -233,7 +233,7 @@ describe('Azure monitor datasource', () => { dataSourceName, visitDashboardAtStart: false, queriesForm: () => { - e2eSelectors.queryEditor.header.select().find('input').type('{selectall}Traces{enter}'); + e2eSelectors.queryEditor.header.select().find('input').type('Traces{enter}'); e2eSelectors.queryEditor.resourcePicker.select.button().click(); e2eSelectors.queryEditor.resourcePicker.search .input() @@ -306,7 +306,7 @@ describe('Azure monitor datasource', () => { e2eSelectors.queryEditor.resourcePicker.advanced.region.input().find('input').type('$region'); e2eSelectors.queryEditor.resourcePicker.advanced.resource.input().find('input').type('$resource'); e2eSelectors.queryEditor.resourcePicker.apply.button().click(); - e2eSelectors.queryEditor.metricsQueryEditor.metricName.input().find('input').type('Blob Capacity{enter}'); + e2eSelectors.queryEditor.metricsQueryEditor.metricName.input().find('input').type('Transactions{enter}'); }, timeout: 10000, }); diff --git a/package.json b/package.json index 411d288d71b..f879e9e7c77 100644 --- a/package.json +++ b/package.json @@ -258,7 +258,7 @@ "@grafana/prometheus": "workspace:*", "@grafana/runtime": "workspace:*", "@grafana/saga-icons": "workspace:*", - "@grafana/scenes": "^4.24.4", + "@grafana/scenes": "4.27.0", "@grafana/schema": "workspace:*", "@grafana/sql": "workspace:*", "@grafana/ui": "workspace:*", diff --git a/packages/grafana-prometheus/src/querybuilder/components/PromQueryBuilder.test.tsx b/packages/grafana-prometheus/src/querybuilder/components/PromQueryBuilder.test.tsx index 5e8fed16607..d5da0babb73 100644 --- a/packages/grafana-prometheus/src/querybuilder/components/PromQueryBuilder.test.tsx +++ b/packages/grafana-prometheus/src/querybuilder/components/PromQueryBuilder.test.tsx @@ -174,20 +174,24 @@ describe('PromQueryBuilder', () => { }); it('shows hints for histogram metrics', async () => { - setup({ + const { container } = setup({ metric: 'histogram_metric_bucket', labels: [], operations: [], }); + await openMetricSelect(container); + await userEvent.click(screen.getByText('histogram_metric_bucket')); await waitFor(() => expect(screen.getByText('hint: add histogram_quantile')).toBeInTheDocument()); }); it('shows hints for counter metrics', async () => { - setup({ + const { container } = setup({ metric: 'histogram_metric_sum', labels: [], operations: [], }); + await openMetricSelect(container); + await userEvent.click(screen.getByText('histogram_metric_sum')); await waitFor(() => expect(screen.getByText('hint: add rate')).toBeInTheDocument()); }); @@ -200,7 +204,7 @@ describe('PromQueryBuilder', () => { for (let i = 0; i < 25; i++) { data.series.push(new MutableDataFrame()); } - setup( + const { container } = setup( { metric: 'histogram_metric_sum', labels: [], @@ -208,6 +212,8 @@ describe('PromQueryBuilder', () => { }, data ); + await openMetricSelect(container); + await userEvent.click(screen.getByText('histogram_metric_sum')); await waitFor(() => expect(screen.getAllByText(/hint:/)).toHaveLength(2)); }); diff --git a/packages/grafana-ui/src/components/Select/SelectBase.test.tsx b/packages/grafana-ui/src/components/Select/SelectBase.test.tsx index 8e7885d34a4..d36fc8de979 100644 --- a/packages/grafana-ui/src/components/Select/SelectBase.test.tsx +++ b/packages/grafana-ui/src/components/Select/SelectBase.test.tsx @@ -65,39 +65,6 @@ describe('SelectBase', () => { expect(screen.queryByText('Test label')).not.toBeInTheDocument(); }); - describe('with custom values', () => { - it('allows editing a custom SelectableValue', async () => { - render( - - ); - - await userEvent.click(screen.getByRole('combobox')); - await userEvent.type(screen.getByRole('combobox'), '{backspace}{backspace}{enter}'); - expect(onChangeHandler).toHaveBeenCalled(); - expect(onChangeHandler.mock.calls[0][0]).toEqual( - expect.objectContaining({ label: 'my custom val', value: 'my custom val' }) - ); - }); - - it('allows editing a custom basic value', async () => { - render(); - - await userEvent.click(screen.getByRole('combobox')); - await userEvent.type(screen.getByRole('combobox'), '{backspace}{backspace}{enter}'); - expect(onChangeHandler).toHaveBeenCalled(); - expect(onChangeHandler.mock.calls[0][0]).toEqual( - expect.objectContaining({ label: 'my custom val', value: 'my custom val' }) - ); - }); - }); - describe('when openMenuOnFocus prop', () => { describe('is provided', () => { it('opens on focus', () => { diff --git a/packages/grafana-ui/src/components/Select/SelectBase.tsx b/packages/grafana-ui/src/components/Select/SelectBase.tsx index 1f5c155045c..34eccfc4aa5 100644 --- a/packages/grafana-ui/src/components/Select/SelectBase.tsx +++ b/packages/grafana-ui/src/components/Select/SelectBase.tsx @@ -205,8 +205,6 @@ export function SelectBase({ } } - const [internalInputValue, setInternalInputValue] = useState(''); - const commonSelectProps = { 'aria-label': ariaLabel, 'data-testid': dataTestid, @@ -282,35 +280,6 @@ export function SelectBase({ creatableProps.onCreateOption = onCreateOption; creatableProps.createOptionPosition = createOptionPosition; creatableProps.isValidNewOption = isValidNewOption; - - // code needed to allow editing a custom value once entered - // we only want to do this for single selects, not multi - if (!isMulti) { - creatableProps.inputValue = internalInputValue; - creatableProps.onMenuOpen = () => { - // make sure we call the base onMenuOpen if it exists - commonSelectProps.onMenuOpen?.(); - - // restore the input state to the selected value - setInternalInputValue(selectedValue?.[0]?.label ?? ''); - }; - creatableProps.onInputChange = (val, actionMeta) => { - // make sure we call the base onInputChange - commonSelectProps.onInputChange(val, actionMeta); - - // update the input value state on change since we're explicitly controlling it - if (actionMeta.action === 'input-change') { - setInternalInputValue(val); - } - }; - creatableProps.onMenuClose = () => { - // make sure we call the base onMenuClose if it exists - commonSelectProps.onMenuClose?.(); - - // clear the input state on menu close, else react-select won't show the SingleValue component correctly - setInternalInputValue(''); - }; - } } // Instead of having AsyncSelect, as a separate component we render ReactAsyncSelect diff --git a/public/app/plugins/datasource/elasticsearch/components/hooks/useCreatableSelectPersistedBehaviour.test.tsx b/public/app/plugins/datasource/elasticsearch/components/hooks/useCreatableSelectPersistedBehaviour.test.tsx index d9625d38048..bd4d1acd91c 100644 --- a/public/app/plugins/datasource/elasticsearch/components/hooks/useCreatableSelectPersistedBehaviour.test.tsx +++ b/public/app/plugins/datasource/elasticsearch/components/hooks/useCreatableSelectPersistedBehaviour.test.tsx @@ -112,7 +112,6 @@ describe('useCreatableSelectPersistedBehaviour', () => { await userEvent.click(input); // we expect 2 elemnts having "Option 2": the input itself and the option. - expect(screen.getByText('Option 2')).toBeInTheDocument(); - expect(screen.getByRole('combobox')).toHaveValue('Option 2'); + expect(screen.getAllByText('Option 2')).toHaveLength(2); }); }); diff --git a/public/app/plugins/datasource/jaeger/components/SearchForm.test.tsx b/public/app/plugins/datasource/jaeger/components/SearchForm.test.tsx index 9ff8bfa1ab9..bc1bae92086 100644 --- a/public/app/plugins/datasource/jaeger/components/SearchForm.test.tsx +++ b/public/app/plugins/datasource/jaeger/components/SearchForm.test.tsx @@ -157,7 +157,6 @@ describe('SearchForm', () => { await user.click(asyncOperationSelect); jest.advanceTimersByTime(3000); - await user.clear(asyncOperationSelect); await user.type(asyncOperationSelect, '$'); const operationOption = await screen.findByText('$operation'); expect(operationOption).toBeDefined(); diff --git a/yarn.lock b/yarn.lock index d95afecb595..2686f4a3e4b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3506,9 +3506,9 @@ __metadata: languageName: unknown linkType: soft -"@grafana/scenes@npm:^4.24.4": - version: 4.26.3 - resolution: "@grafana/scenes@npm:4.26.3" +"@grafana/scenes@npm:4.27.0": + version: 4.27.0 + resolution: "@grafana/scenes@npm:4.27.0" dependencies: "@grafana/e2e-selectors": "npm:^11.0.0" "@leeoniya/ufuzzy": "npm:^1.0.14" @@ -3523,7 +3523,7 @@ __metadata: "@grafana/ui": ^10.4.1 react: ^18.0.0 react-dom: ^18.0.0 - checksum: 10/259bf1d9a202ce24820baf4f653844eadc12752d7a4ad21d94b1cd19b5c18a6bad1352cf13df2186372093a9bcbfe40d0a9a7866479f374787cfb174a53272af + checksum: 10/db072541da50cece2ccc2cd8965daa0ac83a77ab423aa576b9bfd08bb227727d5187bf4e27b0f8578d6fd1ba8fe38e2dafc73c0e74b627ff926915cf744830a2 languageName: node linkType: hard @@ -16692,7 +16692,7 @@ __metadata: "@grafana/prometheus": "workspace:*" "@grafana/runtime": "workspace:*" "@grafana/saga-icons": "workspace:*" - "@grafana/scenes": "npm:^4.24.4" + "@grafana/scenes": "npm:4.27.0" "@grafana/schema": "workspace:*" "@grafana/sql": "workspace:*" "@grafana/tsconfig": "npm:^1.3.0-rc1"