-
Notifications
You must be signed in to change notification settings - Fork 107
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
EPMRPP-96415 || Default number of items per page is displayed when us… #4065
Conversation
…er returns to the page
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## feature/orgs #4065 +/- ##
=============================================
Coverage 61.05% 61.05%
=============================================
Files 82 82
Lines 914 914
Branches 131 130 -1
=============================================
Hits 558 558
Misses 326 326
Partials 30 30 ☔ View full report in Codecov by Sentry. |
@@ -105,6 +105,7 @@ export const prevPagePropertiesSelector = ( | |||
|
|||
export const createQueryParametersSelector = ({ | |||
namespace: staticNamespace, | |||
alternativeNamespace, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the existing staticNamespace
cannot be used instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When everyone switches to the same pagination format, then you can leave one option.
@@ -29,6 +29,7 @@ export const withPagination = ({ | |||
paginationSelector = defaultPaginationSelector, | |||
namespace, | |||
namespaceSelector, | |||
alternativeNamespace, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When everyone switches to the same pagination format, then you can leave one option.
|
||
export const totalElementsSelector = (paginationSelector) => (state) => | ||
paginationSelector(state).totalElements; | ||
export const totalPagesSelector = (paginationSelector) => (state) => | ||
paginationSelector(state).totalPages; | ||
|
||
export const defaultPaginationSelector = () => DEFAULT_PAGINATION; | ||
|
||
export const createParametersSelector = ({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As this not related just to pagination, I suggest to move this selector to the pages/selectors
and name it as createAlternativeQueryParametersSelector
to be consistent in naming.
In this case this alternative common query selector will be responsible for collecting and processing all types of query parameters (pagination, sorting, etc)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
@@ -25,4 +25,5 @@ export const loadingSelector = (state) => domainSelector(state).loading || false | |||
|
|||
export const querySelector = createQueryParametersSelector({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should it be replaced by createAlternativeQueryParametersSelector
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quality Gate passedIssues Measures |
…er returns to the page
PR Checklist
develop
for features/bugfixes, other if mentioned in the task)npm run lint
) prior to submission? Enable the git hook on commit in your IDE to run it and format the code automatically.npm run build
)?manage:translations
script?Visuals