From d4599d8245d55ae0ca5d7760200cc59ebb5c279d Mon Sep 17 00:00:00 2001 From: Vlad Jimenez Date: Wed, 12 Oct 2022 18:29:25 -0700 Subject: [PATCH 1/6] Improve a11y support for table sorting --- lib/css/components/tables.less | 15 ++++++ lib/ts/controllers/s-table.ts | 83 ++++++++++++++++++++++++---------- 2 files changed, 75 insertions(+), 23 deletions(-) diff --git a/lib/css/components/tables.less b/lib/css/components/tables.less index 5500af65de..7be6f7429f 100644 --- a/lib/css/components/tables.less +++ b/lib/css/components/tables.less @@ -66,6 +66,21 @@ color: var(--fc-dark); } + th.s-table--sortable-column { + padding: 0; + + button.s-table--clickable-header { + appearance: none; + background-color: transparent; + border: 0; + color: currentColor; + cursor: pointer; + padding: var(--su8); + text-align: left; + width: 100%; + } + } + // Custom styles when in a table head thead th { vertical-align: bottom; diff --git a/lib/ts/controllers/s-table.ts b/lib/ts/controllers/s-table.ts index ff7aa975cb..3bff877228 100644 --- a/lib/ts/controllers/s-table.ts +++ b/lib/ts/controllers/s-table.ts @@ -1,41 +1,50 @@ import * as Stacks from "../stacks"; +/** + * The string values of these enumerations should correspond with `aria-sort` valid values. + * + * @see https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-sort#values + */ +enum SortOrder { + Ascending = 'ascending', + Descending = 'descending', + None = 'none', +} + export class TableController extends Stacks.StacksController { static targets = ["column"]; - readonly columnTarget!: Element; - readonly columnTargets!: Element[]; + readonly columnTarget!: HTMLTableCellElement; + readonly columnTargets!: HTMLTableCellElement[]; - setCurrentSort(headElem: Element, direction: "asc" | "desc" | "none") { - if (["asc", "desc", "none"].indexOf(direction) < 0) { - throw "direction must be one of asc, desc, or none" - } - // eslint-disable-next-line @typescript-eslint/no-this-alias - const controller = this; - this.columnTargets.forEach(function (target) { - const isCurrrent = target === headElem; + connect() { + this.columnTargets.forEach(this.ensureHeadersAreClickable); + } - target.classList.toggle("is-sorted", isCurrrent && direction !== "none"); + private setCurrentSort(headElem: Element, direction: SortOrder) { + this.columnTargets.forEach((target: HTMLTableCellElement) => { + const isCurrent = target === headElem; + const classSuffix = isCurrent + ? (direction === SortOrder.Ascending ? 'asc' : 'desc') + : SortOrder.None; - target.querySelectorAll(".js-sorting-indicator").forEach(function (icon) { - const visible = isCurrrent ? direction : "none"; - icon.classList.toggle("d-none", !icon.classList.contains("js-sorting-indicator-" + visible)); + target.classList.toggle("is-sorted", isCurrent && direction !== SortOrder.None); + + target.querySelectorAll(".js-sorting-indicator").forEach((icon) => { + icon.classList.toggle("d-none", !icon.classList.contains("js-sorting-indicator-" + classSuffix)); }); - if (!isCurrrent || direction === "none") { - controller.removeElementData(target, "sort-direction"); - } else { - controller.setElementData(target, "sort-direction", direction); - } + target.setAttribute('aria-sort', direction); }); }; sort(evt: Event) { // eslint-disable-next-line @typescript-eslint/no-this-alias const controller = this; - const colHead = evt.currentTarget; - if (!(colHead instanceof HTMLTableCellElement)) { + const sortTriggerEl = evt.currentTarget; + if (!(sortTriggerEl instanceof HTMLButtonElement)) { throw "invalid event target"; } + const colHead = sortTriggerEl.parentElement as HTMLTableCellElement; const table = this.element as HTMLTableElement; const tbody = table.tBodies[0]; @@ -52,7 +61,7 @@ export class TableController extends Stacks.StacksController { // the default behavior when clicking a header is to sort by this column in ascending // direction, *unless* it is already sorted that way - const direction = this.getElementData(colHead, "sort-direction") === "asc" ? -1 : 1; + const direction = colHead.getAttribute('aria-sort') === SortOrder.Ascending ? -1 : 1; const rows = Array.from(table.tBodies[0].rows); @@ -127,9 +136,37 @@ export class TableController extends Stacks.StacksController { // update the UI and set the `data-sort-direction` attribute if appropriate, so that the next click // will cause sorting in descending direction - this.setCurrentSort(colHead, direction === 1 ? "asc" : "desc"); + this.setCurrentSort(colHead, direction === 1 ? SortOrder.Ascending : SortOrder.Descending); } + /** + * Transform legacy header markup into the new markup. + * + * @param headerEl + * @private + */ + private ensureHeadersAreClickable(headerEl: HTMLTableCellElement) { + const headerAction = headerEl.getAttribute('data-action'); + const headerViolatesA11y = headerAction !== null && headerAction.substring(0, 5) === 'click'; + const lacksSortableClass = !headerEl.classList.contains('s-table--sortable-column'); + + if (lacksSortableClass) { + headerEl.classList.add('s-table--sortable-column'); + } + + if (headerViolatesA11y) { + if (process.env.NODE_ENV !== 'production') { + console.warn('s-table :: a `` should not have a data-action="click->..." attribute.'); + } + + headerEl.removeAttribute('data-action'); + headerEl.innerHTML = ` + + `; + } + } } function buildIndex(section: HTMLTableSectionElement): HTMLTableCellElement[][] { From c6830af443c7b877e59c95b02b51322e48ce5532 Mon Sep 17 00:00:00 2001 From: Vlad Jimenez Date: Tue, 25 Oct 2022 16:39:24 -0700 Subject: [PATCH 2/6] Remove extra classes; add column highlighting --- docs/_data/tables.json | 2 +- docs/product/components/tables.html | 64 +++++++++++++++----------- lib/css/components/tables.less | 31 ++++++------- lib/ts/controllers/s-table.ts | 71 ++++++++++++++++------------- 4 files changed, 93 insertions(+), 75 deletions(-) diff --git a/docs/_data/tables.json b/docs/_data/tables.json index 2aafbc9066..3b68ebb323 100755 --- a/docs/_data/tables.json +++ b/docs/_data/tables.json @@ -62,7 +62,7 @@ }, { "attribute": "data-action=\"click->s-table#sort\"", - "applies": "th", + "applies": "button", "description": "Causes a click on the header cell to sort by this column" }, { diff --git a/docs/product/components/tables.html b/docs/product/components/tables.html index db9bf03df1..55403aa5e5 100644 --- a/docs/product/components/tables.html +++ b/docs/product/components/tables.html @@ -771,19 +771,25 @@ - - - @@ -805,23 +811,29 @@
- Season - @Svg.ArrowUpSm.With("js-sorting-indicator js-sorting-indicator-asc d-none") - @Svg.ArrowDownSm.With("js-sorting-indicator js-sorting-indicator-desc d-none") - @Svg.ArrowUpDownSm.With("js-sorting-indicator js-sorting-indicator-none") + + - Starts in month - … + + - Typical temperature in °C - … + +
- - - diff --git a/lib/css/components/tables.less b/lib/css/components/tables.less index 7be6f7429f..d800baa9ed 100644 --- a/lib/css/components/tables.less +++ b/lib/css/components/tables.less @@ -66,21 +66,6 @@ color: var(--fc-dark); } - th.s-table--sortable-column { - padding: 0; - - button.s-table--clickable-header { - appearance: none; - background-color: transparent; - border: 0; - color: currentColor; - cursor: pointer; - padding: var(--su8); - text-align: left; - width: 100%; - } - } - // Custom styles when in a table head thead th { vertical-align: bottom; @@ -295,11 +280,25 @@ color: var(--fc-light); cursor: pointer; + &[data-s-table-target="column"] { + padding: 0; + } + // If an anchor is used, it should appear like a normal header - a { + a, + button { color: inherit; } + button { + appearance: none; + background-color: transparent; + border: 0; + padding: var(--su8); + text-align: left; + width: 100%; + } + // Selected state &.is-sorted { color: var(--black-900); diff --git a/lib/ts/controllers/s-table.ts b/lib/ts/controllers/s-table.ts index 3bff877228..123be543de 100644 --- a/lib/ts/controllers/s-table.ts +++ b/lib/ts/controllers/s-table.ts @@ -11,32 +11,19 @@ enum SortOrder { None = 'none', } +const sortedHeaderBackground = 'bg-powder-200'; +const sortedColumnBackground = 'bg-powder-100'; + export class TableController extends Stacks.StacksController { - static targets = ["column"]; readonly columnTarget!: HTMLTableCellElement; readonly columnTargets!: HTMLTableCellElement[]; + static targets = ["column"]; + connect() { this.columnTargets.forEach(this.ensureHeadersAreClickable); } - private setCurrentSort(headElem: Element, direction: SortOrder) { - this.columnTargets.forEach((target: HTMLTableCellElement) => { - const isCurrent = target === headElem; - const classSuffix = isCurrent - ? (direction === SortOrder.Ascending ? 'asc' : 'desc') - : SortOrder.None; - - target.classList.toggle("is-sorted", isCurrent && direction !== SortOrder.None); - - target.querySelectorAll(".js-sorting-indicator").forEach((icon) => { - icon.classList.toggle("d-none", !icon.classList.contains("js-sorting-indicator-" + classSuffix)); - }); - - target.setAttribute('aria-sort', direction); - }); - }; - sort(evt: Event) { // eslint-disable-next-line @typescript-eslint/no-this-alias const controller = this; @@ -91,7 +78,7 @@ export class TableController extends Stacks.StacksController { // unless the to-be-sorted-by value is explicitly provided on the element via this attribute, // the value we're using is the cell's text, trimmed of any whitespace const explicit = controller.getElementData(cell, "sort-val"); - const d = typeof explicit === "string" ? explicit : cell.textContent!.trim(); + const d = explicit != null ? explicit : cell.textContent!.trim(); if ((d !== "") && (`${parseInt(d, 10)}` !== d)) { anyNonInt = true; @@ -124,9 +111,16 @@ export class TableController extends Stacks.StacksController { }); // this is the actual reordering of the table rows - data.forEach(function (tup) { - const row = rows[tup[1]]; + data.forEach(([_, rowIndex]) => { + const row = rows[rowIndex]; row.parentElement!.removeChild(row); + + for (let i = 0; i < row.cells.length; i++) { + const cell = row.cells.item(i); + + cell?.classList.toggle(sortedColumnBackground, i === colno); + } + if (firstBottomRow) { tbody.insertBefore(row, firstBottomRow); } else { @@ -136,32 +130,45 @@ export class TableController extends Stacks.StacksController { // update the UI and set the `data-sort-direction` attribute if appropriate, so that the next click // will cause sorting in descending direction - this.setCurrentSort(colHead, direction === 1 ? SortOrder.Ascending : SortOrder.Descending); + this.updateSortedColumnStyles(colHead, direction === 1 ? SortOrder.Ascending : SortOrder.Descending); + } + + private updateSortedColumnStyles(targetColumnHeader: Element, direction: SortOrder): void { + // Loop through all sortable columns and remove their sorting direction + // (if any), and only leave/set a sorting on `targetColumnHeader`. + this.columnTargets.forEach((header: HTMLTableCellElement) => { + const isCurrent = header === targetColumnHeader; + const classSuffix = isCurrent + ? (direction === SortOrder.Ascending ? 'asc' : 'desc') + : SortOrder.None; + + header.classList.toggle('is-sorted', isCurrent && direction !== SortOrder.None); + header.classList.toggle(sortedHeaderBackground, isCurrent); + header.setAttribute('aria-sort', direction); + header.querySelectorAll('.js-sorting-indicator').forEach((icon) => { + icon.classList.toggle('d-none', !icon.classList.contains('js-sorting-indicator-' + classSuffix)); + }); + }); } /** * Transform legacy header markup into the new markup. * * @param headerEl - * @private */ private ensureHeadersAreClickable(headerEl: HTMLTableCellElement) { const headerAction = headerEl.getAttribute('data-action'); - const headerViolatesA11y = headerAction !== null && headerAction.substring(0, 5) === 'click'; - const lacksSortableClass = !headerEl.classList.contains('s-table--sortable-column'); - - if (lacksSortableClass) { - headerEl.classList.add('s-table--sortable-column'); - } - if (headerViolatesA11y) { + // Legacy markup that violates accessibility practices; change the DOM + if (headerAction !== null && headerAction.substring(0, 5) === 'click') { if (process.env.NODE_ENV !== 'production') { - console.warn('s-table :: a ` - + {% endfor %} From f99bc57ab714f0c9df826cc11ceb2e1979f6792e Mon Sep 17 00:00:00 2001 From: Vlad Jimenez Date: Wed, 26 Oct 2022 09:37:46 -0700 Subject: [PATCH 5/6] aria-sort shouldn't always be set on every header --- lib/ts/controllers/s-table.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/ts/controllers/s-table.ts b/lib/ts/controllers/s-table.ts index 123be543de..4859d75d69 100644 --- a/lib/ts/controllers/s-table.ts +++ b/lib/ts/controllers/s-table.ts @@ -24,7 +24,7 @@ export class TableController extends Stacks.StacksController { this.columnTargets.forEach(this.ensureHeadersAreClickable); } - sort(evt: Event) { + sort(evt: PointerEvent) { // eslint-disable-next-line @typescript-eslint/no-this-alias const controller = this; const sortTriggerEl = evt.currentTarget; @@ -144,10 +144,15 @@ export class TableController extends Stacks.StacksController { header.classList.toggle('is-sorted', isCurrent && direction !== SortOrder.None); header.classList.toggle(sortedHeaderBackground, isCurrent); - header.setAttribute('aria-sort', direction); header.querySelectorAll('.js-sorting-indicator').forEach((icon) => { icon.classList.toggle('d-none', !icon.classList.contains('js-sorting-indicator-' + classSuffix)); }); + + if (isCurrent) { + header.setAttribute('aria-sort', direction); + } else { + header.removeAttribute('aria-sort'); + } }); } From 946a9687a9e02fa4662242be200c0ab6b3db378b Mon Sep 17 00:00:00 2001 From: Vlad Jimenez Date: Fri, 28 Oct 2022 17:38:03 -0700 Subject: [PATCH 6/6] Remove column highlighting on sort --- lib/ts/controllers/s-table.ts | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/lib/ts/controllers/s-table.ts b/lib/ts/controllers/s-table.ts index 4859d75d69..175a176d5f 100644 --- a/lib/ts/controllers/s-table.ts +++ b/lib/ts/controllers/s-table.ts @@ -11,9 +11,6 @@ enum SortOrder { None = 'none', } -const sortedHeaderBackground = 'bg-powder-200'; -const sortedColumnBackground = 'bg-powder-100'; - export class TableController extends Stacks.StacksController { readonly columnTarget!: HTMLTableCellElement; readonly columnTargets!: HTMLTableCellElement[]; @@ -115,12 +112,6 @@ export class TableController extends Stacks.StacksController { const row = rows[rowIndex]; row.parentElement!.removeChild(row); - for (let i = 0; i < row.cells.length; i++) { - const cell = row.cells.item(i); - - cell?.classList.toggle(sortedColumnBackground, i === colno); - } - if (firstBottomRow) { tbody.insertBefore(row, firstBottomRow); } else { @@ -143,7 +134,6 @@ export class TableController extends Stacks.StacksController { : SortOrder.None; header.classList.toggle('is-sorted', isCurrent && direction !== SortOrder.None); - header.classList.toggle(sortedHeaderBackground, isCurrent); header.querySelectorAll('.js-sorting-indicator').forEach((icon) => { icon.classList.toggle('d-none', !icon.classList.contains('js-sorting-indicator-' + classSuffix)); });
- Season - {% icon "ArrowUpSm", "js-sorting-indicator js-sorting-indicator-asc d-none" %} - {% icon "ArrowDownSm", "js-sorting-indicator js-sorting-indicator-desc d-none" %} - {% icon "ArrowUpDownSm", "js-sorting-indicator js-sorting-indicator-none" %} + + - Starts in month - {% icon "ArrowUpSm", "js-sorting-indicator js-sorting-indicator-asc d-none" %} - {% icon "ArrowDownSm", "js-sorting-indicator js-sorting-indicator-desc d-none" %} - {% icon "ArrowUpDownSm", "js-sorting-indicator js-sorting-indicator-none" %} + + - Typical temperature in °C - {% icon "ArrowUpSm", "js-sorting-indicator js-sorting-indicator-asc d-none" %} - {% icon "ArrowDownSm", "js-sorting-indicator js-sorting-indicator-desc d-none" %} - {% icon "ArrowUpDownSm", "js-sorting-indicator js-sorting-indicator-none" %} + +
` should not have a data-action="click->..." attribute.'); + console.warn('s-table :: a `` should not have a `data-action="click->..."` attribute. https://stackoverflow.design/product/components/tables/#javascript-example'); + console.warn('target: ', headerEl); } headerEl.removeAttribute('data-action'); headerEl.innerHTML = ` - `; From 57a45157cbb8714eafb1dadee2a8906bb3e2c0a8 Mon Sep 17 00:00:00 2001 From: Vlad Jimenez Date: Tue, 25 Oct 2022 16:46:03 -0700 Subject: [PATCH 3/6] Add quick comment --- lib/css/components/tables.less | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/css/components/tables.less b/lib/css/components/tables.less index d800baa9ed..300941fbdf 100644 --- a/lib/css/components/tables.less +++ b/lib/css/components/tables.less @@ -280,11 +280,12 @@ color: var(--fc-light); cursor: pointer; + // If this column is sortable, then the padding will come from the button &[data-s-table-target="column"] { padding: 0; } - // If an anchor is used, it should appear like a normal header + // If an anchor is used, it should appear like a normal header a, button { color: inherit; From 5d3adefa1da34fc679aae22cb6cde96b4c38cbfa Mon Sep 17 00:00:00 2001 From: Vlad Jimenez Date: Tue, 25 Oct 2022 17:36:34 -0700 Subject: [PATCH 4/6] Fix missing ending tag --- docs/product/components/tables.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/product/components/tables.html b/docs/product/components/tables.html index 55403aa5e5..9c27001db5 100644 --- a/docs/product/components/tables.html +++ b/docs/product/components/tables.html @@ -756,7 +756,7 @@ {% for item in tables.data-attributes %}
{{ item.attribute }}{{ item.applies }}{{ item.applies }} {{ item.description }}