React-table: Creating onClick event in getTdProps or getTrProps overrides Expander functionality

[colwynmyself]

Problem Description

If I create an Expander column for my table and then add getTdProps: { onClick: (some function) } to the table, the expander is never opened. I imagine this is because the event isn't being passed along to the expander after the onClick event fires. I've also noticed this issue with getTrProps, but only when I call setState inside the onClick function.

Code Snippet(s) or Codepen (https://codepen.io/colwynmyself/pen/gxaqRW?editors=0010)

Steps to Reproduce

1. Comment out the onClick handler in the Codepen and the expander will work. If you leave it in, the expander doesn't work.

System Information

• Chrome 64-bit (59.0.3071.115)
• Node 6.11.1
• Ubuntu 16.04
• NPM 3.10.10
• Yarn 0.27.5

[colwynmyself]

Btw I tracked it down to these lines: https://github.com/tannerlinsley/react-table/blob/master/src/index.js#L712

                  {...interactionProps}
                  {...tdProps.rest}

interactionProps and tdProps.rest both have onClick handlers, and the tdProps.rest one takes priority since it comes second. I'm working on a fix for this right now, but I'd love some input from you on how you would like it solved.

Right now I'm just doing a dumb check if both objects exist and have onClick handlers, call them both. But it could be improved, or maybe interactionProps.onClick can be an extra property provided to getTdProps that can be called. So the signature looks more like:

                    getTdProps={(state, row, col, instance) => ({
                        onClick: (event, cb) => {
                            // do some stuff with the event
                            cb()
                        }
                    })}

[colwynmyself]

I have a fork here: https://github.com/colwynmyself/react-table/tree/onclick-handling

There aren't any crazy changes, though I'm not totally familiar with your code so while I believe I left the interface the same (except adding a cb that is passed back with getTdProps.onClick when both onClick handlers exist) it's definitely worth double checking. I'm using this fork on my project and it's working well.

[tannerlinsley]

There would be a use case for both sides of this coin. Sometimes you may want to override the original onClick functionality, other times you may want to add to it. I think we need to figure out a way to balance the two options.

[colwynmyself]

Tthat was sort of the idea of passing the callback into the getTdProps.onClick function that if called would trigger the expandable event. I do see your point that that doesn't allow for overriding the getTdProps.onClick function with expandable.

Basically with this change right now, when you have a getTdProps.onClick function and expandable enabled you can:

• [x] Trigger just getTdProps.onClick
• [x] Trigger getTdProps.onClick and expandable, in any order
• [ ] Trigger just expandable

And it would be ideal to be able to override the getTdProps.onClick function with the expandable function. Maybe we could add an option like prioritizeExpand that will override getTdProps.onClick with expandable if they both exist?

[tannerlinsley]

Here's a snippet from the new docs:

<ReactTable
  getTdProps={(state, rowInfo, column, instance) => {
    return {
      onClick: (e, handleOriginal) => {
        // IMPORTANT! React-Table uses onClick internally to trigger
        // events like expanding SubComponents and pivots.
        // By default a custom 'onClick' handler will override this functionality.
        // If you want to fire the original onClick handler, call the
        // 'handleOriginal' function.
        if (handleOriginal) {
          handleOriginal()
        }
      }
    }
  }}
/>

[achowdhury7]

Hi
Can anyone verify whether this is working?
Here is a snippet of my code:

const renderSubComponent = (row) =>
      (
        <Accordion>
          {renderExpandedViewPanels()}
        </Accordion>
      )

  if (props.subComponents) {
    subComponents.SubComponent = renderSubComponent
    subComponents.getTdProps = (state, rowInfo, column, instance) => ({
      onClick: (e) => {
        if (!expandedView.data.hasOwnProperty(rowInfo.original['OIT-SessionID'])) {
          fetchSessionView(rowInfo)
        }
      }
    })
  }

  return (
    <ReactTable
      data={data}
      columns={columns}
      showPageSizeOptions={showPageSizeOptions}
      minRows={pageSize}
      pageSize={pageSize}
      loading={loading}
      className={className}
      showPagination={false}
      {...subComponents}
    />
  )

The onClick handler in getTdProps() overrides the expander. When I try to use getTrProps() instead, the onClick handler fires and on subsequent clicks, the expander works.
Is there something I'm doing wrong? Thanks in advance!

EDIT: I changed the onClick handler code to this :

       onClick: (e, cb) => {
           if (!expandedView.data.hasOwnProperty(rowInfo.original['OIT-SessionID'])) {
             fetchSessionView(rowInfo)
           }

           if (cb) cb()
        }

But the result is the same.

[ernani]

I am always getting callback is not a function here, my release is "react-table": "^6.5.3"

[ernani]

Got it, it seems that the original function aka callback is only present in getTdProps, in getTrProps it is just an event!

[ernani]

I've added this workaround here:

        // check if we have an original function for clicking
        if (originalFunctionOrEvent && typeof originalFunctionOrEvent === 'function') {
          // call it
          originalFunctionOrEvent()
        } else {
          // log
          console.log('originalFunctionOrEvent is an event: ', originalFunctionOrEvent)
        }

[crumblix]

I encountered this problem as well. It still seems to be an issue when (as OP states) you attempt to setState inside getTdProps. You can either handleOriginal or setState but not both.

I'm attempting to select the entire row onClick rather than having to use the checkBoxes. Clicking on a row for my implementation means clearing any other rows that might be selected and just selecting this current row. To select multiple you would need to use the checkboxes.

My workaround is this. I leave getTdProps to to the handleOriginal and I moved the setState to a cell rendering routine i.e.

...
{
    Header: "Name",
    accessor: "name",
    Cell: (row) => this.renderCell(row)
}
...

And then in renderCell ...

  renderCell(row) {
    if (this.state.selection.indexOf(row.original.id) >= 0) {
      return (<div onClick={this.onTextClick.bind(this, row)} style={{ cursor: "default", fontWeight: "bold" }}>{this.renderValue(row.column.id, row.value)}&nbsp;</div>);
    } else {
      return (<div onClick={this.onTextClick.bind(this, row)} style={{ cursor: "default", fontWeight: "normal" }}>{this.renderValue(row.column.id, row.value)}&nbsp;</div>);
    }
  }

Finally onTextClick ...

  onTextClick(row) {
    let id = row && row.original && row.original.id ? row.original.id : null;
    if (id) {
      let data= this.props.data;
      let selectedItem = null;
      if (data) {
        for (let i = 0; i < data.length; i++) {
          if (data[i].id === id) {
            selectedItem = data[i];
            break;
          }
        }
      }
      if (selectedItem) {
        this.setState({ selection: [selectedItem.id,] });
      }
    }
  }

(Assuming the data for your table is passed in via this.props.data.)

The renderValue routine just converts the value to text ... but it's just a slightly improved toString function really.

Note that there is a little hack ... I have to add a   in case renderValue returns an empty string otherwise the onClick method won't fire.

Also note that the row won't be selected if you click the expander (I'm using the custom expander version), but I can live with that.

That's the best I've been able to come up with, hope it helps someone and if someone has a better solution I'd love to hear it! :)