Skip to content

Commit

Permalink
Fix aria roles for the table component
Browse files Browse the repository at this point in the history
Move role="grid" from the inner grid to the table and add
role="row" to all table rows. Fixes #606.
  • Loading branch information
jchen527 committed Mar 4, 2017
1 parent 3fc7b9d commit c506d71
Show file tree
Hide file tree
Showing 7 changed files with 49 additions and 4 deletions.
1 change: 1 addition & 0 deletions docs/Grid.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ A windowed grid of elements. `Grid` only renders cells necessary to fill itself
| overscanColumnCount | Number | | Number of columns to render before/after the visible slice of the grid. This can help reduce flickering during scrolling on certain browsers/devices. |
| overscanIndicesGetter | Function | | Responsible for calculating the number of cells to overscan before and after a specified range [Learn more](#overscanindicesgetter) |
| overscanRowCount | Number | | Number of rows to render above/below the visible slice of the grid. This can help reduce flickering during scrolling on certain browsers/devices. |
| role | String | | Optional override of ARIA role default; defaults to `grid`. |
| rowCount | Number || Number of rows in grid. |
| rowHeight | Number or Function || Either a fixed row height (number) or a function that returns the height of a row given its index: `({ index: number }): number` |
| scrollingResetTimeInterval | Number | | Wait this amount of time after the last scroll event before resetting Grid `pointer-events`; defaults to 150ms. |
Expand Down
13 changes: 13 additions & 0 deletions source/Grid/Grid.jest.js
Original file line number Diff line number Diff line change
Expand Up @@ -1465,6 +1465,19 @@ describe('Grid', () => {
})
})

describe('role', () => {
it('should have grid role by default', () => {
const rendered = findDOMNode(render(getMarkup()))
expect(rendered.getAttribute('role')).toEqual('grid')
})

it('should allow role to be overridden', () => {
const role = null
const rendered = findDOMNode(render(getMarkup({ role })))
expect(rendered.getAttribute('role')).toEqual(role)
})
})

describe('pure', () => {
it('should not re-render unless props have changed', () => {
let cellRendererCalled = false
Expand Down
9 changes: 8 additions & 1 deletion source/Grid/Grid.js
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,11 @@ export default class Grid extends PureComponent {
*/
overscanRowCount: PropTypes.number.isRequired,

/**
* ARIA role for the grid element.
*/
role: PropTypes.string,

/**
* Either a fixed row height (number) or a function that returns the height of a row given its index.
* Should implement the following interface: ({ index: number }): number
Expand Down Expand Up @@ -223,6 +228,7 @@ export default class Grid extends PureComponent {
overscanColumnCount: 0,
overscanIndicesGetter: defaultOverscanIndicesGetter,
overscanRowCount: 10,
role: 'grid',
scrollingResetTimeInterval: DEFAULT_SCROLLING_RESET_TIME_INTERVAL,
scrollToAlignment: 'auto',
scrollToColumn: -1,
Expand Down Expand Up @@ -634,6 +640,7 @@ export default class Grid extends PureComponent {
height,
id,
noContentRenderer,
role,
style,
tabIndex,
width
Expand Down Expand Up @@ -687,7 +694,7 @@ export default class Grid extends PureComponent {
className={cn('ReactVirtualized__Grid', className)}
id={id}
onScroll={this._onScroll}
role='grid'
role={role}
style={{
...gridStyle,
...style
Expand Down
25 changes: 23 additions & 2 deletions source/Table/Table.jest.js
Original file line number Diff line number Diff line change
Expand Up @@ -876,13 +876,35 @@ describe('Table', () => {
})

describe('a11y properties', () => {
it('should set aria role on the table', () => {
const node = findDOMNode(render(getMarkup()))
expect(node.getAttribute('role')).toEqual('grid')
})

it('should set aria role on the header row', () => {
const rendered = findDOMNode(render(getMarkup()))
const row = rendered.querySelector('.ReactVirtualized__Table__headerRow')
expect(row.getAttribute('role')).toEqual('row')
})

it('should not set aria role on the grid', () => {
const rendered = findDOMNode(render(getMarkup()))
const grid = rendered.querySelector('.ReactVirtualized__Table__Grid')
expect(grid.getAttribute('role')).toEqual(null)
})

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

it('should attach a11y properties to a row if :onRowClick is specified', () => {
const rendered = findDOMNode(render(getMarkup({
onRowClick: () => {}
})))
const row = rendered.querySelector('.ReactVirtualized__Table__row')
expect(row.getAttribute('aria-label')).toEqual('row')
expect(row.getAttribute('role')).toEqual('row')
expect(row.tabIndex).toEqual(0)
})

Expand All @@ -892,7 +914,6 @@ describe('Table', () => {
})))
const row = rendered.querySelector('.ReactVirtualized__Table__row')
expect(row.getAttribute('aria-label')).toEqual(null)
expect(row.getAttribute('role')).toEqual(null)
expect(row.tabIndex).toEqual(-1)
})

Expand Down
2 changes: 2 additions & 0 deletions source/Table/Table.js
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,7 @@ export default class Table extends PureComponent {
<div
className={cn('ReactVirtualized__Table', className)}
id={id}
role='grid'
style={style}
>
{!disableHeader && (
Expand Down Expand Up @@ -335,6 +336,7 @@ export default class Table extends PureComponent {
onScroll={this._onScroll}
onSectionRendered={this._onSectionRendered}
ref={this._setRef}
role={null}
scrollbarWidth={scrollbarWidth}
scrollToRow={scrollToIndex}
style={{
Expand Down
1 change: 1 addition & 0 deletions source/Table/defaultHeaderRowRenderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ export default function defaultHeaderRowRenderer ({
}: HeaderRowRendererParams) {
return <div
className={className}
role='row'
style={style}
>
{columns}
Expand Down
2 changes: 1 addition & 1 deletion source/Table/defaultRowRenderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ export default function defaultRowRenderer ({
onRowMouseOut
) {
a11yProps['aria-label'] = 'row'
a11yProps.role = 'row'
a11yProps.tabIndex = 0

if (onRowClick) {
Expand All @@ -49,6 +48,7 @@ export default function defaultRowRenderer ({
{...a11yProps}
className={className}
key={key}
role='row'
style={style}
>
{columns}
Expand Down

0 comments on commit c506d71

Please sign in to comment.