Repository navigation
Page parameter name - #820
Conversation
…be used to access the page_name
…be used to access the pageParameterName
|
Do we really need to pass the entire Pagination object? I think passing only the options array can be enough. |
|
Hi @garak in this comment on the issue you said
Did I miss-understand what you had meant? Or not implement it in the way you expected? Essentially I took the code you proposed, updated it slightly for changed variable names, then modified the templates to pass in that third parameter. |
|
Well, I wrote that code a few months ago. Probably I would ask the same question to the past myself: why passing the entire object, when we can pass only the options? Moreover, considering that the object is only used to retrieve the options themselves. |
How would you like me to proceed? I can modify it such that we pass only that parameter (which for reference is how is used to work in v5), or we can continue with this approach? |
|
Change the method to accept an array, pass |
|
@garak thank you for your feedback - I've pushed another commit which I hope as implemented things as you intended. |
|
|
||
| $data['route'] = $pagination->getRoute(); | ||
| $data['query'] = \array_merge($pagination->getParams(), $queryParams); | ||
| $data['paginatorOptions'] = $pagination->getPaginatorOptions(); |
There was a problem hiding this comment.
let's go with "option" as name, it's simpler
There was a problem hiding this comment.
Changed as requested
|
* @return array |
||
| */ | ||
| public function getQueryParams(array $query, int $page): array | ||
| public function getQueryParams(array $query, int $page, ?array $paginatorOptions = []): array |
There was a problem hiding this comment.
same name change here. Moreover, we don't need to accept a null value
There was a problem hiding this comment.
Changed as requested
…Options object to just options
|
Thank you for your feedback @garak - I've renamed the variable as requested. I hope that the change to make the |
|
* @return array |
||
| */ | ||
| public function getQueryParams(array $query, int $page): array | ||
| public function getQueryParams(array $query, int $page, array $options): array |
There was a problem hiding this comment.
sorry @steveWinter I was about to release this change, but I realized this new argument would break any custom template not passing it.
We need to fix it with a default value of []
2 participants
This PR resolves #815 in the manner suggested by @garak in that issue - passing the entire SlidingPagination object to the templates to allow the pageParameterName to be accessed.
In the future it would likely be possible to remove some of the local variables which are created as the template is being rendered since they could be accessed directly from the SlidingPagination, however I wanted to keep this PR to solving just the issue at hand.