Skip to content

Commit

Permalink
feat: add warnings to the button component for accessibility
Browse files Browse the repository at this point in the history
  • Loading branch information
Chisomchima committed Feb 15, 2024
1 parent bb83b95 commit 1da0fa5
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 2 deletions.
4 changes: 2 additions & 2 deletions collections/forms/i18n/en.pot
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ msgstr ""
"Content-Type: text/plain; charset=utf-8\n"
"Content-Transfer-Encoding: 8bit\n"
"Plural-Forms: nplurals=2; plural=(n != 1)\n"
"POT-Creation-Date: 2024-02-12T14:58:56.792Z\n"
"PO-Revision-Date: 2024-02-12T14:58:56.792Z\n"
"POT-Creation-Date: 2024-02-15T10:07:40.463Z\n"
"PO-Revision-Date: 2024-02-15T10:07:40.463Z\n"

msgid "Upload file"
msgstr "Upload file"
Expand Down
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, 'warn').mockImplementation()
})

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

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

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

it('does not warn if aria-label or title is present', () => {
render(
<Button ariaLabel="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 or ariaLabel attribute.'
)
})

it('No warning if there are no children but arial label and title', () => {
render(
<Button ariaLabel="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
7 changes: 7 additions & 0 deletions components/button/src/button/button.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,13 @@ 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.warn(
'Button component has no children but is missing title or 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

0 comments on commit 1da0fa5

Please sign in to comment.