From 4ef7f997c505e39d56bfca84f663c2a58349616e Mon Sep 17 00:00:00 2001
From: Mohammer5 <jan-gerke@dhis2.org>
Date: Mon, 25 Nov 2024 21:42:44 +0800
Subject: [PATCH] fix(select a11y): handle remaining TODO comments & rename
 internal components

---
 .../__stories__/WithoutOptionsAndLoading.js   | 28 +++++++++++++++
 .../__stories__/WithoutSelection.js           |  1 +
 .../single-select-a11y/__stories__/options.js |  2 +-
 .../menu/{menu-filter.js => filter.js}        |  4 +--
 .../menu/{menu-loading.js => loading.js}      |  4 +--
 .../src/single-select-a11y/menu/menu.js       | 35 ++++++-------------
 .../{menu-options-list.js => options-list.js} | 12 +++----
 ...-value-clear-button.js => clear-button.js} |  8 +++--
 ...lected-value-container.js => container.js} |  4 +--
 ...ed-value-placeholder.js => placeholder.js} |  8 ++---
 .../{selected-value-prefix.js => prefix.js}   |  4 +--
 .../selected-value/selected-value.js          | 25 ++++++-------
 .../single-select-a11y/single-select-a11y.js  |  7 ++++
 .../single-select-a11y.prod.stories.js        |  1 +
 14 files changed, 78 insertions(+), 65 deletions(-)
 create mode 100644 components/select/src/single-select-a11y/__stories__/WithoutOptionsAndLoading.js
 rename components/select/src/single-select-a11y/menu/{menu-filter.js => filter.js} (96%)
 rename components/select/src/single-select-a11y/menu/{menu-loading.js => loading.js} (93%)
 rename components/select/src/single-select-a11y/menu/{menu-options-list.js => options-list.js} (90%)
 rename components/select/src/single-select-a11y/selected-value/{selected-value-clear-button.js => clear-button.js} (91%)
 rename components/select/src/single-select-a11y/selected-value/{selected-value-container.js => container.js} (97%)
 rename components/select/src/single-select-a11y/selected-value/{selected-value-placeholder.js => placeholder.js} (80%)
 rename components/select/src/single-select-a11y/selected-value/{selected-value-prefix.js => prefix.js} (85%)

diff --git a/components/select/src/single-select-a11y/__stories__/WithoutOptionsAndLoading.js b/components/select/src/single-select-a11y/__stories__/WithoutOptionsAndLoading.js
new file mode 100644
index 000000000..1b8236cdf
--- /dev/null
+++ b/components/select/src/single-select-a11y/__stories__/WithoutOptionsAndLoading.js
@@ -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={[]}
+        />
+    )
+}
diff --git a/components/select/src/single-select-a11y/__stories__/WithoutSelection.js b/components/select/src/single-select-a11y/__stories__/WithoutSelection.js
index aa17177f9..abcf0fe83 100644
--- a/components/select/src/single-select-a11y/__stories__/WithoutSelection.js
+++ b/components/select/src/single-select-a11y/__stories__/WithoutSelection.js
@@ -7,6 +7,7 @@ export const WithoutSelection = () => {
 
     return (
         <SingleSelectA11y
+            inputMaxHeight="50px"
             idPrefix="a11y"
             value={value}
             onChange={(nextValue) => setValue(nextValue)}
diff --git a/components/select/src/single-select-a11y/__stories__/options.js b/components/select/src/single-select-a11y/__stories__/options.js
index bcbde0f18..20b185602 100644
--- a/components/select/src/single-select-a11y/__stories__/options.js
+++ b/components/select/src/single-select-a11y/__stories__/options.js
@@ -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: '',
     },
     {
diff --git a/components/select/src/single-select-a11y/menu/menu-filter.js b/components/select/src/single-select-a11y/menu/filter.js
similarity index 96%
rename from components/select/src/single-select-a11y/menu/menu-filter.js
rename to components/select/src/single-select-a11y/menu/filter.js
index 828ed0ec5..a16ba3c6b 100644
--- a/components/select/src/single-select-a11y/menu/menu-filter.js
+++ b/components/select/src/single-select-a11y/menu/filter.js
@@ -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,
diff --git a/components/select/src/single-select-a11y/menu/menu-loading.js b/components/select/src/single-select-a11y/menu/loading.js
similarity index 93%
rename from components/select/src/single-select-a11y/menu/menu-loading.js
rename to components/select/src/single-select-a11y/menu/loading.js
index 936db6391..fc57e0a0f 100644
--- a/components/select/src/single-select-a11y/menu/menu-loading.js
+++ b/components/select/src/single-select-a11y/menu/loading.js
@@ -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,
 }
diff --git a/components/select/src/single-select-a11y/menu/menu.js b/components/select/src/single-select-a11y/menu/menu.js
index 9d2506b28..26b751f23 100644
--- a/components/select/src/single-select-a11y/menu/menu.js
+++ b/components/select/src/single-select-a11y/menu/menu.js
@@ -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;
diff --git a/components/select/src/single-select-a11y/menu/menu-options-list.js b/components/select/src/single-select-a11y/menu/options-list.js
similarity index 90%
rename from components/select/src/single-select-a11y/menu/menu-options-list.js
rename to components/select/src/single-select-a11y/menu/options-list.js
index 3fbf66bc2..3e49d2f31 100644
--- a/components/select/src/single-select-a11y/menu/menu-options-list.js
+++ b/components/select/src/single-select-a11y/menu/options-list.js
@@ -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,
diff --git a/components/select/src/single-select-a11y/selected-value/selected-value-clear-button.js b/components/select/src/single-select-a11y/selected-value/clear-button.js
similarity index 91%
rename from components/select/src/single-select-a11y/selected-value/selected-value-clear-button.js
rename to components/select/src/single-select-a11y/selected-value/clear-button.js
index 06c17a702..d91b96fcb 100644
--- a/components/select/src/single-select-a11y/selected-value/selected-value-clear-button.js
+++ b/components/select/src/single-select-a11y/selected-value/clear-button.js
@@ -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 }
diff --git a/components/select/src/single-select-a11y/selected-value/selected-value-container.js b/components/select/src/single-select-a11y/selected-value/container.js
similarity index 97%
rename from components/select/src/single-select-a11y/selected-value/selected-value-container.js
rename to components/select/src/single-select-a11y/selected-value/container.js
index cb7886a46..34aa5e263 100644
--- a/components/select/src/single-select-a11y/selected-value/selected-value-container.js
+++ b/components/select/src/single-select-a11y/selected-value/container.js
@@ -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,
diff --git a/components/select/src/single-select-a11y/selected-value/selected-value-placeholder.js b/components/select/src/single-select-a11y/selected-value/placeholder.js
similarity index 80%
rename from components/select/src/single-select-a11y/selected-value/selected-value-placeholder.js
rename to components/select/src/single-select-a11y/selected-value/placeholder.js
index ddd43f7b1..858fbfcc5 100644
--- a/components/select/src/single-select-a11y/selected-value/selected-value-placeholder.js
+++ b/components/select/src/single-select-a11y/selected-value/placeholder.js
@@ -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,
diff --git a/components/select/src/single-select-a11y/selected-value/selected-value-prefix.js b/components/select/src/single-select-a11y/selected-value/prefix.js
similarity index 85%
rename from components/select/src/single-select-a11y/selected-value/selected-value-prefix.js
rename to components/select/src/single-select-a11y/selected-value/prefix.js
index 332699147..1a71c426a 100644
--- a/components/select/src/single-select-a11y/selected-value/selected-value-prefix.js
+++ b/components/select/src/single-select-a11y/selected-value/prefix.js
@@ -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,
diff --git a/components/select/src/single-select-a11y/selected-value/selected-value.js b/components/select/src/single-select-a11y/selected-value/selected-value.js
index 19da6c378..6a30e0df3 100644
--- a/components/select/src/single-select-a11y/selected-value/selected-value.js
+++ b/components/select/src/single-select-a11y/selected-value/selected-value.js
@@ -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,
diff --git a/components/select/src/single-select-a11y/single-select-a11y.js b/components/select/src/single-select-a11y/single-select-a11y.js
index 1fe43f274..49187de4e 100644
--- a/components/select/src/single-select-a11y/single-select-a11y.js
+++ b/components/select/src/single-select-a11y/single-select-a11y.js
@@ -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,
 
diff --git a/components/select/src/single-select-a11y/single-select-a11y.prod.stories.js b/components/select/src/single-select-a11y/single-select-a11y.prod.stories.js
index 20e3134f2..994cd4593 100644
--- a/components/select/src/single-select-a11y/single-select-a11y.prod.stories.js
+++ b/components/select/src/single-select-a11y/single-select-a11y.prod.stories.js
@@ -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'