Table - selection callback requires rowProps

[priley86]

Some consumers will need the rowProps passed into the table's rows prop passed back in the onSelect callback. This would be immediately useful if you need to know a system id or some other object identifier passed in for the row selected (rather than trying to convert simple rowIndex). Just opening this to target this issue specifically.

  function selectClick(event) {
    const selected = rowIndex === undefined ? event.target.checked : rowData && !rowData.selected;
    onSelect && onSelect(event, selected, rowId); //rowData missing
  }

Details:
#1250 (review)

[rachael-phillips]

@priley86 do you mind telling me more about priorities around this issue? Is this blocking you currently?

[priley86]

hey @rachael-phillips - I have started a P.O.C. to showcase table/table selections (but still a W.I.P.). I think this issue is being resolved alongside #1441 currently (that PR seems to resolve it now). I can definitely make use of the table actions there too once it is merged. The minor bugfix in #1503 is also relevant.

@suomiy is also prototyping inline edit extension table for CNV. #1520, #1521 and #1227 would all support this. #1520 and #1521 should come first though. We could definitely use help reviewing where possible.

thanks for your help!

[priley86]

some further notes on this...after review, #1441 does not seem to handle this and only targets the ActionsColumn onSelect. I will need something similar for the Table's onSelect and onSort callbacks though:

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

Line 24 in 0b637fe

onSelect(event, isSelected, rowId) {

onSelect should also return rowData and extraParams for the row selected:

onSelect(event, isSelected, rowId, rowData, extraParams) {

On a similar note, I am also seeing that onSort does currently not return the column passed in:

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

Line 21 in 0b637fe

onSort(_event, index, direction) {

could be:

onSort(_event, index, direction, column) {

This may be easier to digest if the consumers are not storing column rows/props in state and instead have them calculated by each view. It is just an optional return that is a nice-to-have.

For the full context of this, you can see my POC started here:
https://github.com/priley86/web-ui/commits/react-tables-latest

I am fine w/ leaving this as one issue and tackling both here. #1441 is still quite useful for my Kebab actions though! Hope this further explains this issue though.

cc: @suomiy

[rachael-phillips]

Thanks for the update @priley86 !

[tlabaj]

closed by PR #1542