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 pagination after increasing items per page #8854

Merged
merged 2 commits into from
Apr 19, 2023

Conversation

JammingBen
Copy link
Contributor

@JammingBen JammingBen commented Apr 18, 2023

Description

Fixes an issue where the file list incorrectly showed no items after paginating and increasing the amount of items per page. This was achieved by resetting the page query back to 1 if the user is on a page > 1.

Related Issue

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests

@JammingBen JammingBen self-assigned this Apr 18, 2023
@JammingBen JammingBen force-pushed the fix-pagination-after-item-per-page-increase branch from 0669deb to e86f9ec Compare April 18, 2023 13:48
@JammingBen JammingBen marked this pull request as ready for review April 19, 2023 08:46
@@ -130,6 +148,21 @@ export default defineComponent({
}
}

const setItemsPerPage = (itemsPerPage: string) => {
const resetPagination = itemsPerPage > unref(itemsPerPageQuery) && unref(currentPage) > 1
Copy link
Member

Choose a reason for hiding this comment

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

IMO we should go back to page 1 whenever the items per page change. So not only if the items per page increase, but instead on any change

Copy link
Member

Choose a reason for hiding this comment

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

I thought we already had this kind of reset in the past. Don't know if it got lost when we moved the pagination params from the store into the composable+query. 🤔 do you know or did you find any orphaned code fragments regarding the page reset?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IMO we should go back to page 1 whenever the items per page change. So not only if the items per page increase, but instead on any change

Sounds good as well 👍

do you know or did you find any orphaned code fragments regarding the page reset?

I didn't see anything regarding this. Where could such code be located? AFAIK ViewOptions is the only component where we set items-per-page.

Copy link
Member

Choose a reason for hiding this comment

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

can't find anything either

@@ -0,0 +1,6 @@
Bugfix: Pagination after increasing items per page

An issue where the file list incorrectly showed no items after paginating and increasing the amount of items per page has been fixed.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
An issue where the file list incorrectly showed no items after paginating and increasing the amount of items per page has been fixed.
An issue where the file list incorrectly showed no items after paginating and changing the amount of items per page has been fixed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Was about to do the same, but the bug report is still right though, no? The issue only occurred when increasing the amount of items.

Copy link
Member

Choose a reason for hiding this comment

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

yes :D fine by me either way

@sonarcloud
Copy link

sonarcloud bot commented Apr 19, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

74.1% 74.1% Coverage
0.0% 0.0% Duplication

@kulmann kulmann merged commit e8a8690 into master Apr 19, 2023
@delete-merged-branch delete-merged-branch bot deleted the fix-pagination-after-item-per-page-increase branch April 19, 2023 11:20
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.

Pagination works incorrect after changing the number of elements on page 2
2 participants