From d4599d8245d55ae0ca5d7760200cc59ebb5c279d Mon Sep 17 00:00:00 2001 From: Vlad Jimenez Date: Wed, 12 Oct 2022 18:29:25 -0700 Subject: [PATCH 01/23] 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 02/23] 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 05/23] 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 06/23] 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)); }); From 4e37924c38a5845fa4823ccbc6a8429a070629a7 Mon Sep 17 00:00:00 2001 From: Dan Cormier Date: Fri, 24 Feb 2023 18:04:14 -0500 Subject: [PATCH 07/23] Port table.less changes to refactored styles --- lib/css/components/table.less | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/lib/css/components/table.less b/lib/css/components/table.less index fc11a794a9..3fb90e913d 100644 --- a/lib/css/components/table.less +++ b/lib/css/components/table.less @@ -127,10 +127,24 @@ &&__sortable { thead th { - a { // If an anchor is used, it should appear like a normal header + a, + button { // If an anchor is used, it should appear like a normal header color: inherit; } + button { + appearance: none; + background-color: transparent; + border: 0; + padding: var(--su8); + text-align: left; + width: 100%; + } + + &[data-s-table-target="column"] { // If this column is sortable, then the padding will come from the button + padding: 0; + } + &.is-sorted { // Selected state color: var(--black-900); } From 87271fc55fcb641e6bdacb41c00200f5e8c42f2f Mon Sep 17 00:00:00 2001 From: Dan Cormier Date: Fri, 24 Feb 2023 18:08:10 -0500 Subject: [PATCH 08/23] test(table): add test for sortable table --- lib/test/s-table.test.ts | 129 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 129 insertions(+) create mode 100644 lib/test/s-table.test.ts diff --git a/lib/test/s-table.test.ts b/lib/test/s-table.test.ts new file mode 100644 index 0000000000..830771d27a --- /dev/null +++ b/lib/test/s-table.test.ts @@ -0,0 +1,129 @@ +import { html, fixture, expect } from "@open-wc/testing"; +import { screen } from "@testing-library/dom"; +import userEvent from "@testing-library/user-event"; +import "../ts/index"; + +const user = userEvent.setup(); + +const testSortableTable = () => { + const testSortabledIcons = (modifier: string) => html` + + + + `; + + return html` +
+
- 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 03/23] 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 04/23] 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 }}
+ + + + + + + + + + + + + + + + + +
+ + + + + +
WinterDecember2
SpringMarch13
SummerJune25
FallSeptember13
Average temperature13
+ + `; +}; + +const testAriaHiddenSvgs = (allSvgs: string[], visible?: string) => { + return allSvgs.forEach((svg) => { + const svgEl = screen.getByTestId(svg); + + if (svg === visible) { + expect(svgEl).to.have.attribute("aria-hidden", "false"); + } else { + expect(svgEl).to.have.attribute("aria-hidden", "true"); + } + }); +} + +describe("s-table", () => { + it("table sort updates on click", async () => { + await fixture(testSortableTable()); + + const sortButtonSeason = screen.getByTestId("test-sortable-season-button"); + const sortButtonMonth = screen.getByTestId("test-sortable-month-button"); + + // const table = screen.getByTestId("test-sortable-table"); + const allSvgs = [ + "test-sortable-season-svg-asc", + "test-sortable-season-svg-desc", + "test-sortable-season-svg-none", + "test-sortable-month-svg-asc", + "test-sortable-month-svg-desc", + "test-sortable-month-svg-none", + ]; + + testAriaHiddenSvgs(allSvgs); + + // Cycle through all season sort options + await user.click(sortButtonSeason); + testAriaHiddenSvgs(allSvgs, "test-sortable-season-svg-asc"); + await user.click(sortButtonSeason); + testAriaHiddenSvgs(allSvgs, "test-sortable-season-svg-desc"); + await user.click(sortButtonSeason); + testAriaHiddenSvgs(allSvgs, "test-sortable-season-svg-none"); + await user.click(sortButtonSeason); + testAriaHiddenSvgs(allSvgs); + + // Set month sort then switch to season sort + await user.click(sortButtonMonth); + testAriaHiddenSvgs(allSvgs, "test-sortable-month-svg-asc"); + await user.click(sortButtonSeason); + testAriaHiddenSvgs(allSvgs, "test-sortable-season-svg-asc"); + }); +}); From f984cc08655d49722abf2e98b5b8bd6f56f97406 Mon Sep 17 00:00:00 2001 From: Vlad Jimenez Date: Mon, 27 Mar 2023 15:34:02 -0700 Subject: [PATCH 09/23] Have eslint apply to tests as well --- .eslintrc.js | 5 ++++- lib/tsconfig.test.json | 4 ++++ 2 files changed, 8 insertions(+), 1 deletion(-) create mode 100644 lib/tsconfig.test.json diff --git a/.eslintrc.js b/.eslintrc.js index db2aec6cdf..eee005df2e 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -3,7 +3,10 @@ module.exports = { parser: "@typescript-eslint/parser", parserOptions: { tsconfigRootDir: __dirname, - project: ["./lib/tsconfig.json"], + project: [ + "./lib/tsconfig.json", + "./lib/tsconfig.test.json", + ], }, plugins: ["@typescript-eslint"], extends: [ diff --git a/lib/tsconfig.test.json b/lib/tsconfig.test.json new file mode 100644 index 0000000000..9547d72f23 --- /dev/null +++ b/lib/tsconfig.test.json @@ -0,0 +1,4 @@ +{ + "extends": "./tsconfig.json", + "include": ["test/"] +} From 5cc54b339481a0b92eca0eba584a05dae39bb63c Mon Sep 17 00:00:00 2001 From: Vlad Jimenez Date: Mon, 27 Mar 2023 15:43:31 -0700 Subject: [PATCH 10/23] It's okay to have unused vars for array destructures --- .eslintrc.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.eslintrc.js b/.eslintrc.js index eee005df2e..b75e998d9f 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -20,5 +20,11 @@ module.exports = { "no-console": "error", "no-alert": "error", "no-process-env": "error", + "@typescript-eslint/no-unused-vars": [ + "error", + { + "destructuredArrayIgnorePattern": "^_" + }, + ] }, }; From 5a24eede0783b79f4e7bfc363e185edeeb3f0036 Mon Sep 17 00:00:00 2001 From: Vlad Jimenez Date: Mon, 27 Mar 2023 15:45:08 -0700 Subject: [PATCH 11/23] Prettify table code + start more unit tests --- lib/test/s-table.test.ts | 227 +++++++++++++++++++++++++++------- lib/ts/controllers/s-table.ts | 150 ++++++++++++---------- 2 files changed, 265 insertions(+), 112 deletions(-) diff --git a/lib/test/s-table.test.ts b/lib/test/s-table.test.ts index 830771d27a..c6d7298bc7 100644 --- a/lib/test/s-table.test.ts +++ b/lib/test/s-table.test.ts @@ -1,11 +1,12 @@ import { html, fixture, expect } from "@open-wc/testing"; import { screen } from "@testing-library/dom"; import userEvent from "@testing-library/user-event"; +import { buildIndex, getCellSlot } from "../ts/controllers/s-table"; import "../ts/index"; const user = userEvent.setup(); -const testSortableTable = () => { +const mockSortableTable = () => { const testSortabledIcons = (modifier: string) => html` { class="svg-icon iconArrowUpSm js-sorting-indicator js-sorting-indicator-asc d-none" width="14" height="14" - viewBox="0 0 14 14"> + viewBox="0 0 14 14" + > { class="svg-icon iconArrowDownSm js-sorting-indicator js-sorting-indicator-desc d-none" width="14" height="14" - viewBox="0 0 14 14"> + viewBox="0 0 14 14" + > { class="svg-icon iconArrowUpDownSm js-sorting-indicator js-sorting-indicator-none" width="14" height="14" - viewBox="0 0 14 14"> + viewBox="0 0 14 14" + > `; @@ -44,31 +48,51 @@ const testSortableTable = () => { - WinterDecember2 - SpringMarch13 - SummerJune25 - FallSeptember13 + + Winter + December + 2 + + + Spring + March + 13 + + + Summer + June + 25 + + + Fall + September + 13 + Average temperature 13 @@ -89,41 +113,152 @@ const testAriaHiddenSvgs = (allSvgs: string[], visible?: string) => { expect(svgEl).to.have.attribute("aria-hidden", "true"); } }); -} +}; describe("s-table", () => { - it("table sort updates on click", async () => { - await fixture(testSortableTable()); - - const sortButtonSeason = screen.getByTestId("test-sortable-season-button"); - const sortButtonMonth = screen.getByTestId("test-sortable-month-button"); - - // const table = screen.getByTestId("test-sortable-table"); - const allSvgs = [ - "test-sortable-season-svg-asc", - "test-sortable-season-svg-desc", - "test-sortable-season-svg-none", - "test-sortable-month-svg-asc", - "test-sortable-month-svg-desc", - "test-sortable-month-svg-none", - ]; - - testAriaHiddenSvgs(allSvgs); - - // Cycle through all season sort options - await user.click(sortButtonSeason); - testAriaHiddenSvgs(allSvgs, "test-sortable-season-svg-asc"); - await user.click(sortButtonSeason); - testAriaHiddenSvgs(allSvgs, "test-sortable-season-svg-desc"); - await user.click(sortButtonSeason); - testAriaHiddenSvgs(allSvgs, "test-sortable-season-svg-none"); - await user.click(sortButtonSeason); - testAriaHiddenSvgs(allSvgs); - - // Set month sort then switch to season sort - await user.click(sortButtonMonth); - testAriaHiddenSvgs(allSvgs, "test-sortable-month-svg-asc"); - await user.click(sortButtonSeason); - testAriaHiddenSvgs(allSvgs, "test-sortable-season-svg-asc"); + describe("Stimulus.js controller", () => { + it("table sort updates on click", async () => { + await fixture(mockSortableTable()); + + const sortButtonSeason = screen.getByTestId( + "test-sortable-season-button" + ); + const sortButtonMonth = screen.getByTestId( + "test-sortable-month-button" + ); + + // const table = screen.getByTestId("test-sortable-table"); + const allSvgs = [ + "test-sortable-season-svg-asc", + "test-sortable-season-svg-desc", + "test-sortable-season-svg-none", + "test-sortable-month-svg-asc", + "test-sortable-month-svg-desc", + "test-sortable-month-svg-none", + ]; + + testAriaHiddenSvgs(allSvgs); + + // Cycle through all season sort options + await user.click(sortButtonSeason); + testAriaHiddenSvgs(allSvgs, "test-sortable-season-svg-asc"); + await user.click(sortButtonSeason); + testAriaHiddenSvgs(allSvgs, "test-sortable-season-svg-desc"); + await user.click(sortButtonSeason); + testAriaHiddenSvgs(allSvgs, "test-sortable-season-svg-none"); + await user.click(sortButtonSeason); + testAriaHiddenSvgs(allSvgs); + + // Set month sort then switch to season sort + await user.click(sortButtonMonth); + testAriaHiddenSvgs(allSvgs, "test-sortable-month-svg-asc"); + await user.click(sortButtonSeason); + testAriaHiddenSvgs(allSvgs, "test-sortable-season-svg-asc"); + }); + }); + + describe("Helper functions", () => { + describe("buildIndex()", () => { + it("should make references to individual cells that use `colspan`", async () => { + const table = await fixture(html` + + + + + + + + + + + + + + + + +
FallSeptember13
Winter10
SpringApril
+ `); + + expect(table.tBodies).to.have.length(1); + + const tableBody = table.tBodies[0]; + const tableIndex = buildIndex(tableBody); + + expect(tableIndex).to.satisfy((rows: typeof tableIndex) => + rows.every((row) => row.length === 3) + ); + expect(tableIndex[1][0]).to.equal( + tableIndex[1][1], + "the Winter cell is expected to be referenced twice in this index" + ); + expect(tableIndex[2][1]).to.equal( + tableIndex[2][2], + "the April cell is expected to be referenced twice in this index" + ); + }); + + it("should make references to individual cells that use `rowspan`", async () => { + const table = await fixture(html` + + + + + + + + + + + + + + + + + + + + + +
FallSeptember13
October20
SpringApril46
October20
+ `); + + expect(table.tBodies).to.have.length(1); + + const tableBody = table.tBodies[0]; + const tableIndex = buildIndex(tableBody); + + expect(tableIndex).to.satisfy((rows: typeof tableIndex) => + rows.every((row) => row.length === 3) + ); + expect(tableIndex[1][0]).to.equal( + tableIndex[0][0], + "the Fall cell is expected to be referenced twice across multiple rows" + ); + expect(tableIndex[3][0]).to.equal( + tableIndex[2][0], + "the Spring cell is expected to be referenced twice in this index" + ); + }); + }); + + describe("getCellSlot()", () => { + it("should correctly calculate the column number for each button", async () => { + await fixture(mockSortableTable()); + + [ + "test-sortable-season-button", + "test-sortable-month-button", + "test-sortable-temp-button", + ].forEach((testID, index) => { + const buttonElement = screen.getByTestId(testID); + const columnCell = buttonElement.parentElement; + + expect(columnCell).to.not.be.null; + expect(getCellSlot(columnCell)).to.equal(index); + }); + }); + }); }); }); diff --git a/lib/ts/controllers/s-table.ts b/lib/ts/controllers/s-table.ts index 9cca5fcd5b..7b642732a3 100644 --- a/lib/ts/controllers/s-table.ts +++ b/lib/ts/controllers/s-table.ts @@ -5,10 +5,10 @@ import * as Stacks from "../stacks"; * * @see https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-sort#values */ -enum SortOrder { - Ascending = 'ascending', - Descending = 'descending', - None = 'none', +export enum SortOrder { + Ascending = "ascending", + Descending = "descending", + None = "none", } export class TableController extends Stacks.StacksController { @@ -47,7 +47,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 = - colHead.getAttribute('aria-sort') === SortOrder.Ascending ? -1 : 1; + colHead.getAttribute("aria-sort") === SortOrder.Ascending ? -1 : 1; const rows = Array.from(table.tBodies[0].rows); @@ -77,7 +77,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 = explicit != null ? explicit : cell.textContent!.trim(); + const d = explicit ?? cell.textContent?.trim() ?? ""; if (d !== "" && `${parseInt(d, 10)}` !== d) { anyNonInt = true; @@ -115,7 +115,7 @@ export class TableController extends Stacks.StacksController { // this is the actual reordering of the table rows data.forEach(([_, rowIndex]) => { const row = rows[rowIndex]; - row.parentElement!.removeChild(row); + row.parentElement?.removeChild(row); if (firstBottomRow) { tbody.insertBefore(row, firstBottomRow); @@ -126,100 +126,114 @@ 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.updateSortedColumnStyles(colHead, direction === 1 ? SortOrder.Ascending : SortOrder.Descending); + this.updateSortedColumnStyles( + colHead, + direction === 1 ? SortOrder.Ascending : SortOrder.Descending + ); } - private updateSortedColumnStyles(targetColumnHeader: Element, direction: SortOrder): void { + 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') + ? direction === SortOrder.Ascending + ? "asc" + : "desc" : SortOrder.None; - header.classList.toggle('is-sorted', isCurrent && direction !== SortOrder.None); - header.querySelectorAll('.js-sorting-indicator').forEach((icon) => { - icon.classList.toggle('d-none', !icon.classList.contains('js-sorting-indicator-' + classSuffix)); + header.classList.toggle( + "is-sorted", + isCurrent && direction !== SortOrder.None + ); + 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); + header.setAttribute("aria-sort", direction); } else { - header.removeAttribute('aria-sort'); + header.removeAttribute("aria-sort"); } }); - } + }; /** * Transform legacy header markup into the new markup. * * @param headerEl */ - private ensureHeadersAreClickable(headerEl: HTMLTableCellElement) { - const headerAction = headerEl.getAttribute('data-action'); + private ensureHeadersAreClickable = (headerEl: HTMLTableCellElement) => { + const headerAction = headerEl.getAttribute("data-action"); // 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 `` should not have a `data-action="click->..."` attribute. https://stackoverflow.design/product/components/tables/#javascript-example'); - console.warn('target: ', headerEl); - } + if (headerAction !== null && headerAction.substring(0, 5) === "click") { + headerEl.removeAttribute("data-action"); - headerEl.removeAttribute('data-action'); + // eslint-disable-next-line no-unsanitized/property headerEl.innerHTML = ` `; } - } + }; } -function buildIndex( +export function buildIndex( section: HTMLTableSectionElement ): HTMLTableCellElement[][] { const result = buildIndexOrGetCellSlot(section); - if (!(result instanceof Array)) { + + if (!Array.isArray(result)) { throw "shouldn't happen"; } + return result; } -function getCellSlot(cell: HTMLTableCellElement): number { - if ( - !( - cell.parentElement && - cell.parentElement.parentElement instanceof HTMLTableSectionElement - ) - ) { +export function getCellSlot(cell: HTMLTableCellElement): number { + const tableElement = cell.parentElement?.parentElement; + + if (!(tableElement instanceof HTMLTableSectionElement)) { throw "invalid table"; } - const result = buildIndexOrGetCellSlot( - cell.parentElement.parentElement, - cell - ); + + const result = buildIndexOrGetCellSlot(tableElement, cell); + if (typeof result !== "number") { throw "shouldn't happen"; } + return result; } -// Just because a is the 4th *child* of its doesn't mean it belongs to the 4th *column* -// of the table. Previous cells may have a colspan; cells in previous rows may have a rowspan. -// Because we need to know which header cells and data cells belong together, we have to 1) find out -// which column number (or "slot" as we call it here) the header cell has, and 2) for each row find -// out which cell corresponds to this slot (because those are the rows we're sorting by). -// -// That's what the following function does. If the second argument is not given, it returns an index -// of the table, which is an array of arrays. Each of the sub-arrays corresponds to a table row. The -// indices of the sub-array correspond to column slots; the values are the actual table cell elements. -// For example index[4][3] is the or in row 4, column 3 of the table section ( or ). -// Note that this element is not necessarily even in the 4th (zero-based) -- if it has a rowSpan > 1, -// it may also be in a previous . -// -// If the second argument is given, it's a or that we're trying to find, and the algorithm -// stops as soon as it has found it and the function returns its slot number. +/** + * Just because a is the 4th *child* of its doesn't mean it belongs to the 4th *column* + * of the table. Previous cells may have a colspan; cells in previous rows may have a rowspan. + * Because we need to know which header cells and data cells belong together, we have to 1) find out + * which column number (or "slot" as we call it here) the header cell has, and 2) for each row find + * out which cell corresponds to this slot (because those are the rows we're sorting by). + * + * That's what the following function does. If the second argument is not given, it returns an index + * of the table, which is an array of arrays. Each of the sub-arrays corresponds to a table row. The + * indices of the sub-array correspond to column slots; the values are the actual table cell elements. + * For example index[4][3] is the or in row 4, column 3 of the table section ( or ). + * Note that this element is not necessarily even in the 4th (zero-based) -- if it has a rowSpan > 1, + * it may also be in a previous . + * + * If the second argument is given, it's a or that we're trying to find, and the algorithm + * stops as soon as it has found it and the function returns its slot number. + */ function buildIndexOrGetCellSlot( section: HTMLTableSectionElement, findCell?: HTMLTableCellElement @@ -234,38 +248,39 @@ function buildIndexOrGetCellSlot( const growingRowsLeft: number[] = []; // continue while we have actual 's left *or* we still have rowspan'ed elements that aren't done - while ( - curRow || - growingRowsLeft.some(function (e) { - return e !== 0; - }) - ) { + while (curRow || growingRowsLeft.some((e) => e !== 0)) { const curIndexRow: HTMLTableCellElement[] = []; index.push(curIndexRow); let curSlot = 0; if (curRow) { for ( - let curCellInd = 0; - curCellInd < curRow.children.length; - curCellInd++ + let curCellIdx = 0; + curCellIdx < curRow.children.length; + curCellIdx++ ) { while (growingRowsLeft[curSlot]) { growingRowsLeft[curSlot]--; curIndexRow[curSlot] = growing[curSlot]; curSlot++; } - const cell = curRow.children[curCellInd]; + + const cell = curRow.children[curCellIdx]; + if (!(cell instanceof HTMLTableCellElement)) { throw "invalid table"; } + if (getComputedStyle(cell).display === "none") { continue; } + if (cell === findCell) { return curSlot; } + const nextFreeSlot = curSlot + cell.colSpan; + for (; curSlot < nextFreeSlot; curSlot++) { growingRowsLeft[curSlot] = cell.rowSpan - 1; // if any of these is already growing, the table is broken -- no guarantees of anything growing[curSlot] = cell; @@ -273,18 +288,21 @@ function buildIndexOrGetCellSlot( } } } + while (curSlot < growing.length) { if (growingRowsLeft[curSlot]) { growingRowsLeft[curSlot]--; curIndexRow[curSlot] = growing[curSlot]; } + curSlot++; } + if (curRow) { curRow = curRow.nextElementSibling; } } - return findCell - ? -1 - : index; /* if findCell was given but we end up here, that means it isn't in this section */ + + // if findCell was given, but we end up here, that means it isn't in this section + return findCell ? -1 : index; } From 186b2b49808e28de7e571bf251251fc81238593e Mon Sep 17 00:00:00 2001 From: Vlad Jimenez Date: Tue, 28 Mar 2023 11:17:39 -0700 Subject: [PATCH 12/23] Changes for latest develop changes --- .eslintrc.js | 5 +---- lib/{test/s-table.test.ts => components/table/table.test.ts} | 4 ++-- lib/tsconfig.test.json | 4 ---- 3 files changed, 3 insertions(+), 10 deletions(-) rename lib/{test/s-table.test.ts => components/table/table.test.ts} (99%) delete mode 100644 lib/tsconfig.test.json diff --git a/.eslintrc.js b/.eslintrc.js index b75e998d9f..2e2e9f1872 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -3,10 +3,7 @@ module.exports = { parser: "@typescript-eslint/parser", parserOptions: { tsconfigRootDir: __dirname, - project: [ - "./lib/tsconfig.json", - "./lib/tsconfig.test.json", - ], + project: ["./lib/tsconfig.json"], }, plugins: ["@typescript-eslint"], extends: [ diff --git a/lib/test/s-table.test.ts b/lib/components/table/table.test.ts similarity index 99% rename from lib/test/s-table.test.ts rename to lib/components/table/table.test.ts index c6d7298bc7..76c4f1dbe8 100644 --- a/lib/test/s-table.test.ts +++ b/lib/components/table/table.test.ts @@ -1,8 +1,8 @@ import { html, fixture, expect } from "@open-wc/testing"; import { screen } from "@testing-library/dom"; import userEvent from "@testing-library/user-event"; -import { buildIndex, getCellSlot } from "../ts/controllers/s-table"; -import "../ts/index"; +import { buildIndex, getCellSlot } from "./table"; +import "../../index"; const user = userEvent.setup(); diff --git a/lib/tsconfig.test.json b/lib/tsconfig.test.json deleted file mode 100644 index 9547d72f23..0000000000 --- a/lib/tsconfig.test.json +++ /dev/null @@ -1,4 +0,0 @@ -{ - "extends": "./tsconfig.json", - "include": ["test/"] -} From 255c7d98ea9aff3795419de8cae58adc1af74bdc Mon Sep 17 00:00:00 2001 From: Vlad Jimenez Date: Thu, 30 Mar 2023 21:22:25 -0700 Subject: [PATCH 13/23] Unit tests for column indicators + aria-sort values --- lib/components/table/table.test.ts | 206 +++++++++++++++++++++-------- lib/components/table/table.ts | 14 +- lib/test/test-utils.ts | 11 ++ 3 files changed, 174 insertions(+), 57 deletions(-) diff --git a/lib/components/table/table.test.ts b/lib/components/table/table.test.ts index 76c4f1dbe8..8b1a0962b9 100644 --- a/lib/components/table/table.test.ts +++ b/lib/components/table/table.test.ts @@ -1,13 +1,14 @@ -import { html, fixture, expect } from "@open-wc/testing"; +import { assert, expect, fixture, html } from "@open-wc/testing"; import { screen } from "@testing-library/dom"; import userEvent from "@testing-library/user-event"; -import { buildIndex, getCellSlot } from "./table"; +import { buildIndex, getCellSlot, SortOrder } from "./table"; import "../../index"; +import { AsLiterals } from "../../test/test-utils"; const user = userEvent.setup(); const mockSortableTable = () => { - const testSortabledIcons = (modifier: string) => html` + const sortDirectionIcons = (modifier: string) => html` - - - @@ -103,57 +113,141 @@ const mockSortableTable = () => { `; }; -const testAriaHiddenSvgs = (allSvgs: string[], visible?: string) => { - return allSvgs.forEach((svg) => { - const svgEl = screen.getByTestId(svg); +describe("s-table", () => { + describe("Column sorting indicators", () => { + const allColIds = [ + "test-sortable-season-col", + "test-sortable-month-col", + "test-sortable-temp-col", + ] as const; + type ColId = AsLiterals; - if (svg === visible) { - expect(svgEl).to.have.attribute("aria-hidden", "false"); - } else { - expect(svgEl).to.have.attribute("aria-hidden", "true"); - } - }); -}; + const allSvgIds = [ + "test-sortable-season-svg-asc", + "test-sortable-season-svg-desc", + "test-sortable-season-svg-none", + "test-sortable-month-svg-asc", + "test-sortable-month-svg-desc", + "test-sortable-month-svg-none", + "test-sortable-temp-svg-asc", + "test-sortable-temp-svg-desc", + "test-sortable-temp-svg-none", + ] as const; + type SvgId = AsLiterals; -describe("s-table", () => { - describe("Stimulus.js controller", () => { - it("table sort updates on click", async () => { + const expectThisColumnToBeSorted = ( + columnId?: ColId, + order?: SortOrder + ) => { + allColIds.forEach((colId) => { + const columnEl = screen.getByTestId(colId); + + if (columnId === colId) { + expect(columnEl).to.have.attribute("aria-sort", order); + } else { + expect(columnEl).to.not.have.attribute("aria-sort"); + } + }); + }; + + const expectTheseIndicatorsToBeVisible = (visibleIds: SvgId[]) => { + allSvgIds.forEach((svgId) => { + const svgEl = screen.getByTestId(svgId); + + if (visibleIds.includes(svgId)) { + expect(svgEl).to.not.have.class("d-none"); + } else { + expect(svgEl).to.have.class("d-none"); + } + }); + }; + + beforeEach(async () => { await fixture(mockSortableTable()); + }); + + it("should start toggle correctly between ASC/DESC after initial click", async () => { + const seasonColumn = screen.getByTestId("test-sortable-season-btn"); + + // Starting off in a fresh state, the "None" SVG icon should be visible + expectThisColumnToBeSorted(); + expectTheseIndicatorsToBeVisible([ + "test-sortable-season-svg-none", + "test-sortable-month-svg-none", + "test-sortable-temp-svg-none", + ]); - const sortButtonSeason = screen.getByTestId( - "test-sortable-season-button" + // Cycle through all season sort options + await user.click(seasonColumn); + expectThisColumnToBeSorted( + "test-sortable-season-col", + SortOrder.Ascending ); - const sortButtonMonth = screen.getByTestId( - "test-sortable-month-button" + expectTheseIndicatorsToBeVisible([ + "test-sortable-season-svg-asc", + "test-sortable-month-svg-none", + "test-sortable-temp-svg-none", + ]); + + await user.click(seasonColumn); + expectThisColumnToBeSorted( + "test-sortable-season-col", + SortOrder.Descending ); + expectTheseIndicatorsToBeVisible([ + "test-sortable-season-svg-desc", + "test-sortable-month-svg-none", + "test-sortable-temp-svg-none", + ]); - // const table = screen.getByTestId("test-sortable-table"); - const allSvgs = [ + // Clicking on a sorted column repeatedly will only toggle between ASC/DESC + await user.click(seasonColumn); + expectThisColumnToBeSorted( + "test-sortable-season-col", + SortOrder.Ascending + ); + expectTheseIndicatorsToBeVisible([ "test-sortable-season-svg-asc", - "test-sortable-season-svg-desc", - "test-sortable-season-svg-none", - "test-sortable-month-svg-asc", - "test-sortable-month-svg-desc", "test-sortable-month-svg-none", - ]; + "test-sortable-temp-svg-none", + ]); + }); - testAriaHiddenSvgs(allSvgs); + it('should toggle back a column indicator to "None" when another column is clicked', async () => { + const seasonColumn = screen.getByTestId("test-sortable-season-btn"); + const monthColumn = screen.getByTestId("test-sortable-month-btn"); - // Cycle through all season sort options - await user.click(sortButtonSeason); - testAriaHiddenSvgs(allSvgs, "test-sortable-season-svg-asc"); - await user.click(sortButtonSeason); - testAriaHiddenSvgs(allSvgs, "test-sortable-season-svg-desc"); - await user.click(sortButtonSeason); - testAriaHiddenSvgs(allSvgs, "test-sortable-season-svg-none"); - await user.click(sortButtonSeason); - testAriaHiddenSvgs(allSvgs); + // Starting off in a fresh state, the "None" SVG icon should be visible + expectThisColumnToBeSorted(); + expectTheseIndicatorsToBeVisible([ + "test-sortable-season-svg-none", + "test-sortable-month-svg-none", + "test-sortable-temp-svg-none", + ]); + + // Click season first; ASC indicator for Season column only should be visible + await user.click(seasonColumn); + expectThisColumnToBeSorted( + "test-sortable-season-col", + SortOrder.Ascending + ); + expectTheseIndicatorsToBeVisible([ + "test-sortable-season-svg-asc", + "test-sortable-month-svg-none", + "test-sortable-temp-svg-none", + ]); - // Set month sort then switch to season sort - await user.click(sortButtonMonth); - testAriaHiddenSvgs(allSvgs, "test-sortable-month-svg-asc"); - await user.click(sortButtonSeason); - testAriaHiddenSvgs(allSvgs, "test-sortable-season-svg-asc"); + // Click on month next + await user.click(monthColumn); + expectThisColumnToBeSorted( + "test-sortable-month-col", + SortOrder.Ascending + ); + expectTheseIndicatorsToBeVisible([ + "test-sortable-season-svg-none", + "test-sortable-month-svg-asc", + "test-sortable-temp-svg-none", + ]); }); }); @@ -248,14 +342,16 @@ describe("s-table", () => { await fixture(mockSortableTable()); [ - "test-sortable-season-button", - "test-sortable-month-button", - "test-sortable-temp-button", + "test-sortable-season-btn", + "test-sortable-month-btn", + "test-sortable-temp-btn", ].forEach((testID, index) => { const buttonElement = screen.getByTestId(testID); const columnCell = buttonElement.parentElement; - expect(columnCell).to.not.be.null; + assert.isNotNull(columnCell); + assert.instanceOf(columnCell, HTMLTableCellElement); + expect(getCellSlot(columnCell)).to.equal(index); }); }); diff --git a/lib/components/table/table.ts b/lib/components/table/table.ts index 1d6629ad61..fd3ba993a9 100644 --- a/lib/components/table/table.ts +++ b/lib/components/table/table.ts @@ -12,8 +12,8 @@ export enum SortOrder { } export class TableController extends Stacks.StacksController { - readonly columnTarget!: HTMLTableCellElement; - readonly columnTargets!: HTMLTableCellElement[]; + declare columnTarget: HTMLTableCellElement; + declare columnTargets: HTMLTableCellElement[]; static targets = ["column"]; @@ -189,6 +189,11 @@ export class TableController extends Stacks.StacksController { }; } +/** + * @internal This function is exported for testing purposes but is not a part of our public API + * + * @param section + */ export function buildIndex( section: HTMLTableSectionElement ): HTMLTableCellElement[][] { @@ -201,6 +206,11 @@ export function buildIndex( return result; } +/** + * @internal This function is exported for testing purposes but is not a part of our public API + * + * @param cell + */ export function getCellSlot(cell: HTMLTableCellElement): number { const tableElement = cell.parentElement?.parentElement; diff --git a/lib/test/test-utils.ts b/lib/test/test-utils.ts index f456102bd8..016188dbc7 100644 --- a/lib/test/test-utils.ts +++ b/lib/test/test-utils.ts @@ -440,3 +440,14 @@ const excludeOrSkipTest = ({ }; export { runComponentTest, runComponentTests }; + +/** + * Convert a const array of strings into a union type of the array's values. + * + * @example + * ``` + * const arrayOfStrings = ['Stacky', 'Ben', 'Dan', 'Giamir'] as const; + * type StringLiterals = AsLiterals; // 'Stacky' | 'Ben' | 'Dan' | 'Giamir' + * ``` + */ +export type AsLiterals> = T[number]; From 0232d70c19f598c02db95cc0480371e282525989 Mon Sep 17 00:00:00 2001 From: Vlad Jimenez Date: Thu, 30 Mar 2023 21:56:42 -0700 Subject: [PATCH 14/23] Apparently DOM tests are async? --- lib/components/table/table.test.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/components/table/table.test.ts b/lib/components/table/table.test.ts index 8b1a0962b9..4b4e5cab62 100644 --- a/lib/components/table/table.test.ts +++ b/lib/components/table/table.test.ts @@ -282,11 +282,11 @@ describe("s-table", () => { expect(tableIndex).to.satisfy((rows: typeof tableIndex) => rows.every((row) => row.length === 3) ); - expect(tableIndex[1][0]).to.equal( + await expect(tableIndex[1][0]).to.equal( tableIndex[1][1], "the Winter cell is expected to be referenced twice in this index" ); - expect(tableIndex[2][1]).to.equal( + await expect(tableIndex[2][1]).to.equal( tableIndex[2][2], "the April cell is expected to be referenced twice in this index" ); @@ -326,11 +326,11 @@ describe("s-table", () => { expect(tableIndex).to.satisfy((rows: typeof tableIndex) => rows.every((row) => row.length === 3) ); - expect(tableIndex[1][0]).to.equal( + await expect(tableIndex[1][0]).to.equal( tableIndex[0][0], "the Fall cell is expected to be referenced twice across multiple rows" ); - expect(tableIndex[3][0]).to.equal( + await expect(tableIndex[3][0]).to.equal( tableIndex[2][0], "the Spring cell is expected to be referenced twice in this index" ); @@ -352,7 +352,7 @@ describe("s-table", () => { assert.isNotNull(columnCell); assert.instanceOf(columnCell, HTMLTableCellElement); - expect(getCellSlot(columnCell)).to.equal(index); + await expect(getCellSlot(columnCell)).to.equal(index); }); }); }); From 94760033ca69669d6356d5ba33b4d5c746b6a316 Mon Sep 17 00:00:00 2001 From: Vlad Jimenez Date: Thu, 30 Mar 2023 22:25:42 -0700 Subject: [PATCH 15/23] Make unit tests + eslint happy by void'ing a promise --- lib/components/table/table.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/components/table/table.test.ts b/lib/components/table/table.test.ts index 4b4e5cab62..1612e86d7e 100644 --- a/lib/components/table/table.test.ts +++ b/lib/components/table/table.test.ts @@ -352,7 +352,7 @@ describe("s-table", () => { assert.isNotNull(columnCell); assert.instanceOf(columnCell, HTMLTableCellElement); - await expect(getCellSlot(columnCell)).to.equal(index); + void expect(getCellSlot(columnCell)).to.equal(index); }); }); }); From 10b76fba352b927c703eb868ed9fcb345fc655d2 Mon Sep 17 00:00:00 2001 From: Dan Cormier Date: Tue, 2 May 2023 16:48:26 -0400 Subject: [PATCH 16/23] update th button styling --- lib/components/table/table.less | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/components/table/table.less b/lib/components/table/table.less index 07ffdd549b..a9a1e1973c 100644 --- a/lib/components/table/table.less +++ b/lib/components/table/table.less @@ -130,14 +130,14 @@ button { appearance: none; background-color: transparent; - border: 0; - padding: var(--su8); + border: none; + display: flex; + gap: var(--su-static4); + font-weight: inherit; + margin: calc(var(--_ta-th-p) * -1); + padding: var(--_ta-th-p); text-align: left; - width: 100%; - } - - &[data-s-table-target="column"] { // If this column is sortable, then the padding will come from the button - padding: 0; + width: calc(100% + calc(var(--_ta-th-p) * 2)); } &.is-sorted { // Selected state From c7c8fca53cf1127eebbccb09ead018a6d4e3f98d Mon Sep 17 00:00:00 2001 From: Dan Cormier Date: Wed, 3 May 2023 14:34:50 -0400 Subject: [PATCH 17/23] small tweak --- lib/components/table/table.less | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/components/table/table.less b/lib/components/table/table.less index a9a1e1973c..ec88b7ba66 100644 --- a/lib/components/table/table.less +++ b/lib/components/table/table.less @@ -130,7 +130,7 @@ button { appearance: none; background-color: transparent; - border: none; + border: 0; display: flex; gap: var(--su-static4); font-weight: inherit; From f26ca06b265159b8b5369b915323463552ed2db6 Mon Sep 17 00:00:00 2001 From: Dan Cormier Date: Wed, 3 May 2023 18:29:15 -0400 Subject: [PATCH 18/23] minor styling tweak --- lib/components/table/table.less | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/components/table/table.less b/lib/components/table/table.less index ec88b7ba66..2e0af1d989 100644 --- a/lib/components/table/table.less +++ b/lib/components/table/table.less @@ -131,6 +131,7 @@ appearance: none; background-color: transparent; border: 0; + cursor: pointer; display: flex; gap: var(--su-static4); font-weight: inherit; From ea5b100dfb966615b064422b1c1af25a8cdc25fd Mon Sep 17 00:00:00 2001 From: Dan Cormier Date: Wed, 3 May 2023 18:29:34 -0400 Subject: [PATCH 19/23] Update test to check current and legacy markup --- lib/components/table/table.test.ts | 374 ++++++++++++++--------------- lib/components/table/table.ts | 31 +-- 2 files changed, 187 insertions(+), 218 deletions(-) diff --git a/lib/components/table/table.test.ts b/lib/components/table/table.test.ts index 1612e86d7e..ce3964e2a3 100644 --- a/lib/components/table/table.test.ts +++ b/lib/components/table/table.test.ts @@ -7,79 +7,67 @@ import { AsLiterals } from "../../test/test-utils"; const user = userEvent.setup(); -const mockSortableTable = () => { - const sortDirectionIcons = (modifier: string) => html` - - - - `; + ${label} ${sortDirectionIcons(modifier)} + + +` : html` + +`; +const mockSortableTable = (useButton: boolean) => { return html`
+ + +
+ ${label} ${sortDirectionIcons(modifier)} +
- - - + ${getTh("Season", "season", useButton)} + ${getTh("Month", "month", useButton)} + ${getTh("Temperature", "temp", useButton)} + @@ -114,140 +102,148 @@ const mockSortableTable = () => { }; describe("s-table", () => { - describe("Column sorting indicators", () => { - const allColIds = [ - "test-sortable-season-col", - "test-sortable-month-col", - "test-sortable-temp-col", - ] as const; - type ColId = AsLiterals; - - const allSvgIds = [ - "test-sortable-season-svg-asc", - "test-sortable-season-svg-desc", - "test-sortable-season-svg-none", - "test-sortable-month-svg-asc", - "test-sortable-month-svg-desc", - "test-sortable-month-svg-none", - "test-sortable-temp-svg-asc", - "test-sortable-temp-svg-desc", - "test-sortable-temp-svg-none", - ] as const; - type SvgId = AsLiterals; - - const expectThisColumnToBeSorted = ( - columnId?: ColId, - order?: SortOrder - ) => { - allColIds.forEach((colId) => { - const columnEl = screen.getByTestId(colId); - - if (columnId === colId) { - expect(columnEl).to.have.attribute("aria-sort", order); - } else { - expect(columnEl).to.not.have.attribute("aria-sort"); - } - }); - }; + [true, false].forEach((useButton) => { + describe("Column sorting indicators", () => { + const allColIds = [ + "test-sortable-season-col", + "test-sortable-month-col", + "test-sortable-temp-col", + ] as const; + type ColId = AsLiterals; - const expectTheseIndicatorsToBeVisible = (visibleIds: SvgId[]) => { - allSvgIds.forEach((svgId) => { - const svgEl = screen.getByTestId(svgId); + const allSvgIds = [ + "test-sortable-season-svg-asc", + "test-sortable-season-svg-desc", + "test-sortable-season-svg-none", + "test-sortable-month-svg-asc", + "test-sortable-month-svg-desc", + "test-sortable-month-svg-none", + "test-sortable-temp-svg-asc", + "test-sortable-temp-svg-desc", + "test-sortable-temp-svg-none", + ] as const; + type SvgId = AsLiterals; - if (visibleIds.includes(svgId)) { - expect(svgEl).to.not.have.class("d-none"); - } else { - expect(svgEl).to.have.class("d-none"); - } - }); - }; + const expectThisColumnToBeSorted = ( + columnId?: ColId, + order?: SortOrder + ) => { + allColIds.forEach((colId) => { + const columnEl = screen.getByTestId(colId); - beforeEach(async () => { - await fixture(mockSortableTable()); - }); + if (columnId === colId) { + expect(columnEl).to.have.attribute("aria-sort", order); + } else { + expect(columnEl).to.not.have.attribute("aria-sort"); + } + }); + }; - it("should start toggle correctly between ASC/DESC after initial click", async () => { - const seasonColumn = screen.getByTestId("test-sortable-season-btn"); + const expectTheseIndicatorsToBeVisible = (visibleIds: SvgId[]) => { + allSvgIds.forEach((svgId) => { + const svgEl = screen.getByTestId(svgId); - // Starting off in a fresh state, the "None" SVG icon should be visible - expectThisColumnToBeSorted(); - expectTheseIndicatorsToBeVisible([ - "test-sortable-season-svg-none", - "test-sortable-month-svg-none", - "test-sortable-temp-svg-none", - ]); + if (visibleIds.includes(svgId)) { + expect(svgEl).to.not.have.class("d-none"); + } else { + expect(svgEl).to.have.class("d-none"); + } + }); + }; - // Cycle through all season sort options - await user.click(seasonColumn); - expectThisColumnToBeSorted( - "test-sortable-season-col", - SortOrder.Ascending - ); - expectTheseIndicatorsToBeVisible([ - "test-sortable-season-svg-asc", - "test-sortable-month-svg-none", - "test-sortable-temp-svg-none", - ]); + beforeEach(async () => { + await fixture(mockSortableTable(useButton)); + }); - await user.click(seasonColumn); - expectThisColumnToBeSorted( - "test-sortable-season-col", - SortOrder.Descending - ); - expectTheseIndicatorsToBeVisible([ - "test-sortable-season-svg-desc", - "test-sortable-month-svg-none", - "test-sortable-temp-svg-none", - ]); + it("should start toggle correctly between ASC/DESC after initial click", async () => { + const seasonColumn = useButton + ? screen.getByTestId("test-sortable-season-btn") + : screen.getByTestId("test-sortable-season-col"); - // Clicking on a sorted column repeatedly will only toggle between ASC/DESC - await user.click(seasonColumn); - expectThisColumnToBeSorted( - "test-sortable-season-col", - SortOrder.Ascending - ); - expectTheseIndicatorsToBeVisible([ - "test-sortable-season-svg-asc", - "test-sortable-month-svg-none", - "test-sortable-temp-svg-none", - ]); - }); + // Starting off in a fresh state, the "None" SVG icon should be visible + expectThisColumnToBeSorted(); + expectTheseIndicatorsToBeVisible([ + "test-sortable-season-svg-none", + "test-sortable-month-svg-none", + "test-sortable-temp-svg-none", + ]); - it('should toggle back a column indicator to "None" when another column is clicked', async () => { - const seasonColumn = screen.getByTestId("test-sortable-season-btn"); - const monthColumn = screen.getByTestId("test-sortable-month-btn"); + // Cycle through all season sort options + await user.click(seasonColumn); + expectThisColumnToBeSorted( + "test-sortable-season-col", + SortOrder.Ascending + ); + expectTheseIndicatorsToBeVisible([ + "test-sortable-season-svg-asc", + "test-sortable-month-svg-none", + "test-sortable-temp-svg-none", + ]); - // Starting off in a fresh state, the "None" SVG icon should be visible - expectThisColumnToBeSorted(); - expectTheseIndicatorsToBeVisible([ - "test-sortable-season-svg-none", - "test-sortable-month-svg-none", - "test-sortable-temp-svg-none", - ]); + await user.click(seasonColumn); + expectThisColumnToBeSorted( + "test-sortable-season-col", + SortOrder.Descending + ); + expectTheseIndicatorsToBeVisible([ + "test-sortable-season-svg-desc", + "test-sortable-month-svg-none", + "test-sortable-temp-svg-none", + ]); - // Click season first; ASC indicator for Season column only should be visible - await user.click(seasonColumn); - expectThisColumnToBeSorted( - "test-sortable-season-col", - SortOrder.Ascending - ); - expectTheseIndicatorsToBeVisible([ - "test-sortable-season-svg-asc", - "test-sortable-month-svg-none", - "test-sortable-temp-svg-none", - ]); + // Clicking on a sorted column repeatedly will only toggle between ASC/DESC + await user.click(seasonColumn); + expectThisColumnToBeSorted( + "test-sortable-season-col", + SortOrder.Ascending + ); + expectTheseIndicatorsToBeVisible([ + "test-sortable-season-svg-asc", + "test-sortable-month-svg-none", + "test-sortable-temp-svg-none", + ]); + }); - // Click on month next - await user.click(monthColumn); - expectThisColumnToBeSorted( - "test-sortable-month-col", - SortOrder.Ascending - ); - expectTheseIndicatorsToBeVisible([ - "test-sortable-season-svg-none", - "test-sortable-month-svg-asc", - "test-sortable-temp-svg-none", - ]); + it('should toggle back a column indicator to "None" when another column is clicked', async () => { + const seasonColumn = useButton + ? screen.getByTestId("test-sortable-season-btn") + : screen.getByTestId("test-sortable-season-col"); + const monthColumn = useButton + ? screen.getByTestId("test-sortable-month-btn") + : screen.getByTestId("test-sortable-month-col"); + + // Starting off in a fresh state, the "None" SVG icon should be visible + expectThisColumnToBeSorted(); + expectTheseIndicatorsToBeVisible([ + "test-sortable-season-svg-none", + "test-sortable-month-svg-none", + "test-sortable-temp-svg-none", + ]); + + // Click season first; ASC indicator for Season column only should be visible + await user.click(seasonColumn); + expectThisColumnToBeSorted( + "test-sortable-season-col", + SortOrder.Ascending + ); + expectTheseIndicatorsToBeVisible([ + "test-sortable-season-svg-asc", + "test-sortable-month-svg-none", + "test-sortable-temp-svg-none", + ]); + + // Click on month next + await user.click(monthColumn); + expectThisColumnToBeSorted( + "test-sortable-month-col", + SortOrder.Ascending + ); + expectTheseIndicatorsToBeVisible([ + "test-sortable-season-svg-none", + "test-sortable-month-svg-asc", + "test-sortable-temp-svg-none", + ]); + }); }); }); @@ -339,7 +335,7 @@ describe("s-table", () => { describe("getCellSlot()", () => { it("should correctly calculate the column number for each button", async () => { - await fixture(mockSortableTable()); + await fixture(mockSortableTable(true)); [ "test-sortable-season-btn", diff --git a/lib/components/table/table.ts b/lib/components/table/table.ts index fd3ba993a9..651ff8ffa3 100644 --- a/lib/components/table/table.ts +++ b/lib/components/table/table.ts @@ -17,18 +17,12 @@ export class TableController extends Stacks.StacksController { static targets = ["column"]; - connect() { - this.columnTargets.forEach(this.ensureHeadersAreClickable); - } - sort(evt: PointerEvent) { // eslint-disable-next-line @typescript-eslint/no-this-alias const controller = this; const sortTriggerEl = evt.currentTarget; - if (!(sortTriggerEl instanceof HTMLButtonElement)) { - throw "invalid event target"; - } - const colHead = sortTriggerEl.parentElement as HTMLTableCellElement; + const triggerIsButton = sortTriggerEl instanceof HTMLButtonElement; + const colHead = (triggerIsButton ? sortTriggerEl.parentElement : sortTriggerEl) as HTMLTableCellElement; const table = this.element as HTMLTableElement; const tbody = table.tBodies[0]; @@ -166,27 +160,6 @@ export class TableController extends Stacks.StacksController { } }); }; - - /** - * Transform legacy header markup into the new markup. - * - * @param headerEl - */ - private ensureHeadersAreClickable = (headerEl: HTMLTableCellElement) => { - const headerAction = headerEl.getAttribute("data-action"); - - // Legacy markup that violates accessibility practices; change the DOM - if (headerAction !== null && headerAction.substring(0, 5) === "click") { - headerEl.removeAttribute("data-action"); - - // eslint-disable-next-line no-unsanitized/property - headerEl.innerHTML = ` - - `; - } - }; } /** From f30fb6d8db5c9353655b413f7878aa822b77268f Mon Sep 17 00:00:00 2001 From: Dan Cormier Date: Wed, 3 May 2023 18:37:42 -0400 Subject: [PATCH 20/23] lint --- lib/components/table/table.test.ts | 81 ++++++++++++++++-------------- lib/components/table/table.ts | 4 +- 2 files changed, 47 insertions(+), 38 deletions(-) diff --git a/lib/components/table/table.test.ts b/lib/components/table/table.test.ts index ce3964e2a3..d67d25449b 100644 --- a/lib/components/table/table.test.ts +++ b/lib/components/table/table.test.ts @@ -12,51 +12,59 @@ const iconData = [ direction: "asc", classes: "iconArrowUpSm js-sorting-indicator-asc d-none", pathD: "M3 9h8L7 5 3 9Z", - }, { + }, + { direction: "desc", classes: "iconArrowDownSm js-sorting-indicator-desc d-none", pathD: "M3 5h8L7 9 3 5Z", - }, { + }, + { direction: "none", classes: "iconArrowUpDownSm js-sorting-indicator-none", pathD: "m7 2 4 4H3l4-4Zm0 10 4-4H3l4 4Z", }, ]; -const sortDirectionIcons = (modifier: string) => iconData.map((icon) => html` - -`); - -const getTh = (label: string, modifier: string, useButton: boolean) => useButton ? html` - -` : html` - -`; +const sortDirectionIcons = (modifier: string) => + iconData.map( + (icon) => html` + + ` + ); + +const getTh = (label: string, modifier: string, useButton: boolean) => + useButton + ? html` + + ` + : html` + + `; const mockSortableTable = (useButton: boolean) => { return html` @@ -67,7 +75,6 @@ const mockSortableTable = (useButton: boolean) => { ${getTh("Season", "season", useButton)} ${getTh("Month", "month", useButton)} ${getTh("Temperature", "temp", useButton)} - diff --git a/lib/components/table/table.ts b/lib/components/table/table.ts index 651ff8ffa3..0d346369eb 100644 --- a/lib/components/table/table.ts +++ b/lib/components/table/table.ts @@ -22,7 +22,9 @@ export class TableController extends Stacks.StacksController { const controller = this; const sortTriggerEl = evt.currentTarget; const triggerIsButton = sortTriggerEl instanceof HTMLButtonElement; - const colHead = (triggerIsButton ? sortTriggerEl.parentElement : sortTriggerEl) as HTMLTableCellElement; + const colHead = ( + triggerIsButton ? sortTriggerEl.parentElement : sortTriggerEl + ) as HTMLTableCellElement; const table = this.element as HTMLTableElement; const tbody = table.tBodies[0]; From 4745d16a985eec8dd170be66da46c7fe0f3b6b2f Mon Sep 17 00:00:00 2001 From: Dan Cormier Date: Wed, 3 May 2023 18:45:20 -0400 Subject: [PATCH 21/23] Update test description to reflect legacy markup --- lib/components/table/table.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/components/table/table.test.ts b/lib/components/table/table.test.ts index d67d25449b..d9962f78ec 100644 --- a/lib/components/table/table.test.ts +++ b/lib/components/table/table.test.ts @@ -109,8 +109,11 @@ const mockSortableTable = (useButton: boolean) => { }; describe("s-table", () => { + // Current, legacy markup [true, false].forEach((useButton) => { - describe("Column sorting indicators", () => { + describe(`Column sorting indicators${ + useButton ? "" : " (legacy markup)" + }`, () => { const allColIds = [ "test-sortable-season-col", "test-sortable-month-col", From a141f181d84e46f21da0ec4affb9c827fc3aebc8 Mon Sep 17 00:00:00 2001 From: Dan Cormier Date: Tue, 9 May 2023 13:12:46 -0400 Subject: [PATCH 22/23] add comments addresses https://github.com/StackExchange/Stacks/pull/1280#discussion_r1188642427 --- lib/components/table/table.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/components/table/table.ts b/lib/components/table/table.ts index 0d346369eb..dcc12de7fd 100644 --- a/lib/components/table/table.ts +++ b/lib/components/table/table.ts @@ -21,7 +21,10 @@ export class TableController extends Stacks.StacksController { // eslint-disable-next-line @typescript-eslint/no-this-alias const controller = this; const sortTriggerEl = evt.currentTarget; + // TODO: support *only* button as trigger in next major release const triggerIsButton = sortTriggerEl instanceof HTMLButtonElement; + // the below conditional is here for backward compatibility with the old API + // where we did not advise buttons as sortable column head triggers const colHead = ( triggerIsButton ? sortTriggerEl.parentElement : sortTriggerEl ) as HTMLTableCellElement; From d7a1a6b18f3c823d44608c8ee0cf0d3323b7ae14 Mon Sep 17 00:00:00 2001 From: Dan Cormier Date: Tue, 9 May 2023 13:20:44 -0400 Subject: [PATCH 23/23] Add scope attr to sortable examples --- docs/product/components/tables.html | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/docs/product/components/tables.html b/docs/product/components/tables.html index 9328445e8c..0889965db8 100644 --- a/docs/product/components/tables.html +++ b/docs/product/components/tables.html @@ -748,8 +748,8 @@
- - - - - -
- - - ${label} ${sortDirectionIcons(modifier)} - + + + ${label} ${sortDirectionIcons(modifier)} +
- - + + @@ -772,7 +772,7 @@
AttributeApplied toAttributeApplied to Description
- - -
+ + +