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

Fix for 868 (selectableRowsHeader option) #882

Merged
merged 3 commits into from
Sep 12, 2019
Merged

Fix for 868 (selectableRowsHeader option) #882

merged 3 commits into from
Sep 12, 2019

Conversation

patorjk
Copy link
Collaborator

@patorjk patorjk commented Aug 29, 2019

#868

  • Added the option selectableRowsHeader that allows you to show/hide the header checkbox for selectable rows.

Quick view: https://codesandbox.io/s/github/patorjk/mui-datatables/tree/hide-header-checkbox

@patorjk
Copy link
Collaborator Author

patorjk commented Aug 29, 2019

Poking around a bit, I noticed this PR is actually very similar to this PR #384, though that one is out of date and has a slightly different implementation. If this PR is accepted, it'd be able to close that one too.

@gabrielliwerant gabrielliwerant added the needs review Useful to mark PRs for what's up next to review label Aug 29, 2019
@gabrielliwerant gabrielliwerant self-requested a review August 29, 2019 20:05
@gabrielliwerant
Copy link
Collaborator

gabrielliwerant commented Aug 29, 2019

Looks like this might also address an old PR #384.

P. S.

I think we commented about this at the same time, as I didn't see your comment on it until now. :p

Copy link
Collaborator

@gabrielliwerant gabrielliwerant left a comment

Choose a reason for hiding this comment

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

Just missing one thing, otherwise this looks good.

There are some tests in MUIDataTableHead.test.js that may need to be modified slightly to take account for the new option. E.g. https://github.com/gregnb/mui-datatables/blob/master/test/MUIDataTableHead.test.js#L127 which will now fail if we don't pass it true for selectableRowsHeader in the test, since the default is supplied by MUIDataTable, but we're using TableHead component directly.

Also, needs some conflicts to handle the new getDefaultOptions options function.

@gabrielliwerant gabrielliwerant added needs work new feature and removed needs review Useful to mark PRs for what's up next to review labels Sep 7, 2019
@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) to 76.425% when pulling 118ad0d on patorjk:hide-header-checkbox into 3032a7c on gregnb:master.

Copy link
Collaborator

@gabrielliwerant gabrielliwerant left a comment

Choose a reason for hiding this comment

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

I'm still seeing a test fail locally. I think we need to change https://github.com/gregnb/mui-datatables/blob/master/test/MUIDataTableHead.test.js#L128 to const options = { selectableRows: 'multiple', selectableRowsHeader: true };.

Nope, you're good! My local didn't have your latest changes, sorry.

@gabrielliwerant gabrielliwerant merged commit 6d44805 into gregnb:master Sep 12, 2019
lalong13 pushed a commit to lalong13/mui-datatables that referenced this pull request Jan 15, 2020
* Added selectableRowsHeader option

* fixed for test
@eminence88th
Copy link

Hi @patorjk @gabrielliwerant , how can I hide the checkbox in the header but still show in the rows of data. I just don't want it to be in the header. How can I do that?

@patorjk
Copy link
Collaborator Author

patorjk commented Oct 26, 2020

@eminence88th see the selectableRowsHeader option.

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

Successfully merging this pull request may close these issues.

4 participants