chore: fix linter issues

dhis2 · Oct 17, 2024 · 68a5d57 · 68a5d57
1 parent e9a1d19
commit 68a5d57
Show file tree

Hide file tree

Showing 8 changed files with 78 additions and 58 deletions.
diff --git a/components/select/src/single-select-a11y/features/position/index.js b/components/select/src/single-select-a11y/features/position/index.js
@@ -32,9 +32,6 @@ When('the window is scrolled down', () => {
 })
 
 Then('the top of the menu is aligned with the bottom of the input', () => {
-    const selectDataTest = '[data-test="dhis2-uicore-singleselect"]'
-    const menuDataTest = '[data-test="dhis2-uicore-select-menu-menuwrapper"]'
-
     cy.all(
         () => cy.findByRole('combobox'),
         () => cy.findByRole('listbox')
@@ -55,9 +52,6 @@ Then('the top of the menu is aligned with the bottom of the input', () => {
 })
 
 Then('the bottom of the menu is aligned with the top of the input', () => {
-    const selectDataTest = '[data-test="dhis2-uicore-singleselect"]'
-    const menuDataTest = '[data-test="dhis2-uicore-select-menu-menuwrapper"]'
-
     cy.all(
         () => cy.findByRole('combobox'),
         // We need to get the parent as the menu itself
@@ -78,9 +72,6 @@ Then('the bottom of the menu is aligned with the top of the input', () => {
 })
 
 Then('it is rendered on top of the SingleSelect', () => {
-    const selectDataTest = '[data-test="dhis2-uicore-singleselect"]'
-    const menuDataTest = '[data-test="dhis2-uicore-select-menu-menuwrapper"]'
-
     cy.all(
         () => cy.findByRole('combobox'),
         () => cy.findByRole('listbox')
@@ -102,9 +93,6 @@ Then('it is rendered on top of the SingleSelect', () => {
 Then(
     'the left of the SingleSelect is aligned with the left of the anchor',
     () => {
-        const selectDataTest = '[data-test="dhis2-uicore-singleselect"]'
-        const menuDataTest = '[data-test="dhis2-uicore-select-menu-menuwrapper"]'
-
         cy.all(
             () => cy.findByRole('combobox'),
             () => cy.findByRole('listbox')

diff --git a/components/select/src/single-select-a11y/option.js b/components/select/src/single-select-a11y/option.js
@@ -65,7 +65,6 @@ export function Option({
     dataTest,
     disabled,
     highlighted,
-    selected,
     ...rest
 }) {
     return (

diff --git a/components/select/src/single-select-a11y/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}) => (
+export const ClearButton = ({ onClear, clearText, dataTest }) => (
     <button
         aria-label={i18n.t('{{clearText}}', { clearText })}
         data-test={dataTest}
@@ -60,7 +60,13 @@ ClearButton.propTypes = {
 }
 
 export const SelectedValueClearButton = ({ onClear, clearText, dataTest }) => {
-    const clearButton = <ClearButton onClear={onClear} dataTest={dataTest} clearText={clearText} />
+    const clearButton = (
+        <ClearButton
+            onClear={onClear}
+            dataTest={dataTest}
+            clearText={clearText}
+        />
+    )
 
     if (!clearText) {
         return clearButton

diff --git a/components/select/src/single-select-a11y/selected-value-container.js b/components/select/src/single-select-a11y/selected-value-container.js
@@ -59,7 +59,7 @@ export const SelectedValueContainer = forwardRef(function Container(
             onFocus={onFocus}
             onBlur={onBlur}
             onClick={onClick}
-            onKeyDown={e => {
+            onKeyDown={(e) => {
                 e.stopPropagation()
                 onKeyPress(e)
             }}

diff --git a/components/select/src/single-select-a11y/single-select-a11y.e2e.stories.js b/components/select/src/single-select-a11y/single-select-a11y.e2e.stories.js
@@ -1,4 +1,4 @@
-import React, { useEffect, useMemo, useState } from 'react'
+import React from 'react'
 import { SingleSelectA11y } from './single-select-a11y.js'
 
 export default {

diff --git a/components/select/src/single-select-a11y/single-select-a11y.js b/components/select/src/single-select-a11y/single-select-a11y.js
@@ -47,15 +47,20 @@ export function SingleSelectA11y({
     // Non-stateful
     // ========
     const comboBoxId = `${idPrefix}-combo`
-    const valueLabel = _valueLabel || options.find(option => option.value === value)?.label || ''
+    const valueLabel =
+        _valueLabel ||
+        options.find((option) => option.value === value)?.label ||
+        ''
 
     if (
-        !valueLabel
-        && options.length
-        && !options.find(option => option.value === '')
-        && !placeholder
+        !valueLabel &&
+        options.length &&
+        !options.find((option) => option.value === '') &&
+        !placeholder
     ) {
-        throw new Error('You must either provide a "valueLabel" or include an empty option in the options array')
+        throw new Error(
+            'You must either provide a "valueLabel" or include an empty option in the options array'
+        )
     }
 
     // Stateful
@@ -64,11 +69,9 @@ export function SingleSelectA11y({
     // Using `useState` here so components get notified when the value changes (from null -> div)
     const comboBoxRef = useRef()
     const [focussedOptionIndex, setFocussedOptionIndex] = useState(() => {
-        const foundIndex = options.findIndex(option => option.value === value)
+        const foundIndex = options.findIndex((option) => option.value === value)
 
-        return foundIndex !== -1
-            ? foundIndex
-            : 0
+        return foundIndex !== -1 ? foundIndex : 0
     })
     const [selectRef, setSelectRef] = useState()
     const [expanded, setExpanded] = useState(false)
@@ -80,15 +83,15 @@ export function SingleSelectA11y({
         } else {
             openMenu()
         }
-    }, [expanded])
+    }, [expanded, openMenu, closeMenu])
 
     const selectFocussedOption = useCallback(() => {
         const option = options[focussedOptionIndex]
 
         if (option) {
             onChange(option.value)
         }
-    }, [focussedOptionIndex, options])
+    }, [focussedOptionIndex, options, onChange])
 
     const handleKeyPress = useHandleKeyPress({
         value,
@@ -190,12 +193,12 @@ SingleSelectA11y.propTypes = {
     /** An array of options **/
     options: optionsProp.isRequired,
 
-    /** A callback that will be called with the new value or an empty string **/
-    onChange: PropTypes.func.isRequired,
-
     /** As of now, this component does not support being uncontrolled */
     value: PropTypes.string.isRequired,
 
+    /** A callback that will be called with the new value or an empty string **/
+    onChange: PropTypes.func.isRequired,
+
     /** Will focus the select initially **/
     autoFocus: PropTypes.bool,
 

diff --git a/components/select/src/single-select-a11y/single-select-a11y.test.js b/components/select/src/single-select-a11y/single-select-a11y.test.js
@@ -20,6 +20,10 @@ describe('<SingleSelectA11y />', () => {
         fireEvent.blur(screen.getByRole('combobox'))
 
         // first argument passed to onBlur is a react event
+        //
+        // Reg. eslint: Eslint complains about using hasOwnProperty,
+        // but accessing `.nativeEvent` directly causes react to log an error
+        // eslint-disable-next-line no-prototype-builtins
         expect(onBlur.mock.calls[0][0].hasOwnProperty('nativeEvent')).toBe(true)
         expect(onBlur).toHaveBeenCalledTimes(1)
     })
@@ -40,7 +44,13 @@ describe('<SingleSelectA11y />', () => {
         fireEvent.focus(screen.getByRole('combobox'))
 
         // first argument passed to onFocus is a react event
-        expect(onFocus.mock.calls[0][0].hasOwnProperty('nativeEvent')).toBe(true)
+        //
+        // Reg. eslint: Eslint complains about using hasOwnProperty,
+        // but accessing `.nativeEvent` directly causes react to log an error
+        // eslint-disable-next-line no-prototype-builtins
+        expect(onFocus.mock.calls[0][0].hasOwnProperty('nativeEvent')).toBe(
+            true
+        )
         expect(onFocus).toHaveBeenCalledTimes(1)
     })
 
@@ -130,8 +140,11 @@ describe('<SingleSelectA11y />', () => {
         )
 
         const placeholder = screen.getByText('Placeholder text')
-        const dataTestValue = placeholder.attributes.getNamedItem('data-test').value
-        expect(dataTestValue).toBe('dhis2-singleselecta11y-selectedvalue-placeholder')
+        const dataTestValue =
+            placeholder.attributes.getNamedItem('data-test').value
+        expect(dataTestValue).toBe(
+            'dhis2-singleselecta11y-selectedvalue-placeholder'
+        )
     })
 
     it('should accept a prefix', () => {
@@ -146,8 +159,11 @@ describe('<SingleSelectA11y />', () => {
         )
 
         const placeholder = screen.getByText('Prefix text')
-        const dataTestValue = placeholder.attributes.getNamedItem('data-test').value
-        expect(dataTestValue).toBe('dhis2-singleselecta11y-selectedvalue-prefix')
+        const dataTestValue =
+            placeholder.attributes.getNamedItem('data-test').value
+        expect(dataTestValue).toBe(
+            'dhis2-singleselecta11y-selectedvalue-prefix'
+        )
     })
 
     it('should allow options to be selected', () => {
@@ -174,15 +190,15 @@ describe('<SingleSelectA11y />', () => {
         fireEvent.click(option)
 
         expect(onChange).toHaveBeenCalledTimes(1)
-        expect(onChange).toHaveBeenCalledWith("foo")
+        expect(onChange).toHaveBeenCalledWith('foo')
     })
 
     it('should allow custom options to be selected', () => {
         const onChange = jest.fn()
+
+        // eslint-disable-next-line react/prop-types
         const CustomOption = ({ value, label }) => (
-            <span data-test={`custom-option-${value}`}>
-                {label}
-            </span>
+            <span data-test={`custom-option-${value}`}>{label}</span>
         )
 
         render(
@@ -207,7 +223,7 @@ describe('<SingleSelectA11y />', () => {
         fireEvent.click(customOption)
 
         expect(onChange).toHaveBeenCalledTimes(1)
-        expect(onChange).toHaveBeenCalledWith("foo")
+        expect(onChange).toHaveBeenCalledWith('foo')
     })
 
     it('should be clearable when there is a selected value', () => {
@@ -317,7 +333,7 @@ describe('<SingleSelectA11y />', () => {
         expect(searchInput).toBeInstanceOf(HTMLInputElement)
         expect(searchInput).toBeVisible()
 
-        fireEvent.change(searchInput, { target: { value: 'Search term'} })
+        fireEvent.change(searchInput, { target: { value: 'Search term' } })
 
         expect(onFilterChange).toHaveBeenCalledTimes(1)
         expect(onFilterChange).toHaveBeenCalledWith('Search term')
@@ -336,7 +352,10 @@ describe('<SingleSelectA11y />', () => {
                 value=""
                 valueLabel=""
                 onChange={jest.fn()}
-                options={[{ value: '', label: 'None' }, { value: 'foo', label: 'Foo' }]}
+                options={[
+                    { value: '', label: 'None' },
+                    { value: 'foo', label: 'Foo' },
+                ]}
             />
         )
 
@@ -372,7 +391,9 @@ describe('<SingleSelectA11y />', () => {
         // Is this because of unnecessary re-renders?
         expect(consoleError).toHaveBeenNthCalledWith(
             1,
-            expect.stringContaining('Encountered two children with the same key'),
+            expect.stringContaining(
+                'Encountered two children with the same key'
+            ),
             'foo',
             expect.anything()
         )
@@ -404,14 +425,13 @@ describe('<SingleSelectA11y />', () => {
         expect(combobox).toContainElement(withTextBar)
     })
 
-
     /**************************
-    *                         *
-    *  =====================  *
-    *  Keyboard interactions  *
-    *  =====================  *
-    *                         *
-    **************************/
+     *                         *
+     *  =====================  *
+     *  Keyboard interactions  *
+     *  =====================  *
+     *                         *
+     **************************/
 
     describe.each([
         { key: ' ' },
@@ -497,12 +517,11 @@ describe('<SingleSelectA11y />', () => {
             expect(screen.queryByRole('listbox')).not.toBeNull()
 
             // highlighting the next option
-            fireEvent.keyDown(screen.getByRole('combobox'), { key: 'ArrowDown' })
+            fireEvent.keyDown(screen.getByRole('combobox'), {
+                key: 'ArrowDown',
+            })
 
-            fireEvent.keyDown(
-                screen.getByRole('combobox'),
-                keyDownOptions,
-            )
+            fireEvent.keyDown(screen.getByRole('combobox'), keyDownOptions)
 
             expect(screen.queryByRole('listbox')).toBeNull()
             expect(onChange).toHaveBeenCalledTimes(1)

diff --git a/storybook/src/load-stories.js b/storybook/src/load-stories.js
@@ -52,7 +52,12 @@ exports.loadStories = () => {
         case icons.includes(curcomp): {
             console.info(`custom => Loading stories for '${curcomp}'`)
             return [
-                path.join(ICONS_DIR, 'src', '**', '*.prod.stories.@(js|jsx|mdx)'),
+                path.join(
+                    ICONS_DIR,
+                    'src',
+                    '**',
+                    '*.prod.stories.@(js|jsx|mdx)'
+                ),
             ]
         }
         case constants.includes(curcomp): {