From 127367464edf4ddc2427e3816ea08f03b9694ee3 Mon Sep 17 00:00:00 2001 From: Jocelyn Collado-Kuri Date: Thu, 18 Dec 2025 06:28:29 -0800 Subject: [PATCH] Standardization across Zabbix UI components (#2141) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Throughout Zabbix we did not have a uniform UI - some drop-down were using `Select` others `Combobox` others a custom one that we created. Some had placeholders and others did not. This PR aims to standardize our Zabbix UI across our query, variable and config editors ## Detailed summary - Migrate from `Select` to `Combobox` -> `Select` component is deprecated - Migrate from `HorizontalGroup` to `Stack` -> `HorizontalGroup` is also deprecated - Remove use of "custom" dropdown `MetricPickerMenu` in favor of `Combobox` ensuring uniformity across our drop-down and removing maintenance overhead for us down the line - Standardize placeholders across all inputs Screenshot 2025-12-17 at 1 13 45 PM Screenshot 2025-12-17 at 1 14 05 PM ## Why To have a clean and standard UI and remove use of UI deprecated packages. ## How to test - Query Editor: - By creating a new query in a dashboard or Explore and interacting with all the different query types and drop-downs - All drop-downs should be searchable and have placeholders - Config Editor: - By going to a datasource and ensuring that the dropdown for Datasource (when DB connection is enabled) and Auth type are responsive and working as expected) Fixes: https://github.com/orgs/grafana/projects/457/views/40?pane=issue&itemId=3740545830&issue=grafana%7Coss-big-tent-squad%7C139 --- .changeset/ninety-eggs-bow.md | 5 + src/components/MetricPicker/MetricPicker.tsx | 123 ++-------------- .../MetricPicker/MetricPickerMenu.tsx | 138 ------------------ .../components/AnnotationQueryEditor.tsx | 14 +- src/datasource/components/ConfigEditor.tsx | 14 +- src/datasource/components/QueryEditor.tsx | 16 +- .../QueryEditor/MetricsQueryEditor.tsx | 16 +- .../QueryEditor/ProblemsQueryEditor.tsx | 25 ++-- .../QueryEditor/QueryOptionsEditor.tsx | 53 +++---- .../QueryEditor/ServicesQueryEditor.tsx | 19 +-- .../QueryEditor/TextMetricsQueryEditor.tsx | 20 ++- .../QueryEditor/TriggersQueryEditor.tsx | 31 ++-- .../QueryEditor/UserMacrosQueryEditor.tsx | 10 +- .../components/ExecScriptModal.tsx | 24 ++- 14 files changed, 152 insertions(+), 356 deletions(-) create mode 100644 .changeset/ninety-eggs-bow.md delete mode 100644 src/components/MetricPicker/MetricPickerMenu.tsx diff --git a/.changeset/ninety-eggs-bow.md b/.changeset/ninety-eggs-bow.md new file mode 100644 index 0000000..2599313 --- /dev/null +++ b/.changeset/ninety-eggs-bow.md @@ -0,0 +1,5 @@ +--- +'grafana-zabbix': minor +--- + +Standardization across Zabbix UI components diff --git a/src/components/MetricPicker/MetricPicker.tsx b/src/components/MetricPicker/MetricPicker.tsx index 4baeaf9..84eff49 100644 --- a/src/components/MetricPicker/MetricPicker.tsx +++ b/src/components/MetricPicker/MetricPicker.tsx @@ -1,130 +1,35 @@ -import { css, cx } from '@emotion/css'; -import React, { FormEvent, useCallback, useEffect, useState, useRef } from 'react'; -import { ClickOutsideWrapper, Input, Spinner, useStyles2 } from '@grafana/ui'; -import { MetricPickerMenu } from './MetricPickerMenu'; +import { css } from '@emotion/css'; +import React, { useRef } from 'react'; +import { Combobox, ComboboxOption } from '@grafana/ui'; import { GrafanaTheme2, SelectableValue } from '@grafana/data'; -import { isRegex } from '../../datasource/utils'; export interface Props { value: string; + placeholder: string; isLoading?: boolean; - options: Array>; + options: Array>; width?: number; onChange: (value: string) => void; } -export const MetricPicker = ({ value, options, isLoading, width, onChange }: Props) => { - const [isOpen, setOpen] = useState(false); - const [query, setQuery] = useState(value); - const [filteredOptions, setFilteredOptions] = useState(options); - const [selectedOptionIdx, setSelectedOptionIdx] = useState(-1); - const [offset] = useState({ vertical: 0, horizontal: 0 }); +export const MetricPicker = ({ value, placeholder, options, isLoading, width, onChange }: Props) => { const ref = useRef(null); - const customStyles = useStyles2(getStyles); - - const inputClass = cx({ - [customStyles.inputRegexp]: isRegex(query), - [customStyles.inputVariable]: query.startsWith('$'), - }); - - useEffect(() => { - setFilteredOptions(options); - }, [options]); - - const onOpen = () => { - setOpen(true); - setFilteredOptions(options); - }; - - const onClose = useCallback(() => { - setOpen(false); - }, []); - - // Only call onClose if menu is open. Prevent unnecessary calls for multiple pickers on the page. - const onClickOutside = () => isOpen && onClose(); - - const onInputChange = (v: FormEvent) => { - if (!isOpen) { - setOpen(true); - } - const newQuery = v?.currentTarget?.value; - if (newQuery) { - setQuery(newQuery); - if (value !== newQuery) { - const filtered = options.filter( - (option) => - option.value?.toLowerCase().includes(newQuery.toLowerCase()) || - option.label?.toLowerCase().includes(newQuery.toLowerCase()) - ); - setFilteredOptions(filtered); - } else { - setFilteredOptions(options); - } - } else { - setQuery(''); - setFilteredOptions(options); - } - }; const onMenuOptionSelect = (option: SelectableValue) => { const newValue = option?.value || ''; - setQuery(newValue); onChange(newValue); - onClose(); - }; - - const onBlurInternal = () => { - onChange(query); - }; - - const onKeyDown = (e: React.KeyboardEvent) => { - if (e.key === 'ArrowDown') { - if (!isOpen) { - setOpen(true); - } - e.preventDefault(); - e.stopPropagation(); - const selected = selectedOptionIdx < filteredOptions.length - 1 ? selectedOptionIdx + 1 : 0; - setSelectedOptionIdx(selected); - } else if (e.key === 'ArrowUp') { - if (!isOpen) { - setOpen(true); - } - e.preventDefault(); - e.stopPropagation(); - const selected = selectedOptionIdx > 0 ? selectedOptionIdx - 1 : filteredOptions.length - 1; - setSelectedOptionIdx(selected); - } else if (e.key === 'Enter') { - e.preventDefault(); - e.stopPropagation(); - onMenuOptionSelect(filteredOptions[selectedOptionIdx]); - } }; return (
- - } - width={width} - onKeyDown={onKeyDown} - /> - {isOpen && ( - - )} - + + width={width} + value={value} + options={options ?? []} + onChange={onMenuOptionSelect} + loading={isLoading} + placeholder={placeholder} + />
); }; diff --git a/src/components/MetricPicker/MetricPickerMenu.tsx b/src/components/MetricPicker/MetricPickerMenu.tsx deleted file mode 100644 index 596e074..0000000 --- a/src/components/MetricPicker/MetricPickerMenu.tsx +++ /dev/null @@ -1,138 +0,0 @@ -import { css, cx } from '@emotion/css'; -import React, { FormEvent } from 'react'; -import { GrafanaTheme2, SelectableValue } from '@grafana/data'; -import { CustomScrollbar, getSelectStyles, Icon, Tooltip, useStyles2, useTheme2 } from '@grafana/ui'; -import { MENU_MAX_HEIGHT } from './constants'; - -interface Props { - options: Array>; - onSelect: (option: SelectableValue) => void; - offset: { vertical: number; horizontal: number }; - minWidth?: number; - selected?: number; -} - -export const MetricPickerMenu = ({ options, offset, minWidth, selected, onSelect }: Props) => { - const theme = useTheme2(); - const styles = getSelectStyles(theme); - const customStyles = useStyles2(getStyles(minWidth)); - - return ( -
0 }, - css` - bottom: ${offset.vertical > 0 ? `${offset.vertical}px` : 'unset'}; - top: ${offset.vertical < 0 ? `${Math.abs(offset.vertical)}px` : 'unset'}; - ` - )} - > -
- -
-
- {options?.map((option, i) => ( - - ))} -
-
- -
-
- ); -}; - -interface MenuOptionProps { - data: SelectableValue; - onClick: (value: SelectableValue) => void; - isFocused?: boolean; - disabled?: boolean; - hideDescription?: boolean; -} - -const MenuOption = React.forwardRef>>( - ({ data, isFocused, disabled, onClick, hideDescription }, ref) => { - const theme = useTheme2(); - const styles = getSelectStyles(theme); - const customStyles = useStyles2(getStyles()); - - const wrapperClassName = cx( - styles.option, - customStyles.menuOptionWrapper, - isFocused && styles.optionFocused, - disabled && customStyles.menuOptionDisabled - ); - - const onClickInternal = (event: FormEvent) => { - if (disabled) { - return; - } - event.preventDefault(); - event.stopPropagation(); - onClick(data); - }; - - return ( -
-
- {data.label || data.value} - {!hideDescription && data.description &&
{data.description}
} -
- {data.description && ( - - - - )} -
- ); - } -); - -MenuOption.displayName = 'MenuOption'; - -export const getStyles = (menuWidth?: number) => (theme: GrafanaTheme2) => { - return { - menuWrapper: css` - display: flex; - max-height: 650px; - position: absolute; - z-index: ${theme.zIndex.dropdown}; - overflow: hidden; - min-width: auto; - `, - menu: css` - min-width: ${theme.spacing(menuWidth || 0)}; - - & > div { - padding-top: ${theme.spacing(1)}; - } - `, - menuLeft: css` - right: 0; - flex-direction: row-reverse; - `, - container: css` - padding: ${theme.spacing(1)}; - border: 1px ${theme.colors.border.weak} solid; - border-radius: ${theme.shape.borderRadius(1)}; - background-color: ${theme.colors.background.primary}; - z-index: ${theme.zIndex.modal}; - `, - menuOptionWrapper: css` - padding: ${theme.spacing(0.5)}; - `, - menuOptionBody: css` - font-weight: ${theme.typography.fontWeightRegular}; - padding: ${theme.spacing(0, 1.5, 0, 0)}; - `, - menuOptionDisabled: css` - color: ${theme.colors.text.disabled}; - cursor: not-allowed; - `, - menuOptionInfoSign: css` - color: ${theme.colors.text.disabled}; - `, - }; -}; diff --git a/src/datasource/components/AnnotationQueryEditor.tsx b/src/datasource/components/AnnotationQueryEditor.tsx index d7c6c4a..ed3a28c 100644 --- a/src/datasource/components/AnnotationQueryEditor.tsx +++ b/src/datasource/components/AnnotationQueryEditor.tsx @@ -2,7 +2,7 @@ import _ from 'lodash'; import React, { useEffect, FormEvent } from 'react'; import { useAsyncFn } from 'react-use'; import { AnnotationQuery, SelectableValue } from '@grafana/data'; -import { InlineField, InlineSwitch, Input, Select } from '@grafana/ui'; +import { Combobox, ComboboxOption, InlineField, InlineSwitch, Input } from '@grafana/ui'; import { ZabbixMetricsQuery } from '../types/query'; import { ZabbixQueryEditorProps } from './QueryEditor'; import { QueryEditorRow } from './QueryEditor/QueryEditorRow'; @@ -11,7 +11,7 @@ import { getVariableOptions } from './QueryEditor/utils'; import { prepareAnnotation } from '../migrations'; import { useInterpolatedQuery } from '../hooks/useInterpolatedQuery'; -const severityOptions: Array> = [ +const severityOptions: Array> = [ { value: 0, label: 'Not classified' }, { value: 1, label: 'Information' }, { value: 2, label: 'Warning' }, @@ -47,7 +47,7 @@ export const AnnotationQueryEditor = ({ annotation, onAnnotationChange, datasour const loadHostOptions = async (group: string) => { const hosts = await datasource.zabbix.getAllHosts(group); - let options: Array> = hosts?.map((host) => ({ + let options: Array> = hosts?.map((host) => ({ value: host.name, label: host.name, })); @@ -64,7 +64,7 @@ export const AnnotationQueryEditor = ({ annotation, onAnnotationChange, datasour const loadAppOptions = async (group: string, host: string) => { const apps = await datasource.zabbix.getAllApps(group, host); - let options: Array> = apps?.map((app) => ({ + let options: Array> = apps?.map((app) => ({ value: app.name, label: app.name, })); @@ -138,6 +138,7 @@ export const AnnotationQueryEditor = ({ annotation, onAnnotationChange, datasour options={groupsOptions} isLoading={groupsLoading} onChange={onFilterChange('group')} + placeholder="Group name" /> @@ -147,6 +148,7 @@ export const AnnotationQueryEditor = ({ annotation, onAnnotationChange, datasour options={hostOptions} isLoading={hostsLoading} onChange={onFilterChange('host')} + placeholder="Host name" /> @@ -158,6 +160,7 @@ export const AnnotationQueryEditor = ({ annotation, onAnnotationChange, datasour options={appOptions} isLoading={appsLoading} onChange={onFilterChange('application')} + placeholder="Application name" /> @@ -171,8 +174,7 @@ export const AnnotationQueryEditor = ({ annotation, onAnnotationChange, datasour <> - { {options.jsonData.dbConnectionEnable && ( <> - + - - - - + > = [ +const countByOptions: Array> = [ { value: '', label: 'All triggers' }, { value: 'problems', label: 'Problems' }, { value: 'items', label: 'Items' }, ]; -const severityOptions: Array> = [ +const severityOptions: Array> = [ { value: 0, label: 'Not classified' }, { value: 1, label: 'Information' }, { value: 2, label: 'Warning' }, @@ -54,7 +54,7 @@ export const TriggersQueryEditor = ({ query, datasource, onChange }: Props) => { const loadHostOptions = async (group: string) => { const hosts = await datasource.zabbix.getAllHosts(group); - let options: Array> = hosts?.map((host) => ({ + let options: Array> = hosts?.map((host) => ({ value: host.name, label: host.name, })); @@ -71,7 +71,7 @@ export const TriggersQueryEditor = ({ query, datasource, onChange }: Props) => { const loadAppOptions = async (group: string, host: string) => { const apps = await datasource.zabbix.getAllApps(group, host); - let options: Array> = apps?.map((app) => ({ + let options: Array> = apps?.map((app) => ({ value: app.name, label: app.name, })); @@ -94,7 +94,7 @@ export const TriggersQueryEditor = ({ query, datasource, onChange }: Props) => { const tags: ZBXItemTag[] = _.flatten(items.map((item: ZBXItem) => item.tags || [])); const tagList = _.uniqBy(tags, (t) => t.tag + t.value || '').map((t) => itemTagToString(t)); - let options: Array> = tagList?.map((tag) => ({ + let options: Array> = tagList?.map((tag) => ({ value: tag, label: tag, })); @@ -129,7 +129,7 @@ export const TriggersQueryEditor = ({ query, datasource, onChange }: Props) => { showDisabledItems: query.options.showDisabledItems, }; const items = await datasource.zabbix.getAllItems(group, host, app, itemTag, options); - let itemOptions: Array> = items?.map((item) => ({ + let itemOptions: Array> = items?.map((item) => ({ value: item.name, label: item.name, })); @@ -218,13 +218,7 @@ export const TriggersQueryEditor = ({ query, datasource, onChange }: Props) => { <> - const loadHostOptions = async (group: string) => { const hosts = await datasource.zabbix.getAllHosts(group); - let options: Array> = hosts?.map((host) => ({ + let options: Array> = hosts?.map((host) => ({ value: host.name, label: host.name, })); @@ -53,7 +52,7 @@ export const UserMacrosQueryEditor = ({ query, datasource, onChange }: Props) => const loadMacrosOptions = async (group: string, host: string) => { const macros = await datasource.zabbix.getAllMacros(group, host); - let options: Array> = macros?.map((m) => ({ + let options: Array> = macros?.map((m) => ({ value: m.macro, label: m.macro, })); @@ -102,6 +101,7 @@ export const UserMacrosQueryEditor = ({ query, datasource, onChange }: Props) => options={groupsOptions} isLoading={groupsLoading} onChange={onFilterChange('group')} + placeholder="Group name" /> @@ -111,6 +111,7 @@ export const UserMacrosQueryEditor = ({ query, datasource, onChange }: Props) => options={hostOptions} isLoading={hostsLoading} onChange={onFilterChange('host')} + placeholder="Host name" /> @@ -122,6 +123,7 @@ export const UserMacrosQueryEditor = ({ query, datasource, onChange }: Props) => options={macrosOptions} isLoading={macrosLoading} onChange={onFilterChange('macro')} + placeholder="Macro name" /> diff --git a/src/panel-triggers/components/ExecScriptModal.tsx b/src/panel-triggers/components/ExecScriptModal.tsx index a724f49..5d797a5 100644 --- a/src/panel-triggers/components/ExecScriptModal.tsx +++ b/src/panel-triggers/components/ExecScriptModal.tsx @@ -1,7 +1,17 @@ import React, { PureComponent, ReactNode } from 'react'; import { css } from '@emotion/css'; -import { GrafanaTheme, SelectableValue } from '@grafana/data'; -import { Button, Spinner, Modal, Select, stylesFactory, withTheme, Themeable, ButtonGroup } from '@grafana/ui'; +import { GrafanaTheme } from '@grafana/data'; +import { + Button, + Spinner, + Modal, + stylesFactory, + withTheme, + Themeable, + ButtonGroup, + Combobox, + ComboboxOption, +} from '@grafana/ui'; import { ZBXScript, APIExecuteScriptResponse } from '../../datasource/zabbix/connectors/zabbix_api/types'; import { FAIcon } from '../../components'; @@ -12,8 +22,8 @@ interface Props extends Themeable { } interface State { - selectedScript: SelectableValue; - scriptOptions: Array>; + selectedScript: ComboboxOption; + scriptOptions: Array>; script: ZBXScript; error: boolean; errorMessage: string | ReactNode; @@ -47,7 +57,7 @@ export class ExecScriptModalUnthemed extends PureComponent { async componentDidMount() { const scripts = await this.props.getScripts(); this.scripts = scripts; - const scriptOptions: Array> = scripts.map((s) => { + const scriptOptions: Array> = scripts.map((s) => { return { value: s.scriptid, label: s.name, @@ -61,7 +71,7 @@ export class ExecScriptModalUnthemed extends PureComponent { this.setState({ scriptOptions, selectedScript, script }); } - onChangeSelectedScript = (v: SelectableValue) => { + onChangeSelectedScript = (v: ComboboxOption) => { const script = this.scripts.find((s) => v.value === s.scriptid); this.setState({ selectedScript: v, script, errorMessage: '', loading: false, result: '' }); }; @@ -133,7 +143,7 @@ export class ExecScriptModalUnthemed extends PureComponent { } > -