Skip to content

Commit 7619446

Browse files
Mohammer5Mohammer5
committed
fix(select a11y): handle remaining TODO comments & rename internal components
1 parent cca9228 commit 7619446

16 files changed

+167
-156
lines changed

‎components/select/src/single-select-a11y/__stories__/WithoutOptionsAndLoading.js

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
import React, { useState } from 'react'
2+
import { SingleSelectA11y } from '../single-select-a11y.js'
3+
4+
const options = [
5+
{ label: 'None', value: '' },
6+
{ value: '1', label: 'Option 1' },
7+
{ value: '2', label: 'Option 2' },
8+
{ value: '3', label: 'Option 3' },
9+
]
10+
11+
export const WithoutOptionsAndLoading = () => {
12+
const [value, setValue] = useState('')
13+
14+
return (
15+
<SingleSelectA11y
16+
loading
17+
idPrefix="a11y"
18+
value={value}
19+
valueLabel={
20+
value
21+
? options.find((option) => option.value === value).label
22+
: ''
23+
}
24+
onChange={(nextValue) => setValue(nextValue)}
25+
options={[]}
26+
/>
27+
)
28+
}

‎components/select/src/single-select-a11y/__stories__/WithoutSelection.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ export const WithoutSelection = () => {
77

88
return (
99
<SingleSelectA11y
10+
inputMaxHeight="50px"
1011
idPrefix="a11y"
1112
value={value}
1213
onChange={(nextValue) => setValue(nextValue)}

‎components/select/src/single-select-a11y/menu/menu-filter.js renamed to ‎components/select/src/single-select-a11y/menu/filter.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import PropTypes from 'prop-types'
44
import React from 'react'
55
import i18n from '../../locales/index.js'
66

7-
export function MenuFilter({
7+
export function Filter({
88
dataTest,
99
idPrefix,
1010
label,
@@ -46,7 +46,7 @@ export function MenuFilter({
4646
)
4747
}
4848

49-
MenuFilter.propTypes = {
49+
Filter.propTypes = {
5050
idPrefix: PropTypes.string.isRequired,
5151
value: PropTypes.string.isRequired,
5252
onChange: PropTypes.func.isRequired,

‎components/select/src/single-select-a11y/menu/menu-loading.js renamed to ‎components/select/src/single-select-a11y/menu/loading.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { CircularLoader } from '@dhis2-ui/loader'
33
import PropTypes from 'prop-types'
44
import React from 'react'
55

6-
export function MenuLoading({ message }) {
6+
export function Loading({ message }) {
77
return (
88
<div className="container">
99
<div>
@@ -33,6 +33,6 @@ export function MenuLoading({ message }) {
3333
)
3434
}
3535

36-
MenuLoading.propTypes = {
36+
Loading.propTypes = {
3737
message: PropTypes.string,
3838
}

‎components/select/src/single-select-a11y/menu/menu.js

Lines changed: 10 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,14 @@
11
import { colors, elevations } from '@dhis2/ui-constants'
22
import { Layer } from '@dhis2-ui/layer'
33
import { Popper } from '@dhis2-ui/popper'
4-
import cx from 'classnames'
54
import PropTypes from 'prop-types'
65
import React, { useEffect, useState } from 'react'
76
import { optionProp } from '../shared-prop-types.js'
87
import { Empty } from './empty.js'
9-
import { MenuFilter } from './menu-filter.js'
10-
import { MenuLoading } from './menu-loading.js'
11-
import { MenuOptionsList } from './menu-options-list.js'
8+
import { Filter } from './filter.js'
9+
import { Loading } from './loading.js'
1210
import { NoMatch } from './no-match.js'
11+
import { OptionsList } from './options-list.js'
1312

1413
export function Menu({
1514
comboBoxId,
@@ -42,12 +41,12 @@ export function Menu({
4241
onFilterChange,
4342
onFilterInputKeyDown,
4443
}) {
45-
const [menuWidth, setMenuWidth] = useState('auto')
44+
const [menuWidth, setWidth] = useState('auto')
4645
const dataTestPrefix = `${dataTest}-menu`
4746

4847
useEffect(() => {
4948
if (selectRef) {
50-
const callback = () => setMenuWidth(`${selectRef.offsetWidth}px`)
49+
const callback = () => setWidth(`${selectRef.offsetWidth}px`)
5150
callback() // We want to know the width as soon as the
5251

5352
selectRef.addEventListener('resize', callback)
@@ -75,13 +74,10 @@ export function Menu({
7574
placement="bottom-start"
7675
observeReferenceResize
7776
>
78-
<div
79-
className={cx('menu', { hidden })}
80-
style={{ width: menuWidth, maxHeight }}
81-
>
77+
<div className="menu" style={{ width: menuWidth, maxHeight }}>
8278
{filterable && (
8379
<div className="filter-container">
84-
<MenuFilter
80+
<Filter
8581
idPrefix={idPrefix}
8682
dataTest={`${dataTestPrefix}-filter`}
8783
value={filterValue}
@@ -98,19 +94,14 @@ export function Menu({
9894

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

101-
<div
102-
className={cx('listbox-container', {
103-
'no-options': !options.length,
104-
})}
105-
>
97+
<div className="listbox-container">
10698
<div className="listbox-wrapper">
107-
<MenuOptionsList
99+
<OptionsList
108100
ref={listBoxRef}
109101
comboBoxId={comboBoxId}
110102
customOption={customOption}
111103
dataTest={`${dataTestPrefix}-list`}
112104
disabled={disabled}
113-
expanded={!hidden}
114105
focussedOptionIndex={focussedOptionIndex}
115106
idPrefix={idPrefix}
116107
labelledBy={labelledBy}
@@ -126,7 +117,7 @@ export function Menu({
126117

127118
{loading && (
128119
<div className="menu-loading-container">
129-
<MenuLoading message={loadingText} />
120+
<Loading message={loadingText} />
130121
</div>
131122
)}
132123
</div>
@@ -154,12 +145,6 @@ export function Menu({
154145
overflow: hidden;
155146
}
156147
157-
.no-options {
158-
// @TODO: What should this value be?
159-
// Ask Joe
160-
height: 50px;
161-
}
162-
163148
.listbox-wrapper {
164149
overflow: auto;
165150
flex-grow: 1;

‎components/select/src/single-select-a11y/menu/menu-options-list.js renamed to ‎components/select/src/single-select-a11y/menu/options-list.js

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,10 @@ import { isOptionHidden } from '../is-option-hidden.js'
44
import { optionProp } from '../shared-prop-types.js'
55
import { Option } from './option.js'
66

7-
export const MenuOptionsList = forwardRef(function MenuOptionsList(
7+
export const OptionsList = forwardRef(function OptionsList(
88
{
99
comboBoxId,
1010
customOption,
11-
expanded,
1211
focussedOptionIndex,
1312
idPrefix,
1413
labelledBy,
@@ -29,9 +28,7 @@ export const MenuOptionsList = forwardRef(function MenuOptionsList(
2928
// * the menu opens
3029
useEffect(() => {
3130
const { current: listBox } = ref
32-
const highlightedOption = expanded
33-
? listBox.childNodes[focussedOptionIndex]
34-
: null
31+
const highlightedOption = listBox.childNodes[focussedOptionIndex]
3532

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

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

95-
MenuOptionsList.propTypes = {
92+
OptionsList.propTypes = {
9693
comboBoxId: PropTypes.string.isRequired,
97-
expanded: PropTypes.bool.isRequired,
9894
focussedOptionIndex: PropTypes.number.isRequired,
9995
idPrefix: PropTypes.string.isRequired,
10096
options: PropTypes.arrayOf(optionProp).isRequired,

‎components/select/src/single-select-a11y/selected-value/clear-button.js

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
import { colors, theme } from '@dhis2/ui-constants'
2+
import { Tooltip } from '@dhis2-ui/tooltip'
3+
import PropTypes from 'prop-types'
4+
import React from 'react'
5+
import i18n from '../../locales/index.js'
6+
7+
function ClearButton({ onClear, clearText, dataTest }) {
8+
return (
9+
<button
10+
aria-label={i18n.t('{{clearText}}', { clearText })}
11+
data-test={dataTest}
12+
onClick={(e) => {
13+
e.stopPropagation()
14+
onClear(e)
15+
}}
16+
>
17+
<svg
18+
width="16"
19+
height="16"
20+
viewBox="0 0 16 16"
21+
fill="none"
22+
xmlns="http://www.w3.org/2000/svg"
23+
>
24+
<path
25+
d="M8 16A8 8 0 1 0 8 0a8 8 0 0 0 0 16Zm3.536-5.879a1 1 0 1 1-1.415 1.414l-2.12-2.12-2.122 2.121a1 1 0 1 1-1.415-1.414L6.586 8 4.464 5.878A1 1 0 1 1 5.88 4.464L8 6.586l2.121-2.121a1 1 0 1 1 1.415 1.414L9.415 8l2.12 2.121Z"
26+
fill="none"
27+
/>
28+
</svg>
29+
<style jsx>{`
30+
button {
31+
background: none;
32+
border: none;
33+
display: flex;
34+
align-items: center;
35+
justify-content: center;
36+
cursor: pointer;
37+
height: 24px;
38+
width: 24px;
39+
border-radius: 3px;
40+
}
41+
button svg path {
42+
fill: ${colors.grey500};
43+
}
44+
button:hover svg path {
45+
fill: ${colors.grey800};
46+
}
47+
button:hover {
48+
background: ${colors.grey200};
49+
}
50+
button:focus {
51+
outline: 2px solid ${theme.focus};
52+
}
53+
`}</style>
54+
</button>
55+
)
56+
}
57+
58+
ClearButton.propTypes = {
59+
clearText: PropTypes.string.isRequired,
60+
onClear: PropTypes.func.isRequired,
61+
dataTest: PropTypes.string,
62+
}
63+
64+
function ClearButtonWithTooltip({ onClear, clearText, dataTest }) {
65+
const clearButton = (
66+
<ClearButton
67+
onClear={onClear}
68+
dataTest={dataTest}
69+
clearText={clearText}
70+
/>
71+
)
72+
73+
if (!clearText) {
74+
return clearButton
75+
}
76+
77+
return (
78+
<Tooltip openDelay={500} content={clearText}>
79+
{clearButton}
80+
</Tooltip>
81+
)
82+
}
83+
84+
ClearButtonWithTooltip.propTypes = {
85+
clearText: PropTypes.string.isRequired,
86+
onClear: PropTypes.func.isRequired,
87+
dataTest: PropTypes.string,
88+
}
89+
90+
export { ClearButtonWithTooltip as ClearButton }

‎components/select/src/single-select-a11y/selected-value/selected-value-container.js renamed to ‎components/select/src/single-select-a11y/selected-value/container.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import cx from 'classnames'
33
import PropTypes from 'prop-types'
44
import React, { forwardRef, useCallback, useEffect } from 'react'
55

6-
export const SelectedValueContainer = forwardRef(function Container(
6+
export const Container = forwardRef(function Container(
77
{
88
children,
99
comboBoxId,
@@ -124,7 +124,7 @@ export const SelectedValueContainer = forwardRef(function Container(
124124
)
125125
})
126126

127-
SelectedValueContainer.propTypes = {
127+
Container.propTypes = {
128128
children: PropTypes.any.isRequired,
129129
comboBoxId: PropTypes.string.isRequired,
130130
idPrefix: PropTypes.string.isRequired,

‎components/select/src/single-select-a11y/selected-value/selected-value-placeholder.js renamed to ‎components/select/src/single-select-a11y/selected-value/placeholder.js

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,7 @@ import { colors } from '@dhis2/ui-constants'
22
import PropTypes from 'prop-types'
33
import React from 'react'
44

5-
export const SelectedValuePlaceholder = ({
6-
placeholder,
7-
className,
8-
dataTest,
9-
}) => {
10-
if (!placeholder) {
11-
return null
12-
}
13-
5+
export function Placeholder({ placeholder, className, dataTest }) {
146
return (
157
<div className={className} data-test={dataTest}>
168
{placeholder}
@@ -25,7 +17,7 @@ export const SelectedValuePlaceholder = ({
2517
)
2618
}
2719

28-
SelectedValuePlaceholder.propTypes = {
20+
Placeholder.propTypes = {
2921
dataTest: PropTypes.string.isRequired,
3022
className: PropTypes.string,
3123
placeholder: PropTypes.string,

‎components/select/src/single-select-a11y/selected-value/selected-value-prefix.js renamed to ‎components/select/src/single-select-a11y/selected-value/prefix.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { colors, spacers } from '@dhis2/ui-constants'
22
import PropTypes from 'prop-types'
33
import React from 'react'
44

5-
export function SelectedValuePrefix({ prefix, className, dataTest }) {
5+
export function Prefix({ prefix, className, dataTest }) {
66
return (
77
<div className={className} data-test={dataTest}>
88
{prefix}
@@ -19,7 +19,7 @@ export function SelectedValuePrefix({ prefix, className, dataTest }) {
1919
)
2020
}
2121

22-
SelectedValuePrefix.propTypes = {
22+
Prefix.propTypes = {
2323
dataTest: PropTypes.string.isRequired,
2424
className: PropTypes.string,
2525
prefix: PropTypes.string,

0 commit comments

Comments
 (0)