yiisoft / yii-dataview

Data widgets
https://www.yiiframework.com/
BSD 3-Clause "New" or "Revised" License
38 stars 21 forks source link

Pagination: page attribute, wrong count. #29

Closed Portaflex closed 2 months ago

Portaflex commented 7 years ago

I want to keep page for pagination in GridView. So I use Yii::$app->session. But the page for the pagination component is not the actual saved page value but, instead, that value minus one. Why?

What steps will reproduce the problem?

function search ()
    {
        if ($data = Yii::$app->request->get()) Yii::$app->session['cita_params'] = $data;
        $params = Yii::$app->session['cita_params'];
        $query = self::v_cita();
        $dataProvider = new ActiveDataProvider([
                'query' => $query,
                'pagination' => [
                    'pageSize' => 30,
                    'page' => $params['page'] - 1
                ],
        ]);

What is the expected result?

I should be able to use just the saved page number.

What do you get instead?

Additional info

Q A
Yii version 2.0.12
PHP version 5.6
Operating system Linux Mint Debian edition
Offout commented 7 years ago

Obviously, fist page for users is 1, and for programmers it's 0, because of $page is used for offset part of query. https://github.com/yiisoft/yii2/blob/master/framework/data/Pagination.php#L297 I see two ways, or programmer knows that, and uses "-1" if needed, or hard code "-1" in framework. Third way is to show to user ?page=0, I think this is not a good thing. I personally prefer to use -1 in my code, rather than in framework.

Portaflex commented 7 years ago

Thanks for your non-obvious answer, dear Off. Every thing that is agreed by convention is non obvious. You prefer to start with "0" and I would prefer to start with "1" in this particular case. Thanks again.

samdark commented 7 years ago

I think it makes sense to fix it to actually accept page number and not offset-page number that is purely internal thing.

samdark commented 7 years ago

@yiisoft/core-developers what do you think?

klimov-paul commented 7 years ago

I think current implementation is correct.

Pagination class is used during DB query 'limit'/'offset' composition, which is a programming level issue, thus having a zero-based offset for it is justified and simplyfies particular 'page query' implementation. It belongs to 'model' layer and thus should follow common programming practice.

Using non-zero-based offset is a matter of representation: it is used because normal (non developers) people do not start counting from zero. However this representation may be changed. For example: I may want to create some portal explicitly for the developers and use zero-based page offset to keep some original site appearance, or I may want to use verbose words like first, second and so on instead of page number.

rob006 commented 7 years ago

@klimov-paul Even GitHub counts pages from 1: https://developer.github.com/v3/#pagination

Offout commented 7 years ago

I want to warn, that personally I, and, I think, many others, that already write -1, will get BC break if it will accept page number.

Portaflex commented 7 years ago

At DB query level offset = pageSize x (page -1) and limit = pageSize. So, for the first page you have to set offset to null somehow, there is your zero. So, it depends on where you want to substract one unit to $page, at your app code or at our yii code. Nevertheless, I don't mind to use 0. It is just for discussion. Thanks.

samdark commented 7 years ago

@Offout we're talking purely about Yii 2.1.

samdark commented 7 years ago

As @klimov-paul pointed out, it could be adjusted at request level but I'm still thinking that defaulting to page number instead of offset makes sense.

samdark commented 2 months ago

Page number is used in yiisoft/data: https://github.com/yiisoft/data/?tab=readme-ov-file#offset-pagination