Skip to content

Commit

Permalink
fix(core): add better aria accessibility missing on menus and checkbo…
Browse files Browse the repository at this point in the history
…xes (#968)

* fix(core): add better aria accessibility missing on menus and checkboxes

* chore: add better aria accessibility in pagination
  • Loading branch information
ghiscoding committed May 8, 2023
1 parent 2d70a7c commit 8041c11
Show file tree
Hide file tree
Showing 12 changed files with 72 additions and 49 deletions.
51 changes: 28 additions & 23 deletions examples/vite-demo-vanilla-bundle/src/examples/example17.scss
Original file line number Diff line number Diff line change
Expand Up @@ -11,31 +11,36 @@ $control-height: 2.4em;
.scroll-configs input {
width: 50px;
}
.cell-effort-driven {
text-align: center;
}
.slick-group-title[level='0'] {
font-weight: bold;
}
.slick-group-title[level='1'] {
text-decoration: underline;
}
.slick-group-title[level='2'] {
font-style: italic;
}
.slick-row:not(.slick-group) >.cell-unselectable {
background: #ececec !important;
}
.slick-row .slick-cell.frozen:last-child,
.slick-header-column.frozen:last-child,
.slick-headerrow-column.frozen:last-child,
.slick-footerrow-column.frozen:last-child {
border-right: 1px solid red;
}

.slick-row.frozen:last-child .slick-cell {
border-bottom: 1px solid red;
.grid17-1, .grid17-2 {
.cell-effort-driven {
text-align: center;
}
.slick-group-title[level='0'] {
font-weight: bold;
}
.slick-group-title[level='1'] {
text-decoration: underline;
}
.slick-group-title[level='2'] {
font-style: italic;
}
.slick-row:not(.slick-group) >.cell-unselectable {
background: #ececec !important;
}
.slick-row .slick-cell.frozen:last-child,
.slick-header-column.frozen:last-child,
.slick-headerrow-column.frozen:last-child,
.slick-footerrow-column.frozen:last-child {
border-right: 1px solid red;
}

.slick-row.frozen:last-child .slick-cell,
.slick-row.frozen:last-child .slick-cell {
border-bottom: 1px solid red;
}
}

.option-item {
padding: 6px;
}
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ describe('SlickCheckboxSelectColumn Plugin', () => {
expect(plugin).toBeTruthy();
expect(updateColHeaderSpy).toHaveBeenCalledWith(
'_checkbox_selector',
`<input id="header-selector${plugin.selectAllUid}" type="checkbox"><label for="header-selector${plugin.selectAllUid}"></label>`,
`<input id="header-selector${plugin.selectAllUid}" type="checkbox" aria-checked="false"><label for="header-selector${plugin.selectAllUid}"></label>`,
'Select/Deselect All'
);
expect(preventDefaultSpy).toHaveBeenCalled();
Expand Down Expand Up @@ -629,7 +629,7 @@ describe('SlickCheckboxSelectColumn Plugin', () => {
expect(setSelectedRowSpy).not.toHaveBeenCalled();
expect(updateColumnHeaderSpy).toHaveBeenCalledWith(
'_checkbox_selector',
`<input id="header-selector${plugin.selectAllUid}" type="checkbox"><label for="header-selector${plugin.selectAllUid}"></label>`,
`<input id="header-selector${plugin.selectAllUid}" type="checkbox" aria-checked="false"><label for="header-selector${plugin.selectAllUid}"></label>`,
'Select/Deselect All'
);
});
Expand All @@ -655,7 +655,7 @@ describe('SlickCheckboxSelectColumn Plugin', () => {
expect(setSelectedRowSpy).not.toHaveBeenCalled();
expect(updateColumnHeaderSpy).toHaveBeenCalledWith(
'_checkbox_selector',
`<input id="header-selector${plugin.selectAllUid}" type="checkbox"><label for="header-selector${plugin.selectAllUid}"></label>`,
`<input id="header-selector${plugin.selectAllUid}" type="checkbox" aria-checked="false"><label for="header-selector${plugin.selectAllUid}"></label>`,
'Select/Deselect All'
);
});
Expand Down Expand Up @@ -686,7 +686,7 @@ describe('SlickCheckboxSelectColumn Plugin', () => {
expect(setSelectedRowSpy).toHaveBeenCalled();
expect(updateColumnHeaderSpy).toHaveBeenCalledWith(
'_checkbox_selector',
`<input id="header-selector${plugin.selectAllUid}" type="checkbox" checked="checked"><label for="header-selector${plugin.selectAllUid}"></label>`,
`<input id="header-selector${plugin.selectAllUid}" type="checkbox" checked="checked" aria-checked="true"><label for="header-selector${plugin.selectAllUid}"></label>`,
'Select/Deselect All'
);
});
Expand Down Expand Up @@ -718,7 +718,7 @@ describe('SlickCheckboxSelectColumn Plugin', () => {
expect(plugin).toBeTruthy();
expect(updateColumnHeaderSpy).toHaveBeenCalledWith(
'_checkbox_selector',
`<input id="header-selector${plugin.selectAllUid}" type="checkbox" checked="checked"><label for="header-selector${plugin.selectAllUid}"></label>`,
`<input id="header-selector${plugin.selectAllUid}" type="checkbox" checked="checked" aria-checked="true"><label for="header-selector${plugin.selectAllUid}"></label>`,
'Select/Deselect All'
);
});
Expand Down
6 changes: 6 additions & 0 deletions packages/common/src/extensions/extensionCommonUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,19 +40,22 @@ export function handleColumnPickerItemClick(this: SlickColumnPicker | SlickGridM
// when calling setOptions, it will resize with ALL Columns (even the hidden ones)
// we can avoid this problem by keeping a reference to the visibleColumns before setOptions and then setColumns after
const previousVisibleColumns = context.getVisibleColumns();
event.target.ariaChecked = String(event.target.checked);
const isChecked = event.target.checked;
context.grid.setOptions({ forceFitColumns: isChecked });
context.grid.setColumns(previousVisibleColumns);
return;
}

if (event.target.dataset.option === 'syncresize') {
event.target.ariaChecked = String(event.target.checked);
context.grid.setOptions({ syncColumnCellResize: !!(event.target.checked) });
return;
}

if (event.target.type === 'checkbox') {
context._areVisibleColumnDifferent = true;
event.target.ariaChecked = String(event.target.checked);
const isChecked = event.target.checked;
const columnId = event.target.dataset.columnid || '';
const visibleColumns: Column[] = [];
Expand Down Expand Up @@ -126,6 +129,7 @@ export function populateColumnPicker(this: SlickColumnPicker | SlickGridMenu, ad
});
const colIndex = context.grid.getColumnIndex(columnId);
if (colIndex >= 0) {
colInputElm.ariaChecked = 'true';
colInputElm.checked = true;
}
columnLiElm.appendChild(colInputElm);
Expand All @@ -152,6 +156,7 @@ export function populateColumnPicker(this: SlickColumnPicker | SlickGridMenu, ad
fitLiElm.appendChild(
createDomElement('input', {
type: 'checkbox', id: `${context._gridUid}-${menuPrefix}colpicker-forcefit`,
ariaChecked: String(context.gridOptions.forceFitColumns),
checked: context.gridOptions.forceFitColumns,
dataset: { option: 'autoresize' }
})
Expand All @@ -170,6 +175,7 @@ export function populateColumnPicker(this: SlickColumnPicker | SlickGridMenu, ad
syncLiElm.appendChild(
createDomElement('input', {
type: 'checkbox', id: `${context._gridUid}-${menuPrefix}colpicker-syncresize`,
ariaChecked: String(context.gridOptions.syncColumnCellResize),
checked: context.gridOptions.syncColumnCellResize,
dataset: { option: 'syncresize' }
})
Expand Down
2 changes: 1 addition & 1 deletion packages/common/src/extensions/menuBaseClass.ts
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ export class MenuBaseClass<M extends CellMenu | ContextMenu | GridMenu | HeaderM
item.disabled = isItemUsable ? false : true;
}

commandLiElm = createDomElement('li', { className: menuCssPrefix });
commandLiElm = createDomElement('li', { className: menuCssPrefix, role: 'menuitem' });
if (typeof item === 'object' && hasData((item as never)[itemType])) {
commandLiElm.dataset[itemType] = (item as never)?.[itemType];
}
Expand Down
4 changes: 2 additions & 2 deletions packages/common/src/extensions/menuFromCellBaseClass.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ export class MenuFromCellBaseClass<M extends CellMenu | ContextMenu> extends Men

// -- Option List section
if (!(this.addonOptions as CellMenu | ContextMenu).hideOptionSection && isColumnOptionAllowed && optionItems.length > 0) {
const optionMenuElm = createDomElement('div', { className: `${this._menuCssPrefix}-option-list` });
const optionMenuElm = createDomElement('div', { className: `${this._menuCssPrefix}-option-list`, role: 'menu' });
this.populateCommandOrOptionTitle('option', this.addonOptions, optionMenuElm);
if (!this.addonOptions.hideCloseButton) {
this.populateCommandOrOptionCloseBtn('option', closeButtonElm, optionMenuElm);
Expand All @@ -123,7 +123,7 @@ export class MenuFromCellBaseClass<M extends CellMenu | ContextMenu> extends Men

// -- Command List section
if (!(this.addonOptions as CellMenu | ContextMenu).hideCommandSection && isColumnCommandAllowed && commandItems.length > 0) {
const commandMenuElm = createDomElement('div', { className: `${this._menuCssPrefix}-command-list` });
const commandMenuElm = createDomElement('div', { className: `${this._menuCssPrefix}-command-list`, role: 'menu' });
this.populateCommandOrOptionTitle('command', this.addonOptions, commandMenuElm);
if (!this.addonOptions.hideCloseButton && (!isColumnOptionAllowed || optionItems.length === 0 || (this.addonOptions as CellMenu | ContextMenu).hideOptionSection)) {
this.populateCommandOrOptionCloseBtn('command', closeButtonElm, commandMenuElm);
Expand Down
14 changes: 11 additions & 3 deletions packages/common/src/extensions/slickCheckboxSelectColumn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,10 @@ export class SlickCheckboxSelectColumn<T = any> {
const selectAllContainerElm = this.headerRowNode?.querySelector<HTMLSpanElement>('#filter-checkbox-selectall-container');
if (selectAllContainerElm) {
selectAllContainerElm.style.display = 'flex';
selectAllContainerElm.ariaChecked = String(this._isSelectAllChecked);
const selectAllInputElm = selectAllContainerElm.querySelector<HTMLInputElement>('input[type="checkbox"]');
if (selectAllInputElm) {
selectAllInputElm.ariaChecked = String(this._isSelectAllChecked);
selectAllInputElm.checked = this._isSelectAllChecked;
}
}
Expand Down Expand Up @@ -287,7 +289,7 @@ export class SlickCheckboxSelectColumn<T = any> {
emptyElement(args.node);

// <span class="container"><input type="checkbox"><label for="checkbox"></label></span>
const spanElm = createDomElement('span', { id: 'filter-checkbox-selectall-container' });
const spanElm = createDomElement('span', { id: 'filter-checkbox-selectall-container', ariaChecked: 'false' });
spanElm.appendChild(
createDomElement('input', { type: 'checkbox', id: `header-filter-selector${this._selectAll_UID}` })
);
Expand All @@ -305,7 +307,7 @@ export class SlickCheckboxSelectColumn<T = any> {
protected checkboxSelectionFormatter(row: number, cell: number, value: any, columnDef: Column, dataContext: any, grid: SlickGrid) {
if (dataContext && this.checkSelectableOverride(row, dataContext, grid)) {
const UID = this.createUID() + row;
return `<input id="selector${UID}" type="checkbox" ${this._selectedRowsLookup[row] ? `checked="checked"` : ''}><label for="selector${UID}"></label>`;
return `<input id="selector${UID}" type="checkbox" ${this._selectedRowsLookup[row] ? `checked="checked" aria-checked="true"` : 'aria-checked="false"'}><label for="selector${UID}"></label>`;
}
return null;
}
Expand Down Expand Up @@ -359,6 +361,7 @@ export class SlickCheckboxSelectColumn<T = any> {
if (!this._addonOptions.hideInFilterHeaderRow) {
const selectAllElm = this.headerRowNode?.querySelector<HTMLInputElement>(`#header-filter-selector${this._selectAll_UID}`);
if (selectAllElm) {
selectAllElm.ariaChecked = String(this._isSelectAllChecked);
selectAllElm.checked = this._isSelectAllChecked;
}
}
Expand All @@ -367,6 +370,8 @@ export class SlickCheckboxSelectColumn<T = any> {
protected handleClick(e: DOMMouseOrTouchEvent<HTMLInputElement>, args: { row: number; cell: number; grid: SlickGrid; }) {
// clicking on a row select checkbox
if (this._grid.getColumns()[args.cell].id === this._addonOptions.columnId && e.target.type === 'checkbox') {
e.target.ariaChecked = String(e.target.checked);

// if editing, try to commit
if (this._grid.getEditorLock().isActive() && !this._grid.getEditorLock().commitCurrentEdit()) {
e.preventDefault();
Expand All @@ -382,6 +387,8 @@ export class SlickCheckboxSelectColumn<T = any> {

protected handleHeaderClick(e: DOMMouseOrTouchEvent<HTMLInputElement>, args: { column: Column; node: HTMLDivElement; grid: SlickGrid; }) {
if (args.column.id === this._addonOptions.columnId && e.target.type === 'checkbox') {
e.target.ariaChecked = String(e.target.checked);

// if editing, try to commit
if (this._grid.getEditorLock().isActive() && !this._grid.getEditorLock().commitCurrentEdit()) {
e.preventDefault();
Expand Down Expand Up @@ -504,6 +511,7 @@ export class SlickCheckboxSelectColumn<T = any> {
if (!this._addonOptions.hideInFilterHeaderRow) {
const selectAllElm = this.headerRowNode?.querySelector<HTMLInputElement>(`#header-filter-selector${this._selectAll_UID}`);
if (selectAllElm) {
selectAllElm.ariaChecked = String(this._isSelectAllChecked);
selectAllElm.checked = this._isSelectAllChecked;
}
}
Expand All @@ -520,7 +528,7 @@ export class SlickCheckboxSelectColumn<T = any> {
}

protected renderSelectAllCheckbox(isSelectAllChecked: boolean) {
const checkedStr = isSelectAllChecked ? ` checked="checked"` : '';
const checkedStr = isSelectAllChecked ? ` checked="checked" aria-checked="true"` : ' aria-checked="false"';
this._grid.updateColumnHeader(
this._addonOptions.columnId || '',
`<input id="header-selector${this._selectAll_UID}" type="checkbox"${checkedStr}><label for="header-selector${this._selectAll_UID}"></label>`,
Expand Down
4 changes: 2 additions & 2 deletions packages/common/src/extensions/slickColumnPicker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ export class SlickColumnPicker {
this._eventHandler.subscribe(this.grid.onColumnsReordered, updateColumnPickerOrder.bind(this) as EventListener);

this._menuElm = createDomElement('div', {
className: `slick-column-picker ${this._gridUid}`,
className: `slick-column-picker ${this._gridUid}`, role: 'menu',
style: { display: 'none' },
});
this._menuElm.setAttribute('aria-expanded', 'false');
Expand All @@ -112,7 +112,7 @@ export class SlickColumnPicker {
addColumnTitleElementWhenDefined.call(this, this._menuElm);
addCloseButtomElement.call(this, this._menuElm);

this._listElm = createDomElement('div', { className: 'slick-column-picker-list' });
this._listElm = createDomElement('div', { className: 'slick-column-picker-list', role: 'menu' });
this._bindEventService.bind(this._menuElm, 'click', handleColumnPickerItemClick.bind(this) as EventListener);

// Hide the menu on outside click.
Expand Down
14 changes: 8 additions & 6 deletions packages/common/src/extensions/slickGridMenu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ export class SlickGridMenu extends MenuBaseClass<GridMenu> {
// user could pass a title on top of the columns list
addColumnTitleElementWhenDefined.call(this, this._menuElm);

this._listElm = createDomElement('div', { className: 'slick-column-picker-list' });
this._listElm = createDomElement('div', { className: 'slick-column-picker-list', role: 'menu' });

// update all columns on any of the column title button click from column picker
this._bindEventService.bind(this._menuElm, 'click', handleColumnPickerItemClick.bind(this) as EventListener);
Expand Down Expand Up @@ -203,7 +203,7 @@ export class SlickGridMenu extends MenuBaseClass<GridMenu> {

const showButton = this._gridMenuOptions.showButton ?? this._defaults.showButton;
if (showButton) {
this._gridMenuButtonElm = createDomElement('button', { className: 'slick-grid-menu-button' });
this._gridMenuButtonElm = createDomElement('button', { className: 'slick-grid-menu-button', ariaLabel: 'Grid Menu' });
if (this._gridMenuOptions?.iconCssClass) {
this._gridMenuButtonElm.classList.add(...this._gridMenuOptions.iconCssClass.split(' '));
}
Expand All @@ -219,11 +219,13 @@ export class SlickGridMenu extends MenuBaseClass<GridMenu> {
this.translateTitleLabels(this._gridMenuOptions);
this.translateTitleLabels(this.sharedService.gridOptions.gridMenu);

this._menuElm = document.createElement('div');
this._menuElm.classList.add('slick-grid-menu', this._gridUid);
this._menuElm.style.display = 'none';
this._menuElm = createDomElement('div', {
role: 'menu',
className: `slick-grid-menu ${this._gridUid}`,
style: { display: 'none' }
});

this._commandMenuElm = createDomElement('div', { className: 'slick-menu-command-list' });
this._commandMenuElm = createDomElement('div', { className: 'slick-menu-command-list', role: 'menu' });
this._menuElm.appendChild(this._commandMenuElm);

this.recreateCommandList(this._gridMenuOptions, {
Expand Down
4 changes: 2 additions & 2 deletions packages/common/src/extensions/slickHeaderMenu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ export class SlickHeaderMenu extends MenuBaseClass<HeaderMenu> {

if (!this._menuElm) {
this._menuElm = createDomElement('div', {
className: 'slick-header-menu',
className: 'slick-header-menu', role: 'menu',
style: { minWidth: `${this.addonOptions.minWidth}px` },
});
this._menuElm.setAttribute('aria-expanded', 'true');
Expand Down Expand Up @@ -178,7 +178,7 @@ export class SlickHeaderMenu extends MenuBaseClass<HeaderMenu> {
return;
}

const headerButtonDivElm = createDomElement('div', { className: 'slick-header-menu-button' });
const headerButtonDivElm = createDomElement('div', { className: 'slick-header-menu-button', ariaLabel: 'Header Menu' });

if (this.addonOptions.buttonCssClass) {
headerButtonDivElm.classList.add(...this.addonOptions.buttonCssClass.split(' '));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ describe('Slick-Pagination Component', () => {

expect(translateService.getCurrentLanguage()).toBe('en');
expect(removeExtraSpaces(pageInfoFromTo.innerHTML)).toBe('<span class="item-from" data-test="item-from" aria-label="Page Item From">10</span>-<span class="item-to" data-test="item-to" aria-label="Page Item To">15</span> <span class="text-of">of</span> ');
expect(removeExtraSpaces(pageInfoTotalItems.innerHTML)).toBe('<span class="total-items" data-test="total-items">95</span> <span class="text-items">items</span> ');
expect(removeExtraSpaces(pageInfoTotalItems.innerHTML)).toBe('<span class="total-items" data-test="total-items" aria-label="Total Items">95</span> <span class="text-items">items</span> ');
component.dispose();
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ describe('Slick-Pagination Component', () => {

expect(translateService.getCurrentLanguage()).toBe('en');
expect(removeExtraSpaces(pageInfoFromTo.innerHTML)).toBe('<span class="item-from" data-test="item-from" aria-label="Page Item From">10</span>-<span class="item-to" data-test="item-to" aria-label="Page Item To">15</span> <span class="text-of">of</span> ');
expect(removeExtraSpaces(pageInfoTotalItems.innerHTML)).toBe('<span class="total-items" data-test="total-items">95</span> <span class="text-items">items</span> ');
expect(removeExtraSpaces(pageInfoTotalItems.innerHTML)).toBe('<span class="total-items" data-test="total-items" aria-label="Total Items">95</span> <span class="text-items">items</span> ');
expect(itemsPerPage.selectedOptions[0].value).toBe('5');
});

Expand Down Expand Up @@ -256,7 +256,7 @@ describe('with different i18n locale', () => {

it('should throw an error when enabling translate without a Translate Service', () => {
mockGridOptions.enableTranslate = true;
expect(() => new SlickPaginationComponent(paginationServiceStub, eventPubSubService, sharedService, null))
expect(() => new SlickPaginationComponent(paginationServiceStub, eventPubSubService, sharedService, null as any))
.toThrow('[Slickgrid-Universal] requires a Translate Service to be installed and configured when the grid option "enableTranslate" is enabled.');
});

Expand All @@ -269,7 +269,7 @@ describe('with different i18n locale', () => {
const pageInfoTotalItems = document.querySelector('.page-info-total-items') as HTMLSpanElement;
expect(translateService.getCurrentLanguage()).toBe('fr');
expect(removeExtraSpaces(pageInfoFromTo.innerHTML)).toBe(`<span class="item-from" data-test="item-from" aria-label="Page Item From">10</span>-<span class="item-to" data-test="item-to" aria-label="Page Item To">15</span> <span class="text-of">de</span> `);
expect(removeExtraSpaces(pageInfoTotalItems.innerHTML)).toBe(`<span class="total-items" data-test="total-items">95</span> <span class="text-items">éléments</span> `);
expect(removeExtraSpaces(pageInfoTotalItems.innerHTML)).toBe(`<span class="total-items" data-test="total-items" aria-label="Total Items">95</span> <span class="text-items">éléments</span> `);
done();
}, 50);
});
Expand Down
Loading

0 comments on commit 8041c11

Please sign in to comment.