diff --git a/.eslintrc.js b/.eslintrc.js index db2aec6cdf..2e2e9f1872 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -17,5 +17,11 @@ module.exports = { "no-console": "error", "no-alert": "error", "no-process-env": "error", + "@typescript-eslint/no-unused-vars": [ + "error", + { + "destructuredArrayIgnorePattern": "^_" + }, + ] }, }; 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 86f0e76aa5..0889965db8 100644 --- a/docs/product/components/tables.html +++ b/docs/product/components/tables.html @@ -748,8 +748,8 @@ - - + + @@ -757,7 +757,7 @@ {% for item in tables.data-attributes %} - + {% endfor %} @@ -772,19 +772,25 @@
AttributeApplied toAttributeApplied to Description
{{ item.attribute }}{{ item.applies }}{{ item.applies }} {{ item.description }}
- - - @@ -806,23 +812,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 - … + +
- - - @@ -882,8 +894,8 @@ - - + + diff --git a/lib/components/table/table.less b/lib/components/table/table.less index b0b8efe21e..2e0af1d989 100644 --- a/lib/components/table/table.less +++ b/lib/components/table/table.less @@ -122,10 +122,25 @@ &&__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; + cursor: pointer; + 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: calc(100% + calc(var(--_ta-th-p) * 2)); + } + &.is-sorted { // Selected state color: var(--black-900); } diff --git a/lib/components/table/table.test.ts b/lib/components/table/table.test.ts new file mode 100644 index 0000000000..d9962f78ec --- /dev/null +++ b/lib/components/table/table.test.ts @@ -0,0 +1,366 @@ +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, SortOrder } from "./table"; +import "../../index"; +import { AsLiterals } from "../../test/test-utils"; + +const user = userEvent.setup(); + +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 mockSortableTable = (useButton: boolean) => { + 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" %} + +
Status Owner SourceViewsAppliesViewsApplies
+ + + ${label} ${sortDirectionIcons(modifier)} +
+ + + ${getTh("Season", "season", useButton)} + ${getTh("Month", "month", useButton)} + ${getTh("Temperature", "temp", useButton)} + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
WinterDecember2
SpringMarch13
SummerJune25
FallSeptember13
Average temperature13
+ + `; +}; + +describe("s-table", () => { + // Current, legacy markup + [true, false].forEach((useButton) => { + describe(`Column sorting indicators${ + useButton ? "" : " (legacy markup)" + }`, () => { + 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"); + } + }); + }; + + 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(useButton)); + }); + + 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"); + + // 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", + ]); + + // 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", + ]); + + 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", + ]); + + // 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", + ]); + }); + + 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", + ]); + }); + }); + }); + + 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) + ); + await expect(tableIndex[1][0]).to.equal( + tableIndex[1][1], + "the Winter cell is expected to be referenced twice in this index" + ); + await 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) + ); + await expect(tableIndex[1][0]).to.equal( + tableIndex[0][0], + "the Fall cell is expected to be referenced twice across multiple rows" + ); + await 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(true)); + + [ + "test-sortable-season-btn", + "test-sortable-month-btn", + "test-sortable-temp-btn", + ].forEach((testID, index) => { + const buttonElement = screen.getByTestId(testID); + const columnCell = buttonElement.parentElement; + + assert.isNotNull(columnCell); + assert.instanceOf(columnCell, HTMLTableCellElement); + + void expect(getCellSlot(columnCell)).to.equal(index); + }); + }); + }); + }); +}); diff --git a/lib/components/table/table.ts b/lib/components/table/table.ts index 22d45ad9d8..dcc12de7fd 100644 --- a/lib/components/table/table.ts +++ b/lib/components/table/table.ts @@ -1,51 +1,33 @@ import * as Stacks from "../../stacks"; -export class TableController extends Stacks.StacksController { - static targets = ["column"]; - readonly columnTarget!: Element; - readonly columnTargets!: Element[]; - - 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; +/** + * 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 + */ +export enum SortOrder { + Ascending = "ascending", + Descending = "descending", + None = "none", +} - target.classList.toggle( - "is-sorted", - isCurrrent && direction !== "none" - ); +export class TableController extends Stacks.StacksController { + declare columnTarget: HTMLTableCellElement; + declare columnTargets: HTMLTableCellElement[]; - 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 - ) - ); - }); - - if (!isCurrrent || direction === "none") { - controller.removeElementData(target, "sort-direction"); - } else { - controller.setElementData(target, "sort-direction", direction); - } - }); - } + static targets = ["column"]; - sort(evt: Event) { + sort(evt: PointerEvent) { // eslint-disable-next-line @typescript-eslint/no-this-alias const controller = this; - const colHead = evt.currentTarget; - if (!(colHead instanceof HTMLTableCellElement)) { - throw "invalid event target"; - } + 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; const table = this.element as HTMLTableElement; const tbody = table.tBodies[0]; @@ -64,7 +46,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; + colHead.getAttribute("aria-sort") === SortOrder.Ascending ? -1 : 1; const rows = Array.from(table.tBodies[0].rows); @@ -94,10 +76,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 ?? cell.textContent?.trim() ?? ""; if (d !== "" && `${parseInt(d, 10)}` !== d) { anyNonInt = true; @@ -133,9 +112,10 @@ 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); + if (firstBottomRow) { tbody.insertBefore(row, firstBottomRow); } else { @@ -145,54 +125,103 @@ 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.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.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"); + } + }); + }; } -function buildIndex( +/** + * @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[][] { 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 - ) - ) { +/** + * @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; + + 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 @@ -207,38 +236,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; @@ -246,18 +276,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; } 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];