search

getgrav / grav-plugin-pagination

Grav Pagination Plugin
https://getgrav.org
MIT License
27 stars 20 forks source link

Where does $uri->currentPage() come from? #47

Open NicoHood opened 4 years ago

NicoHood commented 4 years ago

The plugin uses $uri->currentPage() in multiple lines. For example: https://github.com/getgrav/grav-plugin-pagination/blob/240fd7b38218cf881f322c480c60e8aa30d24669/pagination.php#L151

It uses a grav API: https://learn.getgrav.org/16/api#class-grav-common-uri

I am wondering: How does it work? Does the Grav API read the URI and searches for page:<number>? To me this sounds strange, as the pagination should only be provided by the plugin, not by grav itself. I would have expected, that this is a plugin function, not a common API function.

Could you please clarify?

mahagr commented 4 years ago

https://github.com/getgrav/grav/blob/develop/system/src/Grav/Common/Uri.php#L542-L547

So yes, it looks into the current URL and returns the value of /page:1 parameter.

To be honest, I don't think that we should have a pagination plugin at all. My reasoning is that pagination should be in Grav core and then used by whatever data type you happen to have. Also you may have pagination inside some sidebar content which is using AJAX to load extra information. In this case, the logic in the plugin is too limited. There are already pagination classes added to Grav itself: https://github.com/getgrav/grav/tree/develop/system/src/Grav/Framework/Pagination

NicoHood commented 4 years ago

I agree. There should be either an independent plugin or a full working builtin solution. I guess you have that on the roadmap or somewhere? Should we keep this open?

Edit: I want to add, that having a plugin is nice in the way to easily copy and modify the plugin. For example I find it confusing using extra urls for pagination instead of query parameters. Is there a special reason for that design decision? At least with a plugin you could overcome this issue (and thatswhy I stumbled across this function).