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

Make pagination more extendable #4132

Merged
merged 4 commits into from
Dec 11, 2019
Merged

Make pagination more extendable #4132

merged 4 commits into from
Dec 11, 2019

Conversation

Kmaschta
Copy link
Contributor

@Kmaschta Kmaschta commented Dec 10, 2019

Closes #4131
Closes #4008

@Kmaschta Kmaschta added the RFR Ready For Review label Dec 10, 2019
@WiXSL
Copy link
Contributor

WiXSL commented Dec 10, 2019

Thank you. This also closes #4008

Copy link
Collaborator

@djhi djhi left a comment

Choose a reason for hiding this comment

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

Nice! Missing documentation though ;)

@fzaninotto fzaninotto added this to the 3.1.0 milestone Dec 10, 2019
@Kmaschta
Copy link
Contributor Author

Done!

@@ -16,6 +16,8 @@ const Pagination = ({
total,
setPage,
setPerPage,
ActionsComponent,
Copy link
Member

Choose a reason for hiding this comment

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

not sure about this name. I think in other places (or in material-ui), component props start with lowercase and don't include the term "component"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right: list, create, show etc, are component (and not elements) yet their props are lowercase and without component at the end.
I submitted the change

@fzaninotto fzaninotto merged commit 7f9b7ce into next Dec 11, 2019
@fzaninotto fzaninotto deleted the extend-pagination branch December 11, 2019 07:57
fzaninotto added a commit that referenced this pull request Dec 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFR Ready For Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants