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

Paginator regression between v4.48 and v4.55 #9835

Closed
patrickomeara opened this issue Apr 7, 2024 · 7 comments · Fixed by #9843
Closed

Paginator regression between v4.48 and v4.55 #9835

patrickomeara opened this issue Apr 7, 2024 · 7 comments · Fixed by #9843
Labels

Comments

@patrickomeara
Copy link

patrickomeara commented Apr 7, 2024

Bug description

I've just upgraded from v4.48 to v4.55 and the code I used to get the paginator is no longer working.

        @php($articles = Statamic::tag('collection:articles')->paginate(10))
        {!! $articles['paginate']['auto_links']['paginator']->links() !!}

Is there a recommended way of using Statamic::tag() to get a custom pagination view?

When on v4.48 the custom view is used resources/views/vendor/pagination/tailwind.blade.php. But now the LengthAwarePaginator does not seem to be in the return array at all. Using $articles['paginate']['auto_links'] doesn't use the custom view.

How to reproduce

        @php($articles = Statamic::tag('collection:articles')->paginate(10))
        {!! $articles['paginate']['auto_links']['paginator']->links() !!}

Logs

No response

Environment

Environment
Application Name: Site
Laravel Version: 10.48.4
PHP Version: 8.3.3
Composer Version: 2.7.1
Environment: local
Debug Mode: ENABLED
URL: site.test
Maintenance Mode: OFF

Cache
Config: NOT CACHED
Events: NOT CACHED
Routes: NOT CACHED
Views: NOT CACHED

Drivers
Broadcasting: log
Cache: statamic
Database: sqlite
Logs: single
Mail: mailgun
Queue: sync
Session: file

Statamic
Addons: 1
Antlers: runtime
Sites: 1
Stache Watcher: Enabled
Static Caching: Disabled
Version: 4.55.0 PRO

Statamic Addons
statamic/seo-pro: 5.4.1

Installation

Existing Laravel app

Antlers Parser

None

Additional details

No response

@ryanmitchell
Copy link
Contributor

Should it not be $articles = Statamic::tag('collection:articles')->paginate(10)->fetch() ?

@patrickomeara
Copy link
Author

Neither work.

The issue was from this change where the view is cast to a string.

36a955b

I can't see any access to the LengthAwarePaginator after calling paginate(), am I missing something?

@ryanmitchell
Copy link
Contributor

Can you use {!! $articles['paginate']['links'] !!} instead? It seems to be doing the same thing you are trying.

@patrickomeara
Copy link
Author

That is an array for me, here is the full $articles['paginate'] array.

"paginate" => array:8 [▶
    "total_items" => 43
    "items_per_page" => 10
    "total_pages" => 5
    "current_page" => 4
    "prev_page" => "https://site.test/articles?page=3"
    "next_page" => "https://site.test/articles?page=5"
    "auto_links" => """
      <nav>\n
              <ul class="pagination">\n
                  \n
                                  <li>\n
                          <a href="https://site.test/articles?page=3" rel="prev" aria-label="&laquo; Previous">&lsaquo;</a>\n
                      </li>\n
                  \n
                  \n
                                  \n
                      \n
                      \n
                                                                                              <li><a href="https://site.test/articles?page=1">1</a></li>\n
                                                                                                      <li><a href="https://site.test/articles?page=2">2</a></li>\n
                                                                                                      <li><a href="https://site.test/articles?page=3">3</a></li>\n
                                                                                                      <li class="active" aria-current="page"><span>4</span></li>\n
                                                                                                      <li><a href="https://site.test/articles?page=5">5</a></li>\n
                                                                              \n
                  \n
                                  <li>\n
                          <a href="https://site.test/articles?page=5" rel="next" aria-label="Next &raquo;">&rsaquo;</a>\n
                      </li>\n
                          </ul>\n
          </nav>\n
      """
    "links" => array:2 [▶
      "all" => array:5 [▶
        0 => array:2 [▶
          "page" => 1
          "url" => "https://site.test/articles?page=1"
        ]
        1 => array:2 [▶
          "page" => 2
          "url" => "https://site.test/articles?page=2"
        ]
        2 => array:2 [▶
          "page" => 3
          "url" => "https://site.test/articles?page=3"
        ]
        3 => array:2 [▶
          "page" => 4
          "url" => "https://site.test/articles?page=4"
        ]
        4 => array:2 [▶
          "page" => 5
          "url" => "https://site.test/articles?page=5"
        ]
      ]
      "segments" => array:3 [▶
        "first" => array:5 [▶
          0 => array:2 [▶
            "page" => 1
            "url" => "https://site.test/articles?page=1"
          ]
          1 => array:2 [▶
            "page" => 2
            "url" => "https://site.test/articles?page=2"
          ]
          2 => array:2 [▶
            "page" => 3
            "url" => "https://site.test/articles?page=3"
          ]
          3 => array:2 [▶
            "page" => 4
            "url" => "https://site.test/articles?page=4"
          ]
          4 => array:2 [▶
            "page" => 5
            "url" => "https://site.test/articles?page=5"
          ]
        ]
        "slider" => []
        "last" => []
      ]
    ]

Is there a reason why the default paginator view is explicitly set in OutputsItems?

Changing that line to the below means I can just use {!! $articles['paginate']['auto_links'] !!} as I assume is intended.

            'auto_links' => (string) $paginator->render(),

@duncanmcclean
Copy link
Member

duncanmcclean commented Apr 8, 2024

This is how I'd handle pagination with Blade:

@php($articles = Statamic::tag('collection:articles')->paginate(10)->fetch())
{!! $articles['paginate']['auto_links'] !!}

I don't think we ever intended people doing what you were doing and changing the pagination view with ->paginator()->render('pagination::tailwind').

As you mentioned, the pagination view is currently hard coded to pagination::default. I'm not sure why (& can't find where it was introduced). However, we could probably stop explicitly specifying the view in OutputsItems and let you customize the view in your AppServiceProvider instead:

use Illuminate\Pagination\Paginator;

public function boot(): void
{
    Paginator::defaultView('pagination::tailwind');
}

Would that work for you?

@patrickomeara
Copy link
Author

Yep that works @duncanmcclean

@jasonvarga
Copy link
Member

jasonvarga commented Apr 8, 2024

That'll be a breaking change so it'll be in v5.

For now, you can override it yourself.

Create a new paginator class in app/LengthAwarePaginator.php

<?php

namespace App;

class LengthAwarePaginator extends \Statamic\Extensions\Pagination\LengthAwarePaginator
{
    public function render($view = null, $data = [])
    {
        return static::viewFactory()->make(static::$defaultView, array_merge($data, [
            'paginator' => $this,
            'elements' => $this->elements(),
        ]));
    }
}

In your AppServiceProvider, register it:

use Statamic\Extensions\Pagination\LengthAwarePaginator;

public function register()
{
    $this->app->extend(LengthAwarePaginator::class, function ($paginator) {
        return new \App\LengthAwarePaginator(
            $paginator->getCollection(),
            $paginator->total(),
            $paginator->perPage(),
            $paginator->currentPage(),
            $paginator->getOptions()
        );
    });
}

When you upgrade to v5 you can undo this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants