Merge pull request #17546 from luchillo17/fix/BCKSTG-152-keyboard-navigation---home---

Fix controlled Select keyboard open/close behavior

.changeset/stupid-seas-stare.md

@@ -0,0 +1,5 @@
+---
+'@backstage/core-components': patch
+---
+
+Fix accessibility issue on controlled select input on tab navigation + keyboard enter/space action.

packages/core-components/src/components/Select/Select.test.tsx

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import { fireEvent, render } from '@testing-library/react';
+import { fireEvent, render, within } from '@testing-library/react';
 import React from 'react';
 import { SelectComponent as Select } from './Select';
 
@@ -45,7 +45,7 @@ describe('<Select />', () => {
     expect(input.textContent).toBe('All results');
 
     // Simulate click on input
-    fireEvent.click(input);
+    fireEvent.mouseDown(within(input).getByRole('button'));
 
     expect(getByText('test 1')).toBeInTheDocument();
     const option = getByText('test 1');

packages/core-components/src/components/Select/Select.tsx

@@ -16,7 +16,6 @@
 import Box from '@material-ui/core/Box';
 import Checkbox from '@material-ui/core/Checkbox';
 import Chip from '@material-ui/core/Chip';
-import ClickAwayListener from '@material-ui/core/ClickAwayListener';
 import FormControl from '@material-ui/core/FormControl';
 import InputBase from '@material-ui/core/InputBase';
 import InputLabel from '@material-ui/core/InputLabel';
@@ -171,7 +170,7 @@ export function SelectComponent(props: SelectProps) {
     onChange(event.target.value as SelectedItems);
   };
 
-  const handleClick = (event: React.ChangeEvent<any>) => {
+  const handleOpen = (event: React.ChangeEvent<any>) => {
     if (disabled) {
       event.preventDefault();
       return;
@@ -184,7 +183,7 @@ export function SelectComponent(props: SelectProps) {
     });
   };
 
-  const handleClickAway = () => {
+  const handleClose = () => {
     setOpen(false);
   };
 
@@ -196,88 +195,85 @@ export function SelectComponent(props: SelectProps) {
 
   return (
     <Box className={classes.root}>
-      <ClickAwayListener onClickAway={handleClickAway}>
-        <FormControl className={classes.formControl}>
-          <InputLabel className={classes.formLabel}>{label}</InputLabel>
-          <Select
-            aria-label={label}
-            value={value}
-            native={native}
-            disabled={disabled}
-            data-testid="select"
-            displayEmpty
-            multiple={multiple}
-            margin={margin}
-            onChange={handleChange}
-            onClick={handleClick}
-            open={isOpen}
-            input={<BootstrapInput />}
-            label={label}
-            tabIndex={0}
-            renderValue={s =>
-              multiple && (value as any[]).length !== 0 ? (
-                <Box className={classes.chips}>
-                  {(s as string[]).map(selectedValue => (
-                    <Chip
-                      key={items.find(el => el.value === selectedValue)?.value}
-                      label={
-                        items.find(el => el.value === selectedValue)?.label
-                      }
-                      clickable
-                      onDelete={handleDelete(selectedValue)}
-                      className={classes.chip}
-                    />
-                  ))}
-                </Box>
-              ) : (
-                <Typography>
-                  {(value as any[]).length === 0
-                    ? placeholder || ''
-                    : items.find(el => el.value === s)?.label}
-                </Typography>
-              )
-            }
-            IconComponent={() =>
-              !isOpen ? <ClosedDropdown /> : <OpenedDropdown />
-            }
-            MenuProps={{
-              anchorOrigin: {
-                vertical: 'bottom',
-                horizontal: 'left',
-              },
-              transformOrigin: {
-                vertical: 'top',
-                horizontal: 'left',
-              },
-              getContentAnchorEl: null,
-            }}
-          >
-            {placeholder && !multiple && (
-              <MenuItem value={[]}>{placeholder}</MenuItem>
-            )}
-            {native
-              ? items &&
-                items.map(item => (
-                  <option value={item.value} key={item.value}>
-                    {item.label}
-                  </option>
-                ))
-              : items &&
-                items.map(item => (
-                  <MenuItem key={item.value} value={item.value}>
-                    {multiple && (
-                      <Checkbox
-                        color="primary"
-                        checked={(value as any[]).includes(item.value) || false}
-                        className={classes.checkbox}
-                      />
-                    )}
-                    {item.label}
-                  </MenuItem>
+      <FormControl className={classes.formControl}>
+        <InputLabel className={classes.formLabel}>{label}</InputLabel>
+        <Select
+          aria-label={label}
+          value={value}
+          native={native}
+          disabled={disabled}
+          data-testid="select"
+          displayEmpty
+          multiple={multiple}
+          margin={margin}
+          onChange={handleChange}
+          open={isOpen}
+          onOpen={handleOpen}
+          onClose={handleClose}
+          input={<BootstrapInput />}
+          label={label}
+          tabIndex={0}
+          renderValue={s =>
+            multiple && (value as any[]).length !== 0 ? (
+              <Box className={classes.chips}>
+                {(s as string[]).map(selectedValue => (
+                  <Chip
+                    key={items.find(el => el.value === selectedValue)?.value}
+                    label={items.find(el => el.value === selectedValue)?.label}
+                    clickable
+                    onDelete={handleDelete(selectedValue)}
+                    className={classes.chip}
+                  />
                 ))}
-          </Select>
-        </FormControl>
-      </ClickAwayListener>
+              </Box>
+            ) : (
+              <Typography>
+                {(value as any[]).length === 0
+                  ? placeholder || ''
+                  : items.find(el => el.value === s)?.label}
+              </Typography>
+            )
+          }
+          IconComponent={() =>
+            !isOpen ? <ClosedDropdown /> : <OpenedDropdown />
+          }
+          MenuProps={{
+            anchorOrigin: {
+              vertical: 'bottom',
+              horizontal: 'left',
+            },
+            transformOrigin: {
+              vertical: 'top',
+              horizontal: 'left',
+            },
+            getContentAnchorEl: null,
+          }}
+        >
+          {placeholder && !multiple && (
+            <MenuItem value={[]}>{placeholder}</MenuItem>
+          )}
+          {native
+            ? items &&
+              items.map(item => (
+                <option value={item.value} key={item.value}>
+                  {item.label}
+                </option>
+              ))
+            : items &&
+              items.map(item => (
+                <MenuItem key={item.value} value={item.value}>
+                  {multiple && (
+                    <Checkbox
+                      color="primary"
+                      checked={(value as any[]).includes(item.value) || false}
+                      className={classes.checkbox}
+                    />
+                  )}
+                  {item.label}
+                </MenuItem>
+              ))}
+        </Select>
+      </FormControl>
     </Box>
   );
 }

plugins/catalog-graph/src/components/CatalogGraphPage/CurveFilter.test.tsx

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import { render, waitFor, screen } from '@testing-library/react';
+import { render, waitFor, screen, within } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
 import React from 'react';
 import { CurveFilter } from './CurveFilter';
@@ -33,7 +33,9 @@ describe('<CurveFilter/>', () => {
 
     expect(screen.getByText('Step Before')).toBeInTheDocument();
 
-    await userEvent.click(screen.getByTestId('select'));
+    await userEvent.click(
+      within(screen.getByTestId('select')).getByRole('button'),
+    );
     await userEvent.click(screen.getByText('Monotone X'));
 
     await waitFor(() => {

plugins/catalog-graph/src/components/CatalogGraphPage/DirectionFilter.test.tsx

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import { render, waitFor, screen } from '@testing-library/react';
+import { render, waitFor, screen, within } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
 import React from 'react';
 import { Direction } from '../EntityRelationsGraph';
@@ -37,7 +37,9 @@ describe('<DirectionFilter/>', () => {
 
     expect(screen.getByText('Right to left')).toBeInTheDocument();
 
-    await userEvent.click(screen.getByTestId('select'));
+    await userEvent.click(
+      within(screen.getByTestId('select')).getByRole('button'),
+    );
     await userEvent.click(screen.getByText('Top to bottom'));
 
     await waitFor(() => {

plugins/catalog-react/src/components/EntityKindPicker/EntityKindPicker.test.tsx

@@ -19,7 +19,7 @@ import { Entity } from '@backstage/catalog-model';
 import { ApiProvider } from '@backstage/core-app-api';
 import { alertApiRef } from '@backstage/core-plugin-api';
 import { renderWithEffects, TestApiRegistry } from '@backstage/test-utils';
-import { fireEvent, waitFor, screen } from '@testing-library/react';
+import { fireEvent, waitFor, screen, within } from '@testing-library/react';
 import { capitalize } from 'lodash';
 import { default as React } from 'react';
 import { catalogApiRef } from '../../api';
@@ -87,7 +87,7 @@ describe('<EntityKindPicker/>', () => {
     expect(screen.getByText('Kind')).toBeInTheDocument();
 
     const input = screen.getByTestId('select');
-    fireEvent.click(input);
+    fireEvent.mouseDown(within(input).getByRole('button'));
 
     await waitFor(() => screen.getByText('Domain'));
 
@@ -115,7 +115,7 @@ describe('<EntityKindPicker/>', () => {
       </ApiProvider>,
     );
     const input = screen.getByTestId('select');
-    fireEvent.click(input);
+    fireEvent.mouseDown(within(input).getByRole('button'));
 
     await waitFor(() => screen.getByText('Domain'));
     fireEvent.click(screen.getByText('Domain'));
@@ -171,7 +171,7 @@ describe('<EntityKindPicker/>', () => {
     );
 
     const input = screen.getByTestId('select');
-    fireEvent.click(input);
+    fireEvent.mouseDown(within(input).getByRole('button'));
 
     expect(
       screen.getByRole('option', { name: 'Component' }),
@@ -196,7 +196,7 @@ describe('<EntityKindPicker/>', () => {
     expect(screen.getByText('Frob')).toBeInTheDocument();
 
     const input = screen.getByTestId('select');
-    fireEvent.click(input);
+    fireEvent.mouseDown(within(input).getByRole('button'));
     expect(screen.getByRole('option', { name: 'Domain' })).toBeInTheDocument();
   });
 });

plugins/catalog-react/src/components/EntityTypePicker/EntityTypePicker.test.tsx

@@ -15,7 +15,7 @@
  */
 
 import React from 'react';
-import { fireEvent, waitFor, screen } from '@testing-library/react';
+import { fireEvent, waitFor, screen, within } from '@testing-library/react';
 import { Entity } from '@backstage/catalog-model';
 import { EntityTypePicker } from './EntityTypePicker';
 import { MockEntityListContextProvider } from '../../testUtils/providers';
@@ -95,7 +95,7 @@ describe('<EntityTypePicker/>', () => {
     expect(screen.getByText('Type')).toBeInTheDocument();
 
     const input = screen.getByTestId('select');
-    fireEvent.click(input);
+    fireEvent.mouseDown(within(input).getByRole('button'));
 
     await waitFor(() => screen.getByText('service'));
 
@@ -119,7 +119,7 @@ describe('<EntityTypePicker/>', () => {
       </ApiProvider>,
     );
     const input = screen.getByTestId('select');
-    fireEvent.click(input);
+    fireEvent.mouseDown(within(input).getByRole('button'));
 
     await waitFor(() => screen.getByText('service'));
     fireEvent.click(screen.getByText('service'));
@@ -128,7 +128,7 @@ describe('<EntityTypePicker/>', () => {
       type: new EntityTypeFilter(['service']),
     });
 
-    fireEvent.click(input);
+    fireEvent.mouseDown(within(input).getByRole('button'));
     fireEvent.click(screen.getByText('all'));
 
     expect(updateFilters).toHaveBeenLastCalledWith({ type: undefined });

plugins/xcmetrics/src/components/BuildListFilter/BuildListFilter.test.tsx

@@ -18,7 +18,7 @@ import { renderInTestApp, TestApiProvider } from '@backstage/test-utils';
 import userEvent from '@testing-library/user-event';
 import { BuildListFilter } from './BuildListFilter';
 import { BuildFilters, xcmetricsApiRef } from '../../api';
-import { RenderResult } from '@testing-library/react';
+import { RenderResult, within } from '@testing-library/react';
 
 jest.mock('../../api/XcmetricsClient');
 const client = require('../../api/XcmetricsClient');
@@ -50,13 +50,13 @@ const renderWithFiltersVisible = async (
 
 const setStatusFilter = async (rendered: RenderResult, option: string) => {
   const statusSelect = rendered.getAllByTestId('select')[0];
-  await userEvent.click(statusSelect);
+  await userEvent.click(within(statusSelect).getByRole('button'));
   await userEvent.click((await rendered.findAllByText(option))[0]);
 };
 
 const setProjectFilter = async (rendered: RenderResult, option: string) => {
   const statusSelect = rendered.getAllByTestId('select')[1];
-  await userEvent.click(statusSelect);
+  await userEvent.click(within(statusSelect).getByRole('button'));
   const options = await rendered.findAllByText(option);
   await userEvent.click(options[options.length - 1]);
 };