Skip to content

Commit

Permalink
fix(select a11y): handle remaining TODO comments & rename internal co…
Browse files Browse the repository at this point in the history
…mponents
  • Loading branch information
Mohammer5 committed Nov 25, 2024
1 parent 98514fd commit 4ef7f99
Show file tree
Hide file tree
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
Expand Up @@ -7,6 +7,7 @@ export const WithoutSelection = () => {

return (
<SingleSelectA11y
inputMaxHeight="50px"
idPrefix="a11y"
value={value}
onChange={(nextValue) => setValue(nextValue)}
Expand Down
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: '',
},
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -46,7 +46,7 @@ export function MenuFilter({
)
}

MenuFilter.propTypes = {
Filter.propTypes = {
idPrefix: PropTypes.string.isRequired,
value: PropTypes.string.isRequired,
onChange: PropTypes.func.isRequired,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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}
Expand All @@ -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}
Expand All @@ -126,7 +117,7 @@ export function Menu({

{loading && (
<div className="menu-loading-container">
<MenuLoading message={loadingText} />
<Loading message={loadingText} />
</div>
)}
</div>
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand All @@ -44,7 +41,7 @@ export const MenuOptionsList = forwardRef(function MenuOptionsList(
highlightedOption.scrollIntoView()
}
}
}, [expanded, focussedOptionIndex, ref])
}, [focussedOptionIndex, ref])

return (
<div
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down Expand Up @@ -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}
Expand All @@ -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
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -25,7 +21,7 @@ export const SelectedValuePlaceholder = ({
)
}

SelectedValuePlaceholder.propTypes = {
Placeholder.propTypes = {
dataTest: PropTypes.string.isRequired,
className: PropTypes.string,
placeholder: PropTypes.string,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand All @@ -19,7 +19,7 @@ export function SelectedValuePrefix({ prefix, className, dataTest }) {
)
}

SelectedValuePrefix.propTypes = {
Prefix.propTypes = {
dataTest: PropTypes.string.isRequired,
className: PropTypes.string,
prefix: PropTypes.string,
Expand Down
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,
Expand All @@ -21,6 +21,7 @@ export function SelectedValue({
error,
expanded,
hasSelection,
inputMaxHeight,
labelledBy,
placeholder,
prefix,
Expand All @@ -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}
Expand All @@ -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}
/>
Expand All @@ -77,7 +73,7 @@ export function SelectedValue({

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

Expand All @@ -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,
Expand Down
Loading

0 comments on commit 4ef7f99

Please sign in to comment.