Skip to content

Commit f8f8801

Browse files
authored
Merge pull request #3211 from salesforce-ux/fix/data-table-checkbox-radiogroup
Fix(data-table): checkbox and radio button grouping in data tables
2 parents abfed04 + 7951b8a commit f8f8801

File tree

8 files changed

+73
-70
lines changed

8 files changed

+73
-70
lines changed

gulpfile.js

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -90,14 +90,14 @@ const a11y = withName('a11y')(done => {
9090

9191
const vnu = withName('vnu')(() => accessibility.vnu(getComponents()));
9292

93-
// gulp lint:a11y
94-
// gulp lint:a11y --components path
95-
// gulp lint:a11y --components path,tabs,data-tables
93+
// npm run gulp -- lint:a11y
94+
// npm run gulp -- lint:a11y --components path
95+
// npm run gulp -- lint:a11y --components path,tabs,data-tables
9696
gulp.task('lint:a11y', gulp.series('generate:examples:wrapped', a11y));
9797

98-
// gulp lint:vnu
99-
// gulp lint:vnu --components path
100-
// gulp lint:vnu --components path,tabs,data-tables
98+
// npm run gulp -- lint:vnu
99+
// npm run gulp -- lint:vnu --components path
100+
// npm run gulp -- lint:vnu --components path,tabs,data-tables
101101
gulp.task('lint:vnu', gulp.series('generate:examples:wrapped', vnu));
102102

103103
gulp.task('lint:sass', lint.sass);

ui/components/checkbox/base/example.jsx

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,16 @@ export let Checkbox = props => {
6060
defaultChecked={props.checked}
6161
aria-describedby={props.errorId}
6262
tabIndex={props.tabIndex}
63+
aria-labelledby={
64+
props.labelId && props.groupId
65+
? props.labelId + ' ' + props.groupId
66+
: null
67+
}
6368
/>
6469
<label
6570
className={classNames('slds-checkbox__label')}
6671
htmlFor={props.id ? props.id : uniqueId}
72+
id={props.labelId}
6773
>
6874
<span className="slds-checkbox_faux" />
6975
{props.label && (

ui/components/data-tables/__tests__/__snapshots__/renders_an_advanced_data_table_with_radio_group.json

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

ui/components/data-tables/__tests__/index.spec.jsx

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -82,19 +82,14 @@ it('renders a product table', () =>
8282
it('renders an advanced data table with radio group', () =>
8383
matchesMarkupAndStyle(
8484
<AdvancedDataTable>
85-
<Thead
86-
columns={columns}
87-
hasNoSelectability
88-
radioGroupId="radio-group-header"
89-
label="Choose a Row to Select"
90-
/>
85+
<Thead columns={columns} isSingleSelect />
9186
<tbody>
9287
{_.times(rows.length, i => (
9388
<AdvancedDataTableTr
9489
key={i}
9590
index={i + 1}
9691
{...rows[i]}
97-
radioGroupId="radio-group-header"
92+
isSingleSelect
9893
/>
9994
))}
10095
</tbody>

ui/components/data-tables/advanced/example.jsx

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -345,19 +345,14 @@ export let examples = [
345345
label: 'Radio Group',
346346
element: (
347347
<Table>
348-
<Thead
349-
columns={columns}
350-
hasNoSelectability
351-
radioGroupId={radioGroupCommonId}
352-
label="Choose a Row to Select"
353-
/>
348+
<Thead columns={columns} isSingleSelect />
354349
<tbody>
355350
{_.times(rows.length, i => (
356351
<AdvancedDataTableTr
357352
key={i}
358353
index={i + 1}
359354
{...rows[i]}
360-
radioGroupId={radioGroupCommonId}
355+
isSingleSelect
361356
/>
362357
))}
363358
</tbody>

ui/components/data-tables/index.jsx

Lines changed: 26 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ import { Score } from '../dynamic-icons/score/example';
1818
import SvgIcon from '../../shared/svg-icon';
1919
import { Radio } from '../radio-group/base/example';
2020

21+
const checkboxRadioGroupHeaderId = 'check-group-header';
22+
2123
export const InlineEditTableContainer = props => (
2224
<div className="slds-table_edit_container slds-is-relative">
2325
{props.children}
@@ -51,18 +53,18 @@ export const AdvancedDataTable = props => (
5153
/**
5254
* @name Thead - thead block for advanced, inline, and product edit grids
5355
* @param {*} props
56+
* @prop {array} columnHeaderIcons - List of column names->icon name blocks which show an icon
5457
* @prop {array} columns - Grid columns
5558
* @prop {boolean} actionableMode - Specifies whether the grid is in actionable or navigation mode
56-
* @prop {array} columnHeaderIcons - List of column names->icon name blocks which show an icon
5759
* @prop {boolean} hasErrorColumn - Specifies whether the grid has a errors column
5860
* @prop {boolean} hasFocus - Specifies whether a cell in the thead is in user focus
59-
* @prop {boolean} hasNoSelectability - Specifies whether the thead should not contain a "select all" checkbox
6061
* @prop {boolean} hasMenus - Specifies whether the cells in the thead have a menu button
62+
* @prop {boolean} hasNoSelectability - Specifies whether the thead should not contain a "select all" checkbox
63+
* @prop {boolean} isSingleSelect - Specifies if the row selection uses radio buttons
6164
* @prop {boolean} selectAll - Specifies whether the select all checkbox is marked
6265
* @prop {string} mainColumnWidth - Specifies width of main columns
6366
* @prop {string} singleColumnWidth - Specifies width of a specific column
6467
* @prop {string} sortDirection - Specifies the sort direction of a specific column
65-
* @prop {string} radioGroupId - common id of radio group
6668
*/
6769
export const Thead = props => {
6870
const selectAllColumnWidth = props.hasErrorColumn ? '2rem' : '3.25rem';
@@ -73,7 +75,7 @@ export const Thead = props => {
7375
<tr className="slds-line-height_reset">
7476
{props.hasErrorColumn ? <ErrorsTh /> : null}
7577

76-
{props.hasNoSelectability ? null : (
78+
{props.hasNoSelectability || props.isSingleSelect ? null : (
7779
<SelectAllTh
7880
actionableMode={props.actionableMode}
7981
checked={props.selectAll}
@@ -82,13 +84,9 @@ export const Thead = props => {
8284
/>
8385
)}
8486

85-
{props.radioGroupId ? (
86-
<RadioGroupTh
87-
style={{ width: selectAllColumnWidth }}
88-
radioGroupId={props.radioGroupId}
89-
label={props.label}
90-
/>
91-
) : null}
87+
{props.isSingleSelect && (
88+
<RadioGroupTh style={{ width: selectAllColumnWidth }} />
89+
)}
9290

9391
{_.times(props.columns.length, i => (
9492
<Th
@@ -278,12 +276,17 @@ export let Th = props => {
278276
*/
279277
export const SelectAllTh = props => (
280278
<th style={props.style} className={props.className} scope="col">
279+
<span id={checkboxRadioGroupHeaderId} className="slds-assistive-text">
280+
Choose a row
281+
</span>
281282
<div className="slds-th__action slds-th__action_form">
282283
<Checkbox
283284
tabIndex={props.actionableMode ? '0' : '-1'}
285+
labelId="check-select-all-label"
284286
label="Select All"
285287
hideLabel
286288
checked={props.checked ? true : null}
289+
groupId={checkboxRadioGroupHeaderId}
287290
/>
288291
</div>
289292
</th>
@@ -293,17 +296,12 @@ export const SelectAllTh = props => (
293296
* @name RadioGroupTh - Radio group column header
294297
* @param {*} props
295298
* @prop {object} style - React style object
296-
* @prop {string} radioGroupId - common id of radio group
297-
* @prop {string} label - header text for radio group column
298299
*/
299300
export const RadioGroupTh = props => (
300-
<th
301-
className={props.className}
302-
scope="col"
303-
style={props.style}
304-
id={props.radioGroupId}
305-
>
306-
<span className="slds-assistive-text">{props.label}</span>
301+
<th className={props.className} scope="col" style={props.style}>
302+
<span id="radio-group-header" className="slds-assistive-text">
303+
Choose a row to select
304+
</span>
307305
</th>
308306
);
309307

@@ -337,6 +335,7 @@ export const ErrorsTh = props => (
337335
* @prop {boolean} actionableMode - Specifies whether the grid is in actionable or navigation mode
338336
* @prop {boolean} hasFocus - Specifies whether a specific cell is in focus
339337
* @prop {boolean} hasScore - Specifies whether a row has a score cell
338+
* @prop {boolean} isSingleSelect - Specifies whether to use a radio button for selection or not
340339
* @prop {boolean} rowSelected
341340
* @prop {integer} index - Row index in the Grid
342341
* @prop {string} accountName
@@ -349,7 +348,6 @@ export const ErrorsTh = props => (
349348
* @prop {string} contact
350349
* @prop {string} recordName
351350
* @prop {string} stage
352-
* @prop {string} radioGroupId - common id of radio group
353351
*/
354352
export const AdvancedDataTableTr = props => (
355353
<AdvancedDataTableTrElement
@@ -361,7 +359,7 @@ export const AdvancedDataTableTr = props => (
361359
inputTabIndex={props.actionableMode ? '0' : '-1'}
362360
checked={props.rowSelected}
363361
index={props.index}
364-
radioGroupId={props.radioGroupId}
362+
isSingleSelect={props.isSingleSelect}
365363
/>
366364
<ReadOnlyBodyTh
367365
actionableMode={props.actionableMode}
@@ -484,11 +482,11 @@ export const AdvancedDataTableBodyTh = props => {
484482
* @prop {boolean} checked - Set checked on the cell checkbox
485483
* @prop {boolean} hasFocus - Determines whether the cell is in user focus
486484
* @prop {boolean} isEditable - Determines whether the cell is editable
485+
* @prop {boolean} isSingleSelect - Specifies whether to use a radio button for selection or not
487486
* @prop {integer} cellTabIndex - Set tabindex on the cell
488-
* @prop {integer} inputTabIndex - Set tabindex on the checkbox
489487
* @prop {integer} index - Grid row index
488+
* @prop {integer} inputTabIndex - Set tabindex on the checkbox
490489
* @prop {string} className
491-
* @prop {string} radioGroupId - common id of radio group
492490
*/
493491
export const SelectRowTd = props => (
494492
<AdvancedDataTableTd
@@ -497,22 +495,24 @@ export const SelectRowTd = props => (
497495
isEditable={props.isEditable}
498496
tabIndex={props.cellTabIndex}
499497
>
500-
{props.radioGroupId ? (
498+
{props.isSingleSelect ? (
501499
<Radio
502500
checked={props.checked}
503501
hideLabel
504502
id={`radio-0${props.index}`}
505503
labelId={`radio-button-label-0${props.index}`}
506504
label={`Select item ${props.index}`}
507-
radioGroupId={props.radioGroupId}
505+
groupId="radio-group-header"
508506
tabIndex={props.inputTabIndex}
509507
/>
510508
) : (
511509
<Checkbox
512510
checked={props.checked}
513511
hideLabel
512+
labelId={`check-button-label-0${props.index}`}
514513
id={`checkbox-0${props.index}`}
515514
label={`Select item ${props.index}`}
515+
groupId={checkboxRadioGroupHeaderId}
516516
tabIndex={props.inputTabIndex}
517517
/>
518518
)}

ui/components/data-tables/responsive/example.jsx

Lines changed: 28 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,15 @@
44
import React from 'react';
55
import ButtonIcon from '../../button-icons/';
66
import SvgIcon from '../../../shared/svg-icon';
7+
import { Checkbox } from '../../checkbox/base/example';
78
import classNames from 'classnames';
89

910
/// ////////////////////////////////////////
1011
// Partial(s)
1112
/// ////////////////////////////////////////
1213

14+
const checkboxRadioGroupHeaderId = 'check-group-header';
15+
1316
let Table = props => (
1417
<table
1518
className={classNames('slds-table slds-table_bordered', props.className)}
@@ -18,23 +21,20 @@ let Table = props => (
1821
</table>
1922
);
2023

21-
let Checkbox = props => (
22-
<label className="slds-checkbox">
23-
<input
24-
type="checkbox"
25-
name="options"
26-
disabled={props.disabled}
27-
defaultChecked={props.checked}
28-
/>
29-
<span className="slds-checkbox_faux" />
30-
<span className="slds-assistive-text">{props.label}</span>
31-
</label>
32-
);
33-
3424
let HeadRowData = props => (
3525
<tr className="slds-text-title_caps">
3626
<th className="slds-cell-shrink" scope="col">
37-
<Checkbox label="Select All" checked={props.checked} />
27+
<span className="slds-assistive-text" id={checkboxRadioGroupHeaderId}>
28+
Choose a row to select
29+
</span>
30+
<Checkbox
31+
hideLabel
32+
labelId={'check-button-label-all'}
33+
id={'checkbox-all'}
34+
label={'Select all'}
35+
groupId={checkboxRadioGroupHeaderId}
36+
checked={props.checked}
37+
/>
3838
</th>
3939
<th scope="col">
4040
<div className="slds-truncate" title="Close Date">
@@ -127,7 +127,14 @@ let HeadRowData = props => (
127127
let RowData = props => (
128128
<tr className="slds-hint-parent">
129129
<td className="slds-cell-shrink" data-label="Select Row">
130-
<Checkbox label="Select Row" checked={props.checked} />
130+
<Checkbox
131+
hideLabel
132+
labelId={`check-button-label-0${props.index}`}
133+
id={`checkbox-0${props.index}`}
134+
label={`Select item ${props.index}`}
135+
groupId={checkboxRadioGroupHeaderId}
136+
checked={props.checked}
137+
/>
131138
</td>
132139
<th scope="row" data-label="Opportunity Name">
133140
<div className="slds-truncate" title={props.title}>
@@ -187,8 +194,8 @@ let Overflow = props => (
187194
<HeadRowData />
188195
</thead>
189196
<tbody>
190-
<RowData title="Cloudhub" />
191-
<RowData title="Cloudhub + Anypoint Connectors" />
197+
<RowData index="1" title="Cloudhub" />
198+
<RowData index="2" title="Cloudhub + Anypoint Connectors" />
192199
</tbody>
193200
</Table>
194201
</div>
@@ -200,8 +207,8 @@ let Stacked = props => (
200207
<HeadRowData />
201208
</thead>
202209
<tbody>
203-
<RowData title="Cloudhub" />
204-
<RowData title="Cloudhub + Anypoint Connectors" />
210+
<RowData index="1" title="Cloudhub" />
211+
<RowData index="2" title="Cloudhub + Anypoint Connectors" />
205212
</tbody>
206213
</Table>
207214
);
@@ -212,8 +219,8 @@ let Horizontal = props => (
212219
<HeadRowData />
213220
</thead>
214221
<tbody>
215-
<RowData title="Cloudhub" />
216-
<RowData title="Cloudhub + Anypoint Connectors" />
222+
<RowData index="1" title="Cloudhub" />
223+
<RowData index="2" title="Cloudhub + Anypoint Connectors" />
217224
</tbody>
218225
</Table>
219226
);

ui/components/radio-group/base/example.jsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,8 @@ export let Radio = props => {
4747
tabIndex={props.tabIndex}
4848
aria-describedby={props.errorId}
4949
aria-labelledby={
50-
props.labelId && props.radioGroupId
51-
? props.labelId + ' ' + props.radioGroupId
50+
props.labelId && props.groupId
51+
? props.labelId + ' ' + props.groupId
5252
: null
5353
}
5454
/>

0 commit comments

Comments
 (0)