Grafana-UI: Add id to Select to make it easier to test (#31230)

* Prettier formatting

* Grafana-UI: Add support for id and inputId props to Select

* Grafana-UI: Add aria-label to Select

* Grafana-UI: InlineField and Field get ID from inputId prop

For Select

* Fix tests using TagFilter

* Update Select prop documentation

* Update Field tests to use screen instead

* Fix the last few tests

package.json

@@ -175,6 +175,7 @@
     "postcss-reporter": "6.0.1",
     "prettier": "2.2.1",
     "react-hot-loader": "4.8.0",
+    "react-select-event": "^5.1.0",
     "react-test-renderer": "16.12.0",
     "redux-mock-store": "1.5.4",
     "regexp-replace-loader": "1.0.1",
@@ -216,7 +217,6 @@
     "@types/react-virtualized-auto-sizer": "1.0.0",
     "@types/sockjs-client": "^1.1.1",
     "@types/uuid": "8.3.0",
-    "@types/hoist-non-react-statics": "3.3.1",
     "@welldone-software/why-did-you-render": "4.0.6",
     "abortcontroller-polyfill": "1.4.0",
     "angular": "1.8.2",

packages/grafana-ui/src/components/Forms/Field.test.tsx

@@ -0,0 +1,37 @@
+import React from 'react';
+import { render, screen } from '@testing-library/react';
+import { Input } from '../Input/Input';
+import { Field } from './Field';
+import { Select } from '../Select/Select';
+
+describe('Field', () => {
+  it('renders the label', () => {
+    render(
+      <Field label="My label">
+        <Input id="my-text-input" />
+      </Field>
+    );
+
+    expect(screen.getByText('My label')).toBeInTheDocument();
+  });
+
+  it('renders with the id of its children', () => {
+    render(
+      <Field label="My label">
+        <Input id="my-text-input" />
+      </Field>
+    );
+
+    expect(screen.getByLabelText('My label')).toBeInTheDocument();
+  });
+
+  it('renders with the inputId of its children', () => {
+    render(
+      <Field label="My other label">
+        <Select inputId="my-select-input" onChange={() => {}} />
+      </Field>
+    );
+
+    expect(screen.getByLabelText('My other label')).toBeInTheDocument();
+  });
+});

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

@@ -4,6 +4,7 @@ import { stylesFactory, useTheme } from '../../themes';
 import { css, cx } from 'emotion';
 import { GrafanaTheme } from '@grafana/data';
 import { FieldValidationMessage } from './FieldValidationMessage';
+import { getChildId } from '../../utils/children';
 
 export interface FieldProps extends HTMLAttributes<HTMLDivElement> {
   /** Form input element, i.e Input or Switch */
@@ -62,16 +63,9 @@ export const Field: React.FC<FieldProps> = ({
   ...otherProps
 }) => {
   const theme = useTheme();
-  let inputId;
   const styles = getFieldStyles(theme);
+  const inputId = getChildId(children);
 
-  // Get the first, and only, child to retrieve form input's id
-  const child = React.Children.map(children, (c) => c)[0];
-
-  if (child) {
-    // Retrieve input's id to apply on the label for correct click interaction
-    inputId = (child as React.ReactElement<{ id?: string }>).props.id;
-  }
   const labelElement =
     typeof label === 'string' ? (
       <Label htmlFor={inputId} description={description}>

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

@@ -0,0 +1,37 @@
+import React from 'react';
+import { render, screen } from '@testing-library/react';
+import { Input } from '../Input/Input';
+import { InlineField } from './InlineField';
+import { Select } from '../Select/Select';
+
+describe('InlineField', () => {
+  it('renders the label', () => {
+    render(
+      <InlineField label="My label">
+        <Input id="my-text-input" />
+      </InlineField>
+    );
+
+    expect(screen.getByText('My label')).toBeInTheDocument();
+  });
+
+  it('renders with the id of its children', () => {
+    render(
+      <InlineField label="My label">
+        <Input id="my-text-input" />
+      </InlineField>
+    );
+
+    expect(screen.getByLabelText('My label')).toBeInTheDocument();
+  });
+
+  it('renders with the inputId of its children', () => {
+    render(
+      <InlineField label="My other label">
+        <Select inputId="my-select-input" onChange={() => {}} />
+      </InlineField>
+    );
+
+    expect(screen.getByLabelText('My other label')).toBeInTheDocument();
+  });
+});

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

@@ -5,6 +5,7 @@ import { useTheme } from '../../themes';
 import { InlineLabel } from './InlineLabel';
 import { PopoverContent } from '../Tooltip/Tooltip';
 import { FieldProps } from './Field';
+import { getChildId } from '../../utils/children';
 
 export interface Props extends Omit<FieldProps, 'css' | 'horizontal' | 'description' | 'error'> {
   /** Content for the label's tooltip */
@@ -32,12 +33,8 @@ export const InlineField: FC<Props> = ({
 }) => {
   const theme = useTheme();
   const styles = getStyles(theme, grow);
-  const child = React.Children.only(children);
+  const inputId = getChildId(children);
-  let inputId;
 
-  if (child) {
-    inputId = (child as React.ReactElement<{ id?: string }>).props.id;
-  }
   const labelElement =
     typeof label === 'string' ? (
       <InlineLabel width={labelWidth} tooltip={tooltip} htmlFor={inputId} transparent={transparent}>

packages/grafana-ui/src/components/Select/Select.mdx

@@ -1,6 +1,6 @@
-import { Props, Preview } from "@storybook/addon-docs/blocks";
+import { Props, Preview } from '@storybook/addon-docs/blocks';
-import { Select, AsyncSelect, MultiSelect, AsyncMultiSelect } from "./Select";
+import { Select, AsyncSelect, MultiSelect, AsyncMultiSelect } from './Select';
-import { generateOptions } from "./mockOptions";
+import { generateOptions } from './mockOptions';
 
 # Select variants
 
@@ -27,13 +27,13 @@ There are four properties for each option:
 
 ```jsx
 const options = [
-  { label: "Basic option", value: 0 },
+  { label: 'Basic option', value: 0 },
-  { label: "Option with description", value: 1, description: "this is a description" },
+  { label: 'Option with description', value: 1, description: 'this is a description' },
   {
-    label: "Option with description and image",
+    label: 'Option with description and image',
     value: 2,
-    description: "This is a very elaborate description, describing all the wonders in the world.",
+    description: 'This is a very elaborate description, describing all the wonders in the world.',
-    imgUrl: "https://placekitten.com/40/40",
+    imgUrl: 'https://placekitten.com/40/40',
   },
 ];
 ```
@@ -90,7 +90,7 @@ Where the async function could look like this:
 
 ```tsx
 const loadAsyncOptions = () => {
-  return new Promise<Array<SelectableValue<string>>>(resolve => {
+  return new Promise<Array<SelectableValue<string>>>((resolve) => {
     setTimeout(() => {
       resolve(options);
     }, 2000);
@@ -103,7 +103,7 @@ const loadAsyncOptions = () => {
 Possible to Select multiple values at the same time.
 
 ```tsx
-import { MultiSelect } from "@grafana/ui";
+import { MultiSelect } from '@grafana/ui';
 
 const multiSelect = () => {
   const [value, setValue] = useState<Array<SelectableValue<string>>>([]);
@@ -113,7 +113,7 @@ const multiSelect = () => {
       <MultiSelect
         options={options}
         value={value}
-        onChange={v => {
+        onChange={(v) => {
           setValue(v);
         }}
       />
@@ -126,6 +126,36 @@ const multiSelect = () => {
 
 Like MultiSelect but handles data asynchronously with the `loadOptions` prop.
 
-## Props
+# Testing
+
+Using React Testing Library, you can select the `<Select />` using its matching label, such as the label assigned with the `inputId` prop. Use the `react-select-event` package to select values from the options.
+
+```tsx
+import { render, screen } from '@testing-library/react';
+import selectEvent from 'react-select-event';
+import { Select } from '@grafana/ui';
+
+it('should call onChange', () => {
+  const onChange = jest.fn();
+
+  render(
+    <>
+      <label htmlFor="my-select">My select</label>
+      <Select onChange={onChange} options={options} inputId="my-select" />
+    </>
+  );
+
+  const selectEl = screen.getByLabelText('My select');
+  expect(selectEl).toBeInTheDocument();
+
+  await selectEvent.select(selectEl, 'Option 2');
+  expect(onChange).toHaveBeenCalledWith({
+    label: 'Option 2',
+    value: 2,
+  });
+});
+```
+
+# Props
 
 <Props of={Select} />

packages/grafana-ui/src/components/Select/SelectBase.test.tsx

@@ -1,5 +1,7 @@
 import React from 'react';
 import { mount, ReactWrapper } from 'enzyme';
+import { render, screen } from '@testing-library/react';
+import selectEvent from 'react-select-event';
 import { SelectBase } from './SelectBase';
 import { SelectableValue } from '@grafana/data';
 import { MultiValueContainer } from './MultiValue';
@@ -28,6 +30,19 @@ describe('SelectBase', () => {
     expect(noopt).toHaveLength(1);
   });
 
+  it('is selectable via its label text', async () => {
+    const onChange = jest.fn();
+
+    render(
+      <>
+        <label htmlFor="my-select">My select</label>
+        <SelectBase onChange={onChange} options={options} inputId="my-select" />
+      </>
+    );
+
+    expect(screen.getByLabelText('My select')).toBeInTheDocument();
+  });
+
   describe('when openMenuOnFocus prop', () => {
     describe('is provided', () => {
       it('opens on focus', () => {
@@ -158,22 +173,23 @@ describe('SelectBase', () => {
 
   describe('options', () => {
     it('renders menu with provided options', () => {
-      const container = mount(<SelectBase options={options} onChange={onChangeHandler} isOpen />);
+      render(<SelectBase options={options} onChange={onChangeHandler} isOpen />);
-      const menuOptions = container.find({ 'aria-label': 'Select option' });
+      const menuOptions = screen.getAllByLabelText('Select option');
       expect(menuOptions).toHaveLength(2);
     });
-    it('call onChange handler when option is selected', () => {
+
+    it('call onChange handler when option is selected', async () => {
       const spy = jest.fn();
-      const handler = (value: SelectableValue<number>) => spy(value);
-      const container = mount(<SelectBase options={options} onChange={handler} isOpen />);
-      const menuOptions = container.find({ 'aria-label': 'Select option' });
-      expect(menuOptions).toHaveLength(2);
-      const menuOption = menuOptions.first();
-      menuOption.simulate('click');
 
-      expect(spy).toBeCalledWith({
+      render(<SelectBase onChange={spy} options={options} aria-label="My select" />);
-        label: 'Option 1',
+
-        value: 1,
+      const selectEl = screen.getByLabelText('My select');
+      expect(selectEl).toBeInTheDocument();
+
+      await selectEvent.select(selectEl, 'Option 2');
+      expect(spy).toHaveBeenCalledWith({
+        label: 'Option 2',
+        value: 2,
       });
     });
   });

packages/grafana-ui/src/components/Select/SelectBase.tsx

@@ -90,6 +90,7 @@ const CustomControl = (props: any) => {
 
 export function SelectBase<T>({
   allowCustomValue = false,
+  'aria-label': ariaLabel,
   autoFocus = false,
   backspaceRemovesValue = true,
   cacheOptions,
@@ -106,8 +107,10 @@ export function SelectBase<T>({
   inputValue,
   invalid,
   isClearable = false,
+  id,
   isLoading = false,
   isMulti = false,
+  inputId,
   isOpen,
   isOptionDisabled,
   isSearchable = true,
@@ -168,6 +171,7 @@ export function SelectBase<T>({
   }
 
   const commonSelectProps = {
+    'aria-label': ariaLabel,
     autoFocus,
     backspaceRemovesValue,
     captureMenuScroll: false,
@@ -181,10 +185,12 @@ export function SelectBase<T>({
     inputValue,
     invalid,
     isClearable,
+    id,
     // Passing isDisabled as react-select accepts this prop
     isDisabled: disabled,
     isLoading,
     isMulti,
+    inputId,
     isOptionDisabled,
     isSearchable,
     maxMenuHeight,

packages/grafana-ui/src/components/Select/types.ts

@@ -7,6 +7,8 @@ export type InputActionMeta = {
 };
 
 export interface SelectCommonProps<T> {
+  /** Aria label applied to the input field */
+  ['aria-label']?: string;
   allowCustomValue?: boolean;
   /** Focus is set to the Select when rendered*/
   autoFocus?: boolean;
@@ -18,15 +20,19 @@ export interface SelectCommonProps<T> {
   defaultValue?: any;
   disabled?: boolean;
   filterOption?: (option: SelectableValue, searchQuery: string) => boolean;
-  /**   Function for formatting the text that is displayed when creating a new value*/
+  /** Function for formatting the text that is displayed when creating a new value*/
   formatCreateLabel?: (input: string) => string;
   getOptionLabel?: (item: SelectableValue<T>) => React.ReactNode;
   getOptionValue?: (item: SelectableValue<T>) => string;
   inputValue?: string;
   invalid?: boolean;
   isClearable?: boolean;
+  /** The id to set on the SelectContainer component. To set the id for a label (with htmlFor), @see inputId instead */
+  id?: string;
   isLoading?: boolean;
   isMulti?: boolean;
+  /** The id of the search input. Use this to set a matching label with htmlFor */
+  inputId?: string;
   isOpen?: boolean;
   /** Disables the possibility to type into the input*/
   isSearchable?: boolean;

packages/grafana-ui/src/utils/children.ts

@@ -0,0 +1,19 @@
+import React, { ReactElement } from 'react';
+
+/** Returns the ID value of the first, and only, child element  */
+export function getChildId(children: ReactElement): string | undefined {
+  let inputId: unknown;
+
+  // Get the first, and only, child to retrieve form input's id
+  const child = React.Children.only(children);
+
+  // Retrieve input's id to apply on the label for correct click interaction
+  // For some components (like Select), we want to get the ID from a different prop
+  if ('id' in child?.props) {
+    inputId = child.props.id;
+  } else if ('inputId' in child.props) {
+    inputId = child?.props.inputId;
+  }
+
+  return typeof inputId === 'string' ? inputId : undefined;
+}

public/app/core/components/TagFilter/TagFilter.tsx

@@ -93,13 +93,13 @@ export const TagFilter: FC<Props> = ({
   };
 
   return (
-    <div className={styles.tagFilter} aria-label="Tag filter">
+    <div className={styles.tagFilter}>
       {isClearable && tags.length > 0 && (
         <span className={styles.clear} onClick={() => onTagChange([])}>
           Clear tags
         </span>
       )}
-      <AsyncSelect {...selectOptions} prefix={<Icon name="tag-alt" />} />
+      <AsyncSelect {...selectOptions} prefix={<Icon name="tag-alt" />} aria-label="Tag filter" />
     </div>
   );
 };

public/app/features/search/components/DashboardSearch.test.tsx

@@ -1,6 +1,6 @@
 import React from 'react';
 import { render, fireEvent, screen, waitFor, act } from '@testing-library/react';
-import userEvent from '@testing-library/user-event';
+import selectEvent from 'react-select-event';
 import * as SearchSrv from 'app/core/services/search_srv';
 import * as MockSearchSrv from 'app/core/services/__mocks__/search_srv';
 import { DashboardSearch, Props } from './DashboardSearch';
@@ -94,12 +94,9 @@ describe('DashboardSearch', () => {
     setup();
 
     await waitFor(() => screen.getByLabelText('Tag filter'));
-    // Get the actual element for the underlying Select component, since Select doesn't accept aria- props
-    const tagComponent = screen.getByLabelText('Tag filter').querySelector('div') as Node;
-    fireEvent.keyDown(tagComponent, { keyCode: 40 });
 
-    const firstTag = await screen.findByText('tag1');
+    const tagComponent = screen.getByLabelText('Tag filter');
-    userEvent.click(firstTag);
+    await selectEvent.select(tagComponent, 'tag1');
 
     expect(tagComponent).toBeInTheDocument();
 

public/app/features/search/components/SearchResultsFilter.test.tsx

@@ -1,5 +1,6 @@
 import React from 'react';
 import { fireEvent, render, screen } from '@testing-library/react';
+import selectEvent from 'react-select-event';
 import { Props, SearchResultsFilter } from './SearchResultsFilter';
 import { SearchLayout } from '../types';
 
@@ -80,9 +81,8 @@ describe('SearchResultsFilter', () => {
       query: { ...searchQuery, tag: [] },
     });
     const tagComponent = await screen.findByLabelText('Tag filter');
+    await selectEvent.select(tagComponent, 'tag1');
 
-    fireEvent.keyDown(tagComponent.querySelector('div') as Node, { keyCode: 40 });
-    fireEvent.click(await screen.findByText('tag1'));
     expect(mockFilterByTags).toHaveBeenCalledTimes(1);
     expect(mockFilterByTags).toHaveBeenCalledWith(['tag1']);
   });

yarn.lock

@@ -5725,6 +5725,20 @@
     resolve-from "^5.0.0"
     store2 "^2.7.1"
 
+"@testing-library/dom@>=7":
+  version "7.29.4"
+  resolved "https://registry.yarnpkg.com/@testing-library/dom/-/dom-7.29.4.tgz#1647c2b478789621ead7a50614ad81ab5ae5b86c"
+  integrity sha512-CtrJRiSYEfbtNGtEsd78mk1n1v2TUbeABlNIcOCJdDfkN5/JTOwQEbbQpoSRxGqzcWPgStMvJ4mNolSuBRv1NA==
+  dependencies:
+    "@babel/code-frame" "^7.10.4"
+    "@babel/runtime" "^7.12.5"
+    "@types/aria-query" "^4.2.0"
+    aria-query "^4.2.2"
+    chalk "^4.1.0"
+    dom-accessibility-api "^0.5.4"
+    lz-string "^1.4.4"
+    pretty-format "^26.6.2"
+
 "@testing-library/dom@^7.26.6":
   version "7.26.6"
   resolved "https://registry.yarnpkg.com/@testing-library/dom/-/dom-7.26.6.tgz#d558db63070a3acea5bea7e2497e631cd12541cc"
@@ -22310,6 +22324,13 @@ react-reverse-portal@^2.0.1:
   resolved "https://registry.yarnpkg.com/react-reverse-portal/-/react-reverse-portal-2.0.1.tgz#23b18292c531fb7b343d85a614c15a995838ba31"
   integrity sha512-sj/D9nSHspqV8i8hWkTSZ5Ohnrqk2A5fkDKw4Xe/zV4OfF1UYwmbzrxLdmNRdKkWgQwnXIxaa2E3FC7QYdZAeA==
 
+react-select-event@^5.1.0:
+  version "5.1.0"
+  resolved "https://registry.yarnpkg.com/react-select-event/-/react-select-event-5.1.0.tgz#d45ef68f2a9c872903e8c9725f3ae6e7576f7be0"
+  integrity sha512-D5DzJlYCdZsGbDVFMQFynrG0OLalJM3ZzDT7KQADNVWE604JCeQF9bIuvPZqVD7IzhnPsFzOUCsilzDA6w6WRQ==
+  dependencies:
+    "@testing-library/dom" ">=7"
+
 react-select@^3.0.8:
   version "3.2.0"
   resolved "https://registry.yarnpkg.com/react-select/-/react-select-3.2.0.tgz#de9284700196f5f9b5277c5d850a9ce85f5c72fe"