diff --git a/lerna.json b/lerna.json index 266e598404..9e2d676126 100644 --- a/lerna.json +++ b/lerna.json @@ -5,9 +5,7 @@ } }, "npmClient": "yarn", - "packages": [ - "packages/*" - ], + "packages": ["packages/*"], "ignoreChanges": [ "packages/stylelint-config-design-system/**", "packages/eslint-config-design-system/**" diff --git a/packages/design-system/src/components/Table/Table.jsx b/packages/design-system/src/components/Table/Table.jsx index 62c524a606..f6f745080e 100644 --- a/packages/design-system/src/components/Table/Table.jsx +++ b/packages/design-system/src/components/Table/Table.jsx @@ -75,6 +75,7 @@ export class Table extends React.PureComponent { return React.Children.map(this.props.children, (child) => { if (isTableCaption(child)) { // Extend props on TableCaption before rendering. + // TODO: Use React Context when all products are on React v16.8 or higher if (this.props.scrollable) { return React.cloneElement(child, { _id: this.captionID, @@ -113,10 +114,9 @@ export class Table extends React.PureComponent { className ); - // Make table container focusable and display scroll notice when table width exceeds viewport. - // Set attribute `tabIndex = 0` to make table container focusable and enable keyboard support of using the arrow keys. - // Also, it provides context for screen reader users as they are able to focus on the region. - // Do this by using table's to label the scrollable region using aria-labelleby + // Makes table container focusable and displays the scrollable notice when table width exceeds viewport + // by setting `tabIndex = 0` attribute. + // This provides context for screen readers to the table's via aria-labelleby const attributeScrollable = scrollable && { className: 'ds-c-table__wrapper', role: 'region', diff --git a/packages/design-system/src/components/Table/TableBody.jsx b/packages/design-system/src/components/Table/TableBody.jsx index 0ccc66a5dc..069b852d7d 100644 --- a/packages/design-system/src/components/Table/TableBody.jsx +++ b/packages/design-system/src/components/Table/TableBody.jsx @@ -5,6 +5,7 @@ export const TableBody = ({ children, _stackable, ...tableBodyProps }) => { const renderChildren = () => { return React.Children.map(children, (child) => { // Extend props before rendering. + // TODO: Use React Context when all products are on React v16.8 or higher if (child) { return React.cloneElement(child, { _stackable: _stackable, diff --git a/packages/design-system/src/components/Table/TableCaption.jsx b/packages/design-system/src/components/Table/TableCaption.jsx index f2084be4b0..d018ebbf3e 100644 --- a/packages/design-system/src/components/Table/TableCaption.jsx +++ b/packages/design-system/src/components/Table/TableCaption.jsx @@ -25,7 +25,7 @@ TableCaption.propTypes = { */ children: PropTypes.node, /** - * @hide-prop Additional classes to be added to the caption element. + * Additional classes to be added to the caption element. */ className: PropTypes.string, /** diff --git a/packages/design-system/src/components/Table/TableCell.jsx b/packages/design-system/src/components/Table/TableCell.jsx index 37a482ee75..4e54742ab2 100644 --- a/packages/design-system/src/components/Table/TableCell.jsx +++ b/packages/design-system/src/components/Table/TableCell.jsx @@ -54,13 +54,14 @@ export const TableCell = ({ } let defaultScope = scope; - if (!defaultScope) { - defaultScope = _isTableHeadChild ? 'col' : 'row'; + if (!defaultScope && _isTableHeadChild) { + defaultScope = 'col'; } const alignClassName = align ? `ds-u-text-align--${align}` : null; const classes = classNames(alignClassName, className); + // The data attributes `data-title` is access by CSS to generates row header content for stacked table return ( - {children} + {/* Fix unstyled anonymous element on IE11 Grid https://www.w3.org/TR/css-grid-1/#grid-items */} + {children} ); }; @@ -93,22 +95,27 @@ TableCell.propTypes = { * Additional classes to be added to the row element. */ className: PropTypes.string, + /** + * When provided, this will render the passed in component as the HTML element. + * If this prop is undefined, it renders a `` element if the parent component is `TableHead`, + * otherwise, it renders a `` element. + */ + component: PropTypes.oneOf(['td', 'th']), /** * `TableCell` must define a `headers` prop for stackable tables with a `` element. - * The `headers` prop is needed to associate header and data cells for screen readers. + * The `headers` prop associates header and data cells for screen readers. * `headers` consist of a list of space-separated ids that each correspond to a `` element. - * [Read more on the headers attribute](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/td#Attributes). + * [Read more about the headers attribute](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/td#Attributes). */ headers: PropTypes.string, /** - * `TableCell` must define an `id` prop for stackable tables with a `` element . - * [Read more on the headers attribute](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/td#Attributes). + * `TableCell` must define an `id` prop for stackable tables with a `` element. * The `id` prop associates header and data cells for screen readers. */ id: PropTypes.string, /** - * If this prop is not defined, the component sets a scope attribute of `col` when the parent - * component is `TableHead` or otherwise a scope attribute of `row`. + * If this prop is undefined, the component sets a scope attribute of `col` when the parent + * component is `TableHead` to identify the header cell is a header for a column. */ scope: PropTypes.oneOf(['row', 'col', 'rowgroup', 'colgroup']), /** @@ -116,15 +123,10 @@ TableCell.propTypes = { */ stackedClassName: PropTypes.string, /** - * Table data cell's corresponding header title, this stacked title is displayed when a responsive table - * is vertically stacked. + * Table data cell's corresponding header title, this stacked title is displayed as the row header + * when a responsive table is vertically stacked. */ stackedTitle: PropTypes.string, - /** - * If this prop is not defined, the component renders a `` element - * when the parent component is `TableHead` or otherwise a `` element. - */ - component: PropTypes.oneOf(['td', 'th']), /** * @hide-prop This gets set from the parent `TableHead` component */ diff --git a/packages/design-system/src/components/Table/TableCell.test.jsx b/packages/design-system/src/components/Table/TableCell.test.jsx index 2d441c648d..c877500dbd 100644 --- a/packages/design-system/src/components/Table/TableCell.test.jsx +++ b/packages/design-system/src/components/Table/TableCell.test.jsx @@ -95,13 +95,6 @@ describe('TableCell', function () { expect(table.prop('role')).toBe('rowheader'); }); - it('sets a table scope="row"', () => { - const { wrapper } = renderBody(); - const table = wrapper.find('th'); - - expect(table.prop('scope')).toBe('row'); - }); - it('renders a table row data element', () => { const { wrapper } = renderBody(); const table = wrapper.find('td'); @@ -115,12 +108,5 @@ describe('TableCell', function () { expect(table.prop('role')).toBe('cell'); }); - - it('sets a table scope="row"', () => { - const { wrapper } = renderBody(); - const table = wrapper.find('td'); - - expect(table.prop('scope')).toBe('row'); - }); }); }); diff --git a/packages/design-system/src/components/Table/TableHead.jsx b/packages/design-system/src/components/Table/TableHead.jsx index fc45f3e233..64daa65cc5 100644 --- a/packages/design-system/src/components/Table/TableHead.jsx +++ b/packages/design-system/src/components/Table/TableHead.jsx @@ -5,6 +5,7 @@ export const TableHead = ({ children, _stackable, ...tableHeadProps }) => { const renderChildren = () => { return React.Children.map(children, (child) => { // Extend props before rendering. + // TODO: Use React Context when all products are on React v16.8 or higher if (child) { return React.cloneElement(child, { _isTableHeadChild: true, diff --git a/packages/design-system/src/components/Table/__snapshots__/TableCell.test.jsx.snap b/packages/design-system/src/components/Table/__snapshots__/TableCell.test.jsx.snap index 7fd0e89b55..993c026793 100644 --- a/packages/design-system/src/components/Table/__snapshots__/TableCell.test.jsx.snap +++ b/packages/design-system/src/components/Table/__snapshots__/TableCell.test.jsx.snap @@ -15,9 +15,10 @@ exports[`TableCell TableBody wrap: data cell - default props renders TableC - Cell a + + Cell a + data cell - default props renders TableC - Cell b + + Cell b + @@ -59,7 +61,9 @@ exports[`TableCell TableHead wrap: header cell - default props renders a ta role="columnheader" scope="col" > - Column a + + Column a + header cell - default props renders a ta role="columnheader" scope="col" > - Column b + + Column b + diff --git a/packages/design-system/src/components/Table/__snapshots__/TableHead.test.jsx.snap b/packages/design-system/src/components/Table/__snapshots__/TableHead.test.jsx.snap index 1edf4edfbd..f1156e4eca 100644 --- a/packages/design-system/src/components/Table/__snapshots__/TableHead.test.jsx.snap +++ b/packages/design-system/src/components/Table/__snapshots__/TableHead.test.jsx.snap @@ -25,7 +25,9 @@ exports[`TableHead renders additional attributes 1`] = ` role="columnheader" scope="col" > - Column a + + Column a + - Column b + + Column b + diff --git a/packages/design-system/src/styles/components/_Table.scss b/packages/design-system/src/styles/components/_Table.scss index b05f21496d..37b9356c5a 100644 --- a/packages/design-system/src/styles/components/_Table.scss +++ b/packages/design-system/src/styles/components/_Table.scss @@ -8,7 +8,7 @@ $table-stacked-header-max-width: 160px !default; @mixin table-stacked { thead tr { - /* Hide column headings when responsive table stacked vertically on smaller viewports */ + // Hide column headings when responsive table stacked vertically on narrow viewports // ds-u-visibility--screen-reader border: 0; clip: rect(0, 0, 0, 0); @@ -21,7 +21,7 @@ $table-stacked-header-max-width: 160px !default; word-wrap: normal; } - tr, + // Set to display: block for narrow viewports caption { display: block; } @@ -30,26 +30,34 @@ $table-stacked-header-max-width: 160px !default; th { border: 0; border-top: 1px solid; + + /* autoprefixer grid: on */ display: grid; // CSS grid doesn't support word-wrap. Use minmax as a workaround // https://css-tricks.com/css-grid-in-ie-debunking-common-ie-grid-misconceptions/#fit-content-is-not-ie-friendly-but grid-template-columns: minmax(0, $table-stacked-header-max-width) 1fr; position: relative; - // stylelint-disable selector-no-qualifying-type + // The psuedo element content is generated to display the row header `data-title` for the Tablecell content + /* stylelint-disable-next-line selector-no-qualifying-type */ &[data-title]::before { content: attr(data-title); - // stylelint-enable selector-no-qualifying-type - flex: none; font-weight: bold; + + /* autoprefixer grid: on */ + grid-column: 1; max-width: $table-stacked-header-max-width; - // allow for spacing on right side of data-title column width: calc(100% - #{$spacer-2}); - word-wrap: break-word; } - > * { + // TableCell content is wrapped in a span for IE11 grid support + > span { + // Fixes IE11 display bug https://css-tricks.com/css-grid-in-ie-debunking-common-ie-grid-misconceptions/ + display: block; + + /* autoprefixer grid: on */ grid-column: 2; + width: fit-content; } } diff --git a/packages/design-system/src/types/Table/TableCell.d.ts b/packages/design-system/src/types/Table/TableCell.d.ts index b9c6fa4821..7787e223ef 100644 --- a/packages/design-system/src/types/Table/TableCell.d.ts +++ b/packages/design-system/src/types/Table/TableCell.d.ts @@ -14,8 +14,9 @@ export interface TableCellProps { */ children?: React.ReactNode; /** - * If this prop is not defined, the component renders a `` element - * when the parent component is `TableHead` or otherwise a `` element. + * When provided, this will render the passed in component as the HTML element. + * If this prop is undefined, it renders a `` element if the parent component is `TableHead`, + * otherwise, it renders a `` element. */ component?: TableCellComponent; /** @@ -24,20 +25,19 @@ export interface TableCellProps { className?: string; /** * `TableCell` must define a `headers` prop for stackable tables with a `` element. - * The `headers` prop is needed to associate header and data cells for screen readers. + * The `headers` prop associates header and data cells for screen readers. * `headers` consist of a list of space-separated ids that each correspond to a `` element. - * [Read more on the headers attribute](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/td#Attributes). + * [Read more about the headers attribute](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/td#Attributes). */ headers?: string; /** - * `TableCell` must define an `id` prop for stackable tables with a `` element . - * [Read more on the headers attribute](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/td#Attributes). + * `TableCell` must define an `id` prop for stackable tables with a `` element. * The `id` prop associates header and data cells for screen readers. */ id?: string; /** - * If this prop is not defined, the component sets a scope attribute of `col` when the parent - * component is `TableHead` or otherwise a scope attribute of `row`. + * If this prop is undefined, the component sets a scope attribute of `col` when the parent + * component is `TableHead` to identify the header cell is a header for a column. */ scope?: TableCellScope; /** @@ -45,8 +45,8 @@ export interface TableCellProps { */ stackedClassName?: string; /** - * Table data cell's corresponding header title, this stacked title is displayed when a responsive table - * is vertically stacked. + * Table data cell's corresponding header title, this stacked title is displayed as the row header + * when a responsive table is vertically stacked. */ stackedTitle?: string; /**