fix(table): remove content-based column width sizing

BREAKING CHANGE: This change removes the CellMeasurer component used to size table column width to the widest piece of content in the column Before: Not passing a function to the "width" prop on a Column component would result in the column's width being as wide as the widest piece of content in that column After: Not passing a function to the "width" prop on a Column component results in that column being sized in proportion to the width of the area the table is displayed in. For example: If a table is 1000px wide and has 5 columns, the column/s without a width prop would be 200px wide (1000px/5) Closes DCOS-50093
d2iq · Mar 18, 2019 · 5bba31c · 5bba31c
1 parent 5f96cac
commit 5bba31c
Show file tree

Hide file tree

Showing 8 changed files with 518 additions and 503 deletions.
diff --git a/package-lock.json b/package-lock.json
diff --git a/packages/table/components/CheckboxTable.tsx b/packages/table/components/CheckboxTable.tsx
@@ -3,6 +3,9 @@ import { ThemeProvider } from "emotion-theming";
 import { Table, Column, Cell, HeaderCell } from "..";
 import { TableProps } from "./Table";
 import CheckboxInput from "../../checkboxInput/components/CheckboxInput";
+import { WidthArgs } from "./Column";
+import { display } from "../../shared/styles/styleUtils";
+import { spaceM } from "../../design-tokens/build/js/designTokens";
 
 export interface CheckboxTableProps extends TableProps {
   /**
@@ -48,6 +51,7 @@ class CheckboxTable extends React.PureComponent<
           data.length - Object.keys(disabledRows).length
     };
   }
+  private checkboxRef = React.createRef<HTMLDivElement>();
 
   constructor(props) {
     super(props);
@@ -59,6 +63,7 @@ class CheckboxTable extends React.PureComponent<
     this.getSelectedRows = this.getSelectedRows.bind(this);
     this.checkboxCellRenderer = this.checkboxCellRenderer.bind(this);
     this.getHeaderCheckbox = this.getHeaderCheckbox.bind(this);
+    this.getCheckboxColWidth = this.getCheckboxColWidth.bind(this);
   }
 
   render() {
@@ -74,13 +79,22 @@ class CheckboxTable extends React.PureComponent<
           <Column
             header={<HeaderCell>{this.getHeaderCheckbox()}</HeaderCell>}
             cellRenderer={this.checkboxCellRenderer}
+            width={this.getCheckboxColWidth}
           />
           {this.props.children as any}
         </Table>
       </ThemeProvider>
     );
   }
 
+  private getCheckboxColWidth(_args: WidthArgs) {
+    const checkbox = this.checkboxRef.current;
+
+    return checkbox
+      ? checkbox.getBoundingClientRect().width + parseInt(spaceM, 10)
+      : 24;
+  }
+
   private checkboxCellRenderer(rowData) {
     const {
       data,
@@ -140,20 +154,22 @@ class CheckboxTable extends React.PureComponent<
     };
 
     return (
-      <CheckboxInput
-        id="headerCheckbox"
-        inputLabel="Toggle all rows"
-        showInputLabel={false}
-        checked={
-          (Boolean(selectedLength) && selectedLength === maxSelectedLength) ||
-          this.state.headerChecked
-        }
-        indeterminate={
-          Boolean(selectedLength) && selectedLength < maxSelectedLength
-        }
-        disabled={!Boolean(maxSelectedLength)}
-        onChange={handleChange}
-      />
+      <div ref={this.checkboxRef} className={display("inline-block")}>
+        <CheckboxInput
+          id="headerCheckbox"
+          inputLabel="Toggle all rows"
+          showInputLabel={false}
+          checked={
+            (Boolean(selectedLength) && selectedLength === maxSelectedLength) ||
+            this.state.headerChecked
+          }
+          indeterminate={
+            Boolean(selectedLength) && selectedLength < maxSelectedLength
+          }
+          disabled={!Boolean(maxSelectedLength)}
+          onChange={handleChange}
+        />
+      </div>
     );
   }
 

diff --git a/packages/table/components/Column.tsx b/packages/table/components/Column.tsx
@@ -17,6 +17,8 @@ export interface ColumnProps {
   cellRenderer: (data: any, width: number) => React.ReactNode;
   /**
    * width should return a column width in pixels.
+   * If no function is provided, columns will distribute
+   * their width evenly
    */
   width?: (args: WidthArgs) => number;
   /**

diff --git a/packages/table/components/Table.tsx b/packages/table/components/Table.tsx
@@ -1,13 +1,7 @@
 import * as React from "react";
 import { cx, css } from "emotion";
 import styled from "react-emotion";
-import {
-  AutoSizer,
-  MultiGrid,
-  GridCellProps,
-  CellMeasurer,
-  CellMeasurerCache
-} from "react-virtualized";
+import { AutoSizer, MultiGrid, GridCellProps } from "react-virtualized";
 
 import {
   headerCss,
@@ -152,9 +146,7 @@ export class Table<T> extends React.PureComponent<TableProps, TableState> {
               currentIndex,
               remainingWidth
             })
-          : this.cellMeasureCache.columnWidth({ index: currentIndex }) + 1;
-        // Adding 1 pixel to the calculated columnWidth for when we use `text-overflow: ellipsis`.
-        // In rare cases, the browser will try and truncate the text too soon
+          : width / totalColumns;
 
         const clampedWidth = clampWidth(
           calculatedWidth,
@@ -173,12 +165,6 @@ export class Table<T> extends React.PureComponent<TableProps, TableState> {
     }
   );
 
-  private cellMeasureCache = new CellMeasurerCache({
-    defaultHeight: this.props.rowHeight || ROW_HEIGHT,
-    defaultWidth: 150,
-    fixedHeight: true
-  });
-
   constructor(props) {
     super(props);
 
@@ -195,16 +181,6 @@ export class Table<T> extends React.PureComponent<TableProps, TableState> {
     };
   }
 
-  public componentDidUpdate(prevProps: TableProps) {
-    if (
-      this.props.children !== prevProps.children ||
-      this.props.data !== prevProps.data
-    ) {
-      this.cellMeasureCache.clearAll();
-      this.updateGridSize();
-    }
-  }
-
   public render() {
     return (
       <AutoSizer
@@ -285,19 +261,7 @@ export class Table<T> extends React.PureComponent<TableProps, TableState> {
   }
 
   private cellRenderer(args: GridCellProps) {
-    const { columnIndex, rowIndex, key, parent } = args;
-
-    return (
-      <CellMeasurer
-        cache={this.cellMeasureCache}
-        columnIndex={columnIndex}
-        key={key}
-        parent={parent}
-        rowIndex={rowIndex}
-      >
-        {this.getCell(args)}
-      </CellMeasurer>
-    );
+    return this.getCell(args);
   }
 
   private getCell(args: GridCellProps) {

diff --git a/packages/table/stories/Table.stories.tsx b/packages/table/stories/Table.stories.tsx
@@ -92,7 +92,7 @@ storiesOf("Table", module)
       </Table>
     </div>
   ))
-  .addWithInfo("column width match content", () => (
+  .addWithInfo("default column widths", () => (
     <div
       style={{
         height: "175px",
@@ -104,28 +104,33 @@ storiesOf("Table", module)
         <Column
           header={<HeaderCell>name</HeaderCell>}
           cellRenderer={nameCellRenderer}
+          growToFill={true}
+          minWidth={100}
+          maxWidth={150}
         />
         <Column
           header={<HeaderCell>role</HeaderCell>}
           cellRenderer={roleCellRenderer}
+          growToFill={true}
         />
         <Column
           header={<HeaderCell>state</HeaderCell>}
           cellRenderer={stateCellRenderer}
+          growToFill={true}
+          minWidth={100}
+          maxWidth={150}
         />
         <Column
           header={<HeaderCell>Very Long</HeaderCell>}
           cellRenderer={veryLongRenderer}
-          maxWidth={300}
+          growToFill={true}
         />
         <Column
           header={<HeaderCell textAlign="right">zip code</HeaderCell>}
           cellRenderer={zipcodeCellRenderer}
-        />
-        <Column
-          header={<HeaderCell>city</HeaderCell>}
-          cellRenderer={cityCellRenderer}
-          maxWidth={200}
+          growToFill={true}
+          minWidth={100}
+          maxWidth={150}
         />
       </Table>
     </div>
@@ -142,6 +147,9 @@ storiesOf("Table", module)
         <Column
           header={<HeaderCell>name</HeaderCell>}
           cellRenderer={nameCellRenderer}
+          growToFill={true}
+          minWidth={100}
+          maxWidth={150}
         />
         <Column
           header={<HeaderCell>role</HeaderCell>}
@@ -151,20 +159,21 @@ storiesOf("Table", module)
         <Column
           header={<HeaderCell>state</HeaderCell>}
           cellRenderer={stateCellRenderer}
+          growToFill={true}
+          minWidth={100}
+          maxWidth={150}
         />
         <Column
           header={<HeaderCell>Very Long</HeaderCell>}
           cellRenderer={veryLongRenderer}
-          maxWidth={300}
+          growToFill={true}
         />
         <Column
           header={<HeaderCell textAlign="right">zip code</HeaderCell>}
           cellRenderer={zipcodeCellRenderer}
-        />
-        <Column
-          header={<HeaderCell>city</HeaderCell>}
-          cellRenderer={cityCellRenderer}
           growToFill={true}
+          minWidth={100}
+          maxWidth={150}
         />
       </Table>
     </div>

diff --git a/packages/table/tests/CheckboxTable.test.tsx b/packages/table/tests/CheckboxTable.test.tsx
@@ -177,7 +177,7 @@ describe("CheckboxTable", () => {
 
     expect(onChangeFn).not.toHaveBeenCalled();
     checkboxComponent
-      .find("input")
+      .find("#headerCheckbox")
       .simulate("change", { target: { checked: true } });
 
     expect(onChangeFn).toHaveBeenCalledWith(
@@ -187,7 +187,7 @@ describe("CheckboxTable", () => {
       }, {})
     );
     checkboxComponent
-      .find("input")
+      .find("#headerCheckbox")
       .simulate("change", { target: { checked: false } });
     expect(onChangeFn).toHaveBeenCalledWith({});
   });