Elasticsearch: fix a11y issues in datasource settings (#43706)

packages/grafana-ui/src/components/Forms/InlineField.tsx

@@ -1,11 +1,12 @@
 import React, { FC } from 'react';
 import { cx, css } from '@emotion/css';
-import { GrafanaTheme } from '@grafana/data';
+import { GrafanaTheme2 } from '@grafana/data';
-import { useTheme } from '../../themes';
+import { useTheme2 } from '../../themes';
 import { InlineLabel } from './InlineLabel';
 import { PopoverContent } from '../Tooltip/Tooltip';
 import { FieldProps } from './Field';
 import { getChildId } from '../../utils/reactUtils';
+import { FieldValidationMessage } from './FieldValidationMessage';
 
 export interface Props extends Omit<FieldProps, 'css' | 'horizontal' | 'description' | 'error'> {
   /** Content for the label's tooltip */
@@ -16,6 +17,8 @@ export interface Props extends Omit<FieldProps, 'css' | 'horizontal' | 'descript
   grow?: boolean;
   /** Make field's background transparent */
   transparent?: boolean;
+  /** Error message to display */
+  error?: string | null;
   htmlFor?: string;
 }
 
@@ -30,10 +33,11 @@ export const InlineField: FC<Props> = ({
   className,
   htmlFor,
   grow,
+  error,
   transparent,
   ...htmlProps
 }) => {
-  const theme = useTheme();
+  const theme = useTheme2();
   const styles = getStyles(theme, grow);
   const inputId = htmlFor ?? getChildId(children);
 
@@ -49,14 +53,21 @@ export const InlineField: FC<Props> = ({
   return (
     <div className={cx(styles.container, className)} {...htmlProps}>
       {labelElement}
+      <div className={styles.childContainer}>
         {React.cloneElement(children, { invalid, disabled, loading })}
+        {invalid && error && (
+          <div className={cx(styles.fieldValidationWrapper)}>
+            <FieldValidationMessage>{error}</FieldValidationMessage>
+          </div>
+        )}
+      </div>
     </div>
   );
 };
 
 InlineField.displayName = 'InlineField';
 
-const getStyles = (theme: GrafanaTheme, grow?: boolean) => {
+const getStyles = (theme: GrafanaTheme2, grow?: boolean) => {
   return {
     container: css`
       display: flex;
@@ -65,7 +76,13 @@ const getStyles = (theme: GrafanaTheme, grow?: boolean) => {
       text-align: left;
       position: relative;
       flex: ${grow ? 1 : 0} 0 auto;
-      margin: 0 ${theme.spacing.xs} ${theme.spacing.xs} 0;
+      margin: 0 ${theme.spacing(0.5)} ${theme.spacing(0.5)} 0;
+    `,
+    childContainer: css`
+      flex: ${grow ? 1 : 0} 0 auto;
+    `,
+    fieldValidationWrapper: css`
+      margin-top: ${theme.spacing(0.5)};
     `,
   };
 };

public/app/core/components/TransformersUI/configFromQuery/ConfigFromQueryTransformerEditor.test.tsx

@@ -38,7 +38,7 @@ describe('ConfigFromQueryTransformerEditor', () => {
   it('Should be able to select config frame by refId', async () => {
     setup();
 
-    let select = (await screen.findByText('Config query')).nextSibling!;
+    let select = (await screen.findByText('Config query')).nextSibling!.firstChild!;
     await fireEvent.keyDown(select, { keyCode: 40 });
     await selectOptionInTest(select as HTMLElement, 'A');
 

public/app/plugins/datasource/elasticsearch/configuration/ElasticDetails.test.tsx

@@ -1,92 +1,89 @@
 import React from 'react';
-import { last } from 'lodash';
-import { mount } from 'enzyme';
 import { ElasticDetails } from './ElasticDetails';
 import { createDefaultConfigOptions } from './mocks';
-import { LegacyForms } from '@grafana/ui';
+import { render, screen } from '@testing-library/react';
-const { Select } = LegacyForms;
+import selectEvent from 'react-select-event';
 
 describe('ElasticDetails', () => {
-  it('should render without error', () => {
+  describe('Max concurrent Shard Requests', () => {
-    mount(<ElasticDetails onChange={() => {}} value={createDefaultConfigOptions()} />);
+    it('should render "Max concurrent Shard Requests" if version >= 5.6.0', () => {
+      render(<ElasticDetails onChange={() => {}} value={createDefaultConfigOptions({ esVersion: '5.6.0' })} />);
+      expect(screen.getByLabelText('Max concurrent Shard Requests')).toBeInTheDocument();
     });
 
-  it('should render "Max concurrent Shard Requests" if version high enough', () => {
+    it('should not render "Max concurrent Shard Requests" if version < 5.6.0', () => {
-    const wrapper = mount(<ElasticDetails onChange={() => {}} value={createDefaultConfigOptions()} />);
+      render(<ElasticDetails onChange={() => {}} value={createDefaultConfigOptions({ esVersion: '5.0.0' })} />);
-    expect(wrapper.find('input[aria-label="Max concurrent Shard Requests input"]').length).toBe(1);
+      expect(screen.queryByLabelText('Max concurrent Shard Requests')).not.toBeInTheDocument();
     });
-
-  it('should not render "Max concurrent Shard Requests" if version is low', () => {
-    const options = createDefaultConfigOptions();
-    options.jsonData.esVersion = '5.0.0';
-    const wrapper = mount(<ElasticDetails onChange={() => {}} value={options} />);
-    expect(wrapper.find('input[aria-label="Max concurrent Shard Requests input"]').length).toBe(0);
   });
 
-  it('should change database on interval change when not set explicitly', () => {
+  it('should change database on interval change when not set explicitly', async () => {
     const onChangeMock = jest.fn();
-    const wrapper = mount(<ElasticDetails onChange={onChangeMock} value={createDefaultConfigOptions()} />);
+    render(<ElasticDetails onChange={onChangeMock} value={createDefaultConfigOptions()} />);
-    const selectEl = wrapper.find({ label: 'Pattern' }).find(Select);
+    const selectEl = screen.getByLabelText('Pattern');
-    selectEl.props().onChange({ value: 'Daily', label: 'Daily' }, { action: 'select-option', option: undefined });
 
-    expect(onChangeMock.mock.calls[0][0].jsonData.interval).toBe('Daily');
+    await selectEvent.select(selectEl, 'Daily', { container: document.body });
-    expect(onChangeMock.mock.calls[0][0].database).toBe('[logstash-]YYYY.MM.DD');
+
+    expect(onChangeMock).toHaveBeenLastCalledWith(
+      expect.objectContaining({
+        database: '[logstash-]YYYY.MM.DD',
+        jsonData: expect.objectContaining({ interval: 'Daily' }),
+      })
+    );
   });
 
-  it('should change database on interval change if pattern is from example', () => {
+  it('should change database on interval change if pattern is from example', async () => {
     const onChangeMock = jest.fn();
     const options = createDefaultConfigOptions();
     options.database = '[logstash-]YYYY.MM.DD.HH';
-    const wrapper = mount(<ElasticDetails onChange={onChangeMock} value={options} />);
+    render(<ElasticDetails onChange={onChangeMock} value={options} />);
+    const selectEl = screen.getByLabelText('Pattern');
 
-    const selectEl = wrapper.find({ label: 'Pattern' }).find(Select);
+    await selectEvent.select(selectEl, 'Monthly', { container: document.body });
-    selectEl.props().onChange({ value: 'Monthly', label: 'Monthly' }, { action: 'select-option', option: undefined });
 
-    expect(onChangeMock.mock.calls[0][0].jsonData.interval).toBe('Monthly');
+    expect(onChangeMock).toHaveBeenLastCalledWith(
-    expect(onChangeMock.mock.calls[0][0].database).toBe('[logstash-]YYYY.MM');
+      expect.objectContaining({
+        database: '[logstash-]YYYY.MM',
+        jsonData: expect.objectContaining({ interval: 'Monthly' }),
+      })
+    );
   });
 
   describe('version change', () => {
     const testCases = [
-      { version: '5.0.0', expectedMaxConcurrentShardRequests: 256 },
+      { version: '5.x', expectedMaxConcurrentShardRequests: 256 },
-      { version: '5.0.0', maxConcurrentShardRequests: 50, expectedMaxConcurrentShardRequests: 50 },
+      { version: '5.x', maxConcurrentShardRequests: 50, expectedMaxConcurrentShardRequests: 50 },
-      { version: '5.6.0', expectedMaxConcurrentShardRequests: 256 },
+      { version: '5.6+', expectedMaxConcurrentShardRequests: 256 },
-      { version: '5.6.0', maxConcurrentShardRequests: 256, expectedMaxConcurrentShardRequests: 256 },
+      { version: '5.6+', maxConcurrentShardRequests: 256, expectedMaxConcurrentShardRequests: 256 },
-      { version: '5.6.0', maxConcurrentShardRequests: 5, expectedMaxConcurrentShardRequests: 256 },
+      { version: '5.6+', maxConcurrentShardRequests: 5, expectedMaxConcurrentShardRequests: 256 },
-      { version: '5.6.0', maxConcurrentShardRequests: 200, expectedMaxConcurrentShardRequests: 200 },
+      { version: '5.6+', maxConcurrentShardRequests: 200, expectedMaxConcurrentShardRequests: 200 },
-      { version: '7.0.0', expectedMaxConcurrentShardRequests: 5 },
+      { version: '7.0+', expectedMaxConcurrentShardRequests: 5 },
-      { version: '7.0.0', maxConcurrentShardRequests: 256, expectedMaxConcurrentShardRequests: 5 },
+      { version: '7.0+', maxConcurrentShardRequests: 256, expectedMaxConcurrentShardRequests: 5 },
-      { version: '7.0.0', maxConcurrentShardRequests: 5, expectedMaxConcurrentShardRequests: 5 },
+      { version: '7.0+', maxConcurrentShardRequests: 5, expectedMaxConcurrentShardRequests: 5 },
-      { version: '7.0.0', maxConcurrentShardRequests: 6, expectedMaxConcurrentShardRequests: 6 },
+      { version: '7.0+', maxConcurrentShardRequests: 6, expectedMaxConcurrentShardRequests: 6 },
     ];
 
-    const onChangeMock = jest.fn();
-    const options = createDefaultConfigOptions();
-    const wrapper = mount(<ElasticDetails onChange={onChangeMock} value={options} />);
-
     testCases.forEach((tc) => {
-      it(`sets maxConcurrentShardRequests = ${tc.maxConcurrentShardRequests} if version = ${tc.version},`, () => {
+      const onChangeMock = jest.fn();
-        wrapper.setProps({
+      it(`sets maxConcurrentShardRequests=${tc.expectedMaxConcurrentShardRequests} if version=${tc.version},`, async () => {
-          onChange: onChangeMock,
+        render(
-          value: {
+          <ElasticDetails
-            ...options,
+            onChange={onChangeMock}
-            jsonData: {
+            value={createDefaultConfigOptions({
-              ...options.jsonData,
               maxConcurrentShardRequests: tc.maxConcurrentShardRequests,
-            },
+              esVersion: '2.0.0',
-          },
+            })}
-        });
+          />
-
-        const selectEl = wrapper.find({ label: 'Version' }).find(Select);
-        selectEl
-          .props()
-          .onChange(
-            { value: tc.version, label: tc.version.toString() },
-            { action: 'select-option', option: undefined }
         );
 
-        expect(last(onChangeMock.mock.calls)[0].jsonData.maxConcurrentShardRequests).toBe(
+        const selectEl = screen.getByLabelText('ElasticSearch version');
-          tc.expectedMaxConcurrentShardRequests
+
+        await selectEvent.select(selectEl, tc.version, { container: document.body });
+
+        expect(onChangeMock).toHaveBeenCalledWith(
+          expect.objectContaining({
+            jsonData: expect.objectContaining({ maxConcurrentShardRequests: tc.expectedMaxConcurrentShardRequests }),
+          })
         );
       });
     });

public/app/plugins/datasource/elasticsearch/configuration/ElasticDetails.tsx

@@ -1,6 +1,5 @@
 import React from 'react';
-import { EventsWithValidation, regexValidation, LegacyForms } from '@grafana/ui';
+import { FieldSet, InlineField, Input, Select, InlineSwitch } from '@grafana/ui';
-const { Switch, Select, Input, FormField } = LegacyForms;
 import { ElasticsearchOptions, Interval } from '../types';
 import { DataSourceSettings, SelectableValue } from '@grafana/data';
 import { gte, lt, valid } from 'semver';
@@ -45,59 +44,45 @@ export const ElasticDetails = ({ value, onChange }: Props) => {
       : undefined;
   return (
     <>
-      <h3 className="page-heading">Elasticsearch details</h3>
+      <FieldSet label="Elasticsearch details">
-
+        <InlineField label="Index name" labelWidth={26}>
-      <div className="gf-form-group">
+          <Input
-        <div className="gf-form-inline">
+            id="es_config_indexName"
-          <div className="gf-form">
-            <FormField
-              labelWidth={10}
-              inputWidth={15}
-              label="Index name"
             value={value.database || ''}
             onChange={changeHandler('database', value, onChange)}
-              placeholder={'es-index-name'}
+            width={24}
+            placeholder="es-index-name"
             required
           />
-          </div>
+        </InlineField>
 
-          <div className="gf-form">
+        <InlineField label="Pattern" labelWidth={26}>
-            <FormField
-              labelWidth={10}
-              label="Pattern"
-              inputEl={
           <Select
-                  menuShouldPortal
+            inputId="es_config_indexPattern"
-                  options={indexPatternTypes}
-                  onChange={intervalHandler(value, onChange)}
             value={indexPatternTypes.find(
-                    (pattern) =>
+              (pattern) => pattern.value === (value.jsonData.interval === undefined ? 'none' : value.jsonData.interval)
-                      pattern.value === (value.jsonData.interval === undefined ? 'none' : value.jsonData.interval)
             )}
+            options={indexPatternTypes}
+            onChange={intervalHandler(value, onChange)}
+            width={24}
+            menuShouldPortal
           />
-              }
+        </InlineField>
-            />
-          </div>
-        </div>
 
-        <div className="gf-form max-width-25">
+        <InlineField label="Time field name" labelWidth={26}>
-          <FormField
+          <Input
-            labelWidth={10}
+            id="es_config_timeField"
-            inputWidth={15}
-            label="Time field name"
             value={value.jsonData.timeField || ''}
             onChange={jsonDataChangeHandler('timeField', value, onChange)}
+            width={24}
+            placeholder="@timestamp"
             required
           />
-        </div>
+        </InlineField>
 
-        <div className="gf-form">
+        <InlineField label="ElasticSearch version" labelWidth={26}>
-          <FormField
-            labelWidth={10}
-            label="Version"
-            inputEl={
           <Select
-                menuShouldPortal
+            inputId="es_config_version"
             options={[customOption, ...esVersions].filter(isTruthy)}
             onChange={(option) => {
               const maxConcurrentShardRequests = getMaxConcurrenShardRequestOrDefault(
@@ -114,71 +99,61 @@ export const ElasticDetails = ({ value, onChange }: Props) => {
               });
             }}
             value={currentVersion || customOption}
+            width={24}
+            menuShouldPortal
           />
-            }
+        </InlineField>
-          />
+
-        </div>
         {gte(value.jsonData.esVersion, '5.6.0') && (
-          <div className="gf-form max-width-30">
+          <InlineField label="Max concurrent Shard Requests" labelWidth={26}>
-            <FormField
+            <Input
-              aria-label={'Max concurrent Shard Requests input'}
+              id="es_config_shardRequests"
-              labelWidth={15}
-              label="Max concurrent Shard Requests"
               value={value.jsonData.maxConcurrentShardRequests || ''}
               onChange={jsonDataChangeHandler('maxConcurrentShardRequests', value, onChange)}
+              width={24}
             />
-          </div>
+          </InlineField>
         )}
-        <div className="gf-form-inline">
+
-          <div className="gf-form">
+        <InlineField
-            <FormField
-              labelWidth={10}
           label="Min time interval"
-              inputEl={
+          labelWidth={26}
-                <Input
-                  className={'width-6'}
-                  value={value.jsonData.timeInterval || ''}
-                  onChange={jsonDataChangeHandler('timeInterval', value, onChange)}
-                  placeholder="10s"
-                  validationEvents={{
-                    [EventsWithValidation.onBlur]: [
-                      regexValidation(
-                        /^\d+(ms|[Mwdhmsy])$/,
-                        'Value is not valid, you can use number with time unit specifier: y, M, w, d, h, m, s'
-                      ),
-                    ],
-                  }}
-                />
-              }
           tooltip={
             <>
-                  A lower limit for the auto group by time interval. Recommended to be set to write frequency, for
+              A lower limit for the auto group by time interval. Recommended to be set to write frequency, for example{' '}
-                  example <code>1m</code> if your data is written every minute.
+              <code>1m</code> if your data is written every minute.
             </>
           }
+          error="Value is not valid, you can use number with time unit specifier: y, M, w, d, h, m, s"
+          invalid={!!value.jsonData.timeInterval && !/^\d+(ms|[Mwdhmsy])$/.test(value.jsonData.timeInterval)}
+        >
+          <Input
+            id="es_config_minTimeInterval"
+            value={value.jsonData.timeInterval || ''}
+            onChange={jsonDataChangeHandler('timeInterval', value, onChange)}
+            width={24}
+            placeholder="10s"
           />
-          </div>
+        </InlineField>
-        </div>
+
-        <div className="gf-form-inline">
+        <InlineField label="X-Pack enabled" labelWidth={26}>
-          <Switch
+          <InlineSwitch
-            label="X-Pack enabled"
+            id="es_config_xpackEnabled"
-            labelClass="width-10"
             checked={value.jsonData.xpack || false}
             onChange={jsonDataSwitchChangeHandler('xpack', value, onChange)}
           />
-        </div>
+        </InlineField>
 
         {gte(value.jsonData.esVersion, '6.6.0') && value.jsonData.xpack && (
-          <div className="gf-form-inline">
+          <InlineField label="Include Frozen Indices" labelWidth={26}>
-            <Switch
+            <InlineSwitch
-              label="Include frozen indices"
+              id="es_config_frozenIndices"
-              labelClass="width-10"
               checked={value.jsonData.includeFrozen ?? false}
               onChange={jsonDataSwitchChangeHandler('includeFrozen', value, onChange)}
             />
-          </div>
+          </InlineField>
         )}
-      </div>
+      </FieldSet>
     </>
   );
 };

public/app/plugins/datasource/elasticsearch/configuration/LogsConfig.test.tsx

@@ -1,26 +1,22 @@
 import React from 'react';
-import { mount, shallow } from 'enzyme';
+import { render, screen, fireEvent } from '@testing-library/react';
 import { LogsConfig } from './LogsConfig';
 import { createDefaultConfigOptions } from './mocks';
-import { LegacyForms } from '@grafana/ui';
-const { FormField } = LegacyForms;
 
 describe('ElasticDetails', () => {
-  it('should render without error', () => {
-    mount(<LogsConfig onChange={() => {}} value={createDefaultConfigOptions().jsonData} />);
-  });
-
-  it('should render fields', () => {
-    const wrapper = shallow(<LogsConfig onChange={() => {}} value={createDefaultConfigOptions().jsonData} />);
-    expect(wrapper.find(FormField).length).toBe(2);
-  });
-
   it('should pass correct data to onChange', () => {
     const onChangeMock = jest.fn();
-    const wrapper = mount(<LogsConfig onChange={onChangeMock} value={createDefaultConfigOptions().jsonData} />);
+    const expectedMessageField = '@message';
-    const inputEl = wrapper.find(FormField).at(0).find('input');
+    const expectedLevelField = '@level';
-    (inputEl.getDOMNode() as any).value = 'test_field';
+
-    inputEl.simulate('change');
+    render(<LogsConfig onChange={onChangeMock} value={createDefaultConfigOptions().jsonData} />);
-    expect(onChangeMock.mock.calls[0][0].logMessageField).toBe('test_field');
+    const messageField = screen.getByLabelText('Message field name');
+    const levelField = screen.getByLabelText('Level field name');
+
+    fireEvent.change(messageField, { target: { value: expectedMessageField } });
+    expect(onChangeMock).toHaveBeenLastCalledWith(expect.objectContaining({ logMessageField: expectedMessageField }));
+
+    fireEvent.change(levelField, { target: { value: expectedLevelField } });
+    expect(onChangeMock).toHaveBeenLastCalledWith(expect.objectContaining({ logLevelField: expectedLevelField }));
   });
 });

public/app/plugins/datasource/elasticsearch/configuration/LogsConfig.tsx

@@ -1,6 +1,5 @@
 import React from 'react';
-import { LegacyForms } from '@grafana/ui';
+import { Input, InlineField, FieldSet } from '@grafana/ui';
-const { FormField } = LegacyForms;
 import { ElasticsearchOptions } from '../types';
 
 type Props = {
@@ -19,28 +18,25 @@ export const LogsConfig = (props: Props) => {
   };
 
   return (
-    <>
+    <FieldSet label="Logs">
-      <h3 className="page-heading">Logs</h3>
+      <InlineField label="Message field name" labelWidth={22}>
-
+        <Input
-      <div className="gf-form-group">
+          id="es_logs-config_logMessageField"
-        <div className="gf-form max-width-30">
-          <FormField
-            labelWidth={11}
-            label="Message field name"
           value={value.logMessageField}
           onChange={changeHandler('logMessageField')}
           placeholder="_source"
+          width={24}
         />
-        </div>
+      </InlineField>
-        <div className="gf-form max-width-30">
+
-          <FormField
+      <InlineField label="Level field name" labelWidth={22}>
-            labelWidth={11}
+        <Input
-            label="Level field name"
+          id="es_logs-config_logLevelField"
           value={value.logLevelField}
           onChange={changeHandler('logLevelField')}
+          width={24}
         />
-        </div>
+      </InlineField>
-      </div>
+    </FieldSet>
-    </>
   );
 };

public/app/plugins/datasource/elasticsearch/configuration/mocks.ts

@@ -2,7 +2,9 @@ import { DataSourceSettings } from '@grafana/data';
 import { ElasticsearchOptions } from '../types';
 import { createDatasourceSettings } from '../../../../features/datasources/mocks';
 
-export function createDefaultConfigOptions(): DataSourceSettings<ElasticsearchOptions> {
+export function createDefaultConfigOptions(
+  options?: Partial<ElasticsearchOptions>
+): DataSourceSettings<ElasticsearchOptions> {
   return createDatasourceSettings<ElasticsearchOptions>({
     timeField: '@time',
     esVersion: '7.0.0',
@@ -11,5 +13,6 @@ export function createDefaultConfigOptions(): DataSourceSettings<ElasticsearchOp
     maxConcurrentShardRequests: 300,
     logMessageField: 'test.message',
     logLevelField: 'test.level',
+    ...options,
   });
 }

public/app/plugins/datasource/testdata/QueryEditor.test.tsx

@@ -37,7 +37,7 @@ describe('Test Datasource Query Editor', () => {
   it('should switch scenario and display its default values', async () => {
     const { rerender } = setup();
 
-    let select = (await screen.findByText('Scenario')).nextSibling!;
+    let select = (await screen.findByText('Scenario')).nextSibling!.firstChild!;
     await fireEvent.keyDown(select, { keyCode: 40 });
     const scs = screen.getAllByLabelText('Select option');