Skip to content

Commit

Permalink
Merge pull request #1511 from dhis2/accessibility-updates-from-alpha
Browse files Browse the repository at this point in the history
feat: accessibility updates from alpha
  • Loading branch information
kabaros authored Jun 3, 2024
2 parents 6962afc + 389e61a commit d8aea01
Show file tree
Hide file tree
Showing 13 changed files with 339 additions and 16 deletions.
46 changes: 46 additions & 0 deletions components/button/src/button/__tests__/Button.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,52 @@ import React from 'react'
import { Button } from '../button.js'

describe('<Button>', () => {
let consoleSpy

beforeEach(() => {
consoleSpy = jest.spyOn(console, 'debug').mockImplementation()
})

afterEach(() => {
consoleSpy.mockRestore()
})

describe('warning for missing aria-label and title', () => {
it('No warning if children exist but aria-label and title is missing', () => {
render(<Button>Children content</Button>)

expect(consoleSpy).not.toHaveBeenCalled()
})

it('does not warn if aria-label and title is present', () => {
render(
<Button aria-label="Test" title="Test">
Children content
</Button>
)

expect(consoleSpy).not.toHaveBeenCalled()
})

it('warns if no children are present with no arial-label and title', () => {
render(<Button>{/* No children */}</Button>)

expect(consoleSpy).toHaveBeenCalledWith(
'Button component has no children but is missing title and ariaLabel attribute.'
)
})

it('No warning if there are no children but arial label and title', () => {
render(
<Button aria-label="Test" title="Test">
{/* No children */}
</Button>
)

expect(consoleSpy).not.toHaveBeenCalled()
})
})

it('renders a default data-test attribute', () => {
const dataTest = 'dhis2-uicore-button'
const wrapper = mount(<Button dataTest={dataTest} />)
Expand Down
8 changes: 8 additions & 0 deletions components/button/src/button/button.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,14 @@ export const Button = ({
}
}, [initialFocus, ref.current])

Check warning on line 38 in components/button/src/button/button.js

View workflow job for this annotation

GitHub Actions / lint

React Hook useEffect has an unnecessary dependency: 'ref.current'. Either exclude it or remove the dependency array. Mutable values like 'ref.current' aren't valid dependencies because mutating them doesn't re-render the component

const { 'aria-label': ariaLabel, title } = otherProps

if (!children && !title && !ariaLabel) {
console.debug(
'Button component has no children but is missing title and ariaLabel attribute.'
)
}

const handleClick = (event) => onClick && onClick({ value, name }, event)
const handleBlur = (event) => onBlur && onBlur({ value, name }, event)
const handleFocus = (event) => onFocus && onFocus({ value, name }, event)
Expand Down
24 changes: 23 additions & 1 deletion components/button/src/split-button/split-button.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import PropTypes from 'prop-types'
import React, { Component } from 'react'
import css from 'styled-jsx/css'
import { Button } from '../index.js'
import i18n from '../locales/index.js'

const rightButton = css.resolve`
button {
Expand All @@ -18,8 +19,25 @@ class SplitButton extends Component {
state = {
open: false,
}

anchorRef = React.createRef()

componentDidMount() {
document.addEventListener('keydown', this.handleKeyDown)
}

componentWillUnmount() {
document.removeEventListener('keydown', this.handleKeyDown)
}
handleKeyDown = (event) => {
event.preventDefault()
if (event.key === 'Escape' && this.state.open) {
event.stopPropagation()
this.setState({ open: false })
this.anchorRef.current && this.anchorRef.current.focus()
}
}

onClick = (payload, event) => {
if (this.props.onClick) {
this.props.onClick(
Expand All @@ -33,7 +51,9 @@ class SplitButton extends Component {
}
}

onToggle = () => this.setState({ open: !this.state.open })
onToggle = () => {
this.setState((prevState) => ({ open: !prevState.open }))
}

render() {
const { open } = this.state
Expand Down Expand Up @@ -94,6 +114,8 @@ class SplitButton extends Component {
tabIndex={tabIndex}
className={cx(className, rightButton.className)}
dataTest={`${dataTest}-toggle`}
title={i18n.t('Toggle dropdown')}
aria-label={i18n.t('Toggle dropdown')}
>
{arrow}
</Button>
Expand Down
85 changes: 85 additions & 0 deletions components/button/src/split-button/split-button.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
import { render, fireEvent, cleanup, waitFor } from '@testing-library/react'
import React from 'react'
import '@testing-library/jest-dom/extend-expect'
import { SplitButton } from './split-button.js'

describe('SplitButton', () => {
afterEach(cleanup)

it('renders button with children', () => {
const { getByText } = render(<SplitButton>Click me</SplitButton>)
expect(getByText('Click me')).toBeInTheDocument()
})

it('toggles dropdown when left button is clicked', () => {
const { getByTestId, queryByTestId } = render(<SplitButton />)
const toggleButton = getByTestId('dhis2-uicore-splitbutton-toggle')

fireEvent.click(toggleButton)
expect(
queryByTestId('dhis2-uicore-splitbutton-menu')
).toBeInTheDocument()

fireEvent.click(toggleButton)
expect(
queryByTestId('dhis2-uicore-splitbutton-menu')
).not.toBeInTheDocument()
})

it('renders dropdown content when open is true', () => {
const { getByTestId } = render(
<SplitButton component={<div>Dropdown Content</div>} />
)

const toggleButton = getByTestId('dhis2-uicore-splitbutton-toggle')
fireEvent.click(toggleButton)

expect(getByTestId('dhis2-uicore-splitbutton-menu')).toBeInTheDocument()
})

it("does not close dropdown 'Enter' key is pressed", async () => {
const { getByTestId } = render(
<SplitButton component={<div>Dropdown Content</div>} />
)

const toggleButton = getByTestId('dhis2-uicore-splitbutton-toggle')
fireEvent.click(toggleButton)
expect(getByTestId('dhis2-uicore-splitbutton-menu')).toBeInTheDocument()

fireEvent.keyDown(document, { key: 'Enter' })

// Use waitFor to wait for the DOM to update
await waitFor(() => {
expect(
getByTestId('dhis2-uicore-splitbutton-menu')
).toBeInTheDocument()
})
})

it('closes dropdown when escape key is pressed', async () => {
const { getByTestId, queryByTestId } = render(
<SplitButton component={<div>Dropdown Content</div>} />
)

const toggleButton = getByTestId('dhis2-uicore-splitbutton-toggle')
fireEvent.click(toggleButton)
expect(getByTestId('dhis2-uicore-splitbutton-menu')).toBeInTheDocument()

fireEvent.keyDown(document, { key: 'Escape' })

// Use waitFor to wait for the DOM to update
await waitFor(() => {
expect(
queryByTestId('dhis2-uicore-splitbutton-menu')
).not.toBeInTheDocument()
})
})

it('adds title and aria-label attributes to the right button', () => {
const { getByTestId } = render(<SplitButton />)
const toggleButton = getByTestId('dhis2-uicore-splitbutton-toggle')

expect(toggleButton).toHaveAttribute('title', 'Toggle dropdown')
expect(toggleButton).toHaveAttribute('aria-label', 'Toggle dropdown')
})
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { mount } from 'enzyme'
import React from 'react'
import { CircularLoader } from '../circular-loader.js'

describe('Circular Loader', () => {
it('renders the circular loader with aria label', () => {
const wrapper = mount(
<CircularLoader
dataTest={'circular-loader-test'}
aria-label="Circular Loader"
/>
)
const actual = wrapper.find({ 'data-test': 'circular-loader-test' })
expect(actual.prop('role')).toBe('progressbar')
expect(actual.prop('aria-label')).toBe('Circular Loader')
})

it('renders the circular loader without aria label', () => {
const wrapper = mount(
<CircularLoader dataTest={'circular-loader-test'} />
)
const actual = wrapper.find({ 'data-test': 'circular-loader-test' })
expect(actual.prop('aria-label')).toBe(undefined)
expect(actual.prop('role')).toBe('progressbar')
})
})
3 changes: 3 additions & 0 deletions components/loader/src/circular-loader/circular-loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const CircularLoader = ({
invert,
className,
dataTest,
'aria-label': ariaLabel,
}) => (
<div
role="progressbar"
Expand All @@ -20,6 +21,7 @@ const CircularLoader = ({
invert,
})}
data-test={dataTest}
aria-label={ariaLabel}
>
<style jsx>{`
div {
Expand Down Expand Up @@ -67,6 +69,7 @@ CircularLoader.defaultProps = {
}

CircularLoader.propTypes = {
'aria-label': PropTypes.string,
className: PropTypes.string,
dataTest: PropTypes.string,
extrasmall: sharedPropTypes.sizePropType,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,13 @@ export default {
const Template = (args) => <CircularLoader {...args} />

export const Default = Template.bind({})
Default.args = { 'aria-label': 'Default Loader' }

export const Small = Template.bind({})
Small.args = { small: true }
Small.args = { small: true, 'aria-label': 'Small Loader' }

export const Large = Template.bind({})
Large.args = { large: true }
Large.args = { large: true, 'aria-label': 'Large Loader' }

export const ExtraSmall = Template.bind({})
ExtraSmall.args = { extrasmall: true }
ExtraSmall.args = { extrasmall: true, 'aria-label': 'ExtraSmall Loader' }
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import { mount } from 'enzyme'
import React from 'react'
import { LinearLoader } from '../linear-loader.js'

describe('Linear Loader', () => {
it('renders the linear loader with provided aria attributes', () => {
const wrapper = mount(
<LinearLoader
dataTest={'linear-loader-test'}
aria-label="Linear Loader"
amount={15}
/>
)
const actual = wrapper.find({ 'data-test': 'linear-loader-test' })
expect(actual.prop('role')).toBe('progressbar')
expect(actual.prop('aria-label')).toBe('Linear Loader')
expect(actual.prop('aria-valuenow')).toBe(15)
})

it('renders the linear loader without aria label', () => {
const wrapper = mount(
<LinearLoader dataTest={'linear-loader-test'} amount={45} />
)
const actual = wrapper.find({ 'data-test': 'linear-loader-test' })
expect(actual.prop('role')).toBe('progressbar')
expect(actual.prop('aria-label')).toBe(undefined)
expect(actual.prop('aria-valuenow')).toBe(45)
})
})
4 changes: 4 additions & 0 deletions components/loader/src/linear-loader/linear-loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,13 @@ const LinearLoader = ({
invert,
className,
dataTest,
'aria-label': ariaLabel,
}) => {
return (
<div
role="progressbar"
aria-valuenow={amount}
aria-label={ariaLabel}
className={cx(className, { invert })}
data-test={dataTest}
>
Expand Down Expand Up @@ -78,6 +81,7 @@ LinearLoader.defaultProps = {
LinearLoader.propTypes = {
/** The progression in percent without the '%' sign */
amount: PropTypes.number.isRequired,
'aria-label': PropTypes.string,
className: PropTypes.string,
dataTest: PropTypes.string,
/** Use inverted color scheme */
Expand Down
16 changes: 12 additions & 4 deletions components/loader/src/linear-loader/linear-loader.stories.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,16 @@ export default {
componentSubtitle: subtitle,
docs: { description: { component: description } },
},
argTypes: { margin: { table: { defaultValue: { summary: '12px' } } } },
argTypes: {
margin: { table: { defaultValue: { summary: '12px' } } },
},
}

export const Determinate = (args) => <LinearLoader {...args} />
Determinate.args = { amount: 60 }
Determinate.args = {
amount: 60,
'aria-label': 'Determinate Loader',
}

export const OverlayPage = (args) => (
<Layer level={layers.blocking} translucent>
Expand All @@ -37,7 +42,7 @@ export const OverlayPage = (args) => (
</Center>
</Layer>
)
OverlayPage.args = { amount: 30 }
OverlayPage.args = { amount: 30, 'aria-label': 'Loader with Overlay Page' }
OverlayPage.parameters = { docs: { inlineStories: false } }

export const OverlayComponent = (args) => (
Expand All @@ -49,7 +54,10 @@ export const OverlayComponent = (args) => (
</Cover>
</div>
)
OverlayComponent.args = { amount: 80 }
OverlayComponent.args = {
amount: 80,
'aria-label': 'Loader with Overlay Component',
}

export const RTL = (args) => (
<div dir="rtl">
Expand Down
2 changes: 2 additions & 0 deletions components/loader/types/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ export interface CircularLoaderProps {
invert?: boolean
large?: boolean
small?: boolean
'aria-label'?: string
}

export const CircularLoader: React.FC<CircularLoaderProps>
Expand All @@ -30,6 +31,7 @@ export interface LinearLoaderProps {
* The width of the entire indicator
*/
width?: string
'aria-label'?: string
}

export const LinearLoader: React.FC<LinearLoaderProps>
Loading

0 comments on commit d8aea01

Please sign in to comment.