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

Pagination improvements, choice of items per page #2832

Merged
merged 10 commits into from
Aug 1, 2024
Merged

Conversation

janvhs
Copy link
Member

@janvhs janvhs commented Jul 29, 2024

Description

Add support for choosing between multiple selections of items per page when doing pagination.
This affects all tables in the code-base.

How was this tested?

Jest and Storybook

Add a documentation PR or write that no changes are required for the documentation.

  • DONE

@janvhs
Copy link
Member Author

janvhs commented Jul 29, 2024

Test's are still failing but maybe you want to have a look regardless?

Copy link
Contributor

@dottorblaster dottorblaster left a comment

Choose a reason for hiding this comment

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

Just some sparse comments but I think this is really good.

I noticed this is appearing on all the tables now, do we want this to happen? Do we want to have a prop to toggle this behavior? I don't have hard feelings about the direction.

assets/js/common/Table/Pagination.jsx Show resolved Hide resolved
assets/js/common/Table/Pagination.jsx Outdated Show resolved Hide resolved
assets/js/common/Table/Table.jsx Outdated Show resolved Hide resolved
assets/js/common/Table/Table.stories.jsx Outdated Show resolved Hide resolved
Copy link
Contributor

@dottorblaster dottorblaster left a comment

Choose a reason for hiding this comment

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

LGTM, feel free to rebase and merge 👍

@janvhs janvhs merged commit 7ce69a2 into main Aug 1, 2024
26 checks passed
@janvhs janvhs deleted the TRNT-3151-3152 branch August 1, 2024 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants