Skip to content

Commit caa55a3

Browse files
committed
fix(ui-table,ui-codemods): set caption prop to required
BREAKING CHANGE: Table's caption have to be set from now on.
1 parent b9067f3 commit caa55a3

File tree

9 files changed

+159
-5
lines changed

9 files changed

+159
-5
lines changed

docs/contributor-docs/upgrade-guide.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,8 @@ The **`<CodeEditor>` component** from the `ui-code-editor` package has been **re
7474
| `Body` | `hover` | is now stored in `TableContext` |
7575
| `Body` | `headers` | is now stored in `TableContext` |
7676

77+
[Table](#Table)'s `caption` prop is now required.
78+
7779
---
7880

7981
## API Changes
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
import { Table } from '@instructure/ui-table'
2+
3+
export default function Example() {
4+
return (
5+
<Table>
6+
<thead>
7+
<tr>
8+
<th>Header</th>
9+
</tr>
10+
</thead>
11+
<tbody>
12+
<tr>
13+
<td>Data</td>
14+
</tr>
15+
</tbody>
16+
</Table>
17+
)
18+
}
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
import { Table } from '@instructure/ui-table'
2+
3+
export default function Example() {
4+
return (
5+
<Table caption="My table">
6+
<thead>
7+
<tr>
8+
<th>Header</th>
9+
</tr>
10+
</thead>
11+
<tbody>
12+
<tr>
13+
<td>Data</td>
14+
</tr>
15+
</tbody>
16+
</Table>
17+
)
18+
}

packages/ui-codemods/lib/__node_tests__/codemods.test.ts

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@ import removeAsFromInstUISettingsProvider from '../removeAsFromInstUISettingsPro
2828
import renameCanvasThemesCodemod from '../renameCanvasThemesCodemod'
2929
import updateCodeEditorImport from '../updateCodeEditorImport'
3030
import renameGetComputedStyleToGetCSSStyleDeclaration from '../renameGetComputedStyleToGetCSSStyleDeclaration'
31+
import { printWarning } from '../utils/codemodHelpers'
32+
import warnTableCaptionMissing from '../warnTableCaptionMissing'
33+
import { vi, expect } from 'vitest'
3134

3235
describe('test codemods', () => {
3336
it('test InstUI v10 color codemods', () => {
@@ -49,4 +52,38 @@ describe('test codemods', () => {
4952
it('test renaming "getComputedStyle" to getCSSStyleDeclaration', () => {
5053
runTest(renameGetComputedStyleToGetCSSStyleDeclaration)
5154
})
55+
56+
it('test Table caption warning codemod', async () => {
57+
const fs = await import('fs/promises')
58+
const noCaptionInput = await fs.readFile(
59+
require.resolve(
60+
'./__testfixtures__/warnTableCaptionMissing/noCaption.input.tsx'
61+
),
62+
'utf-8'
63+
)
64+
const withCaptionInput = await fs.readFile(
65+
require.resolve(
66+
'./__testfixtures__/warnTableCaptionMissing/withCaption.input.tsx'
67+
),
68+
'utf-8'
69+
)
70+
// Test: should warn if Table is missing caption
71+
const spy = vi.spyOn(printWarning, 'call')
72+
warnTableCaptionMissing(
73+
{ source: noCaptionInput, path: 'noCaption.input.tsx' } as any,
74+
{ jscodeshift: require('jscodeshift') } as any,
75+
{}
76+
)
77+
expect(spy).toHaveBeenCalled()
78+
spy.mockClear()
79+
80+
// Test: should NOT warn if Table has caption
81+
warnTableCaptionMissing(
82+
{ source: withCaptionInput, path: 'withCaption.input.tsx' } as any,
83+
{ jscodeshift: require('jscodeshift') } as any,
84+
{}
85+
)
86+
expect(spy).not.toHaveBeenCalled()
87+
spy.mockRestore()
88+
})
5289
})

packages/ui-codemods/lib/index.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,14 @@ import removeAsFromInstUISettingsProvider from './removeAsFromInstUISettingsProv
2828
import { renameCanvasThemes } from './renameCanvasThemesCodemod'
2929
import updateCodeEditorImport from './updateCodeEditorImport'
3030
import renameGetComputedStyleToGetCSSStyleDeclaration from './renameGetComputedStyleToGetCSSStyleDeclaration'
31+
import warnTableCaptionMissing from './warnTableCaptionMissing'
3132

3233
export {
3334
updateV10Breaking,
3435
instUIv11Codemods,
3536
removeAsFromInstUISettingsProvider,
3637
renameCanvasThemes,
3738
updateCodeEditorImport,
38-
renameGetComputedStyleToGetCSSStyleDeclaration
39+
renameGetComputedStyleToGetCSSStyleDeclaration,
40+
warnTableCaptionMissing
3941
}
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
/*
2+
* The MIT License (MIT)
3+
*
4+
* Copyright (c) 2015 - present Instructure, Inc.
5+
*
6+
* Permission is hereby granted, free of charge, to any person obtaining a copy
7+
* of this software and associated documentation files (the "Software"), to deal
8+
* in the Software without restriction, including without limitation the rights
9+
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
10+
* copies of the Software, and to permit persons to whom the Software is
11+
* furnished to do so, subject to the following conditions:
12+
*
13+
* The above copyright notice and this permission notice shall be included in all
14+
* copies or substantial portions of the Software.
15+
*
16+
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
17+
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
18+
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
19+
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
20+
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
21+
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
22+
* SOFTWARE.
23+
*/
24+
import type { FileInfo, API, Options } from 'jscodeshift'
25+
import {
26+
findImport,
27+
findElement,
28+
findAttribute,
29+
printWarning
30+
} from './utils/codemodHelpers'
31+
32+
const TABLE_IMPORT_PATHS = ['@instructure/ui-table', '@instructure/ui']
33+
34+
export default function warnTableCaptionMissing(
35+
file: FileInfo,
36+
api: API,
37+
options: Options
38+
) {
39+
const j = api.jscodeshift
40+
const root = j(file.source)
41+
const filePath = file.path
42+
43+
// Find the local name for Table (could be aliased)
44+
const tableName = findImport(j, root, 'Table', TABLE_IMPORT_PATHS)
45+
if (!tableName) return file.source
46+
47+
// Find all <Table> elements
48+
const tableElements = findElement(filePath, j, root, tableName)
49+
50+
tableElements.forEach((path) => {
51+
// Check if caption attribute exists
52+
const hasCaption =
53+
findAttribute(
54+
filePath,
55+
j,
56+
j(path.value.openingElement),
57+
'caption'
58+
).size() > 0
59+
if (!hasCaption) {
60+
printWarning(
61+
filePath,
62+
path.value.loc?.start.line,
63+
`<${tableName}> is missing required 'caption' prop.`
64+
)
65+
}
66+
})
67+
68+
return file.source
69+
}

packages/ui-table/src/Table/__tests__/Table.test.tsx

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,8 @@ describe('<Table />', async () => {
8080

8181
it('applies a fixed column layout', async () => {
8282
await renderTable({
83-
layout: 'fixed'
83+
layout: 'fixed',
84+
caption: 'Test table'
8485
})
8586
const table = screen.getByRole('table')
8687

@@ -89,7 +90,8 @@ describe('<Table />', async () => {
8990

9091
it('passes hover to table row', async () => {
9192
renderTable({
92-
hover: true
93+
hover: true,
94+
caption: 'Test table'
9395
})
9496
const tableRows = screen.getAllByRole('row')
9597

@@ -119,7 +121,8 @@ describe('<Table />', async () => {
119121

120122
it('can render table in stacked layout', async () => {
121123
renderTable({
122-
layout: 'stacked'
124+
layout: 'stacked',
125+
caption: 'Test table'
123126
})
124127
const stackedTable = screen.getByRole('table')
125128

packages/ui-table/src/Table/index.tsx

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ import type { TableProps } from './props'
4444

4545
import { allowedProps } from './props'
4646
import TableContext from './TableContext'
47+
import { error } from '@instructure/console'
4748

4849
/**
4950
---
@@ -110,6 +111,10 @@ class Table extends Component<TableProps> {
110111
const isStacked = layout === 'stacked'
111112
const headers = isStacked ? this.getHeaders() : undefined
112113

114+
if (!caption) {
115+
error(false, `[Table] required prop caption is not set.`)
116+
}
117+
113118
return (
114119
<TableContext.Provider
115120
value={{

packages/ui-table/src/Table/props.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ type TableOwnProps = {
3636
* Provide a screen reader friendly description. Anything passed to this
3737
* prop will be wrapped by `<ScreenReaderContent>` when it is rendered.
3838
*/
39-
caption?: React.ReactNode
39+
caption: React.ReactNode
4040
/**
4141
* Valid values are `0`, `none`, `auto`, `xxx-small`, `xx-small`, `x-small`,
4242
* `small`, `medium`, `large`, `x-large`, `xx-large`. Apply these values via

0 commit comments

Comments
 (0)