-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[a11y] Update dropdown component for keyboard usage #261
Changes from 8 commits
fc9ed72
2071c4c
24a06b9
b3fa7f9
aa637c0
579cba7
0282172
36a3349
f0566d3
ceeb947
fba381d
e027a6f
ebc41c2
0506d64
72e7a4e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,151 @@ | ||
import React from 'react'; | ||
import PropTypes from 'prop-types'; | ||
import { css } from '@emotion/core'; | ||
import useResetFocus from 'src/utils/accessibility/useResetFocus'; | ||
|
||
import OffClickWrapper from '../offClickWrapper'; | ||
import ChevronIcon from '../../svgs/icons/chevron-icon.svg'; | ||
import { | ||
DropdownContainer, | ||
dropdownInputStyle, | ||
IconContainer, | ||
DropdownOptionsContainer, | ||
DropdownOption, | ||
} from './style'; | ||
|
||
import { OptionType } from './index'; | ||
|
||
type DesktopDropdownProps = { | ||
value?: string; | ||
options: OptionType[]; | ||
currentOption: OptionType; | ||
textAlign: 'left' | 'center'; | ||
closeDropdown: () => void; | ||
onOptionClick: (event: any) => void; | ||
onSelectClick: () => void; | ||
isOpen: boolean; | ||
optionsContainerMaxHeight?: string; | ||
}; | ||
|
||
const DesktopDropdown = ({ | ||
value, | ||
options, | ||
textAlign, | ||
currentOption, | ||
closeDropdown, | ||
onOptionClick, | ||
onSelectClick, | ||
isOpen, | ||
optionsContainerMaxHeight, | ||
}: DesktopDropdownProps) => { | ||
const { initialFocus, resetFocus } = useResetFocus<HTMLDivElement>(); | ||
|
||
return ( | ||
<OffClickWrapper | ||
onOffClick={closeDropdown} | ||
css={css` | ||
width: 100%; | ||
`} | ||
> | ||
<DropdownContainer textAlign={textAlign}> | ||
<div | ||
id="select-input-box" | ||
onClick={onSelectClick} | ||
onKeyDown={(event: React.KeyboardEvent) => { | ||
ipc103 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (event.key === 'Enter') { | ||
onSelectClick(); | ||
} | ||
}} | ||
tabIndex={0} | ||
aria-label="Open dropdown option" | ||
aria-haspopup="listbox" | ||
role="button" | ||
ref={initialFocus} | ||
> | ||
<div css={dropdownInputStyle({ textAlign })}> | ||
{currentOption && currentOption.label} | ||
</div> | ||
<IconContainer> | ||
<ChevronIcon width={10} height={10} rotate={isOpen ? 90 : 0} /> | ||
</IconContainer> | ||
</div> | ||
|
||
<DropdownOptionsContainer | ||
isOpen={isOpen} | ||
optionsContainerMaxHeight={optionsContainerMaxHeight} | ||
role="listbox" | ||
aria-activedescendant={value} | ||
tabIndex={isOpen ? 0 : -1} | ||
aria-labelledby="select-input-box" | ||
> | ||
{options.map(option => { | ||
const { | ||
value: optionValue, disabled, label, ...rest | ||
} = option; | ||
|
||
return ( | ||
<DropdownOption | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did you play around at all with trying to achieve the (kind of) opposite of I also have not dug into the internals of all this as much as you have, so I'm not use if that kind of functionality is consistent with "best practices" or we'd be potentially introducing hacky behavior that we ought to achieve with other code. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a good idea - I'm going to write up a separate ticket for this and will investigate further. Looking at the |
||
key={optionValue} | ||
value={optionValue} | ||
selected={value === optionValue} | ||
disabled={!!disabled} | ||
onClick={onOptionClick} | ||
onKeyDown={event => { | ||
if (event.key === 'Enter') { | ||
onOptionClick(event); | ||
resetFocus(); | ||
} | ||
}} | ||
role="option" | ||
aria-selected={value === optionValue} | ||
tabIndex={isOpen && !disabled ? 0 : -1} | ||
// eslint-disable-next-line react/jsx-props-no-spreading | ||
{...rest} | ||
> | ||
{label} | ||
</DropdownOption> | ||
); | ||
})} | ||
</DropdownOptionsContainer> | ||
</DropdownContainer> | ||
</OffClickWrapper> | ||
); | ||
}; | ||
|
||
DesktopDropdown.defaultProps = { | ||
value: undefined, | ||
options: [{ value: null, label: '' }], | ||
currentOption: { value: null, label: '' }, | ||
textAlign: 'left', | ||
closeDropdown: () => undefined, | ||
onOptionClick: () => undefined, | ||
onSelectClick: () => undefined, | ||
isOpen: false, | ||
}; | ||
|
||
DesktopDropdown.propTypes = { | ||
// eslint-disable-next-line react/forbid-prop-types | ||
value: PropTypes.any, | ||
options: PropTypes.arrayOf( | ||
PropTypes.shape({ | ||
// eslint-disable-next-line react/forbid-prop-types | ||
value: PropTypes.any, | ||
label: PropTypes.string, | ||
disabled: PropTypes.bool, | ||
}), | ||
), | ||
textAlign: PropTypes.oneOf(['left', 'center']), | ||
currentOption: PropTypes.shape({ | ||
// eslint-disable-next-line react/forbid-prop-types | ||
value: PropTypes.any, | ||
label: PropTypes.string, | ||
disabled: PropTypes.bool, | ||
}), | ||
closeDropdown: PropTypes.func, | ||
ipc103 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
onOptionClick: PropTypes.func, | ||
onSelectClick: PropTypes.func, | ||
isOpen: PropTypes.bool, | ||
optionsContainerMaxHeight: PropTypes.string.isRequired, | ||
}; | ||
|
||
export default DesktopDropdown; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,20 @@ import MobileDropdown from './mobileDropdown'; | |
import DesktopDropdown from './desktopDropdown'; | ||
import allowNullPropType from '../../utils/allowNullPropType'; | ||
|
||
export type OptionType = { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This file is just TS conversion changes. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ipc103 I think we'll also want to convert this to a Function component. We're typing Fortunately the only non-render methods aren't lifecycle methods so it should be pretty straightforward to convert. |
||
value: string; | ||
label: string; | ||
disabled?: boolean; | ||
}; | ||
|
||
type DropdownProps = { | ||
value?: string; | ||
onChange: (option: OptionType) => void; | ||
options: OptionType[]; | ||
optionsContainerMaxHeight?: string; | ||
textAlign: 'left' | 'center'; | ||
ipc103 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}; | ||
|
||
const defaultProps = { | ||
textAlign: 'left', | ||
onChange: () => undefined, | ||
|
@@ -28,7 +42,11 @@ const propTypes = { | |
optionsContainerMaxHeight: PropTypes.string, | ||
}; | ||
|
||
class Dropdown extends React.Component { | ||
class Dropdown extends React.Component<DropdownProps> { | ||
static defaultProps = defaultProps; | ||
|
||
static propTypes = propTypes; | ||
|
||
state = { isOpen: false }; | ||
|
||
onSelectClick = () => { | ||
|
@@ -37,10 +55,10 @@ class Dropdown extends React.Component { | |
this.setState({ isOpen: !isOpen }); | ||
}; | ||
|
||
onSelectChange = event => { | ||
onSelectChange = (event: React.ChangeEvent<HTMLSelectElement>) => { | ||
const { onChange } = this.props; | ||
|
||
const { value, selectedOptions } = event.target; | ||
const {target} = event; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did this not auto-format to be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It did not - and apparently I've gotten very dependent on our linting to fix my bad whitespace habits 😅 |
||
const { value, selectedOptions } = target; | ||
|
||
if (selectedOptions && selectedOptions.length) { | ||
const { label } = selectedOptions[0]; | ||
|
@@ -50,15 +68,15 @@ class Dropdown extends React.Component { | |
this.closeDropdown(); | ||
}; | ||
|
||
onOptionClick = event => { | ||
onOptionClick = (event: React.MouseEvent<HTMLLIElement>) => { | ||
const { onChange } = this.props; | ||
|
||
if (event.target.hasAttribute('disabled')) { | ||
const target = event.target as HTMLSelectElement; | ||
ipc103 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (target.hasAttribute('disabled')) { | ||
return; | ||
} | ||
|
||
const value = event.target.getAttribute('value'); | ||
const label = event.target.innerText; | ||
const value = target.getAttribute('value') as string; | ||
const label = target.innerText; | ||
onChange({ value, label }); | ||
this.closeDropdown(); | ||
}; | ||
|
@@ -95,7 +113,4 @@ class Dropdown extends React.Component { | |
} | ||
} | ||
|
||
Dropdown.defaultProps = defaultProps; | ||
Dropdown.propTypes = propTypes; | ||
|
||
export default Dropdown; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
File CHANGELOG:
useResetFocus
hook to move focus back to the button when an option is selectedDropdownOptionsContainer
ul andDropdownOption
li.enter
key.