[WNMGDS-650] Table followup (#853)

* Add css grid prefix for IE11 * Enabale grid autoplacement for ie11 * Remove unnecessary lines * Set control comment on css file autoprefixer grid: on * Fix unstyled anonymous element on IE11 Grid * Update formating * Fix ie11 superimposing elements on a grid cell * Update comments * Update snapshots * Update snapshots * Update comment and header scope * Update test for scope property
CMSgov · Oct 28, 2020 · 2e331c2 · 2e331c2
1 parent ededa2b
commit 2e331c2
Show file tree

Hide file tree

Showing 11 changed files with 70 additions and 64 deletions.
diff --git 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
@@ -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 <caption> 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 <caption> 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
@@ -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
@@ -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
@@ -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 (
     <Component
       className={classes}
@@ -71,7 +72,8 @@ export const TableCell = ({
       data-title={stackedTitle}
       {...tableCellProps}
     >
-      {children}
+      {/* Fix unstyled anonymous element on IE11 Grid https://www.w3.org/TR/css-grid-1/#grid-items */}
+      <span>{children}</span>
     </Component>
   );
 };
@@ -93,38 +95,38 @@ 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 `<th>` element if the parent component is `TableHead`,
+   * otherwise, it renders a `<td>` element.
+   */
+  component: PropTypes.oneOf(['td', 'th']),
   /**
    * `TableCell` must define a `headers` prop for stackable tables with a `<td>` 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 `<td>` 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 `<th>` 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 `<th>` 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']),
   /**
    * Additional classes to be added to the stacked Title element.
    */
   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 `<th>` element
-   * when the parent component is `TableHead` or otherwise a `<td>` 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
@@ -95,13 +95,6 @@ describe('TableCell', function () {
       expect(table.prop('role')).toBe('rowheader');
     });
 
-    it('sets a table <th> scope="row"', () => {
-      const { wrapper } = renderBody();
-      const table = wrapper.find('th');
-
-      expect(table.prop('scope')).toBe('row');
-    });
-
     it('renders a table <td> 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 <td> 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
@@ -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
@@ -15,9 +15,10 @@ exports[`TableCell TableBody wrap: <td> data cell - default props renders TableC
             <th
               className="ds-u-text-align--left"
               role="rowheader"
-              scope="row"
             >
-              Cell a
+              <span>
+                Cell a
+              </span>
             </th>
           </TableCell>
           <TableCell
@@ -26,9 +27,10 @@ exports[`TableCell TableBody wrap: <td> data cell - default props renders TableC
             <td
               className="ds-u-text-align--left"
               role="cell"
-              scope="row"
             >
-              Cell b
+              <span>
+                Cell b
+              </span>
             </td>
           </TableCell>
         </tr>
@@ -59,7 +61,9 @@ exports[`TableCell TableHead wrap: <th> header cell - default props renders a ta
               role="columnheader"
               scope="col"
             >
-              Column a
+              <span>
+                Column a
+              </span>
             </th>
           </TableCell>
           <TableCell
@@ -72,7 +76,9 @@ exports[`TableCell TableHead wrap: <th> header cell - default props renders a ta
               role="columnheader"
               scope="col"
             >
-              Column b
+              <span>
+                Column b
+              </span>
             </th>
           </TableCell>
         </tr>

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
@@ -25,7 +25,9 @@ exports[`TableHead renders additional attributes 1`] = `
               role="columnheader"
               scope="col"
             >
-              Column a
+              <span>
+                Column a
+              </span>
             </th>
           </TableCell>
           <TableCell
@@ -38,7 +40,9 @@ exports[`TableHead renders additional attributes 1`] = `
               role="columnheader"
               scope="col"
             >
-              Column b
+              <span>
+                Column b
+              </span>
             </th>
           </TableCell>
         </tr>

diff --git 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
@@ -14,8 +14,9 @@ export interface TableCellProps {
    */
   children?: React.ReactNode;
   /**
-   * If this prop is not defined, the component renders a `<th>` element
-   * when the parent component is `TableHead` or otherwise a `<td>` element.
+   * When provided, this will render the passed in component as the HTML element.
+   * If this prop is undefined, it renders a `<th>` element if the parent component is `TableHead`,
+   * otherwise, it renders a `<td>` element.
    */
   component?: TableCellComponent;
   /**
@@ -24,29 +25,28 @@ export interface TableCellProps {
   className?: string;
   /**
    * `TableCell` must define a `headers` prop for stackable tables with a `<td>` 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 `<td>` 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 `<th>` 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 `<th>` 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;
   /**
    * Additional classes to be added to the stacked Title element.
    */
   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;
   /**