From cd49fa7dfd9e834cff74881ed7ed44bd6b5c4747 Mon Sep 17 00:00:00 2001 From: Drake Harper Date: Wed, 29 Jul 2026 15:05:24 -0600 Subject: [PATCH] fix(ui-table): don't describe a sort state on unsorted v1 tables `getCaptionText` lost its early return for the unsorted case, so a v1 Table given a plain string caption has ' Sorted by undefined (undefined)' appended to both its and its accessible name whenever no column is sorted: aria-label="Movies Sorted by undefined (undefined)" That is user-facing for screen reader users. Restore the early return so an unsorted table uses its caption as given, and keep the caption function path untouched. Found while upgrading canvas-lms from 11.7.4-SECURITY.3 to 11.7.4, where it broke 10 vitest files and 6 Selenium specs that locate tables by accessible name. Adds a v1 test file. The existing Table tests import `@instructure/ui-table/latest` and only ever pass a caption function, so the v1 string-caption path had no coverage. --- .../src/Table/v1/__tests__/Table.test.tsx | 92 +++++++++++++++++++ packages/ui-table/src/Table/v1/index.tsx | 10 +- 2 files changed, 100 insertions(+), 2 deletions(-) create mode 100644 packages/ui-table/src/Table/v1/__tests__/Table.test.tsx diff --git a/packages/ui-table/src/Table/v1/__tests__/Table.test.tsx b/packages/ui-table/src/Table/v1/__tests__/Table.test.tsx new file mode 100644 index 0000000000..47fe539450 --- /dev/null +++ b/packages/ui-table/src/Table/v1/__tests__/Table.test.tsx @@ -0,0 +1,92 @@ +/* + * The MIT License (MIT). + * + * Copyright (c) 2015 - present Instructure, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +import { render, screen } from '@testing-library/react' +import { MockInstance, vi } from 'vitest' +// v1 accepts a plain ReactNode caption, in addition to the caption function +// that v2 requires +import { Table } from '../index.js' +import type { TableColHeaderProps } from '../ColHeader/props' +import '@testing-library/jest-dom' + +describe(' (v1)', () => { + let consoleErrorMock: MockInstance + + beforeEach(() => { + // Mocking console to prevent test output pollution + consoleErrorMock = vi.spyOn(console, 'error').mockImplementation(() => {}) + }) + + afterEach(() => { + consoleErrorMock.mockRestore() + }) + + const renderTable = (colHeaderProps?: TableColHeaderProps) => + render( +
+ + + + Foo + + Bar + + + + + +
+ ) + + describe('with a string caption', () => { + it('uses the caption verbatim when no column is sorted', async () => { + const { container } = renderTable() + + expect(container.querySelector('caption')).toHaveTextContent(/^Movies$/) + expect(screen.getByRole('table', { name: 'Movies' })).toBeInTheDocument() + }) + + it('does not describe a sort state that does not exist', async () => { + const { container } = renderTable() + + expect(container.querySelector('caption')).not.toHaveTextContent( + 'Sorted by' + ) + expect(container.querySelector('caption')).not.toHaveTextContent( + 'undefined' + ) + }) + + it('appends the sort state when a column is sorted', async () => { + const { container } = renderTable({ + id: 'foo', + sortDirection: 'ascending' + }) + + expect(container.querySelector('caption')).toHaveTextContent( + 'Movies Sorted by Foo (ascending)' + ) + }) + }) +}) diff --git a/packages/ui-table/src/Table/v1/index.tsx b/packages/ui-table/src/Table/v1/index.tsx index 0aa63d6c9a..d30121e3a5 100644 --- a/packages/ui-table/src/Table/v1/index.tsx +++ b/packages/ui-table/src/Table/v1/index.tsx @@ -161,7 +161,7 @@ class Table extends Component { const headerText = typeof colHeader.props.children === 'string' ? colHeader.props.children - : (colHeader.props.children?.props?.children ?? '') + : colHeader.props.children?.props?.children ?? '' return { header: headerText, direction: colHeader.props.sortDirection } } } @@ -174,7 +174,13 @@ class Table extends Component { if (typeof caption === 'function') { return caption(sortInfo?.header ?? '', sortInfo?.direction ?? 'none') } - const sortText = ` Sorted by ${sortInfo?.header} (${sortInfo?.direction})` + // An unsorted table has no sort state to describe, so use the caption as + // given. Falling through would append ' Sorted by undefined (undefined)' + // to the accessible name of every unsorted table. + if (!sortInfo) { + return caption as string + } + const sortText = ` Sorted by ${sortInfo.header} (${sortInfo.direction})` return caption ? caption + sortText : sortText.trim() }