From f0095d84e370bfbdb34b50e74d26a6e85e437ec0 Mon Sep 17 00:00:00 2001 From: Sonia Aguilar <33540275+soniaAguilarPeiron@users.noreply.github.com> Date: Fri, 5 Sep 2025 17:51:19 +0200 Subject: [PATCH] Alerting: Load labels in drop-downs without blocking the interaction with the form inputs (#110648) * load labels in dropdown async without blockig the interaction with labels inputs * fix tests * update translations * remove fetchOptions unused property * revert removing wrong lines * Preferch rules for autocomplete and use caching * use selectedKey for getting gops values * revert removed lines --------- Co-authored-by: Konrad Lalik --- .../unified/components/AlertLabelDropdown.tsx | 65 +++---- .../GrafanaFolderAndLabelsStep.tsx | 12 +- .../rule-editor/labels/LabelsField.test.tsx | 72 +++----- .../rule-editor/labels/LabelsField.tsx | 165 +++++++++--------- .../rule-editor/useAlertRuleSuggestions.tsx | 2 +- public/locales/en-US/grafana.json | 6 - 6 files changed, 152 insertions(+), 170 deletions(-) diff --git a/public/app/features/alerting/unified/components/AlertLabelDropdown.tsx b/public/app/features/alerting/unified/components/AlertLabelDropdown.tsx index e016aa6169d..ff70fce4203 100644 --- a/public/app/features/alerting/unified/components/AlertLabelDropdown.tsx +++ b/public/app/features/alerting/unified/components/AlertLabelDropdown.tsx @@ -1,61 +1,52 @@ import { css } from '@emotion/css'; import { FC, forwardRef } from 'react'; -import { GroupBase, OptionsOrGroups, createFilter } from 'react-select'; import { SelectableValue } from '@grafana/data'; import { t } from '@grafana/i18n'; -import { Field, Select, useStyles2 } from '@grafana/ui'; +import { Combobox, ComboboxOption, Field, useStyles2 } from '@grafana/ui'; export interface AlertLabelDropdownProps { onChange: (newValue: SelectableValue) => void; onOpenMenu?: () => void; - options: SelectableValue[]; + options: ComboboxOption[]; defaultValue?: SelectableValue; type: 'key' | 'value'; + isLoading?: boolean; } -const _customFilter = createFilter({ ignoreCase: false }); -function customFilter(opt: SelectableValue, searchQuery: string) { - return _customFilter( - { - label: opt.label ?? '', - value: opt.value ?? '', - data: {}, - }, - searchQuery - ); -} - -const handleIsValidNewOption = ( - inputValue: string, - _: SelectableValue | null, - options: OptionsOrGroups, GroupBase>> -) => { - const exactValueExists = options.some((el) => el.label === inputValue); - const valueIsNotEmpty = inputValue.trim().length; - return !Boolean(exactValueExists) && Boolean(valueIsNotEmpty); -}; const AlertLabelDropdown: FC = forwardRef( - function LabelPicker({ onChange, options, defaultValue, type, onOpenMenu = () => {} }, ref) { + function LabelPicker({ onChange, options, defaultValue, type, onOpenMenu = () => {}, isLoading = false }, ref) { const styles = useStyles2(getStyles); + const handleChange = (option: ComboboxOption | null) => { + if (option) { + onChange({ + label: option.label || option.value, + value: option.value, + description: option.description, + }); + } + }; + + const currentValue = defaultValue + ? { + label: defaultValue.label || defaultValue.value, + value: defaultValue.value, + description: defaultValue.description, + } + : undefined; + return (
- + placeholder={t('alerting.alert-label-dropdown.placeholder-select', 'Choose {{type}}', { type })} - width={29} - className="ds-picker select-container" - backspaceRemovesValue={false} - onChange={onChange} - onOpenMenu={onOpenMenu} - filterOption={customFilter} - isValidNewOption={handleIsValidNewOption} + width={25} options={options} - maxMenuHeight={500} - noOptionsMessage={t('alerting.label-picker.no-options-message', 'No labels found')} - defaultValue={defaultValue} - allowCustomValue + value={currentValue} + onChange={handleChange} + createCustomValue={true} + data-testid={`alertlabel-${type}-combobox`} />
diff --git a/public/app/features/alerting/unified/components/rule-editor/GrafanaFolderAndLabelsStep.tsx b/public/app/features/alerting/unified/components/rule-editor/GrafanaFolderAndLabelsStep.tsx index a870a6f5b45..ced959c1a14 100644 --- a/public/app/features/alerting/unified/components/rule-editor/GrafanaFolderAndLabelsStep.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/GrafanaFolderAndLabelsStep.tsx @@ -1,9 +1,11 @@ -import { useState } from 'react'; +import { useEffect, useState } from 'react'; import { useFormContext } from 'react-hook-form'; import { Trans, t } from '@grafana/i18n'; import { Stack, Text } from '@grafana/ui'; +import { alertRuleApi } from '../../api/alertRuleApi'; +import { GRAFANA_RULER_CONFIG } from '../../api/featureDiscoveryApi'; import { KBObjectArray, RuleFormValues } from '../../types/rule-form'; import { GRAFANA_RULES_SOURCE_NAME } from '../../utils/datasource'; @@ -13,12 +15,20 @@ import { RuleEditorSection } from './RuleEditorSection'; import { LabelsEditorModal } from './labels/LabelsEditorModal'; import { LabelsFieldInForm } from './labels/LabelsFieldInForm'; +const { usePrefetch } = alertRuleApi; + /** Precondition: rule is Grafana managed. */ export function GrafanaFolderAndLabelsStep() { const { setValue, getValues } = useFormContext(); const [showLabelsEditor, setShowLabelsEditor] = useState(false); + const prefechRulerRules = usePrefetch('rulerRules'); + + useEffect(() => { + prefechRulerRules({ rulerConfig: GRAFANA_RULER_CONFIG }); + }, [prefechRulerRules]); + function onCloseLabelsEditor(labelsToUpdate?: KBObjectArray) { if (labelsToUpdate) { setValue('labels', labelsToUpdate); diff --git a/public/app/features/alerting/unified/components/rule-editor/labels/LabelsField.test.tsx b/public/app/features/alerting/unified/components/rule-editor/labels/LabelsField.test.tsx index a0983cba07f..bdd7fee994e 100644 --- a/public/app/features/alerting/unified/components/rule-editor/labels/LabelsField.test.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/labels/LabelsField.test.tsx @@ -1,6 +1,6 @@ import * as React from 'react'; import { FormProvider, useForm } from 'react-hook-form'; -import { render, screen, waitFor, waitForElementToBeRemoved, within } from 'test/test-utils'; +import { render, screen, waitFor, within } from 'test/test-utils'; import { clearPluginSettingsCache } from 'app/features/plugins/pluginSettings'; @@ -38,7 +38,11 @@ async function renderLabelsWithSuggestions() { ); - await waitForElementToBeRemoved(() => screen.queryByText('Loading existing labels')); + + // Wait for the dropdowns to be rendered + await waitFor(() => { + expect(screen.getAllByTestId('alertlabel-key-picker')).toHaveLength(2); + }); return view; } @@ -79,27 +83,26 @@ describe('LabelsField with suggestions', () => { it('Should display two dropdowns with the existing labels', async () => { await renderLabelsWithSuggestions(); - await waitFor(() => expect(screen.getAllByTestId('alertlabel-key-picker')).toHaveLength(2)); - - expect(screen.getByTestId('labelsInSubform-key-0')).toHaveTextContent('key1'); - expect(screen.getByTestId('labelsInSubform-key-1')).toHaveTextContent('key2'); + expect(screen.getByTestId('labelsInSubform-key-0').querySelector('input')).toHaveValue('key1'); + expect(screen.getByTestId('labelsInSubform-key-1').querySelector('input')).toHaveValue('key2'); expect(screen.getAllByTestId('alertlabel-value-picker')).toHaveLength(2); - expect(screen.getByTestId('labelsInSubform-value-0')).toHaveTextContent('value1'); - expect(screen.getByTestId('labelsInSubform-value-1')).toHaveTextContent('value2'); + expect(screen.getByTestId('labelsInSubform-value-0').querySelector('input')).toHaveValue('value1'); + expect(screen.getByTestId('labelsInSubform-value-1').querySelector('input')).toHaveValue('value2'); }); it('Should delete a key-value combination', async () => { const { user } = await renderLabelsWithSuggestions(); - expect(await screen.findAllByTestId('alertlabel-key-picker')).toHaveLength(2); - expect(await screen.findAllByTestId('alertlabel-value-picker')).toHaveLength(2); + expect(screen.getAllByTestId('alertlabel-key-picker')).toHaveLength(2); + + expect(screen.getAllByTestId('alertlabel-value-picker')).toHaveLength(2); await user.click(screen.getByTestId('delete-label-1')); - expect(await screen.findAllByTestId('alertlabel-key-picker')).toHaveLength(1); - expect(await screen.findAllByTestId('alertlabel-value-picker')).toHaveLength(1); + expect(screen.getAllByTestId('alertlabel-key-picker')).toHaveLength(1); + expect(screen.getAllByTestId('alertlabel-value-picker')).toHaveLength(1); }); it('Should add new key-value dropdowns', async () => { @@ -110,15 +113,15 @@ describe('LabelsField with suggestions', () => { expect(screen.getAllByTestId('alertlabel-key-picker')).toHaveLength(3); - expect(screen.getByTestId('labelsInSubform-key-0')).toHaveTextContent('key1'); - expect(screen.getByTestId('labelsInSubform-key-1')).toHaveTextContent('key2'); - expect(screen.getByTestId('labelsInSubform-key-2')).toHaveTextContent('Choose key'); + expect(screen.getByTestId('labelsInSubform-key-0').querySelector('input')).toHaveValue('key1'); + expect(screen.getByTestId('labelsInSubform-key-1').querySelector('input')).toHaveValue('key2'); + expect(screen.getByTestId('labelsInSubform-key-2').querySelector('input')).toHaveValue(''); expect(screen.getAllByTestId('alertlabel-value-picker')).toHaveLength(3); - expect(screen.getByTestId('labelsInSubform-value-0')).toHaveTextContent('value1'); - expect(screen.getByTestId('labelsInSubform-value-1')).toHaveTextContent('value2'); - expect(screen.getByTestId('labelsInSubform-value-2')).toHaveTextContent('Choose value'); + expect(screen.getByTestId('labelsInSubform-value-0').querySelector('input')).toHaveValue('value1'); + expect(screen.getByTestId('labelsInSubform-value-1').querySelector('input')).toHaveValue('value2'); + expect(screen.getByTestId('labelsInSubform-value-2').querySelector('input')).toHaveValue(''); }); it('Should be able to write new keys and values using the dropdowns', async () => { @@ -127,35 +130,16 @@ describe('LabelsField with suggestions', () => { await waitFor(() => expect(screen.getByText('Add more')).toBeVisible()); await user.click(screen.getByText('Add more')); - const LastKeyDropdown = within(screen.getByTestId('labelsInSubform-key-2')); - const LastValueDropdown = within(screen.getByTestId('labelsInSubform-value-2')); + expect(screen.getAllByTestId('alertlabel-key-picker')).toHaveLength(3); - await user.type(LastKeyDropdown.getByRole('combobox'), 'key3{enter}'); - await user.type(LastValueDropdown.getByRole('combobox'), 'value3{enter}'); + const lastKeyDropdown = within(screen.getByTestId('labelsInSubform-key-2')); + const lastValueDropdown = within(screen.getByTestId('labelsInSubform-value-2')); - expect(screen.getByTestId('labelsInSubform-key-2')).toHaveTextContent('key3'); - expect(screen.getByTestId('labelsInSubform-value-2')).toHaveTextContent('value3'); - }); + await user.type(lastKeyDropdown.getByRole('combobox'), 'key3{enter}'); + await user.type(lastValueDropdown.getByRole('combobox'), 'value3{enter}'); - it('Should be able to write new keys and values using the dropdowns, case sensitive', async () => { - const { user } = await renderLabelsWithSuggestions(); - - await waitFor(() => expect(screen.getAllByTestId('alertlabel-key-picker')).toHaveLength(2)); - expect(screen.getByTestId('labelsInSubform-key-0')).toHaveTextContent('key1'); - expect(screen.getByTestId('labelsInSubform-key-1')).toHaveTextContent('key2'); - expect(screen.getByTestId('labelsInSubform-value-0')).toHaveTextContent('value1'); - expect(screen.getByTestId('labelsInSubform-value-1')).toHaveTextContent('value2'); - - const LastKeyDropdown = within(screen.getByTestId('labelsInSubform-key-1')); - const LastValueDropdown = within(screen.getByTestId('labelsInSubform-value-1')); - - await user.type(LastKeyDropdown.getByRole('combobox'), 'KEY2{enter}'); - expect(screen.getByTestId('labelsInSubform-key-0')).toHaveTextContent('key1'); - expect(screen.getByTestId('labelsInSubform-key-1')).toHaveTextContent('KEY2'); - - await user.type(LastValueDropdown.getByRole('combobox'), 'VALUE2{enter}'); - expect(screen.getByTestId('labelsInSubform-value-0')).toHaveTextContent('value1'); - expect(screen.getByTestId('labelsInSubform-value-1')).toHaveTextContent('VALUE2'); + expect(screen.getByTestId('labelsInSubform-key-2').querySelector('input')).toHaveValue('key3'); + expect(screen.getByTestId('labelsInSubform-value-2').querySelector('input')).toHaveValue('value3'); }); }); diff --git a/public/app/features/alerting/unified/components/rule-editor/labels/LabelsField.tsx b/public/app/features/alerting/unified/components/rule-editor/labels/LabelsField.tsx index 58134774458..a1f1771517e 100644 --- a/public/app/features/alerting/unified/components/rule-editor/labels/LabelsField.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/labels/LabelsField.tsx @@ -4,7 +4,7 @@ import { Controller, FormProvider, useFieldArray, useForm, useFormContext } from import { GrafanaTheme2, SelectableValue } from '@grafana/data'; import { Trans, t } from '@grafana/i18n'; -import { Button, Field, InlineLabel, Input, LoadingPlaceholder, Space, Stack, Text, useStyles2 } from '@grafana/ui'; +import { Button, ComboboxOption, Field, InlineLabel, Input, Space, Stack, Text, useStyles2 } from '@grafana/ui'; import { labelsApi } from '../../../api/labelsApi'; import { usePluginBridge } from '../../../hooks/usePluginBridge'; @@ -29,9 +29,13 @@ const useGetOpsLabelsKeys = (skip: boolean) => { function mapLabelsToOptions( items: Iterable = [], labelsInSubForm?: Array<{ key: string; value: string }> -): Array> { +): Array> { const existingKeys = new Set(labelsInSubForm ? labelsInSubForm.map((label) => label.key) : []); - return Array.from(items, (item) => ({ label: item, value: item, isDisabled: existingKeys.has(item) })); + return Array.from(items, (item) => ({ + label: item, + value: item, + disabled: existingKeys.has(item), + })); } export interface LabelsInRuleProps { @@ -244,11 +248,11 @@ export function LabelsWithSuggestions({ dataSourceName }: LabelsWithSuggestionsP append({ key: '', value: '' }); }, [append]); + const [selectedKey, setSelectedKey] = useState(''); // check if the labels plugin is installed const { installed: labelsPluginInstalled = false, loading: loadingLabelsPlugin } = usePluginBridge( SupportedPlugin.Labels ); - const [selectedKey, setSelectedKey] = useState(''); const { loading, keysFromExistingAlerts, groupedOptions, getValuesForLabel } = useCombinedLabels( dataSourceName, @@ -262,84 +266,83 @@ export function LabelsWithSuggestions({ dataSourceName }: LabelsWithSuggestionsP return getValuesForLabel(selectedKey); }, [selectedKey, getValuesForLabel]); - const isLoading = loading || loadingLabelsPlugin; - return ( - <> - {isLoading && ( - - )} - {!isLoading && ( - - {fields.map((field, index) => { - return ( -
- - { - return ( - { - onChange(newValue.value); - setSelectedKey(newValue.value); - }} - type="key" - /> - ); - }} - /> - - = - - { - return ( - { - onChange(newValue.value); - }} - onOpenMenu={() => { - setSelectedKey(labelsInSubform[index].key); - }} - type="value" - /> - ); - }} - /> - + + {fields.map((field, index) => { + return ( +
+ + { + return ( + group.options) + : keysFromExistingAlerts + } + isLoading={loading} + onChange={(newValue: SelectableValue) => { + if (newValue) { + onChange(newValue.value || newValue.label || ''); + setSelectedKey(newValue.value); + } + }} + type="key" + /> + ); + }} + /> + + = + + { + return ( + { + if (newValue) { + onChange(newValue.value || newValue.label || ''); + } + }} + onOpenMenu={() => { + setSelectedKey(labelsInSubform[index].key); + }} + type="value" + /> + ); + }} + /> + - -
- ); - })} - - - )} - + +
+ ); + })} + + ); } @@ -478,7 +481,7 @@ const getStyles = (theme: GrafanaTheme2) => { margin: 0, }), labelInput: css({ - width: '175px', + width: '215px', margin: 0, }), confirmButton: css({ diff --git a/public/app/features/alerting/unified/components/rule-editor/useAlertRuleSuggestions.tsx b/public/app/features/alerting/unified/components/rule-editor/useAlertRuleSuggestions.tsx index e751e80c185..8ab1e621cff 100644 --- a/public/app/features/alerting/unified/components/rule-editor/useAlertRuleSuggestions.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/useAlertRuleSuggestions.tsx @@ -29,7 +29,7 @@ export function useGetLabelsFromDataSourceName(rulesSourceName: string) { useEffect(() => { if (features?.rulerConfig && !prometheusRulesPrimary) { - fetchRulerRules({ rulerConfig: features.rulerConfig }); + fetchRulerRules({ rulerConfig: features.rulerConfig }, true); } }, [features?.rulerConfig, fetchRulerRules]); diff --git a/public/locales/en-US/grafana.json b/public/locales/en-US/grafana.json index 9cd4548f667..013c0255105 100644 --- a/public/locales/en-US/grafana.json +++ b/public/locales/en-US/grafana.json @@ -1703,9 +1703,6 @@ "notes": "Notes", "returns": "Returns" }, - "label-picker": { - "no-options-message": "No labels found" - }, "labels-editor-modal": { "title-edit-labels": "Edit labels" }, @@ -1724,9 +1721,6 @@ "description": "Select a label key/value from the options below, or type a new one and press Enter.", "save": "Save" }, - "labels-with-suggestions": { - "text-loading-existing-labels": "Loading existing labels" - }, "labels-without-suggestions": { "message": { "required": "Required."