feat(Table): support custom components in expandable rows #1520

atiratree · 2019-03-07T18:44:38Z

by using expandable cellFormatter or ExpandableRowContent component directly

exposed isRowExpanded function for extensions to use

#1227 is dependent on this PR.

patternfly-build · 2019-03-07T18:53:31Z

PatternFly-React preview: https://1520-pr-patternfly-react-patternfly.surge.sh

codecov-io · 2019-03-07T19:02:38Z

Codecov Report

Merging #1520 into master will increase coverage by 0.04%.
The diff coverage is 96.29%.

@@            Coverage Diff             @@
##           master    #1520      +/-   ##
==========================================
+ Coverage   83.06%   83.11%   +0.04%     
==========================================
  Files         564      566       +2     
  Lines        6101     6119      +18     
  Branches       59       59              
==========================================
+ Hits         5068     5086      +18     
  Misses       1010     1010              
  Partials       23       23

Flag	Coverage Δ
#patternfly3	`84.89% <ø> (ø)`	⬆️
#patternfly4	`79.81% <96.29%> (+0.15%)`	⬆️
#patternflymisc	`95.65% <ø> (ø)`	⬆️

Impacted Files	Coverage Δ
...t-table/src/components/Table/utils/transformers.js	`100% <ø> (ø)`	⬆️
...-4/react-table/src/components/Table/utils/utils.js	`100% <100%> (ø)`
...ernfly-4/react-table/src/components/Table/Table.js	`88.46% <100%> (+0.46%)`	⬆️
...fly-4/react-table/src/test-helpers/data-helpers.js	`100% <100%> (ø)`
...c/components/Table/utils/decorators/collapsible.js	`100% <100%> (ø)`	⬆️
...ternfly-4/react-table/src/components/Table/Body.js	`95.65% <83.33%> (-0.51%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5fffb9a...c73891c. Read the comment docs.

jschuler · 2019-03-14T17:26:43Z

packages/patternfly-4/react-table/src/components/Table/Body.js

-      ? this.parentsExpanded(rows[parentId].parent)
-      : rows[parentId].isOpen;
-  }
-
  mapCells = (row, rowKey) => {
    const { headerData } = this.props;
    let shiftKey = Boolean(headerData[0] && headerData[0].extraParams.onSelect);


It would be good to see more comments, for example it looks like the shiftKey can take values of 0 | 1 | 2 here and then I am not sure how it is used

I added the comments, refactored that part and fixed a small bug in the process

I hope it is readable now

jschuler · 2019-03-14T17:27:46Z

LGTM but this needs a look by @priley86 @karelhala

priley86 · 2019-03-14T20:39:26Z

thanks for reviewing @jschuler. I don't have any issue w/ this and it seems a nice addition, however, @karelhala is much more familiar.

atiratree · 2019-03-15T17:48:45Z

+ enhanced the TS a bit

by using expandable cellFormatter or ExpandableRowContent component - exposed isRowExpanded function for extensions to use

priley86 · 2019-03-19T14:24:46Z

@jschuler @karelhala - any additional thoughts here?

karelhala

Codewise looks good. I am just a bit concern about breaking current API, can we document that somewhere so consumers will know about that? Also it would be good if we bump the version of table package so everybody knows that this API changed.

dtaylor113 · 2019-03-20T13:54:47Z

Codewise looks good. I am just a bit concern about breaking current API, can we document that somewhere so consumers will know about that? Also it would be good if we bump the version of table package so everybody knows that this API changed.

Hi, is this a breaking change?

karelhala · 2019-03-20T14:09:33Z

Hi, is this a breaking change?

Not really if consumers are going to use the old way it's just going to look differently, but the table will still work and expandable actions will work as well.

dtaylor113 · 2019-03-20T14:52:05Z

great! thanks @karelhala

priley86

no issues on my side

atiratree · 2019-03-20T15:45:37Z

Codewise looks good. I am just a bit concern about breaking current API, can we document that somewhere so consumers will know about that? Also it would be good if we bump the version of table package so everybody knows that this API changed.

Hi, is this a breaking change?

Not really if consumers are going to use the old way it's just going to look differently, but the table will still work and expandable actions will work as well.

Should I update the version? and write something to the CHANGELOG ?

atiratree · 2019-03-20T15:47:55Z

I mean, should the version be 1.3.0 ?

jschuler · 2019-03-20T15:49:45Z

No need to manually increment the version or update the changelog, this should be handled correctly by the build

atiratree mentioned this pull request Mar 7, 2019

feat(Table): support inline editing in PF4 #1227

Merged

6 tasks

atiratree force-pushed the pf4-table-expandable-row branch from c783607 to 592a1cf Compare March 7, 2019 19:43

priley86 mentioned this pull request Mar 7, 2019

Table - selection callback requires rowProps #1472

Closed

tlabaj added PF4 labels Mar 8, 2019

tlabaj requested review from jschuler and dlabaj March 8, 2019 14:05

tlabaj assigned jschuler Mar 8, 2019

jschuler reviewed Mar 14, 2019

View reviewed changes

atiratree force-pushed the pf4-table-expandable-row branch 2 times, most recently from 19b1924 to 1b9158a Compare March 15, 2019 17:45

atiratree force-pushed the pf4-table-expandable-row branch 3 times, most recently from 0c2c957 to e315386 Compare March 18, 2019 09:37

feat(Table): support custom components in expandable rows

c73891c

by using expandable cellFormatter or ExpandableRowContent component - exposed isRowExpanded function for extensions to use

atiratree force-pushed the pf4-table-expandable-row branch from e315386 to c73891c Compare March 18, 2019 09:45

karelhala approved these changes Mar 20, 2019

View reviewed changes

priley86 approved these changes Mar 20, 2019

View reviewed changes

jschuler merged commit a0e61ab into patternfly:master Mar 20, 2019

atiratree mentioned this pull request Mar 21, 2019

fix(Table): reverts unnecessary fix #1623

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(Table): support custom components in expandable rows #1520

feat(Table): support custom components in expandable rows #1520

atiratree commented Mar 7, 2019

patternfly-build commented Mar 7, 2019

codecov-io commented Mar 7, 2019 •

edited

Loading

jschuler Mar 14, 2019

atiratree Mar 15, 2019

atiratree Mar 15, 2019

jschuler commented Mar 14, 2019

priley86 commented Mar 14, 2019

atiratree commented Mar 15, 2019

priley86 commented Mar 19, 2019

karelhala left a comment

dtaylor113 commented Mar 20, 2019

karelhala commented Mar 20, 2019

dtaylor113 commented Mar 20, 2019

priley86 left a comment

atiratree commented Mar 20, 2019

atiratree commented Mar 20, 2019

jschuler commented Mar 20, 2019

feat(Table): support custom components in expandable rows #1520

feat(Table): support custom components in expandable rows #1520

Conversation

atiratree commented Mar 7, 2019

patternfly-build commented Mar 7, 2019

codecov-io commented Mar 7, 2019 • edited Loading

Codecov Report

jschuler Mar 14, 2019

Choose a reason for hiding this comment

atiratree Mar 15, 2019

Choose a reason for hiding this comment

atiratree Mar 15, 2019

Choose a reason for hiding this comment

jschuler commented Mar 14, 2019

priley86 commented Mar 14, 2019

atiratree commented Mar 15, 2019

priley86 commented Mar 19, 2019

karelhala left a comment

Choose a reason for hiding this comment

dtaylor113 commented Mar 20, 2019

karelhala commented Mar 20, 2019

dtaylor113 commented Mar 20, 2019

priley86 left a comment

Choose a reason for hiding this comment

atiratree commented Mar 20, 2019

atiratree commented Mar 20, 2019

jschuler commented Mar 20, 2019

codecov-io commented Mar 7, 2019 •

edited

Loading