Add pagination support to attachment modal#8953
Merged
Conversation
Updated the policy attachment modal to include pagination for listing policies. Adjusted the search function and integrated a paginator for handling large datasets efficiently. This enhances usability when browsing extensive policy lists.
Ben-El
added
the
exclude-changelog
E2E Test Results - DynamoDB Local - Local Block Adapter |
E2E Test Results - Quickstart |
itaigilo
requested changes
Apr 9, 2025
Contributor
There was a problem hiding this comment.
AttachModal is used not only for policy attachment, but by other components as well.
Can you describe the effects of your changes on these other components?
Contributor
Author
There was a problem hiding this comment.
The components that are affected are marked in the pic above,
which are:
- Attach policy modal in users and groups sections
- Add user to group modal.
- Add member to group modal.
Generally speaking, to these 4 modals, I added the pagination functionality.
Contributor
@Ben-El Please avoid (if possible) linking to non-lakeFS repos, since these are not available for OSS contributers. |
Contributor
Also note that:
|
Simplify state management by removing redundant `results` state and directly using API response. Centralize pagination logic and improve search behavior with a dedicated handler for prefix updates.
Replaced custom search logic with paginated API calls to improve scalability and maintainability. Updated group members, user policies, and user groups search functions for consistent use of pagination parameters. Removed unnecessary local state and redundant logic to streamline the code.
Ben-El
changed the title
itaigilo
requested changes
Apr 10, 2025
Contributor
itaigilo
left a comment
itaigilo
left a comment
There was a problem hiding this comment.
Looking good -
Blocking for the members comment.
Also, again, please avoid from linking Issues in private git repos (i.e product),
Since this is an open-source repo.
Refactored the user search functionality to use a locally cached list of all users, reducing redundancy in API calls. Introduced a `searchUsers` function that filters from a pre-fetched user list, paginating results efficiently.
Adjusted the indentation of the DataTable and Paginator components in the auth forms file to improve code clarity and maintain consistency. This change does not alter functionality but enhances code maintainability and readability.
Updated the indentation in the DataTable component for better readability and consistent formatting. No functional changes were introduced.
Corrected inconsistent indentation in the `rowFn` function of the auth forms component. This improves code readability and adheres to the project's formatting standards without altering functionality.
itaigilo
reviewed
Apr 15, 2025
Contributor
itaigilo
left a comment
itaigilo
left a comment
There was a problem hiding this comment.
Looking good!
Almost there, but added a few more comments, to make this cleaner, hopefully.
Centralized page size constant and improved code readability by using shared pagination parameters. Simplified and unified the handling of search and pagination across components.
Updated the `pageSize` constant to `PageSize` in both definition and usage to follow naming conventions. This ensures better alignment with code standards and improves readability.
itaigilo
approved these changes
Apr 15, 2025
Contributor
itaigilo
left a comment
itaigilo
left a comment
There was a problem hiding this comment.
Nice work 💪
Approving to unblock,
But please fix the last comment before merging.
Replaced hardcoded page size value (5) with the `PageSize` constant across multiple components for consistency and maintainability. This ensures centralized management of pagination settings.
idanovo
added
the
include-changelog
Ben-El
added
area/UI
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #8966
Change Description
Updated the attachment modal to include pagination for:
Adjusted the search function and integrated a paginator for handling large datasets efficiently.
This enhances usability when browsing extensive policy/group/member lists.
Testing Details
Was tested manually on local env, verified the pagination controls are showing, and added some more policies locally to increase the list size to see the pagination work appropriately.