-
Notifications
You must be signed in to change notification settings - Fork 858
[EuiBasicTable & EuiInMemoryTable] Fix multiple accessibility/axe errors on EuiBasicTable and EuiInMemoryTable pages #5241
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
Changes from all commits
06fbd23
08ca142
20afa32
51c053b
b517a77
7d4e735
b42992b
6477253
c4f6773
ff2d070
68b48a6
cc50123
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,6 +28,7 @@ import { | |
| EuiTableRowCellCheckbox, | ||
| EuiTableSortMobile, | ||
| EuiTableHeaderMobile, | ||
| EuiScreenReaderOnly, | ||
| } from '../../../../../src/components'; | ||
|
|
||
| import { | ||
|
|
@@ -248,7 +249,8 @@ export default class extends Component { | |
| }, | ||
| { | ||
| id: 'type', | ||
| label: '', | ||
| label: 'Type', | ||
| isVisuallyHiddenLabel: true, | ||
| alignment: LEFT_ALIGNMENT, | ||
| width: '24px', | ||
| cellProvider: (cell) => <EuiIcon type={cell} size="m" />, | ||
|
|
@@ -320,7 +322,8 @@ export default class extends Component { | |
| }, | ||
| { | ||
| id: 'actions', | ||
| label: '', | ||
| label: 'Actions', | ||
| isVisuallyHiddenLabel: true, | ||
| alignment: RIGHT_ALIGNMENT, | ||
| isActionsPopover: true, | ||
| width: '32px', | ||
|
|
@@ -437,8 +440,10 @@ export default class extends Component { | |
| renderSelectAll = (mobile) => { | ||
| return ( | ||
| <EuiCheckbox | ||
| id="selectAllCheckbox" | ||
| label={mobile ? 'Select all' : null} | ||
| id={mobile ? 'selectAllCheckboxMobile' : 'selectAllCheckboxDesktop'} | ||
| label={mobile ? 'Select all rows' : null} | ||
| aria-label="Select all rows" | ||
| title="Select all rows" | ||
| checked={this.areAllItemsSelected()} | ||
| onChange={this.toggleAll.bind(this)} | ||
| type={mobile ? null : 'inList'} | ||
|
|
@@ -473,6 +478,14 @@ export default class extends Component { | |
| {this.renderSelectAll()} | ||
| </EuiTableHeaderCellCheckbox> | ||
| ); | ||
| } else if (column.isVisuallyHiddenLabel) { | ||
| headers.push( | ||
| <EuiTableHeaderCell key={column.id} width={column.width}> | ||
| <EuiScreenReaderOnly> | ||
| <span>{column.label}</span> | ||
| </EuiScreenReaderOnly> | ||
| </EuiTableHeaderCell> | ||
| ); | ||
| } else { | ||
| headers.push( | ||
| <EuiTableHeaderCell | ||
|
|
@@ -511,6 +524,8 @@ export default class extends Component { | |
| checked={this.isItemSelected(item.id)} | ||
| onChange={this.toggleItem.bind(this, item.id)} | ||
| type="inList" | ||
| title="Select this row" | ||
| aria-label="Select this row" | ||
| /> | ||
| </EuiTableRowCellCheckbox> | ||
| ); | ||
|
|
@@ -741,6 +756,7 @@ export default class extends Component { | |
| <EuiSpacer size="m" /> | ||
|
|
||
| <EuiTablePagination | ||
| tableCaption="Custom EuiTable demo" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @constancecchen Question here, is this supposed to be right? In master I'm getting a dev console error that
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I'd offer to open a PR but I'm a little knee-deep in last minute data grid fires ATM, feel free to open an issue and I can hopefully get to it later! |
||
| aria-controls={exampleId} | ||
| activePage={this.pager.getCurrentPageIndex()} | ||
| itemsPerPage={this.state.itemsPerPage} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,7 @@ import { | |
| EuiHealth, | ||
| EuiButton, | ||
| EuiDescriptionList, | ||
| EuiScreenReaderOnly, | ||
| } from '../../../../../src/components'; | ||
|
|
||
| import { RIGHT_ALIGNMENT } from '../../../../../src/services'; | ||
|
|
@@ -160,6 +161,11 @@ export const Table = () => { | |
| align: RIGHT_ALIGNMENT, | ||
| width: '40px', | ||
| isExpander: true, | ||
| name: ( | ||
| <EuiScreenReaderOnly> | ||
| <span>Expand rows</span> | ||
| </EuiScreenReaderOnly> | ||
| ), | ||
|
Comment on lines
+164
to
+168
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This addition was optional with b517a77, but I added it because:
Demo of visually hidden table column heading with SR text: sr-table-heading.mp4 |
||
| render: (item) => ( | ||
| <EuiButtonIcon | ||
| onClick={() => toggleDetails(item)} | ||
|
|
@@ -195,6 +201,7 @@ export const Table = () => { | |
| <Fragment> | ||
| {deleteButton} | ||
| <EuiBasicTable | ||
| tableCaption="Demo of EuiBasicTable with expanding rows" | ||
| items={pageOfItems} | ||
| itemId="id" | ||
| itemIdToExpandedRowMap={itemIdToExpandedRowMap} | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feel free to test locally yourself with
yarn test-a11y!