Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix(select a11y): handle remaining TODO comments & rename internal co…
Browse files Browse the repository at this point in the history
…mponents
Mohammer5 committed Nov 25, 2024
1 parent 98514fd commit 4ef7f99
Showing 14 changed files with 78 additions and 65 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import React, { useState } from 'react'
import { SingleSelectA11y } from '../single-select-a11y.js'

const options = [
{ label: 'None', value: '' },
{ value: '1', label: 'Option 1' },
{ value: '2', label: 'Option 2' },
{ value: '3', label: 'Option 3' },
]

export const WithoutOptionsAndLoading = () => {
const [value, setValue] = useState('')

return (
<SingleSelectA11y
loading
idPrefix="a11y"
value={value}
valueLabel={
value
? options.find((option) => option.value === value).label
: ''
}
onChange={(nextValue) => setValue(nextValue)}
options={[]}
/>
)
}
Original file line number Diff line number Diff line change
@@ -7,6 +7,7 @@ export const WithoutSelection = () => {

return (
<SingleSelectA11y
inputMaxHeight="50px"
idPrefix="a11y"
value={value}
onChange={(nextValue) => setValue(nextValue)}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
export const options = [
{
label: 'None',
label: 'None foo bar baz foo bar baz foo bar baz foo bar baz foo bar baz foo bar baz foo bar baz foo bar baz foo bar baz foo bar baz foo bar baz foo bar baz foo bar baz foo bar baz foo bar baz foo bar baz foo bar baz foo bar baz None foo bar baz foo bar baz foo bar baz foo bar baz foo bar baz foo bar baz foo bar baz foo bar baz foo bar baz foo bar baz foo bar baz foo bar baz foo bar baz foo bar baz foo bar baz foo bar baz foo bar baz foo bar baz',
value: '',
},
{
Original file line number Diff line number Diff line change
@@ -4,7 +4,7 @@ import PropTypes from 'prop-types'
import React from 'react'
import i18n from '../../locales/index.js'

export function MenuFilter({
export function Filter({
dataTest,
idPrefix,
label,
@@ -46,7 +46,7 @@ export function MenuFilter({
)
}

MenuFilter.propTypes = {
Filter.propTypes = {
idPrefix: PropTypes.string.isRequired,
value: PropTypes.string.isRequired,
onChange: PropTypes.func.isRequired,
Original file line number Diff line number Diff line change
@@ -3,7 +3,7 @@ import { CircularLoader } from '@dhis2-ui/loader'
import PropTypes from 'prop-types'
import React from 'react'

export function MenuLoading({ message }) {
export function Loading({ message }) {
return (
<div className="container">
<div>
@@ -33,6 +33,6 @@ export function MenuLoading({ message }) {
)
}

MenuLoading.propTypes = {
Loading.propTypes = {
message: PropTypes.string,
}
35 changes: 10 additions & 25 deletions components/select/src/single-select-a11y/menu/menu.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
import { colors, elevations } from '@dhis2/ui-constants'
import { Layer } from '@dhis2-ui/layer'
import { Popper } from '@dhis2-ui/popper'
import cx from 'classnames'
import PropTypes from 'prop-types'
import React, { useEffect, useState } from 'react'
import { optionProp } from '../shared-prop-types.js'
import { Empty } from './empty.js'
import { MenuFilter } from './menu-filter.js'
import { MenuLoading } from './menu-loading.js'
import { MenuOptionsList } from './menu-options-list.js'
import { Filter } from './filter.js'
import { Loading } from './loading.js'
import { NoMatch } from './no-match.js'
import { OptionsList } from './options-list.js'

export function Menu({
comboBoxId,
@@ -42,12 +41,12 @@ export function Menu({
onFilterChange,
onFilterInputKeyDown,
}) {
const [menuWidth, setMenuWidth] = useState('auto')
const [menuWidth, setWidth] = useState('auto')
const dataTestPrefix = `${dataTest}-menu`

useEffect(() => {
if (selectRef) {
const callback = () => setMenuWidth(`${selectRef.offsetWidth}px`)
const callback = () => setWidth(`${selectRef.offsetWidth}px`)
callback() // We want to know the width as soon as the

selectRef.addEventListener('resize', callback)
@@ -75,13 +74,10 @@ export function Menu({
placement="bottom-start"
observeReferenceResize
>
<div
className={cx('menu', { hidden })}
style={{ width: menuWidth, maxHeight }}
>
<div className="menu" style={{ width: menuWidth, maxHeight }}>
{filterable && (
<div className="filter-container">
<MenuFilter
<Filter
idPrefix={idPrefix}
dataTest={`${dataTestPrefix}-filter`}
value={filterValue}
@@ -98,19 +94,14 @@ export function Menu({

{hasNoFilterMatch && <NoMatch>{noMatchText}</NoMatch>}

<div
className={cx('listbox-container', {
'no-options': !options.length,
})}
>
<div className="listbox-container">
<div className="listbox-wrapper">
<MenuOptionsList
<OptionsList
ref={listBoxRef}
comboBoxId={comboBoxId}
customOption={customOption}
dataTest={`${dataTestPrefix}-list`}
disabled={disabled}
expanded={!hidden}
focussedOptionIndex={focussedOptionIndex}
idPrefix={idPrefix}
labelledBy={labelledBy}
@@ -126,7 +117,7 @@ export function Menu({

{loading && (
<div className="menu-loading-container">
<MenuLoading message={loadingText} />
<Loading message={loadingText} />
</div>
)}
</div>
@@ -154,12 +145,6 @@ export function Menu({
overflow: hidden;
}
.no-options {
// @TODO: What should this value be?
// Ask Joe
height: 50px;
}
.listbox-wrapper {
overflow: auto;
flex-grow: 1;
Original file line number Diff line number Diff line change
@@ -4,11 +4,10 @@ import { isOptionHidden } from '../is-option-hidden.js'
import { optionProp } from '../shared-prop-types.js'
import { Option } from './option.js'

export const MenuOptionsList = forwardRef(function MenuOptionsList(
export const OptionsList = forwardRef(function OptionsList(
{
comboBoxId,
customOption,
expanded,
focussedOptionIndex,
idPrefix,
labelledBy,
@@ -29,9 +28,7 @@ export const MenuOptionsList = forwardRef(function MenuOptionsList(
// * the menu opens
useEffect(() => {
const { current: listBox } = ref
const highlightedOption = expanded
? listBox.childNodes[focussedOptionIndex]
: null
const highlightedOption = listBox.childNodes[focussedOptionIndex]

if (highlightedOption) {
const listBoxParent = listBox.parentNode
@@ -44,7 +41,7 @@ export const MenuOptionsList = forwardRef(function MenuOptionsList(
highlightedOption.scrollIntoView()
}
}
}, [expanded, focussedOptionIndex, ref])
}, [focussedOptionIndex, ref])

return (
<div
@@ -92,9 +89,8 @@ export const MenuOptionsList = forwardRef(function MenuOptionsList(
)
})

MenuOptionsList.propTypes = {
OptionsList.propTypes = {
comboBoxId: PropTypes.string.isRequired,
expanded: PropTypes.bool.isRequired,
focussedOptionIndex: PropTypes.number.isRequired,
idPrefix: PropTypes.string.isRequired,
options: PropTypes.arrayOf(optionProp).isRequired,
Original file line number Diff line number Diff line change
@@ -4,7 +4,7 @@ import PropTypes from 'prop-types'
import React from 'react'
import i18n from '../../locales/index.js'

export const ClearButton = ({ onClear, clearText, dataTest }) => (
const ClearButton = ({ onClear, clearText, dataTest }) => (
<button
aria-label={i18n.t('{{clearText}}', { clearText })}
data-test={dataTest}
@@ -59,7 +59,7 @@ ClearButton.propTypes = {
dataTest: PropTypes.string,
}

export const SelectedValueClearButton = ({ onClear, clearText, dataTest }) => {
const ClearButtonWithTooltip = ({ onClear, clearText, dataTest }) => {
const clearButton = (
<ClearButton
onClear={onClear}
@@ -79,8 +79,10 @@ export const SelectedValueClearButton = ({ onClear, clearText, dataTest }) => {
)
}

SelectedValueClearButton.propTypes = {
ClearButtonWithTooltip.propTypes = {
clearText: PropTypes.string.isRequired,
onClear: PropTypes.func.isRequired,
dataTest: PropTypes.string,
}

export { ClearButtonWithTooltip as ClearButton }
Original file line number Diff line number Diff line change
@@ -3,7 +3,7 @@ import cx from 'classnames'
import PropTypes from 'prop-types'
import React, { forwardRef, useCallback, useEffect } from 'react'

export const SelectedValueContainer = forwardRef(function Container(
export const Container = forwardRef(function Container(
{
children,
comboBoxId,
@@ -124,7 +124,7 @@ export const SelectedValueContainer = forwardRef(function Container(
)
})

SelectedValueContainer.propTypes = {
Container.propTypes = {
children: PropTypes.any.isRequired,
comboBoxId: PropTypes.string.isRequired,
idPrefix: PropTypes.string.isRequired,
Original file line number Diff line number Diff line change
@@ -2,11 +2,7 @@ import { colors } from '@dhis2/ui-constants'
import PropTypes from 'prop-types'
import React from 'react'

export const SelectedValuePlaceholder = ({
placeholder,
className,
dataTest,
}) => {
export const Placeholder = ({ placeholder, className, dataTest }) => {
if (!placeholder) {
return null
}
@@ -25,7 +21,7 @@ export const SelectedValuePlaceholder = ({
)
}

SelectedValuePlaceholder.propTypes = {
Placeholder.propTypes = {
dataTest: PropTypes.string.isRequired,
className: PropTypes.string,
placeholder: PropTypes.string,
Original file line number Diff line number Diff line change
@@ -2,7 +2,7 @@ import { colors, spacers } from '@dhis2/ui-constants'
import PropTypes from 'prop-types'
import React from 'react'

export function SelectedValuePrefix({ prefix, className, dataTest }) {
export function Prefix({ prefix, className, dataTest }) {
return (
<div className={className} data-test={dataTest}>
{prefix}
@@ -19,7 +19,7 @@ export function SelectedValuePrefix({ prefix, className, dataTest }) {
)
}

SelectedValuePrefix.propTypes = {
Prefix.propTypes = {
dataTest: PropTypes.string.isRequired,
className: PropTypes.string,
prefix: PropTypes.string,
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { IconChevronDown16 } from '@dhis2/ui-icons'
import PropTypes from 'prop-types'
import React from 'react'
import { SelectedValueClearButton } from './selected-value-clear-button.js'
import { SelectedValueContainer } from './selected-value-container.js'
import { SelectedValuePlaceholder } from './selected-value-placeholder.js'
import { SelectedValuePrefix } from './selected-value-prefix.js'
import { ClearButton } from './clear-button.js'
import { Container } from './container.js'
import { Placeholder } from './placeholder.js'
import { Prefix } from './prefix.js'

export function SelectedValue({
clearText,
@@ -21,6 +21,7 @@ export function SelectedValue({
error,
expanded,
hasSelection,
inputMaxHeight,
labelledBy,
placeholder,
prefix,
@@ -32,12 +33,10 @@ export function SelectedValue({
onClick,
onFocus,
}) {
// @TODO
const inputMaxHeight = '300px'
const dataTestPrefix = `${dataTest}-selectedvalue`

return (
<SelectedValueContainer
<Container
dataTest={`${dataTestPrefix}-container`}
ref={comboBoxRef}
comboBoxId={comboBoxId}
@@ -58,15 +57,12 @@ export function SelectedValue({
onKeyDown={onKeyDown}
>
{prefix && (
<SelectedValuePrefix
dataTest={`${dataTestPrefix}-prefix`}
prefix={prefix}
/>
<Prefix dataTest={`${dataTestPrefix}-prefix`} prefix={prefix} />
)}

<div className="selected-option-label">
{!valueLabel && !prefix && (
<SelectedValuePlaceholder
<Placeholder
dataTest={`${dataTestPrefix}-placeholder`}
placeholder={placeholder}
/>
@@ -77,7 +73,7 @@ export function SelectedValue({

{hasSelection && clearable && !disabled && (
<div className="clear-button-container">
<SelectedValueClearButton
<ClearButton
clearText={clearText}
dataTest={`${dataTestPrefix}-clear`}
onClear={onClear}
@@ -114,7 +110,7 @@ export function SelectedValue({
border: 0;
}
`}</style>
</SelectedValueContainer>
</Container>
)
}

@@ -135,6 +131,7 @@ SelectedValue.propTypes = {
error: PropTypes.bool,
expanded: PropTypes.bool,
hasSelection: PropTypes.bool,
inputMaxHeight: PropTypes.oneOfType([PropTypes.string, PropTypes.number]),
labelledBy: PropTypes.string,
placeholder: PropTypes.string,
prefix: PropTypes.string,
Original file line number Diff line number Diff line change
@@ -71,6 +71,7 @@ export function SingleSelectA11y({
filterPlaceholder: _filterPlaceholder = '',
filterValue = '',
filterable = false,
inputMaxHeight = '',
labelledBy = '',
loading = false,
menuLoadingText: _menuLoadingText = '',
@@ -128,6 +129,7 @@ export function SingleSelectA11y({
const [selectRef, setSelectRef] = useState()
const [expanded, setExpanded] = useState(false)
const closeMenu = useCallback(() => setExpanded(false), [])

const openMenu = useCallback(() => {
const selectedOptionIndex = options.findIndex(
(option) => option.value === value
@@ -138,6 +140,7 @@ export function SingleSelectA11y({

setExpanded(true)
}, [options, value, focussedOptionIndex, setFocussedOptionIndex])

const toggleMenu = useCallback(() => {
if (expanded) {
closeMenu()
@@ -219,6 +222,7 @@ export function SingleSelectA11y({
expanded={expanded}
hasSelection={!!value}
idPrefix={idPrefix}
inputMaxHeight={inputMaxHeight}
labelledBy={labelledBy}
options={options}
placeholder={placeholder}
@@ -329,6 +333,9 @@ SingleSelectA11y.propTypes = {
/** Whether the select should display a filter input **/
filterable: PropTypes.bool,

/** Max height of the container displaying the selected value **/
inputMaxHeight: PropTypes.oneOfType([PropTypes.string, PropTypes.number]),

/** Should contain the id of the element that labels the select, if applicable **/
labelledBy: PropTypes.string,

Original file line number Diff line number Diff line change
@@ -19,6 +19,7 @@ export { EmptyWithEmptyText } from './__stories__/EmptyWithEmptyText.js'
export { EmptyWithEmptyNode } from './__stories__/EmptyWithEmptyNode.js'
export { WithOptionsAndLoading } from './__stories__/WithOptionsAndLoading.js'
export { WithOptionsAndLoadingText } from './__stories__/WithOptionsAndLoadingText.js'
export { WithoutOptionsAndLoading } from './__stories__/WithoutOptionsAndLoading.js'
export { WithManyOptions } from './__stories__/WithManyOptions.js'
export { WithCustomLowMaxHeight } from './__stories__/WithCustomLowMaxHeight.js'
export { WithOptionsAndDisabled } from './__stories__/WithOptionsAndDisabled.js'

0 comments on commit 4ef7f99

Please sign in to comment.