Skip to content

Commit

Permalink
fix(table): sortable column headers will now use <button> element f…
Browse files Browse the repository at this point in the history
…or a11y purposes (#416)
  • Loading branch information
DRiFTy17 authored Oct 27, 2023
1 parent 52a0bec commit 43f775e
Show file tree
Hide file tree
Showing 5 changed files with 112 additions and 7 deletions.
22 changes: 20 additions & 2 deletions src/lib/table/_mixins.scss
Original file line number Diff line number Diff line change
Expand Up @@ -108,10 +108,14 @@
display: none;
}
}

&--sortable {
@include head-cell-sortable(forge-table-head__cell-sort-icon, forge-table-head__cell__sort-order);


button {
@include head-cell-sortable-button;
}

&.forge-table-head__cell--sorted-ascending {
.forge-table-head__cell-sort-icon {
@include head-cell-sort-icon-ascending;
Expand Down Expand Up @@ -433,6 +437,20 @@
}
}

@mixin head-cell-sortable-button {
cursor: pointer;
border: none;
background: transparent;
text-align: inherit;
margin: inherit;
padding-block: 4px;
padding-inline: 0;
font: inherit;
color: inherit;
width: 100%;
outline-offset: 4px;
}

/// The base table body row styles for row interactions.
@mixin body-row() {
height: variables.$tbody-row-height;
Expand Down
19 changes: 18 additions & 1 deletion src/lib/table/table-foundation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ export class TableFoundation implements ITableFoundation {
private _rowDoubleClickListener: (evt: Event) => void;
private _selectRowListener: (evt: Event) => void;
private _selectAllListener: (evt: Event) => void;
private _sortableHeadCellKeydownListener: EventListener;
private _headRowMouseDownListener: (evt: MouseEvent) => void;
private _headRowContextMenuListener: (evt: MouseEvent) => void;
private _documentMouseMoveListener: (evt: MouseEvent) => void;
Expand Down Expand Up @@ -132,6 +133,7 @@ export class TableFoundation implements ITableFoundation {
this._rowDoubleClickListener = evt => this._onRowDoubleClick(evt);
this._selectRowListener = evt => this._onRowSelected(evt);
this._selectAllListener = evt => this._onSelectAll(evt);
this._sortableHeadCellKeydownListener = (evt: KeyboardEvent) => this._onSortableHeadCellKeydown(evt);
this._headRowMouseDownListener = evt => this._onHeadRowMouseDown(evt);
this._headRowContextMenuListener = evt => this._onHeadRowContextMenu(evt);
this._documentMouseMoveListener = evt => this._onMouseMove(evt);
Expand Down Expand Up @@ -224,6 +226,7 @@ export class TableFoundation implements ITableFoundation {
doubleClickListener: this._allowRowClick ? this._rowDoubleClickListener : null,
selectListener: this._select ? this._selectRowListener : null,
selectAllListener: this._multiselect ? this._selectAllListener : null,
sortableHeadCellKeydownListener: this._sortableHeadCellKeydownListener,
headRowMouseDownListener: this._headRowMouseDownListener,
headRowContextMenuListener: this._headRowContextMenuListener,
filterListener: this._filter ? this._filterListener : null,
Expand Down Expand Up @@ -905,7 +908,11 @@ export class TableFoundation implements ITableFoundation {
}

private _onHeadRowContextMenu(evt: MouseEvent): void {
evt.preventDefault();
// We only handle this event on MacOS due to the `ctrl+click` combination triggering the contextmenu event...
// So we only detect that scenario here to still allow for the default context menu on Mac when right-clicking
if (evt.ctrlKey) {
evt.preventDefault();
}
}

private _onHeadRowMouseDown(evt: MouseEvent): void {
Expand Down Expand Up @@ -979,6 +986,16 @@ export class TableFoundation implements ITableFoundation {
this._headCellMouseDownIndex = undefined;
}

private _onSortableHeadCellKeydown(evt: KeyboardEvent): void {
if (evt.key === ' ' || evt.key === 'Enter') {
const composedPath = getEventPath(evt);
const rowElement = composedPath.find(el => el.tagName === 'TR') as HTMLTableRowElement;
const thElement = composedPath.find(el => el.tagName === 'TH') as HTMLTableCellElement;
const cellIndex = Array.from(rowElement.cells).findIndex(c => c === thElement);
this._onSort(cellIndex);
}
}

/**
* Called when a click event is triggered on the table header row.
* We use this to capture all click events on the row, and determine which
Expand Down
16 changes: 14 additions & 2 deletions src/lib/table/table-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,14 @@ export class TableUtils {
addClass([TABLE_CONSTANTS.classes.TABLE_CELL, TABLE_CONSTANTS.classes.TABLE_HEAD_CELL], th);

// We wrap the header text in a div for ease of alignment
const cellContainer = document.createElement('div');
let cellContainer: HTMLElement;
if (columnConfig.sortable) {
cellContainer = document.createElement('button');
(cellContainer as HTMLButtonElement).type = 'button';
cellContainer.addEventListener('keydown', tableConfiguration.sortableHeadCellKeydownListener);
} else {
cellContainer = document.createElement('div');
}
cellContainer.classList.add(TABLE_CONSTANTS.classes.TABLE_HEAD_CELL_CONTAINER);

// Add tooltip for multisort
Expand Down Expand Up @@ -286,6 +293,7 @@ export class TableUtils {
*/
public static setSortDirection(tableElement: HTMLTableElement, columnIndex: number, sortDirection: SortDirection): void {
const cell = TableUtils._getHeaderCellByIndex(tableElement, columnIndex);
tableElement.querySelectorAll('th[aria-sort]').forEach(th => th.removeAttribute('aria-sort'));
TableUtils._setColumnSortDirection(cell, sortDirection);
}

Expand All @@ -294,7 +302,7 @@ export class TableUtils {
* @param thElement
* @param sortDirection
*/
private static _setColumnSortDirection(thElement: HTMLTableHeaderCellElement, sortDirection: SortDirection | undefined): void {
private static _setColumnSortDirection(thElement: HTMLTableCellElement, sortDirection: SortDirection | undefined): void {
if (thElement.classList.contains(TABLE_CONSTANTS.classes.TABLE_HEAD_CELL_SORTED_ASCENDING)) {
thElement.classList.remove(TABLE_CONSTANTS.classes.TABLE_HEAD_CELL_SORTED_ASCENDING);
}
Expand All @@ -304,8 +312,10 @@ export class TableUtils {
}

if (!sortDirection || sortDirection === SortDirection.Descending) {
thElement.setAttribute('aria-sort', 'descending');
thElement.classList.add(TABLE_CONSTANTS.classes.TABLE_HEAD_CELL_SORTED_DESCENDING);
} else {
thElement.setAttribute('aria-sort', 'ascending');
thElement.classList.add(TABLE_CONSTANTS.classes.TABLE_HEAD_CELL_SORTED_ASCENDING);
}
}
Expand Down Expand Up @@ -1052,6 +1062,8 @@ export class TableUtils {
public static removeColumnSort(tableElement: HTMLTableElement, columnIndex: number): void {
const cell = TableUtils._getHeaderCellByIndex(tableElement, columnIndex);

cell.removeAttribute('aria-sort');

// Remove any existing sort direction classes from the existing th element
if (cell.classList.contains(TABLE_CONSTANTS.classes.TABLE_HEAD_CELL_SORTED_ASCENDING)) {
cell.classList.remove(TABLE_CONSTANTS.classes.TABLE_HEAD_CELL_SORTED_ASCENDING);
Expand Down
1 change: 1 addition & 0 deletions src/lib/table/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export interface ITableConfiguration {
doubleClickListener: ((evt: Event) => void) | null;
selectListener: ((evt: Event) => void) | null;
selectAllListener: ((evt: Event) => void) | null;
sortableHeadCellKeydownListener: EventListener;
headRowMouseDownListener: (evt: Event) => void;
headRowContextMenuListener: (evt: Event) => void;
filterListener: ((value: any, columnIndex: number) => void) | null;
Expand Down
61 changes: 59 additions & 2 deletions src/test/spec/table/table.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,9 @@ describe('TableComponent', function(this: ITestContext) {

expect(firstCell.classList.contains(TABLE_CONSTANTS.classes.TABLE_HEAD_CELL_SORTABLE)).toBe(true, 'Expected sortable class on first cell');
expect(secondCell.classList.contains(TABLE_CONSTANTS.classes.TABLE_HEAD_CELL_SORTABLE)).toBe(true, 'Expected sortable class on second cell');
expect(firstCell.classList.contains(TABLE_CONSTANTS.classes.TABLE_HEAD_CELL_SORTED_DESCENDING)).toBe(true, 'Expected sort ascending class on first cell');
expect(firstCell.classList.contains(TABLE_CONSTANTS.classes.TABLE_HEAD_CELL_SORTED_DESCENDING)).toBe(true, 'Expected sort descending class on first cell');
expect(firstCell.getAttribute('aria-sort')).toBe('descending', 'Expected aria-sort to be ascending');
expect(secondCell.hasAttribute('aria-sort')).toBeFalse();
expect(activelySortedCells.length).toBe(1, 'Expected only 1 actively sorted column');
expect(sortIconElement).toBeDefined();
expect((<HTMLElement>sortIconElement).classList.contains(TABLE_CONSTANTS.classes.TABLE_HEAD_CELL_SORT_ICON_ACTIVE)).toBe(true);
Expand Down Expand Up @@ -917,6 +919,40 @@ describe('TableComponent', function(this: ITestContext) {
expect(callback).toHaveBeenCalled();
});

it('should emit sort event when pressing space key on sortable column', async function(this: ITestContext) {
this.context = setupTestContext();
const testColumns = deepCopy(columns);
testColumns[0].sortable = true;

this.context.component.columnConfigurations = testColumns;

const callback = jasmine.createSpy('callback');
this.context.component.addEventListener(TABLE_CONSTANTS.events.SORT, callback);

const headerRow = getTableHeaderRow(this.context.getTableElement());
const firstCell = headerRow.cells.item(0) as HTMLTableCellElement;
firstCell.querySelector('button')?.dispatchEvent(new KeyboardEvent('keydown', { key: ' ' }));

expect(callback).toHaveBeenCalled();
});

it('should emit sort event when pressing enter key on sortable column', async function(this: ITestContext) {
this.context = setupTestContext();
const testColumns = deepCopy(columns);
testColumns[0].sortable = true;

this.context.component.columnConfigurations = testColumns;

const callback = jasmine.createSpy('callback');
this.context.component.addEventListener(TABLE_CONSTANTS.events.SORT, callback);

const headerRow = getTableHeaderRow(this.context.getTableElement());
const firstCell = headerRow.cells.item(0) as HTMLTableCellElement;
firstCell.querySelector('button')?.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter' }));

expect(callback).toHaveBeenCalled();
});

it('should not emit sort event clicking non-sortable column', function(this: ITestContext) {
this.context = setupTestContext();
this.context.component.columnConfigurations = columns;
Expand All @@ -930,6 +966,20 @@ describe('TableComponent', function(this: ITestContext) {
expect(callback).not.toHaveBeenCalled();
});

it('should not emit sort event pressing enter or space key on non-sortable column', function(this: ITestContext) {
this.context = setupTestContext();
this.context.component.columnConfigurations = columns;

const callback = jasmine.createSpy('callback');
this.context.component.addEventListener(TABLE_CONSTANTS.events.SORT, callback);

const headerRow = getTableHeaderRow(this.context.getTableElement());
headerRow.cells.item(0)!.dispatchEvent(new KeyboardEvent('keydown', { key: ' ' }));
headerRow.cells.item(0)!.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter' }));

expect(callback).not.toHaveBeenCalled();
});

it('should toggle sort direction when clicking same column', function(this: ITestContext) {
this.context = setupTestContext();
const testColumns = deepCopy(columns);
Expand All @@ -942,9 +992,11 @@ describe('TableComponent', function(this: ITestContext) {

clickTableCell(firstCell);
expect(firstCell.classList.contains(TABLE_CONSTANTS.classes.TABLE_HEAD_CELL_SORTED_ASCENDING)).toBe(true);
expect(firstCell.getAttribute('aria-sort')).toBe('ascending');

clickTableCell(firstCell);
expect(firstCell.classList.contains(TABLE_CONSTANTS.classes.TABLE_HEAD_CELL_SORTED_DESCENDING)).toBe(true);
expect(firstCell.getAttribute('aria-sort')).toBe('descending');
});

it('should not sort column when clicking non-sortable column', function(this: ITestContext) {
Expand All @@ -968,6 +1020,7 @@ describe('TableComponent', function(this: ITestContext) {

expect(callback).not.toHaveBeenCalled();
expect(hasSortClass).toBe(false);
expect(secondCell.hasAttribute('aria-sort')).toBeFalse();
expect(sortIconElement).toBeNull();
});

Expand All @@ -990,7 +1043,9 @@ describe('TableComponent', function(this: ITestContext) {
const secondCellSortIcon = secondCell.querySelector(`.${TABLE_CONSTANTS.classes.TABLE_HEAD_CELL_SORT_ICON}`) as HTMLElement;

expect(firstCell.classList.contains(TABLE_CONSTANTS.classes.TABLE_HEAD_CELL_SORTED_ASCENDING)).toBe(false);
expect(firstCell.hasAttribute('aria-sort')).toBeFalse();
expect(secondCell.classList.contains(TABLE_CONSTANTS.classes.TABLE_HEAD_CELL_SORTED_ASCENDING)).toBe(true);
expect(secondCell.getAttribute('aria-sort')).toBe('ascending');
expect(firstCellSortIcon.classList.contains(TABLE_CONSTANTS.classes.TABLE_HEAD_CELL_SORT_ICON_ACTIVE)).toBe(false);
expect(secondCellSortIcon.classList.contains(TABLE_CONSTANTS.classes.TABLE_HEAD_CELL_SORT_ICON_ACTIVE)).toBe(true);
});
Expand Down Expand Up @@ -1018,9 +1073,11 @@ describe('TableComponent', function(this: ITestContext) {
const firstCellSortIcon = firstCell.querySelector(`.${TABLE_CONSTANTS.classes.TABLE_HEAD_CELL_SORT_ICON}`) as HTMLElement;
const secondCellSortIcon = secondCell.querySelector(`.${TABLE_CONSTANTS.classes.TABLE_HEAD_CELL_SORT_ICON}`) as HTMLElement;

expect(firstCell.classList.contains(TABLE_CONSTANTS.classes.TABLE_HEAD_CELL_SORTED_ASCENDING)).toBe(true, 'First column should have descending sort class');
expect(firstCell.classList.contains(TABLE_CONSTANTS.classes.TABLE_HEAD_CELL_SORTED_ASCENDING)).toBe(true, 'First column should have ascending sort class');
expect(firstCell.getAttribute('aria-sort')).toBe('ascending');
expect(secondCell.classList.contains(TABLE_CONSTANTS.classes.TABLE_HEAD_CELL_SORTED_ASCENDING)).toBe(false, 'Second column should not have descending sort class');
expect(secondCell.classList.contains(TABLE_CONSTANTS.classes.TABLE_HEAD_CELL_SORTED_DESCENDING)).toBe(false, 'Second column should not have ascending sort class');
expect(secondCell.hasAttribute('aria-sort')).toBeFalse();
expect(firstCellSortIcon.classList.contains(TABLE_CONSTANTS.classes.TABLE_HEAD_CELL_SORT_ICON_ACTIVE)).toBe(true, 'First column should have active sort icon');
expect(secondCellSortIcon.classList.contains(TABLE_CONSTANTS.classes.TABLE_HEAD_CELL_SORT_ICON_ACTIVE)).toBe(false, 'Second column should not have active sort icon');
});
Expand Down

0 comments on commit 43f775e

Please sign in to comment.