Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix aria roles for table headers and cells #681

Merged
merged 2 commits into from
May 20, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/Column.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ Describes the header and cell contents of a table column.
| flexShrink | Number | | Flex shrink style; defaults to 1 |
| headerClassName | String | | CSS class to apply to this column's header |
| headerRenderer | Function | | Optional callback responsible for rendering a column's header column. [Learn more](#headerrenderer) |
| id | String | | Optional id to set on the column header; used for [`aria-describedby`](https://www.w3.org/TR/wai-aria/states_and_properties#aria-describedby) |
| label | String | | Header label for this column |
| maxWidth | Number | | Maximum width of column; this property will only be used if :flexGrow is greater than 0 |
| minWidth | Number | | Minimum width of column |
Expand Down
3 changes: 3 additions & 0 deletions source/Table/Column.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@ export default class Column extends Component {
*/
headerRenderer: PropTypes.func.isRequired,

/** Optional id to set on the column header */
id: PropTypes.string,

/** Header label for this column */
label: PropTypes.string,

Expand Down
52 changes: 50 additions & 2 deletions source/Table/Table.jest.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ describe('Table', () => {
cellDataGetter,
cellRenderer,
columnData = { data: 123 },
columnID,
columnStyle,
disableSort = false,
headerRenderer,
Expand Down Expand Up @@ -70,6 +71,7 @@ describe('Table', () => {
headerRenderer={headerRenderer}
disableSort={disableSort}
style={columnStyle}
id={columnID}
/>
<Column
label='Email'
Expand Down Expand Up @@ -885,6 +887,21 @@ describe('Table', () => {
expect(row.getAttribute('role')).toEqual('row')
})

it('should set aria role on a cell', () => {
const rendered = findDOMNode(render(getMarkup()))
const cell = rendered.querySelector('.ReactVirtualized__Table__rowColumn')
expect(cell.getAttribute('role')).toEqual('gridcell')
})

it('should set aria-describedby on a cell when the column has an id', () => {
const columnID = 'column-header-test'
const rendered = findDOMNode(render(getMarkup({
columnID
})))
const cell = rendered.querySelector('.ReactVirtualized__Table__rowColumn')
expect(cell.getAttribute('aria-describedby')).toEqual(columnID)
})

it('should attach a11y properties to a row if :onRowClick is specified', () => {
const rendered = findDOMNode(render(getMarkup({
onRowClick: () => {}
Expand All @@ -903,14 +920,46 @@ describe('Table', () => {
expect(row.tabIndex).toEqual(-1)
})

it('should set aria role on a header column', () => {
const rendered = findDOMNode(render(getMarkup()))
const header = rendered.querySelector('.ReactVirtualized__Table__headerColumn')
expect(header.getAttribute('role')).toEqual('columnheader')
})

it('should set aria-sort ascending on a header column if the column is sorted ascending', () => {
const rendered = findDOMNode(render(getMarkup({
sortBy: 'name',
sortDirection: SortDirection.ASC
})))
const header = rendered.querySelector('.ReactVirtualized__Table__headerColumn')
expect(header.getAttribute('aria-sort')).toEqual('ascending')
})

it('should set aria-sort descending on a header column if the column is sorted descending', () => {
const rendered = findDOMNode(render(getMarkup({
sortBy: 'name',
sortDirection: SortDirection.DESC
})))
const header = rendered.querySelector('.ReactVirtualized__Table__headerColumn')
expect(header.getAttribute('aria-sort')).toEqual('descending')
})

it('should set id on a header column when the column has an id', () => {
const columnID = 'column-header-test'
const rendered = findDOMNode(render(getMarkup({
columnID
})))
const header = rendered.querySelector('.ReactVirtualized__Table__headerColumn')
expect(header.getAttribute('id')).toEqual(columnID)
})

it('should attach a11y properties to a header column if sort is enabled', () => {
const rendered = findDOMNode(render(getMarkup({
disableSort: false,
sort: () => {}
})))
const row = rendered.querySelector('.ReactVirtualized__Table__headerColumn')
expect(row.getAttribute('aria-label')).toEqual('Name')
expect(row.getAttribute('role')).toEqual('rowheader')
expect(row.tabIndex).toEqual(0)
})

Expand All @@ -920,7 +969,6 @@ describe('Table', () => {
})))
const row = rendered.querySelector('.ReactVirtualized__Table__headerColumn')
expect(row.getAttribute('aria-label')).toEqual(null)
expect(row.getAttribute('role')).toEqual(null)
expect(row.tabIndex).toEqual(-1)
})
})
Expand Down
25 changes: 22 additions & 3 deletions source/Table/Table.js
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,8 @@ export default class Table extends PureComponent {
cellRenderer,
className,
columnData,
dataKey
dataKey,
id
} = column.props

const cellData = cellDataGetter({ columnData, dataKey, rowData })
Expand All @@ -396,10 +397,18 @@ export default class Table extends PureComponent {
? renderedCell
: null

const a11yProps = {}

if (id) {
a11yProps['aria-describedby'] = id
}

return (
<div
{...a11yProps}
key={`Row${rowIndex}-Col${columnIndex}`}
className={cn('ReactVirtualized__Table__rowColumn', className)}
role='gridcell'
style={style}
title={title}
>
Expand All @@ -410,7 +419,7 @@ export default class Table extends PureComponent {

_createHeader ({ column, index }) {
const { headerClassName, headerStyle, onHeaderClick, sort, sortBy, sortDirection } = this.props
const { dataKey, disableSort, headerRenderer, label, columnData } = column.props
const { dataKey, disableSort, headerRenderer, id, label, columnData } = column.props
const sortEnabled = !disableSort && sort

const classNames = cn(
Expand Down Expand Up @@ -455,17 +464,27 @@ export default class Table extends PureComponent {
}

a11yProps['aria-label'] = column.props['aria-label'] || label || dataKey
a11yProps.role = 'rowheader'
a11yProps.tabIndex = 0
a11yProps.onClick = onClick
a11yProps.onKeyDown = onKeyDown
}

if (sortBy === dataKey) {
a11yProps['aria-sort'] = sortDirection === SortDirection.ASC
? 'ascending'
: 'descending'
}

if (id) {
a11yProps.id = id
}

return (
<div
{...a11yProps}
key={`Header-Col${index}`}
className={classNames}
role='columnheader'
style={style}
>
{renderedHeader}
Expand Down