Skip to content

Commit

Permalink
fix clicking on a custom selection column will trigger selection twice
Browse files Browse the repository at this point in the history
  • Loading branch information
@AllenFang
AllenFang committed Jan 21, 2017
1 parent 19ef7a6 commit eff49cf
Showing 1 changed file with 1 addition and 3 deletions.
4 changes: 1 addition & 3 deletions src/TableRow.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,7 @@ class TableRow extends Component {
}

rowClick = e => {
if (e.target.tagName !== 'INPUT' &&
e.target.tagName !== 'SELECT' &&
e.target.tagName !== 'TEXTAREA') {
if (e.target.tagName === 'TD') {
const rowIndex = this.props.index + 1;
const cellIndex = e.target.cellIndex;
const { selectRow, unselectableRow, isSelected, onSelectRow, onExpandRow } = this.props;
Expand Down

5 comments on commit eff49cf

@jfremy
Copy link
Contributor

@jfremy jfremy commented on eff49cf Feb 1, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fix introduce a regression on lines having "advanced" cell renderers (which do not output pure text but html). For instance I have renderers which output a <span>...</span> and the click will only be registered if I click the row outside of the <span> tag. Clicking inside the <span> will not register

@AllenFang
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jfremy, sorry that's my mistake and cause some bugs, I've fixed on my local and will release soon. thanks

@AllenFang
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jfremy, I've release v2.11.0 for fixing these bug, you can check this out and let me know if the problem still remain, thanks

@jfremy
Copy link
Contributor

@jfremy jfremy commented on eff49cf Feb 2, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing it. Sorry I did not propose a fix with the report
I'll try it right away

@jfremy
Copy link
Contributor

@jfremy jfremy commented on eff49cf Feb 2, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Just tested the failing use case and it works with 2.11.0. Thank you very much

Please sign in to comment.