fix(ui-table): don't append a sort state to unsorted v1 captions - #2671
Open
balzss wants to merge 1 commit into
Open
fix(ui-table): don't append a sort state to unsorted v1 captions#2671balzss wants to merge 1 commit into
balzss wants to merge 1 commit into
Conversation
Contributor
|
Contributor
Visual regression report
Diff images (33)alert.png — baseline no longer producedavatar.png — baseline no longer producedbadge.png — baseline no longer producedbillboard.png — baseline no longer producedbreadcrumb.png — baseline no longer producedbutton-and-derivatives.png — baseline no longer producedbyline.png — baseline no longer producedcalendar.png — baseline no longer producedcheckbox.png — baseline no longer producedcheckboxgroup.png — baseline no longer producedcolorpicker.png — baseline no longer producedcontextview.png — baseline no longer producedcustom-and-lucide-icons.png — baseline no longer produceddateinput-dateinput2.png — baseline no longer produceddatetimeinput.png — baseline no longer produceddiff-demo.png — 6324 pixels differdrilldown.png — baseline no longer producedfiledrop.png — baseline no longer producedform-errors.png — baseline no longer producedheading.png — baseline no longer producedimg.png — baseline no longer producedlink.png — baseline no longer producedmenu.png — baseline no longer producedmetric-pill-tag-timeselect-text.png — baseline no longer producedoptions.png — baseline no longer producedpagination.png — baseline no longer producedprogressbar.png — baseline no longer producedselect-simpleselect.png — baseline no longer producedtable.png — baseline no longer producedtabs.png — baseline no longer producedtooltip.png — baseline no longer producedtreebrowser.png — baseline no longer producedview.png — baseline no longer producedBaselines come from the |
balzss
requested review from
joyenjoyer
and
a lite review from Copilot
and removed request for
Copilot
August 5, 2026 09:28
joyenjoyer
requested changes
Aug 6, 2026
joyenjoyer
left a comment
joyenjoyer
left a comment
Contributor
There was a problem hiding this comment.
Please reduce the commit message, it is quite long and verbose
|
|
||
| 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 |
Contributor
There was a problem hiding this comment.
Overly verbose, messy comment, please remove
balzss
force-pushed
the
fix/ui-table-unsorted-caption-v1
branch
from
August 13, 2026 12:11
1b12f46 to
e7ad1e7
Compare
`getCaptionText` lost its `if (!sortInfo) return caption` early return in aac2b17 (#2574), so every unsorted v1 Table with a plain string caption rendered ' Sorted by undefined (undefined)' into its <caption> and its aria-label. First shipped in v11.7.4; reported by canvas-lms. Restore the early return, leaving the function-caption branch untouched. Also add a v1 test suite — v1 had no coverage, which is why this shipped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
balzss
force-pushed
the
fix/ui-table-unsorted-caption-v1
branch
from
August 13, 2026 12:23
e7ad1e7 to
b97c098
Compare
ToMESSKa
approved these changes
Aug 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
An unsorted v1
Tablewith a plain stringcaptionrendersSorted by undefined (undefined)into both its<caption>and its accessible name:getCaptionTextlost itsif (!sortInfo) return captionguard in aac2b17 (#2574), which restructured it around the newTableCaptionfunction form. Shipped in v11.7.4. v2 is unaffected —captionmust be a function there.Our own docs site is affected too: the
__docs__Properties, Params, Returns and ComponentTheme tables all use v1 with string captions.Changes
Table/v1/index.tsx. The function-caption branch is untouched, and theas stringcast matches SECURITY.3 — the value feedsaria-label, which needs a string.Table/v1/__tests__/Table.test.tsx. v1 had no test coverage at all, which is why this shipped: the existing suite imports/latest(v2), and fix(ui-table): fix caption for table to be responsive to sorting #2574 converted its last string captions to functions.Test Plan
pnpm run test:vitest ui-table— 25 passed. Full unit suite — 2218 passed.build:typesclean.aria-labels, and the function-caption sort form still works.Worth knowing for future tests: the new file imports relatively on purpose.
getWorkspaceAliases()invitest.config.mtsis only wired into the browser project, so a@instructure/ui-table/*import in thewebproject resolves to builtes/output rather than source.Fixes INSTUI-5138
Reported by @drakeaharper in #instui while upgrading canvas-lms onto public 11.7.4, where it broke 10 vitest files and 6 Selenium specs. Supersedes #2666.
🤖 Generated with Claude Code