Accessibility improvements (#2071)

* workaround for mui select focusVisible issue

tmp

* use legend for checkboxfield overall description

* make focus outlines a bit more visible

* add a comment about the typecasting

Author: ChristopherChudzicki

frontends/main/src/app-pages/DashboardPage/DashboardPage.tsx

@@ -171,6 +171,10 @@ const TabsContainer = styled(TabList)(({ theme }) => ({
   a: {
     padding: "0",
     opacity: "1",
+
+    "&:focus-visible": {
+      outlineOffset: "-1px",
+    },
   },
   "&:hover": {
     a: {

frontends/ol-components/package.json

@@ -20,9 +20,9 @@
     "@mitodl/smoot-design": "^3.3.0",
     "@mui/base": "5.0.0-beta.69",
     "@mui/lab": "6.0.0-beta.28",
-    "@mui/material": "^6.4.1",
-    "@mui/material-nextjs": "^6.3.1",
-    "@mui/system": "^6.4.1",
+    "@mui/material": "^6.4.5",
+    "@mui/material-nextjs": "^6.4.3",
+    "@mui/system": "^6.4.3",
     "@remixicon/react": "^4.2.0",
     "@testing-library/dom": "^10.4.0",
     "@types/react-dom": "^19",

frontends/ol-components/src/components/Checkbox/Checkbox.tsx

@@ -64,6 +64,11 @@ const Container = styled.div`
   && input[type="checkbox"] {
     margin: 0;
     margin-right: 4px;
+
+    /* Help avoid focus outline from being cutoff  */
+    :focus-visible {
+      outline-offset: -1px;
+    }
   }
 
   ${containerStyles}

frontends/ol-components/src/components/Checkbox/CheckboxChoiceField.tsx

@@ -36,7 +36,7 @@ const Label = styled(FormLabel)(({ theme }) => ({
   width: "100%",
   color: theme.custom.colors.darkGray2,
   ...theme.typography.subtitle2,
-}))
+})) as typeof FormLabel // https://mui.com/material-ui/guides/typescript/?srsltid=AfmBOoo9kvRiALbxt4kAarRGiKaiJ7tbui5tstoL23DYscJPyk6UaTul#complications-with-the-component-prop
 
 const CheckboxChoiceField: React.FC<CheckboxChoiceFieldProps> = ({
   label,
@@ -59,7 +59,7 @@ const CheckboxChoiceField: React.FC<CheckboxChoiceFieldProps> = ({
       className={className}
       disabled={disabled}
     >
-      {label && <Label>{label}</Label>}
+      {label && <Label component="legend">{label}</Label>}
       <_Container>
         {choices.map((choice) => {
           return (

frontends/ol-components/src/components/SelectField/SelectField.test.tsx

@@ -1,7 +1,8 @@
 import React from "react"
 import { screen } from "@testing-library/react"
-import { SelectField } from "./SelectField"
-import type { SelectFieldProps } from "./SelectField"
+import user from "@testing-library/user-event"
+import { Select, SelectField } from "./SelectField"
+import type { SelectFieldProps, SelectProps } from "./SelectField"
 
 import { faker } from "@faker-js/faker/locale/en"
 import MenuItem from "@mui/material/MenuItem"
@@ -39,3 +40,52 @@ describe("SelectField", () => {
     expect(input).toBeRequired()
   })
 })
+
+describe("Select", () => {
+  const setup = (props?: Partial<SelectProps>) => {
+    const defaults = {
+      name: "test-name",
+      value: "",
+      label: "test-label",
+    }
+    const { rerender: _rerender } = renderWithTheme(
+      <Select {...defaults} {...props}>
+        <MenuItem disabled value="">
+          Please select an op
+        </MenuItem>
+        <MenuItem value="value-1">Option 1</MenuItem>
+        <MenuItem value="value-2">Option 2</MenuItem>
+      </Select>,
+    )
+    const rerender = (newProps: Partial<SelectFieldProps>) => {
+      _rerender(<SelectField {...defaults} {...newProps} />)
+    }
+    return { rerender }
+  }
+
+  /**
+   * This test exists to ensure our workaround for
+   * https://github.com/mui/material-ui/issues/23747
+   * is behaving as expected.
+   */
+  it("Applies class 'pointer-open' to menu if and only if opened via pointer", async () => {
+    setup()
+    const select = screen.getByRole("combobox")
+    const getMenu = () => document.querySelector(".MuiMenu-root")
+
+    // Opened via pointer; has class pointer-open
+    await user.click(select)
+    expect(getMenu()).toHaveClass("pointer-open")
+    expect(document.activeElement).toHaveTextContent("Option 1")
+
+    // close it
+    await user.keyboard("{Escape}")
+    expect(getMenu()).toBe(null)
+    expect(document.activeElement).toBe(select)
+
+    // open via keyboard, does NOT have class pointer-open
+    await user.keyboard("{Enter}")
+    expect(getMenu()).not.toHaveClass("pointer-open")
+    expect(document.activeElement).toHaveTextContent("Option 1")
+  })
+})

frontends/ol-components/src/components/SelectField/SelectField.tsx

@@ -96,6 +96,7 @@ const SelectIcon = styled(RiArrowDownSLine)({
   width: "1em",
 })
 
+const POINTER_CLASSNAME = "pointer-open"
 /**
  * WARNING: You likely do not need this component. Try one of
  *
@@ -105,12 +106,39 @@ const SelectIcon = styled(RiArrowDownSLine)({
  * instead.
  */
 function Select<Value = unknown>({ size, ...props }: SelectProps<Value>) {
+  const menu = React.useRef<HTMLDivElement | null>(null)
   return (
     <MuiSelect
       variant="standard"
       {...props}
       size={size}
       IconComponent={SelectIcon}
+      /**
+       * The next three properties are a workaround to deal with
+       * https://github.com/mui/material-ui/issues/23747
+       *
+       * Note: Another workaround is mentioned in the issue, but breaks
+       * accessibility (https://github.com/mui/material-ui/issues/23747#issuecomment-2596590221)
+       */
+      onPointerDown={() => {
+        // This likely isn't necessasry---the Menu unmounts on close.
+        // But let's not rely on that.
+        menu.current?.classList.remove(POINTER_CLASSNAME)
+      }}
+      onPointerUp={() => {
+        menu.current?.classList.add(POINTER_CLASSNAME)
+      }}
+      MenuProps={{
+        ref: menu,
+        sx: {
+          [`&.${POINTER_CLASSNAME} .MuiMenuItem-root.Mui-focusVisible`]: {
+            outline: "none",
+          },
+        },
+        onKeyDown: () => {
+          menu.current?.classList.remove(POINTER_CLASSNAME)
+        },
+      }}
       input={<SelectInput size={size} />}
     />
   )

frontends/ol-components/src/components/SimpleSelect/SimpleSelect.tsx

@@ -75,7 +75,6 @@ const SimpleSelectField: React.FC<SimpleSelectFieldProps> = ({
 }) => {
   return (
     <SelectField {...others}>
-      <MenuItem style={{ display: "none" }}></MenuItem>
       {options.map(({ value, label, ...itemProps }) => (
         <MenuItem size={others.size} value={value} key={value} {...itemProps}>
           {label}