Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add onColumnClick to Table #1207

Merged
merged 2 commits into from
Oct 3, 2018

Conversation

grahamlyus
Copy link
Contributor

This change adds onColumnClick to Table, works the same way as onHeaderClick.

I was attaching onClick in my Column cellRenderer, but it doesn't work when the cell is empty. Attaching to the div in _createColumn works regardless.

@CaffeinatedFunctionality

I definitely could use this. There are some columns I need the ability to click and others I need to disable as they contain buttons in them

@@ -447,6 +454,10 @@ export default class Table extends React.PureComponent {
rowIndex,
});

const onClick = event => {
onColumnClick && onColumnClick({columnData, dataKey, event});
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think its necessary to pass in the event. We should just invoke the callback with columnData and datakey

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, I am not using event in my handlers, but I added it to make it the same callback parameters as onHeaderClick

@wuweiweiwu
Copy link
Contributor

Can you also add unit tests? To make sure that column clicks invoke the callback with the right parameters?

@grahamlyus
Copy link
Contributor Author

Tests are in source/Table/Table.jest.js, also based on the onHeaderClick tests

@wuweiweiwu
Copy link
Contributor

@grahamlyus my b mustve miss the tests last night.

@wuweiweiwu wuweiweiwu merged commit f79c47e into bvaughn:master Oct 3, 2018
@wuweiweiwu
Copy link
Contributor

Thanks for contributing!

errendir added a commit to IdeaFlowCo/react-virtualized that referenced this pull request Oct 24, 2018
* bvaughn/master: (54 commits)
  Update version and changelog for 9.21.0 release (bvaughn#1252)
  chore: update lockfile
  Update ci badge (bvaughn#1227)
  Allow users to override default table row styles (bvaughn#1175)
  Add onColumnClick to Table (bvaughn#1207)
  remove unused variable in Masonry.example.js (bvaughn#1218)
  Fix Table aria attributes (bvaughn#1208)
  Fix typo in CellMeasurer.DynamicHeightTableColumn.example.js (bvaughn#1190)
  Update usingAutoSizer.md (bvaughn#1186)
  Add an extra check for an e.target.className.indexOf function (bvaughn#1210)
  Fix broken Slack badge image (bvaughn#1205)
  docs(CellMeasurer): fix `import` statement (bvaughn#1187)
  Added new friend (bvaughn#1197)
  Fix createMultiSort bug (bvaughn#1051)
  adding new usecase example and fix some typos (bvaughn#1168)
  Updating version to 9.20.1
  Update changelog for the 9.20.1 release (bvaughn#1167)
  Prevent early debounceScrollEndedCallback when there is a slow render (bvaughn#1141)
  removing sideEffects (bvaughn#1163)
  fix for bvaughn#998 with test cases (bvaughn#1154)
  ...
@eubrunomiguel
Copy link

columnData seems to always be undefined

@christianbalderrama
Copy link

Wanted to follow up as well, columnData is undefined.

@wuweiweiwu
Copy link
Contributor

@christianbalderrama @eubrunomiguel can you open an issue? Thanks!

@B3Kay
Copy link

B3Kay commented Apr 16, 2019

columnData is undefined...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants