Make engine widget accessible (#19988)

.storybook/main.js

@@ -21,6 +21,7 @@ module.exports = {
     resolve: {
       ...storybookConfig.resolve,
       alias: appConfig.resolve.alias,
+      extensions: appConfig.resolve.extensions,
     },
   }),
 };

frontend/src/metabase/admin/databases/components/widgets/EngineWidget/EngineWidget.styled.jsx → frontend/src/metabase/admin/databases/components/widgets/EngineWidget/EngineWidget.styled.tsx

 import styled from "styled-components";
 import { color, lighten } from "metabase/lib/colors";
 import { breakpointMinSmall } from "metabase/styled-components/theme";
-import Icon from "metabase/components/Icon";
-import IconButtonWrapper from "metabase/components/IconButtonWrapper";
 import Button from "metabase/core/components/Button";
+import Icon from "metabase/components/Icon";
 
 export const EngineSearchRoot = styled.div`
   display: block;
 `;
 
-export const EngineListRoot = styled.div`
+export const EngineListRoot = styled.ul`
   display: grid;
   grid-template-columns: 1fr;
   gap: 1.5rem;
@@ -20,13 +19,24 @@ export const EngineListRoot = styled.div`
   }
 `;
 
-export const EngineCardRoot = styled(IconButtonWrapper)`
+export interface EngineCardRootProps {
+  isActive: boolean;
+}
+
+export const EngineCardRoot = styled.li<EngineCardRootProps>`
+  display: flex;
   flex: 1 1 auto;
   flex-direction: column;
+  align-items: center;
+  justify-content: center;
   height: 5.375rem;
   padding: 1rem;
   border: 1px solid ${color("bg-medium")};
+  border-radius: 0.375rem;
   background-color: ${color("white")};
+  cursor: pointer;
+  outline: ${props =>
+    props.isActive ? `2px solid ${color("brand-light")}` : ""};
 
   &:hover {
     border-color: ${color("brand")};
@@ -73,26 +83,44 @@ export const EngineEmptyText = styled.div`
   text-align: center;
 `;
 
-export const EngineExpandButton = styled(Button)`
-  width: 100%;
-`;
-
-export const EngineInfoRoot = styled.div`
+export const EngineButtonRoot = styled.button`
   display: flex;
+  justify-content: space-between;
   align-items: center;
   color: ${color("white")};
+  width: 100%;
   padding: 0.75rem;
   border-radius: 0.5rem;
   border: 1px solid ${color("brand")};
   background-color: ${color("brand")};
+  transition: all 200ms linear;
+  transition-property: color, background-color;
+  cursor: pointer;
+
+  &:hover {
+    color: ${color("white")};
+    background-color: ${lighten("brand", 0.12)};
+  }
+
+  &:focus {
+    outline: 2px solid ${color("brand-light")};
+  }
+
+  &:focus:not(:focus-visible) {
+    outline: none;
+  }
 `;
 
-export const EngineInfoTitle = styled.div`
-  flex: 1 0 auto;
+export const EngineButtonTitle = styled.span`
+  flex: 0 1 auto;
   font-size: 1rem;
   font-weight: bold;
 `;
 
-export const EngineInfoIcon = styled(Icon)`
+export const EngineButtonIcon = styled(Icon)`
   cursor: pointer;
 `;
+
+export const EngineToggleRoot = styled(Button)`
+  width: 100%;
+`;

frontend/src/metabase/admin/databases/components/widgets/EngineWidget/EngineWidget.jsx → frontend/src/metabase/admin/databases/components/widgets/EngineWidget/EngineWidget.tsx

-import React, { useCallback, useMemo, useState } from "react";
-import PropTypes from "prop-types";
+import React, { KeyboardEvent, useCallback, useMemo, useState } from "react";
 import { jt, t } from "ttag";
+import _ from "underscore";
 import { getEngineLogo } from "metabase/lib/engine";
 import Settings from "metabase/lib/settings";
 import TextInput from "metabase/components/TextInput";
 import ExternalLink from "metabase/core/components/ExternalLink";
 import {
+  EngineButtonIcon,
+  EngineButtonRoot,
+  EngineButtonTitle,
   EngineCardIcon,
   EngineCardImage,
   EngineCardRoot,
@@ -13,59 +16,75 @@ import {
   EngineEmptyIcon,
   EngineEmptyStateRoot,
   EngineEmptyText,
-  EngineExpandButton,
-  EngineInfoIcon,
-  EngineInfoRoot,
-  EngineInfoTitle,
   EngineListRoot,
   EngineSearchRoot,
+  EngineToggleRoot,
 } from "./EngineWidget.styled";
+import { EngineField, EngineOption } from "./types";
 
 const DEFAULT_OPTIONS_COUNT = 6;
 
-const EngineWidget = ({ field, options, isHosted }) => {
+export interface EngineWidget {
+  field: EngineField;
+  options: EngineOption[];
+  isHosted?: boolean;
+}
+
+const EngineWidget = ({
+  field,
+  options,
+  isHosted,
+}: EngineWidget): JSX.Element => {
   if (field.value) {
-    return <EngineInfo field={field} options={options} />;
+    return <EngineButton field={field} options={options} />;
   } else {
     return <EngineSearch field={field} options={options} isHosted={isHosted} />;
   }
 };
 
-EngineWidget.propTypes = {
-  field: PropTypes.object.isRequired,
-  options: PropTypes.array.isRequired,
-  isHosted: PropTypes.bool,
-};
+interface EngineButtonProps {
+  field: EngineField;
+  options: EngineOption[];
+}
 
-const EngineInfo = ({ field, options }) => {
+const EngineButton = ({ field, options }: EngineButtonProps): JSX.Element => {
   const option = options.find(option => option.value === field.value);
 
   const handleClick = useCallback(() => {
-    field.onChange(undefined);
+    field.onChange?.(undefined);
   }, [field]);
 
   return (
-    <EngineInfoRoot>
-      <EngineInfoTitle>{option ? option.name : field.value}</EngineInfoTitle>
-      <EngineInfoIcon
+    <EngineButtonRoot autoFocus onClick={handleClick}>
+      <EngineButtonTitle>
+        {option ? option.name : field.value}
+      </EngineButtonTitle>
+      <EngineButtonIcon
         name="close"
         size={18}
         aria-label={t`Remove database`}
-        onClick={handleClick}
       />
-    </EngineInfoRoot>
+    </EngineButtonRoot>
   );
 };
 
-EngineInfo.propTypes = {
-  field: PropTypes.object.isRequired,
-  options: PropTypes.array.isRequired,
-};
+interface EngineSearchProps {
+  field: EngineField;
+  options: EngineOption[];
+  isHosted?: boolean;
+}
 
-const EngineSearch = ({ field, options, isHosted }) => {
+const EngineSearch = ({
+  field,
+  options,
+  isHosted,
+}: EngineSearchProps): JSX.Element => {
+  const rootId = useMemo(() => _.uniqueId(), []);
   const [searchText, setSearchText] = useState("");
+  const [activeIndex, setActiveIndex] = useState<number>();
   const [isExpanded, setIsExpanded] = useState(false);
   const isSearching = searchText.length > 0;
+  const isNavigating = activeIndex != null;
   const hasMoreOptions = options.length > DEFAULT_OPTIONS_COUNT;
 
   const sortedOptions = useMemo(() => {
@@ -77,15 +96,51 @@ const EngineSearch = ({ field, options, isHosted }) => {
     [sortedOptions, isExpanded, isSearching, searchText],
   );
 
+  const optionCount = visibleOptions.length;
+  const activeOption = isNavigating ? visibleOptions[activeIndex] : undefined;
+
+  const handleSearch = useCallback((value: string) => {
+    setSearchText(value);
+    setActiveIndex(undefined);
+  }, []);
+
+  const handleKeyDown = useCallback(
+    (event: KeyboardEvent) => {
+      switch (event.key) {
+        case "Enter":
+          activeOption && field.onChange?.(activeOption.value);
+          event.preventDefault();
+          break;
+        case "ArrowUp":
+        case "ArrowDown":
+          setIsExpanded(true);
+          setActiveIndex(getActiveIndex(event.key, activeIndex, optionCount));
+          event.preventDefault();
+          break;
+      }
+    },
+    [field, activeIndex, activeOption, optionCount],
+  );
+
   return (
-    <EngineSearchRoot>
+    <EngineSearchRoot role="combobox" aria-expanded="true">
       <TextInput
         value={searchText}
         placeholder={t`Search for a database…`}
-        onChange={setSearchText}
+        autoFocus
+        aria-autocomplete="list"
+        aria-controls={getListBoxId(rootId)}
+        aria-activedescendant={getListOptionId(rootId, activeOption)}
+        onChange={handleSearch}
+        onKeyDown={handleKeyDown}
       />
       {visibleOptions.length ? (
-        <EngineList field={field} options={visibleOptions} />
+        <EngineList
+          rootId={rootId}
+          options={visibleOptions}
+          activeIndex={activeIndex}
+          onOptionChange={field.onChange}
+        />
       ) : (
         <EngineEmptyState isHosted={isHosted} />
       )}
@@ -99,36 +154,60 @@ const EngineSearch = ({ field, options, isHosted }) => {
   );
 };
 
-EngineSearch.propTypes = {
-  field: PropTypes.object.isRequired,
-  options: PropTypes.array.isRequired,
-  isHosted: PropTypes.bool,
-};
+interface EngineListProps {
+  rootId: string;
+  options: EngineOption[];
+  activeIndex?: number;
+  onOptionChange?: (value: string) => void;
+}
 
-const EngineList = ({ field, options }) => {
+const EngineList = ({
+  rootId,
+  options,
+  activeIndex,
+  onOptionChange,
+}: EngineListProps): JSX.Element => {
   return (
-    <EngineListRoot>
-      {options.map(option => (
-        <EngineCard key={option.value} field={field} option={option} />
+    <EngineListRoot role="listbox" id={getListBoxId(rootId)}>
+      {options.map((option, optionIndex) => (
+        <EngineCard
+          key={option.value}
+          rootId={rootId}
+          option={option}
+          isActive={optionIndex === activeIndex}
+          onOptionChange={onOptionChange}
+        />
       ))}
     </EngineListRoot>
   );
 };
 
-EngineList.propTypes = {
-  field: PropTypes.object,
-  options: PropTypes.array,
-};
+export interface EngineCardProps {
+  rootId: string;
+  option: EngineOption;
+  isActive: boolean;
+  onOptionChange?: (value: string) => void;
+}
 
-const EngineCard = ({ field, option }) => {
+const EngineCard = ({
+  rootId,
+  option,
+  isActive,
+  onOptionChange,
+}: EngineCardProps): JSX.Element => {
   const logo = getEngineLogo(option.value);
 
   const handleClick = useCallback(() => {
-    field.onChange(option.value);
-  }, [field, option]);
+    onOptionChange?.(option.value);
+  }, [option, onOptionChange]);
 
   return (
-    <EngineCardRoot key={option.value} onClick={handleClick}>
+    <EngineCardRoot
+      role="option"
+      id={getListOptionId(rootId, option)}
+      isActive={isActive}
+      onClick={handleClick}
+    >
       {logo ? (
         <EngineCardImage src={logo} />
       ) : (
@@ -139,12 +218,11 @@ const EngineCard = ({ field, option }) => {
   );
 };
 
-EngineCard.propTypes = {
-  field: PropTypes.object,
-  option: PropTypes.object,
-};
+interface EngineEmptyStateProps {
+  isHosted?: boolean;
+}
 
-const EngineEmptyState = ({ isHosted }) => {
+const EngineEmptyState = ({ isHosted }: EngineEmptyStateProps): JSX.Element => {
   return (
     <EngineEmptyStateRoot>
       <EngineEmptyIcon name="search" size={32} />
@@ -152,7 +230,10 @@ const EngineEmptyState = ({ isHosted }) => {
         <EngineEmptyText>{t`Didn’t find anything`}</EngineEmptyText>
       ) : (
         <EngineEmptyText>{jt`Don’t see your database? Check out our ${(
-          <ExternalLink href={Settings.docsUrl("developers-guide-drivers")}>
+          <ExternalLink
+            key="link"
+            href={Settings.docsUrl("developers-guide-drivers")}
+          >
             {t`Community Drivers`}
           </ExternalLink>
         )} page to see if it’s available for self-hosting.`}</EngineEmptyText>
@@ -161,32 +242,31 @@ const EngineEmptyState = ({ isHosted }) => {
   );
 };
 
-EngineEmptyState.propTypes = {
-  isHosted: PropTypes.bool,
-};
+interface EngineToggleProps {
+  isExpanded: boolean;
+  onExpandedChange: (isExpanded: boolean) => void;
+}
 
-const EngineToggle = ({ isExpanded, onExpandedChange }) => {
+const EngineToggle = ({
+  isExpanded,
+  onExpandedChange,
+}: EngineToggleProps): JSX.Element => {
   const handleClick = useCallback(() => {
     onExpandedChange(!isExpanded);
   }, [isExpanded, onExpandedChange]);
 
   return (
-    <EngineExpandButton
+    <EngineToggleRoot
       primary
       iconRight={isExpanded ? "chevronup" : "chevrondown"}
       onClick={handleClick}
     >
-      {isExpanded ? t`Show less options` : t`Show more options`}
-    </EngineExpandButton>
+      {isExpanded ? t`Show fewer options` : t`Show more options`}
+    </EngineToggleRoot>
   );
 };
 
-EngineToggle.propTypes = {
-  isExpanded: PropTypes.bool,
-  onExpandedChange: PropTypes.func,
-};
-
-const getSortedOptions = options => {
+const getSortedOptions = (options: EngineOption[]): EngineOption[] => {
   return options.sort((a, b) => {
     if (a.index >= 0 && b.index >= 0) {
       return a.index - b.index;
@@ -200,7 +280,12 @@ const getSortedOptions = options => {
   });
 };
 
-const getVisibleOptions = (options, isExpanded, isSearching, searchText) => {
+const getVisibleOptions = (
+  options: EngineOption[],
+  isExpanded: boolean,
+  isSearching: boolean,
+  searchText: string,
+): EngineOption[] => {
   if (isSearching) {
     return options.filter(e => includesIgnoreCase(e.name, searchText));
   } else if (isExpanded) {
@@ -210,8 +295,45 @@ const getVisibleOptions = (options, isExpanded, isSearching, searchText) => {
   }
 };
 
-const includesIgnoreCase = (sourceText, searchText) => {
+const includesIgnoreCase = (
+  sourceText: string,
+  searchText: string,
+): boolean => {
   return sourceText.toLowerCase().includes(searchText.toLowerCase());
 };
 
+const getListBoxId = (rootId: string): string => {
+  return `${rootId}-listbox`;
+};
+
+const getListOptionId = (
+  rootId: string,
+  option?: EngineOption,
+): string | undefined => {
+  return option ? `${rootId}-option-${option.value}` : undefined;
+};
+
+const getActiveIndex = (
+  key: string,
+  activeIndex: number | undefined,
+  optionCount: number,
+): number | undefined => {
+  switch (key) {
+    case "ArrowDown":
+      if (activeIndex != null) {
+        return Math.min(activeIndex + 1, optionCount - 1);
+      } else {
+        return 0;
+      }
+    case "ArrowUp":
+      if (activeIndex != null) {
+        return Math.max(activeIndex - 1, 0);
+      } else {
+        return optionCount;
+      }
+    default:
+      return activeIndex;
+  }
+};
+
 export default EngineWidget;

frontend/src/metabase/admin/databases/components/widgets/EngineWidget/EngineWidget.unit.spec.js → frontend/src/metabase/admin/databases/components/widgets/EngineWidget/EngineWidget.unit.spec.tsx

 import React from "react";
 import { render, screen } from "@testing-library/react";
-import userEvent from "@testing-library/user-event";
+import userEvent, { specialChars } from "@testing-library/user-event";
 import EngineWidget from "./EngineWidget";
+import { EngineField, EngineOption } from "./types";
 
 describe("EngineWidget", () => {
   it("should allow choosing a database", () => {
@@ -35,7 +36,7 @@ describe("EngineWidget", () => {
     expect(screen.getByText("H2")).toBeInTheDocument();
     expect(screen.getByText("Presto")).toBeInTheDocument();
 
-    userEvent.click(screen.getByText("Show less options"));
+    userEvent.click(screen.getByText("Show fewer options"));
     expect(screen.getByText("MySQL")).toBeInTheDocument();
     expect(screen.getByText("H2")).toBeInTheDocument();
     expect(screen.queryByText("Presto")).not.toBeInTheDocument();
@@ -74,14 +75,29 @@ describe("EngineWidget", () => {
 
     expect(screen.getByText(/Didn’t find anything/)).toBeInTheDocument();
   });
+
+  it("should allow selection via keyboard", () => {
+    const field = getField();
+    const options = getOptions();
+
+    render(<EngineWidget field={field} options={options} />);
+
+    const input = screen.getByRole("textbox");
+    userEvent.type(input, "sql");
+    userEvent.type(input, specialChars.arrowDown);
+    userEvent.type(input, specialChars.arrowDown);
+    userEvent.type(input, specialChars.enter);
+
+    expect(field.onChange).toHaveBeenCalledWith("postgres");
+  });
 });
 
-const getField = value => ({
+const getField = (value?: string): EngineField => ({
   value,
   onChange: jest.fn(),
 });
 
-const getOptions = () => [
+const getOptions = (): EngineOption[] => [
   {
     name: "MySQL",
     value: "mysql",

frontend/src/metabase/admin/databases/components/widgets/EngineWidget/types.ts

+export interface EngineField {
+  value?: string;
+  onChange?: (value: string | undefined) => void;
+}
+
+export interface EngineOption {
+  name: string;
+  value: string;
+  index: number;
+}