Select component a11y fixes (#2038)

* fixes disabled option colors for all browsers (Firefox had the most issues) * fixes custom arrow color for disabled state and Windows high contrast mode, and hacks around Firefox quirks * updates SelectInput stories to use FormControl * makes cursor behavior consistent for inputs * removes redundant ARIA attributes from <select> * updates tests and snapshots * adds changeset * fixes media query for forced colors (high contrast mode) * hacks around Firefox Windows high-contrast mode quirk * addresses a11y feedback * fixes linting
primer · May 4, 2022 · 1c2eeb9 · 1c2eeb9
1 parent 1bfaaa1
commit 1c2eeb9
Show file tree

Hide file tree

Showing 10 changed files with 631 additions and 101 deletions.
diff --git a/.changeset/cold-pillows-protect.md b/.changeset/cold-pillows-protect.md
@@ -0,0 +1,5 @@
+---
+'@primer/react': patch
+---
+
+Fixes accessibility bugs in the Select component.
diff --git a/docs/content/Select.mdx b/docs/content/Select.mdx
@@ -16,7 +16,7 @@ import {Select, Text} from '@primer/react'
 ```jsx live
 <FormControl>
   <FormControl.Label>Preferred Primer component interface</FormControl.Label>
-  <Select id="basic">
+  <Select>
     <Select.Option value="figma">Figma</Select.Option>
     <Select.Option value="css">Primer CSS</Select.Option>
     <Select.Option value="prc">Primer React components</Select.Option>
@@ -30,7 +30,7 @@ import {Select, Text} from '@primer/react'
 ```jsx live
 <FormControl>
   <FormControl.Label>Preferred Primer component interface</FormControl.Label>
-  <Select id="grouped">
+  <Select>
     <Select.OptGroup label="GUI">
       <Select.Option value="figma">Figma</Select.Option>
     </Select.OptGroup>
@@ -48,7 +48,7 @@ import {Select, Text} from '@primer/react'
 ```jsx live
 <FormControl>
   <FormControl.Label>Preferred Primer component interface</FormControl.Label>
-  <Select id="withPlaceholder" placeholder="Pick an interface">
+  <Select placeholder="Pick an interface">
     <Select.Option value="figma">Figma</Select.Option>
     <Select.Option value="css">Primer CSS</Select.Option>
     <Select.Option value="prc">Primer React components</Select.Option>

diff --git a/src/Select.tsx b/src/Select.tsx
@@ -1,6 +1,5 @@
 import React from 'react'
 import styled from 'styled-components'
-import {get} from './constants'
 import TextInputWrapper, {StyledWrapperProps} from './_TextInputWrapper'
 
 export type SelectProps = Omit<
@@ -10,19 +9,35 @@ export type SelectProps = Omit<
 
 const StyledSelect = styled.select`
   appearance: none;
-  background-color: transparent;
   border: 0;
   color: currentColor;
+  font-size: inherit;
   outline: none;
   width: 100%;
 
-  option {
-    color: initial;
+  /* Firefox hacks: */
+  /* 1. Makes Firefox's native dropdown menu's background match the theme.
+
+        background-color should be 'transparent', but Firefox uses the background-color on 
+        <select> to determine the background color used for the dropdown menu.
+  */
+  background-color: inherit;
+
+  /* 2. Prevents visible overlap of partially transparent background colors.
+  
+     'colors.input.disabledBg' happens to be partially transparent in light mode, so we use a
+     transparent background-color on a disabled <select>. */
+  &:disabled {
+    background-color: transparent;
   }
 
-  /* colors the select input's placeholder text */
-  &:invalid {
-    color: ${get('colors.fg.subtle')};
+  /* 3. Maintain dark bg color in Firefox on Windows high-contrast mode
+  
+     Firefox makes the <select>'s background color white when setting 'background-color: transparent;' */
+  @media screen and (forced-colors: active) {
+    &:disabled {
+      background-color: -moz-combobox;
+    }
   }
 `
 
@@ -44,18 +59,24 @@ const Select = React.forwardRef<HTMLSelectElement, SelectProps>(
   ({children, disabled, placeholder, size, required, validationStatus, ...rest}: SelectProps, ref) => (
     <TextInputWrapper
       sx={{
-        position: 'relative'
+        overflow: 'hidden',
+        position: 'relative',
+        '@media screen and (forced-colors: active)': {
+          svg: {
+            fill: disabled ? 'GrayText' : 'FieldText'
+          }
+        }
       }}
       size={size}
       validationStatus={validationStatus}
+      disabled={disabled}
     >
       <StyledSelect
         ref={ref}
-        required={required || Boolean(placeholder)}
+        required={required}
         disabled={disabled}
-        aria-required={required}
-        aria-disabled={disabled}
         aria-invalid={validationStatus === 'error' ? 'true' : 'false'}
+        data-hasplaceholder={Boolean(placeholder)}
         {...rest}
       >
         {placeholder && (

diff --git a/src/_TextInputWrapper.tsx b/src/_TextInputWrapper.tsx
@@ -93,11 +93,19 @@ export const TextInputBaseWrapper = styled.span<StyledBaseWrapperProps>`
   border-radius: ${get('radii.2')};
   outline: none;
   box-shadow: ${get('shadows.primer.shadow.inset')};
-  cursor: text;
   display: inline-flex;
   align-items: stretch;
   min-height: 32px;
 
+  input,
+  textarea {
+    cursor: text;
+  }
+
+  select {
+    cursor: pointer;
+  }
+
   &::placeholder {
     color: ${get('colors.fg.subtle')};
   }
@@ -125,10 +133,15 @@ export const TextInputBaseWrapper = styled.span<StyledBaseWrapperProps>`
   ${props =>
     props.disabled &&
     css`
-      cursor: not-allowed;
       color: ${get('colors.primer.fg.disabled')};
       background-color: ${get('colors.input.disabledBg')};
       border-color: ${get('colors.border.default')};
+
+      input,
+      textarea,
+      select {
+        cursor: not-allowed;
+      }
     `}
 
     ${props =>

diff --git a/src/__tests__/Select.test.tsx b/src/__tests__/Select.test.tsx
@@ -73,7 +73,7 @@ describe('Select', () => {
     const placeholderOption = getByText('Pick a choice')
     const select = getByLabelText('Choice')
 
-    expect(select).not.toHaveAttribute('aria-required')
+    expect(select).not.toHaveAttribute('required')
 
     expect(placeholderOption).toBeDefined()
     expect(placeholderOption.tagName.toLowerCase()).toBe('option')
@@ -102,7 +102,7 @@ describe('Select', () => {
     const placeholderOption = getByText('Pick a choice')
     const select = getByLabelText('Choice')
 
-    expect(select).toHaveAttribute('aria-required')
+    expect(select).toHaveAttribute('required')
 
     expect(placeholderOption).toBeDefined()
     expect(placeholderOption.tagName.toLowerCase()).toBe('option')
@@ -130,7 +130,7 @@ describe('Select', () => {
 
     const select = getByLabelText('Choice')
 
-    expect(select).toHaveAttribute('aria-disabled')
+    expect(select).toHaveAttribute('disabled')
     expect(select).toHaveAttribute('disabled')
   })
 })
diff --git a/src/__tests__/__snapshots__/Autocomplete.test.tsx.snap b/src/__tests__/__snapshots__/Autocomplete.test.tsx.snap
@@ -12,7 +12,6 @@ Array [
   border-radius: 6px;
   outline: none;
   box-shadow: inset 0 1px 0 rgba(208,215,222,0.2);
-  cursor: text;
   display: -webkit-inline-box;
   display: -webkit-inline-flex;
   display: -ms-inline-flexbox;
@@ -28,6 +27,15 @@ Array [
   padding-right: 0;
 }
 
+.c0 input,
+.c0 textarea {
+  cursor: text;
+}
+
+.c0 select {
+  cursor: pointer;
+}
+
 .c0::-webkit-input-placeholder {
   color: #6e7781;
 }
@@ -163,7 +171,6 @@ Array [
   border-radius: 6px;
   outline: none;
   box-shadow: inset 0 1px 0 rgba(208,215,222,0.2);
-  cursor: text;
   display: -webkit-inline-box;
   display: -webkit-inline-flex;
   display: -ms-inline-flexbox;
@@ -179,6 +186,15 @@ Array [
   padding-right: 0;
 }
 
+.c0 input,
+.c0 textarea {
+  cursor: text;
+}
+
+.c0 select {
+  cursor: pointer;
+}
+
 .c0::-webkit-input-placeholder {
   color: #6e7781;
 }
@@ -349,7 +365,6 @@ Array [
   border-radius: 6px;
   outline: none;
   box-shadow: inset 0 1px 0 rgba(208,215,222,0.2);
-  cursor: text;
   display: -webkit-inline-box;
   display: -webkit-inline-flex;
   display: -ms-inline-flexbox;
@@ -365,6 +380,15 @@ Array [
   padding-right: 0;
 }
 
+.c0 input,
+.c0 textarea {
+  cursor: text;
+}
+
+.c0 select {
+  cursor: pointer;
+}
+
 .c0::-webkit-input-placeholder {
   color: #6e7781;
 }
@@ -1298,7 +1322,6 @@ Array [
   border-radius: 6px;
   outline: none;
   box-shadow: inset 0 1px 0 rgba(208,215,222,0.2);
-  cursor: text;
   display: -webkit-inline-box;
   display: -webkit-inline-flex;
   display: -ms-inline-flexbox;
@@ -1314,6 +1337,15 @@ Array [
   padding-right: 0;
 }
 
+.c0 input,
+.c0 textarea {
+  cursor: text;
+}
+
+.c0 select {
+  cursor: pointer;
+}
+
 .c0::-webkit-input-placeholder {
   color: #6e7781;
 }
@@ -2157,7 +2189,6 @@ Array [
   border-radius: 6px;
   outline: none;
   box-shadow: inset 0 1px 0 rgba(208,215,222,0.2);
-  cursor: text;
   display: -webkit-inline-box;
   display: -webkit-inline-flex;
   display: -ms-inline-flexbox;
@@ -2173,6 +2204,15 @@ Array [
   padding-right: 0;
 }
 
+.c0 input,
+.c0 textarea {
+  cursor: text;
+}
+
+.c0 select {
+  cursor: pointer;
+}
+
 .c0::-webkit-input-placeholder {
   color: #6e7781;
 }
@@ -3027,7 +3067,6 @@ Array [
   border-radius: 6px;
   outline: none;
   box-shadow: inset 0 1px 0 rgba(208,215,222,0.2);
-  cursor: text;
   display: -webkit-inline-box;
   display: -webkit-inline-flex;
   display: -ms-inline-flexbox;
@@ -3043,6 +3082,15 @@ Array [
   padding-right: 0;
 }
 
+.c0 input,
+.c0 textarea {
+  cursor: text;
+}
+
+.c0 select {
+  cursor: pointer;
+}
+
 .c0::-webkit-input-placeholder {
   color: #6e7781;
 }
@@ -4027,7 +4075,6 @@ Array [
   border-radius: 6px;
   outline: none;
   box-shadow: inset 0 1px 0 rgba(208,215,222,0.2);
-  cursor: text;
   display: -webkit-inline-box;
   display: -webkit-inline-flex;
   display: -ms-inline-flexbox;
@@ -4043,6 +4090,15 @@ Array [
   padding-right: 0;
 }
 
+.c0 input,
+.c0 textarea {
+  cursor: text;
+}
+
+.c0 select {
+  cursor: pointer;
+}
+
 .c0::-webkit-input-placeholder {
   color: #6e7781;
 }