Table rowgroups #393

fczuardi · 2018-06-13T14:12:40Z

closes Make table row expandable/collapsible #391

fczuardi · 2018-06-15T05:15:39Z

still missing from the patch is the small blue "L" icon on the rows that are grouped

alcferreira · 2018-06-15T17:41:08Z

packages/table/src/Table.js

+    // iterate over all rows and sub-rows to create
+    // a hash table of rows indexed by keys in the
+    // format "<index>,<subIndex>"
+    const rowsMap = this.props.data.reduce((entries, row, index) => {


What happens when rows are added to props.data after the component is mounted?

would break selection checkboxes :P

actually is the selectionData passed to selectionHeader that breaks, it wont pass a selected row if the data about that row was added afterwards, I will update the index on component update to fix this (and write tests)

fixed with 77910db

vNakamura · 2018-06-15T14:48:39Z

packages/theme/src/colors.js

  RED_ORANGE: "#ff4411",
  INDIGO_MILK_CAP: "#3498db",
+  INDIGO_MILK_CAP_06: "rgba(52,152,219,0.06)",


How about we use a tool like: https://polished.js.org/docs/#transparentize to generate this color?
If some day we change the value for INDIGO_MILK_CAP, the transparent one will be updated along with it.

fixed on d968410

vNakamura · 2018-06-15T17:16:08Z

packages/table/README.md

@@ -58,6 +58,8 @@ core of the whole thing, it should be an array of objects, each item representin
 | selectionHeader | function ``(selectedRows, clearFunction) => React.node`` | a function that receives an array of selected rows data plus a function to clear selection; and should return a react node to be rendered as an action bar on top of the table, there is a helper component SelectionBar that can be used as the return of this function, or you can create your own... see the stories files for usage examples |
 | width | string | use this to manually change the width of the table |
 | rowHeight | string | use this to manually change the height of the body rows of the table, the package exports a set of named values as ``rowHeights``, you can import them and use ``rowHeights.SMALL`` to have a more compact table |
+| rowGroupKey | string | if you have rows that contains sub-rows as a list under a key, you can pass this property with the name of the key, to have a table with row groups generated |
+| collapsed | boolean | if row groups are used, this flag will add a button column with buttons that works as expand/collapse toggle on the start of row groups. The groups will start collapsed. |


This could be split into 2 props: collapsible and startCollapsed. What do you think?

how will this be used? will we have tables that starts expanded? if so, maybe we can have an expanded for that case

Yeah, we don't need to worry about it now

vNakamura · 2018-06-15T17:43:29Z

packages/table/src/Table.js

+  _renderRow = (row, index, subIndex = "", group = false) => {
+    const { selectableRows, collapsed, children } = this.props;
+    const rowKey = `${index},${subIndex}`;
+    const selected = this.state.selectedRows.indexOf(rowKey) !== -1;


I preferincludes over indexOf because it's declarative.

A downside is that it's not supported by IE.
We could add an indication of this restriction in the readme, so the use of a polyfill would be decided at the application using this component.

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/includes

Anyway, this is minor and I'm OK if we decide to keep indexOf.

fixed on b60cce8

alcferreira · 2018-06-15T18:38:22Z

packages/table/src/Table.js

+        ? [
+            ...oldExpandedRows.slice(0, expandedIndexOf),
+            ...oldExpandedRows.slice(expandedIndexOf + 1)
+          ]


We could use filter here...

const expandedRows = expandedIndexOf !== -1 ? oldExpandedRows.filter((element, index) => index !== expandedIndexOf) : [...oldExpandedRows, index]

fixed on 01af33d