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

Page parameter name #820

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/Helper/Processor.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ public function render(SlidingPaginationInterface $pagination, array $queryParam

$data['route'] = $pagination->getRoute();
$data['query'] = \array_merge($pagination->getParams(), $queryParams);
$data['paginatorOptions'] = $pagination->getPaginatorOptions();
Copy link
Collaborator

Choose a reason for hiding this comment

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

let's go with "option" as name, it's simpler


return \array_merge(
$pagination->getPaginatorOptions() ?? [], // options given to paginator when paginated
Expand Down
14 changes: 8 additions & 6 deletions src/Twig/Extension/PaginationRuntime.php
Original file line number Diff line number Diff line change
Expand Up @@ -109,20 +109,22 @@
}

/**
* @param array<string, mixed> $query
* @param int $page
* @param array<string, mixed> $query
* @param int $page
* @param array<string, mixed>|null $paginatorOptions
* @return array<string, mixed>
*/
public function getQueryParams(array $query, int $page): array
public function getQueryParams(array $query, int $page, ?array $paginatorOptions = []): array
Copy link
Collaborator

Choose a reason for hiding this comment

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

same name change here. Moreover, we don't need to accept a null value

{
$pageName = $paginatorOptions['pageParameterName'] ?? $this->pageName;
if ($page === 1 && $this->skipFirstPageLink) {
if (isset($query[$this->pageName])) {
unset($query[$this->pageName]);
if (isset($query[$pageName])) {
unset($query[$pageName]);
}

return $query;
}

return array_merge($query, [$this->pageName => $page]);
return array_merge($query, [$pageName => $page]);

Check failure on line 128 in src/Twig/Extension/PaginationRuntime.php

View workflow job for this annotation

GitHub Actions / PHPStan

Method Knp\Bundle\PaginatorBundle\Twig\Extension\PaginationRuntime::getQueryParams() should return array<string, mixed> but returns array<int|string, mixed>.
Copy link
Collaborator

Choose a reason for hiding this comment

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

this phpstan failure needs to be addressed

Copy link
Author

Choose a reason for hiding this comment

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

I don't understand how the change which has been made can be the cause of this error? $pageName will be a string, otherwise nothing about the array_merge has changed?

Copy link
Collaborator

Choose a reason for hiding this comment

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

The variable now can come from the options array, so it can be different from before

}
}
16 changes: 8 additions & 8 deletions templates/Pagination/bulma_pagination.html.twig
Original file line number Diff line number Diff line change
Expand Up @@ -13,23 +13,23 @@
{% if pageCount > 1 %}
<nav class="{{ classes|join(' ') }}" role="navigation" aria-label="pagination">
{% if previous is defined %}
<a rel="prev" class="pagination-previous" href="{{ path(route, knp_pagination_query(query, previous)) }}">{{ 'label_previous'|trans({}, 'KnpPaginatorBundle') }}</a>
<a rel="prev" class="pagination-previous" href="{{ path(route, knp_pagination_query(query, previous, paginatorOptions)) }}">{{ 'label_previous'|trans({}, 'KnpPaginatorBundle') }}</a>
{% else %}
<a class="pagination-previous" disabled>{{ 'label_previous'|trans({}, 'KnpPaginatorBundle') }}</a>
{% endif %}

{% if next is defined %}
<a rel="next" class="pagination-next" href="{{ path(route, knp_pagination_query(query, next)) }}">{{ 'label_next'|trans({}, 'KnpPaginatorBundle') }}</a>
<a rel="next" class="pagination-next" href="{{ path(route, knp_pagination_query(query, next, paginatorOptions)) }}">{{ 'label_next'|trans({}, 'KnpPaginatorBundle') }}</a>
{% else %}
<a class="pagination-next" disabled>{{ 'label_next'|trans({}, 'KnpPaginatorBundle') }}</a>
{% endif %}

<ul class="pagination-list">
<li>
{% if current == first %}
<a class="pagination-link is-current" aria-label="Page {{ current }}" aria-current="page" href="{{ path(route, knp_pagination_query(query, first)) }}">1</a>
<a class="pagination-link is-current" aria-label="Page {{ current }}" aria-current="page" href="{{ path(route, knp_pagination_query(query, first, paginatorOptions)) }}">1</a>
{% else %}
<a class="pagination-link" href="{{ path(route, knp_pagination_query(query, first)) }}">1</a>
<a class="pagination-link" href="{{ path(route, knp_pagination_query(query, first, paginatorOptions)) }}">1</a>
{% endif %}
</li>

Expand All @@ -43,9 +43,9 @@
{% if first != page and page != last %}
<li>
{% if page == current %}
<a class="pagination-link is-current" aria-label="Page {{ current }}" aria-current="page" href="{{ path(route, knp_pagination_query(query, page)) }}">{{ page }}</a>
<a class="pagination-link is-current" aria-label="Page {{ current }}" aria-current="page" href="{{ path(route, knp_pagination_query(query, page, paginatorOptions)) }}">{{ page }}</a>
{% else %}
<a class="pagination-link" aria-label="Goto page {{ page }}" href="{{ path(route, knp_pagination_query(query, page)) }}">{{ page }}</a>
<a class="pagination-link" aria-label="Goto page {{ page }}" href="{{ path(route, knp_pagination_query(query, page, paginatorOptions)) }}">{{ page }}</a>
{% endif %}
</li>
{% endif %}
Expand All @@ -59,9 +59,9 @@

<li>
{% if current == last %}
<a class="pagination-link is-current" aria-label="Page {{ current }}" aria-current="page" href="{{ path(route, knp_pagination_query(query, last)) }}">{{ last }}</a>
<a class="pagination-link is-current" aria-label="Page {{ current }}" aria-current="page" href="{{ path(route, knp_pagination_query(query, last, paginatorOptions)) }}">{{ last }}</a>
{% else %}
<a class="pagination-link" href="{{ path(route, knp_pagination_query(query, last)) }}">{{ last }}</a>
<a class="pagination-link" href="{{ path(route, knp_pagination_query(query, last, paginatorOptions)) }}">{{ last }}</a>
{% endif %}
</li>
</ul>
Expand Down
14 changes: 7 additions & 7 deletions templates/Pagination/foundation_v5_pagination.html.twig
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
<ul class="pagination">
{% if previous is defined %}
<li class="arrow">
<a rel="prev" href="{{ path(route, knp_pagination_query(query, previous)) }}">&laquo; {{ 'label_previous'|trans({}, 'KnpPaginatorBundle') }}</a>
<a rel="prev" href="{{ path(route, knp_pagination_query(query, previous, paginatorOptions)) }}">&laquo; {{ 'label_previous'|trans({}, 'KnpPaginatorBundle') }}</a>
</li>
{% else %}
<li class="arrow unavailable">
Expand All @@ -33,11 +33,11 @@

{% if startPage > 1 %}
<li>
<a href="{{ path(route, knp_pagination_query(query, 1)) }}">1</a>
<a href="{{ path(route, knp_pagination_query(query, 1, paginatorOptions)) }}">1</a>
</li>
{% if startPage == 3 %}
<li>
<a href="{{ path(route, knp_pagination_query(query, 2)) }}">2</a>
<a href="{{ path(route, knp_pagination_query(query, 2, paginatorOptions)) }}">2</a>
</li>
{% elseif startPage != 2 %}
<li class="unavailable">
Expand All @@ -49,7 +49,7 @@
{% for page in pagesInRange %}
{% if page != current %}
<li>
<a href="{{ path(route, knp_pagination_query(query, page)) }}">
<a href="{{ path(route, knp_pagination_query(query, page, paginatorOptions)) }}">
{{ page }}
</a>
</li>
Expand All @@ -69,20 +69,20 @@
</li>
{% else %}
<li>
<a href="{{ path(route, knp_pagination_query(query, (pageCount - 1))) }}">
<a href="{{ path(route, knp_pagination_query(query, (pageCount - 1), paginatorOptions)) }}">
{{ pageCount - 1 }}
</a>
</li>
{% endif %}
{% endif %}
<li>
<a href="{{ path(route, knp_pagination_query(query, pageCount)) }}">{{ pageCount }}</a>
<a href="{{ path(route, knp_pagination_query(query, pageCount, paginatorOptions)) }}">{{ pageCount }}</a>
</li>
{% endif %}

{% if next is defined %}
<li class="arrow">
<a rel="next" href="{{ path(route, knp_pagination_query(query, next)) }}">
<a rel="next" href="{{ path(route, knp_pagination_query(query, next, paginatorOptions)) }}">
{{ 'label_next'|trans({}, 'KnpPaginatorBundle') }} &nbsp;&raquo;
</a>
</li>
Expand Down
14 changes: 7 additions & 7 deletions templates/Pagination/foundation_v6_pagination.html.twig
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

{% if previous is defined %}
<li class="pagination-previous">
<a rel="prev" href="{{ path(route, knp_pagination_query(query, previous)) }}">
<a rel="prev" href="{{ path(route, knp_pagination_query(query, previous, paginatorOptions)) }}">
{{ 'label_previous'|trans({}, 'KnpPaginatorBundle') }}
</a>
</li>
Expand All @@ -17,11 +17,11 @@

{% if startPage > 1 %}
<li>
<a href="{{ path(route, knp_pagination_query(query, 1)) }}">1</a>
<a href="{{ path(route, knp_pagination_query(query, 1, paginatorOptions)) }}">1</a>
</li>
{% if startPage == 3 %}
<li>
<a href="{{ path(route, knp_pagination_query(query, 2)) }}">2</a>
<a href="{{ path(route, knp_pagination_query(query, 2, paginatorOptions)) }}">2</a>
</li>
{% elseif startPage != 2 %}
<li class="ellipsis"></li>
Expand All @@ -31,7 +31,7 @@
{% for page in pagesInRange %}
{% if page != current %}
<li>
<a href="{{ path(route, knp_pagination_query(query, page)) }}">
<a href="{{ path(route, knp_pagination_query(query, page, paginatorOptions)) }}">
{{ page }}
</a>
</li>
Expand All @@ -46,20 +46,20 @@
<li class="ellipsis"></li>
{% else %}
<li>
<a href="{{ path(route, knp_pagination_query(query, (pageCount - 1))) }}">
<a href="{{ path(route, knp_pagination_query(query, (pageCount - 1), paginatorOptions)) }}">
{{ pageCount - 1 }}
</a>
</li>
{% endif %}
{% endif %}
<li>
<a href="{{ path(route, knp_pagination_query(query, pageCount)) }}">{{ pageCount }}</a>
<a href="{{ path(route, knp_pagination_query(query, pageCount, paginatorOptions)) }}">{{ pageCount }}</a>
</li>
{% endif %}

{% if next is defined %}
<li class="pagination-next">
<a rel="next" href="{{ path(route, knp_pagination_query(query, next)) }}">
<a rel="next" href="{{ path(route, knp_pagination_query(query, next, paginatorOptions)) }}">
{{ 'label_next'|trans({}, 'KnpPaginatorBundle') }}
</a>
</li>
Expand Down
10 changes: 5 additions & 5 deletions templates/Pagination/materialize_pagination.html.twig
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
<ul class="pagination">
{% if first is defined and current != first %}
<li class="waves-effect">
<a href="{{ path(route, knp_pagination_query(query, first)) }}">
<a href="{{ path(route, knp_pagination_query(query, first, paginatorOptions)) }}">
<i class="material-icons">first_page</i>
</a>
</li>
Expand All @@ -28,7 +28,7 @@

{% if previous is defined %}
<li class="waves-effect">
<a rel="prev" href="{{ path(route, knp_pagination_query(query, previous)) }}">
<a rel="prev" href="{{ path(route, knp_pagination_query(query, previous, paginatorOptions)) }}">
<i class="material-icons">chevron_left</i>
</a>
</li>
Expand All @@ -43,7 +43,7 @@
{% for page in pagesInRange %}
{% if page != current %}
<li class="waves-effect">
<a href="{{ path(route, knp_pagination_query(query, page)) }}">{{ page }}</a>
<a href="{{ path(route, knp_pagination_query(query, page, paginatorOptions)) }}">{{ page }}</a>
</li>
{% else %}
<li class="active">
Expand All @@ -54,7 +54,7 @@

{% if next is defined %}
<li class="waves-effect">
<a rel="next" href="{{ path(route, knp_pagination_query(query, next)) }}">
<a rel="next" href="{{ path(route, knp_pagination_query(query, next, paginatorOptions)) }}">
<i class="material-icons">chevron_right</i>
</a>
</li>
Expand All @@ -68,7 +68,7 @@

{% if last is defined and current != last %}
<li class="waves-effect">
<a href="{{ path(route, knp_pagination_query(query, last)) }}">
<a href="{{ path(route, knp_pagination_query(query, last, paginatorOptions)) }}">
<i class="material-icons">last_page</i>
</a>
</li>
Expand Down
4 changes: 2 additions & 2 deletions templates/Pagination/rel_links.html.twig
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
{% if pageCount > 1 %}
{% if previous is defined %}
<link rel="prev" href="{{ path(route, knp_pagination_query(query, previous)) }}" />
<link rel="prev" href="{{ path(route, knp_pagination_query(query, previous, paginatorOptions)) }}" />
{% endif %}

{% if next is defined %}
<link rel="next" href="{{ path(route, knp_pagination_query(query, next)) }}" />
<link rel="next" href="{{ path(route, knp_pagination_query(query, next, paginatorOptions)) }}" />
{% endif %}
{% endif %}
10 changes: 5 additions & 5 deletions templates/Pagination/semantic_ui_pagination.html.twig
Original file line number Diff line number Diff line change
Expand Up @@ -12,34 +12,34 @@
#}
<div class="ui pagination menu">
{% if first is defined and current != first %}
<a class="icon item" href="{{ path(route, knp_pagination_query(query, first)) }}">
<a class="icon item" href="{{ path(route, knp_pagination_query(query, first, paginatorOptions)) }}">
<i class="angle double left icon"></i>
</a>
{% endif %}

{% if previous is defined %}
<a rel="prev" class="item icon" href="{{ path(route, knp_pagination_query(query, previous)) }}">
<a rel="prev" class="item icon" href="{{ path(route, knp_pagination_query(query, previous, paginatorOptions)) }}">
<i class="angle left icon"></i>
</a>
{% endif %}

{% for page in pagesInRange %}
{% if page != current %}
<a class="item" href="{{ path(route, knp_pagination_query(query, page)) }}">{{ page }}</a>
<a class="item" href="{{ path(route, knp_pagination_query(query, page, paginatorOptions)) }}">{{ page }}</a>
{% else %}
<span class="active item">{{ page }}</span>
{% endif %}

{% endfor %}

{% if next is defined %}
<a rel="next" class="icon item" href="{{ path(route, knp_pagination_query(query, next)) }}">
<a rel="next" class="icon item" href="{{ path(route, knp_pagination_query(query, next, paginatorOptions)) }}">
<i class="angle right icon"></i>
</a>
{% endif %}

{% if last is defined and current != last %}
<a class="icon item" href="{{ path(route, knp_pagination_query(query, last)) }}">
<a class="icon item" href="{{ path(route, knp_pagination_query(query, last, paginatorOptions)) }}">
<i class="angle right double icon"></i>
</a>
{% endif %}
Expand Down
10 changes: 5 additions & 5 deletions templates/Pagination/sliding.html.twig
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,20 @@
<div class="pagination">
{% if first is defined and current != first %}
<span class="first">
<a href="{{ path(route, knp_pagination_query(query, first)) }}">&lt;&lt;</a>
<a href="{{ path(route, knp_pagination_query(query, first, paginatorOptions)) }}">&lt;&lt;</a>
</span>
{% endif %}

{% if previous is defined %}
<span class="previous">
<a rel="prev" href="{{ path(route, knp_pagination_query(query, previous)) }}">&lt;</a>
<a rel="prev" href="{{ path(route, knp_pagination_query(query, previous, paginatorOptions)) }}">&lt;</a>
</span>
{% endif %}

{% for page in pagesInRange %}
{% if page != current %}
<span class="page">
<a href="{{ path(route, knp_pagination_query(query, page)) }}">{{ page }}</a>
<a href="{{ path(route, knp_pagination_query(query, page, paginatorOptions)) }}">{{ page }}</a>
</span>
{% else %}
<span class="current">{{ page }}</span>
Expand All @@ -25,13 +25,13 @@

{% if next is defined %}
<span class="next">
<a rel="next" href="{{ path(route, knp_pagination_query(query, next)) }}">&gt;</a>
<a rel="next" href="{{ path(route, knp_pagination_query(query, next, paginatorOptions)) }}">&gt;</a>
</span>
{% endif %}

{% if last is defined and current != last %}
<span class="last">
<a href="{{ path(route, knp_pagination_query(query, last)) }}">&gt;&gt;</a>
<a href="{{ path(route, knp_pagination_query(query, last, paginatorOptions)) }}">&gt;&gt;</a>
</span>
{% endif %}
</div>
Expand Down
10 changes: 5 additions & 5 deletions templates/Pagination/tailwindcss_pagination.html.twig
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,20 @@
<div class="flex items-baseline flex-row border border-gray-400 rounded-sm w-auto">
{% if first is defined and current != first %}
<span class="bg-white text-blue-600 px-3 py-2 text-lg border-r border-gray-400 font-bold">
<a href="{{ path(route, knp_pagination_query(query, first)) }}">&lt;&lt;</a>
<a href="{{ path(route, knp_pagination_query(query, first, paginatorOptions)) }}">&lt;&lt;</a>
</span>
{% endif %}

{% if previous is defined %}
<span class="bg-white text-blue-600 px-3 text-lg py-2 border-r border-gray-400">
<a rel="prev" href="{{ path(route, knp_pagination_query(query, previous)) }}">&lt;</a>
<a rel="prev" href="{{ path(route, knp_pagination_query(query, previous, paginatorOptions)) }}">&lt;</a>
</span>
{% endif %}

{% for page in pagesInRange %}
{% if page != current %}
<span class="bg-white text-blue-600 px-3 py-2 text-lg border-r border-gray-400">
<a href="{{ path(route, knp_pagination_query(query, page)) }}">{{ page }}</a>
<a href="{{ path(route, knp_pagination_query(query, page, paginatorOptions)) }}">{{ page }}</a>
</span>
{% else %}
<span class="bg-blue-600 text-white px-3 py-2 text-lg font-bold">{{ page }}</span>
Expand All @@ -26,13 +26,13 @@

{% if next is defined %}
<span class="bg-white text-blue-600 px-3 py-2 text-lg border-r border-gray-400">
<a rel="next" href="{{ path(route, knp_pagination_query(query, next)) }}">&gt;</a>
<a rel="next" href="{{ path(route, knp_pagination_query(query, next, paginatorOptions)) }}">&gt;</a>
</span>
{% endif %}

{% if last is defined and current != last %}
<span class="bg-white text-blue-600 px-3 py-2 text-lg border-gray-400 font-bold">
<a href="{{ path(route, knp_pagination_query(query, last)) }}">&gt;&gt;</a>
<a href="{{ path(route, knp_pagination_query(query, last, paginatorOptions)) }}">&gt;&gt;</a>
</span>
{% endif %}
</div>
Expand Down
Loading