From 1fc5712f18a8780bb6797f1b1ff84e5582ffe920 Mon Sep 17 00:00:00 2001
From: Jimmy Somsanith <jsomsanith@talend.com>
Date: Tue, 4 Sep 2018 18:04:37 +0200
Subject: [PATCH 1/2] Fix Table accessibility

Add aria attributes, following https://www.w3.org/TR/2018/NOTE-wai-aria-practices-1.1-20180726/examples/grid/dataGrids.html#rps_label
---
 .gitignore                               |  1 +
 source/Table/Table.jest.js               | 50 +++++++++++++++++++++++-
 source/Table/Table.js                    | 11 +++++-
 source/Table/defaultHeaderRowRenderer.js |  2 +-
 source/Table/defaultRowRenderer.js       |  2 +-
 5 files changed, 62 insertions(+), 4 deletions(-)

diff --git a/.gitignore b/.gitignore
index 78f2cd662..d4821a555 100644
--- a/.gitignore
+++ b/.gitignore
@@ -6,3 +6,4 @@ node_modules
 npm-debug.log
 styles.css
 .vscode
+.idea
diff --git a/source/Table/Table.jest.js b/source/Table/Table.jest.js
index 2fec94102..cc0f872ec 100644
--- a/source/Table/Table.jest.js
+++ b/source/Table/Table.jest.js
@@ -1191,10 +1191,29 @@ describe('Table', () => {
       expect(node.getAttribute('role')).toEqual('grid');
     });
 
+    it('should set aria col/row count on the table', () => {
+      const node = findDOMNode(render(getMarkup()));
+      expect(node.getAttribute('aria-colcount')).toEqual('2');
+      expect(node.getAttribute('aria-rowcount')).toEqual(`${list.size}`);
+    });
+
+    it('should pass down aria labels on the table', () => {
+      const node = findDOMNode(
+        render(
+          getMarkup({
+            'aria-label': 'my-table-label',
+            'aria-labelledby': 'my-table-label-id',
+          }),
+        ),
+      );
+      expect(node.getAttribute('aria-label')).toEqual('my-table-label');
+      expect(node.getAttribute('aria-labelledby')).toEqual('my-table-label-id');
+    });
+
     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');
+      expect(row.getAttribute('role')).toEqual('rowheader');
     });
 
     it('should set appropriate aria role on the grid', () => {
@@ -1209,6 +1228,13 @@ describe('Table', () => {
       expect(row.getAttribute('role')).toEqual('row');
     });
 
+    it('should set aria rowindex on a row', () => {
+      const rendered = findDOMNode(render(getMarkup()));
+      const rows = rendered.querySelectorAll('.ReactVirtualized__Table__row');
+      expect(rows[0].getAttribute('aria-rowindex')).toEqual('1');
+      expect(rows[1].getAttribute('aria-rowindex')).toEqual('2');
+    });
+
     it('should set aria role on a cell', () => {
       const rendered = findDOMNode(render(getMarkup()));
       const cell = rendered.querySelector(
@@ -1217,6 +1243,15 @@ describe('Table', () => {
       expect(cell.getAttribute('role')).toEqual('gridcell');
     });
 
+    it('should set aria colindex on a cell', () => {
+      const rendered = findDOMNode(render(getMarkup()));
+      const cells = rendered.querySelectorAll(
+        '.ReactVirtualized__Table__rowColumn',
+      );
+      expect(cells[0].getAttribute('aria-colindex')).toEqual('1');
+      expect(cells[1].getAttribute('aria-colindex')).toEqual('2');
+    });
+
     it('should set aria-describedby on a cell when the column has an id', () => {
       const columnID = 'column-header-test';
       const rendered = findDOMNode(
@@ -1296,6 +1331,19 @@ describe('Table', () => {
       expect(header.getAttribute('aria-sort')).toEqual('descending');
     });
 
+    it('should set aria-sort to "none" if the column is sortable but not the current sort', () => {
+      const rendered = findDOMNode(
+        render(getMarkup({disableSort: true, sort: jest.fn()})),
+      );
+      const headers = rendered.querySelectorAll(
+        '.ReactVirtualized__Table__headerColumn',
+      );
+      // the first column is not sortable
+      expect(headers[0].getAttribute('aria-sort')).toBe(null);
+      // the second column is sortable
+      expect(headers[1].getAttribute('aria-sort')).toEqual('none');
+    });
+
     it('should set id on a header column when the column has an id', () => {
       const columnID = 'column-header-test';
       const rendered = findDOMNode(
diff --git a/source/Table/Table.js b/source/Table/Table.js
index 4a06e460e..30320f54d 100644
--- a/source/Table/Table.js
+++ b/source/Table/Table.js
@@ -378,19 +378,26 @@ export default class Table extends React.PureComponent {
       };
     });
 
+    const columns = this._getHeaderColumns();
+
     // Note that we specify :rowCount, :scrollbarWidth, :sortBy, and :sortDirection as properties on Grid even though these have nothing to do with Grid.
     // This is done because Grid is a pure component and won't update unless its properties or state has changed.
     // Any property that should trigger a re-render of Grid then is specified here to avoid a stale display.
     return (
       <div
+        aria-label={this.props['aria-label']}
+        aria-labelledby={this.props['aria-labelledby']}
+        aria-colcount={columns.length}
+        aria-rowcount={this.props.rowCount}
         className={cn('ReactVirtualized__Table', className)}
         id={id}
         role="grid"
+        tabIndex="0"
         style={style}>
         {!disableHeader &&
           headerRowRenderer({
             className: cn('ReactVirtualized__Table__headerRow', rowClass),
-            columns: this._getHeaderColumns(),
+            columns,
             style: {
               ...rowStyleObject,
               height: headerHeight,
@@ -456,6 +463,7 @@ export default class Table extends React.PureComponent {
     // See PR https://github.com/bvaughn/react-virtualized/pull/942
     return (
       <div
+        aria-colindex={columnIndex + 1}
         aria-describedby={id}
         className={cn('ReactVirtualized__Table__rowColumn', className)}
         key={'Row' + rowIndex + '-' + 'Col' + columnIndex}
@@ -545,6 +553,7 @@ export default class Table extends React.PureComponent {
       };
 
       headerAriaLabel = column.props['aria-label'] || label || dataKey;
+      headerAriaSort = 'none';
       headerTabIndex = 0;
       headerOnClick = onClick;
       headerOnKeyDown = onKeyDown;
diff --git a/source/Table/defaultHeaderRowRenderer.js b/source/Table/defaultHeaderRowRenderer.js
index a44d82cbc..2d49473c8 100644
--- a/source/Table/defaultHeaderRowRenderer.js
+++ b/source/Table/defaultHeaderRowRenderer.js
@@ -8,7 +8,7 @@ export default function defaultHeaderRowRenderer({
   style,
 }: HeaderRowRendererParams) {
   return (
-    <div className={className} role="row" style={style}>
+    <div className={className} role="rowheader" style={style}>
       {columns}
     </div>
   );
diff --git a/source/Table/defaultRowRenderer.js b/source/Table/defaultRowRenderer.js
index ef3b056fe..836c0ff39 100644
--- a/source/Table/defaultRowRenderer.js
+++ b/source/Table/defaultRowRenderer.js
@@ -18,7 +18,7 @@ export default function defaultRowRenderer({
   rowData,
   style,
 }: RowRendererParams) {
-  const a11yProps = {};
+  const a11yProps = {'aria-rowindex': index + 1};
 
   if (
     onRowClick ||

From 98e94e24941e2884612a2dccb66fa698703ce22a Mon Sep 17 00:00:00 2001
From: Jimmy Somsanith <jsomsanith@talend.com>
Date: Thu, 6 Sep 2018 09:13:21 +0200
Subject: [PATCH 2/2] Fixes after review

---
 source/Table/Table.jest.js               |  2 +-
 source/Table/Table.js                    | 11 ++++++-----
 source/Table/defaultHeaderRowRenderer.js |  2 +-
 3 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/source/Table/Table.jest.js b/source/Table/Table.jest.js
index cc0f872ec..e38b11a22 100644
--- a/source/Table/Table.jest.js
+++ b/source/Table/Table.jest.js
@@ -1213,7 +1213,7 @@ describe('Table', () => {
     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('rowheader');
+      expect(row.getAttribute('role')).toEqual('row');
     });
 
     it('should set appropriate aria role on the grid', () => {
diff --git a/source/Table/Table.js b/source/Table/Table.js
index 30320f54d..8f000442f 100644
--- a/source/Table/Table.js
+++ b/source/Table/Table.js
@@ -19,8 +19,12 @@ import SortDirection from './SortDirection';
  */
 export default class Table extends React.PureComponent {
   static propTypes = {
+    /** This is just set on the grid top element. */
     'aria-label': PropTypes.string,
 
+    /** This is just set on the grid top element. */
+    'aria-labelledby': PropTypes.string,
+
     /**
      * Removes fixed height from the scrollingContainer so that the total height
      * of rows can stretch the window. Intended for use with WindowScroller
@@ -378,8 +382,6 @@ export default class Table extends React.PureComponent {
       };
     });
 
-    const columns = this._getHeaderColumns();
-
     // Note that we specify :rowCount, :scrollbarWidth, :sortBy, and :sortDirection as properties on Grid even though these have nothing to do with Grid.
     // This is done because Grid is a pure component and won't update unless its properties or state has changed.
     // Any property that should trigger a re-render of Grid then is specified here to avoid a stale display.
@@ -387,17 +389,16 @@ export default class Table extends React.PureComponent {
       <div
         aria-label={this.props['aria-label']}
         aria-labelledby={this.props['aria-labelledby']}
-        aria-colcount={columns.length}
+        aria-colcount={React.Children.toArray(children).length}
         aria-rowcount={this.props.rowCount}
         className={cn('ReactVirtualized__Table', className)}
         id={id}
         role="grid"
-        tabIndex="0"
         style={style}>
         {!disableHeader &&
           headerRowRenderer({
             className: cn('ReactVirtualized__Table__headerRow', rowClass),
-            columns,
+            columns: this._getHeaderColumns(),
             style: {
               ...rowStyleObject,
               height: headerHeight,
diff --git a/source/Table/defaultHeaderRowRenderer.js b/source/Table/defaultHeaderRowRenderer.js
index 2d49473c8..a44d82cbc 100644
--- a/source/Table/defaultHeaderRowRenderer.js
+++ b/source/Table/defaultHeaderRowRenderer.js
@@ -8,7 +8,7 @@ export default function defaultHeaderRowRenderer({
   style,
 }: HeaderRowRendererParams) {
   return (
-    <div className={className} role="rowheader" style={style}>
+    <div className={className} role="row" style={style}>
       {columns}
     </div>
   );