From 1b12f46f2f177248bc5b4bcf8aabbf2c75fb6cd6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20S=C3=A1ros?= Date: Fri, 31 Jul 2026 13:25:59 +0200 Subject: [PATCH] fix(ui-table): don't append a sort state to unsorted v1 captions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `getCaptionText` lost its `if (!sortInfo) return caption` early return in aac2b1790f (#2574, INSTUI-5047), which restructured it around the new `TableCaption` function form. Every unsorted v1 Table given a plain string caption then rendered ' Sorted by undefined (undefined)' into both its and its aria-label: aria-label="Movies Sorted by undefined (undefined)" That is a real defect for screen reader users, and it contradicts the prop's own documentation ("A plain ReactNode ... is rendered as-is and the sort state is ignored"). It also affected our own docs site, whose Properties/Params/Returns/ComponentTheme tables all use v1 with string captions. First shipped in v11.7.4; reported by canvas-lms. Restore the early return, leaving the function-caption branch untouched. v1 had no test coverage at all — the existing suite imports `@instructure/ui-table/latest`, which is v2, and the same commit converted its remaining string captions to functions, so nothing guarded this path. Add a v1 suite covering the unsorted and sorted string-caption cases. It imports relatively on purpose: the `web` vitest project has no workspace aliases, so a package import there resolves to built output rather than source. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/Table/v1/__tests__/Table.test.tsx | 97 +++++++++++++++++++ packages/ui-table/src/Table/v1/index.tsx | 10 +- 2 files changed, 105 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..8dce3d75e0 --- /dev/null +++ b/packages/ui-table/src/Table/v1/__tests__/Table.test.tsx @@ -0,0 +1,97 @@ +/* + * 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 as well as the caption function that v2 +// requires. The main `` suite imports `/latest`, which is v2, so the +// string-caption path is only covered here. +// +// Imported relatively so this exercises source: the `web` vitest project has no +// workspace aliases, so `@instructure/ui-table/*` there resolves to the built +// `es/` output instead. +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?: Partial) => + render( +
+ + + + Foo + + Bar + + + + + Cell + Cell + + +
+ ) + + 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() + const caption = container.querySelector('caption') + + expect(caption).not.toHaveTextContent('Sorted by') + expect(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..de9ab5ef97 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. Falling through would + // put ' Sorted by undefined (undefined)' into 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() }