Skip to content

in TablePagination 'actions' classes key is missing #10607

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

Closed
gadkadosh opened this issue Mar 10, 2018 · 8 comments
Closed

in TablePagination 'actions' classes key is missing #10607

gadkadosh opened this issue Mar 10, 2018 · 8 comments
Labels
bug 🐛 Something doesn't work component: table This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process.
Milestone

Comments

@gadkadosh
Copy link

actions classes key is documented, yet seems not to be implemented in the code.
I looked quickly in TablePagination and TablePaginationActions.

@oliviertassinari oliviertassinari added this to the post v1.0.0 milestone Mar 11, 2018
@oliviertassinari oliviertassinari added bug 🐛 Something doesn't work component: table This is the name of the generic UI component, not the React module! labels Mar 11, 2018
@oliviertassinari
Copy link
Member

@gadkadosh It's a good point. I think that we should move the TablePaginationActions's styles to the TablePagination component and inject a classes property to the action child.
https://github.com/mui-org/material-ui/blob/bbbc7a57a26673f9304275f2444a445b56ac4f71/src/Table/TablePaginationActions.js#L9-L13

@oliviertassinari oliviertassinari added the good first issue Great for first contributions. Enable to learn the contribution process. label Mar 11, 2018
@lukePeavey
Copy link
Contributor

lukePeavey commented Mar 16, 2018

Is it ok if I work on this?

@oliviertassinari
Copy link
Member

oliviertassinari commented Mar 18, 2018

@lukePeavey Sorry for the delay. Ideally, I wanted to clear the render prop API story before taking care of this issue: #10476. It can change the way we handle it. But after more thought, I think that we can move forward here. So what do you think of moving the TablePaginationActions.js styles into TablePagination.js and having it providing a "filtered" classes property?

@t49tran
Copy link
Contributor

t49tran commented Mar 28, 2018

Have anyone started working on this issue yet ? If not I can start a PR.

@lukePeavey
Copy link
Contributor

lukePeavey commented Mar 29, 2018 via email

@lukePeavey
Copy link
Contributor

@oliviertassinari Sorry it took me so long to respond!

I didn't initially notice that that you had labeled this as post-v1. If you would rather just wait and address this after the render prop api question is resolved, thats totally fine with me.

I think that we should move the TablePaginationActions's styles to the TablePagination component and inject a classes property to the action child.

I think this makes sense, at least as a short term solution. If you want to go ahead with this, i've made the changes and can submit a PR.

@oliviertassinari
Copy link
Member

@lukePeavey It's label post-v1 as it's not required to cut v1 out (the deadline is in one month). It's a simple change, so if you want to work on it, it's always great :).

@lukePeavey
Copy link
Contributor

Oh ok

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: table This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process.
Projects
None yet
Development

No branches or pull requests

4 participants