yiisoft / jquery-pjax

pushState + ajax = pjax
http://pjax.herokuapp.com
MIT License
144 stars 40 forks source link

$.pjax.reload(); seems to be not working anymore #9

Closed pjacquemin closed 10 years ago

pjacquemin commented 10 years ago

Hi all,

After the last update of jquery.pjax.js (https://github.com/yiisoft/jquery-pjax/pull/8#issuecomment-53869912) my gridview seems not refreshing anymore after clicking on actions button (I followed this tutorial : http://www.yiiframework.com/wiki/655/how-to-use-gridview-with-ajax/) Here is a sample of my code (to display an action column) :

'class' => 'yii\grid\ActionColumn',
            'header' => 'Actions',
            'buttons' => [
                'reset' => function($url, $model) {
                    return Html::a(
                            '<span class="glyphicon glyphicon-repeat"></span>', '', [
                            'onClick' => "
                                    if (confirm('Are you sure you want to reset this?')) {
                                        $.ajax({
                                            cache    : false,
                                            url  : '" . $url . "',
                                            success  : function(response) {
                                                $.pjax.reload({container:'#translationGrid'}); //this line is not working anymore
                                            },
                                        });
                                    }
                                ",
                            'title' => Yii::t('yii', 'Reset')
                        ]
                    );
                }
            ],
            'template' => '{delete} {reset}'

It's look like that the line 389 hasn't the history: true option set :

if(!pjax.options.history) return; //history is set to false here

I propose to add the default value to true to the pjax() options (line145) :

function pjax(options) {
        options = $.extend(true, {history: true}, $.ajaxSettings, pjax.defaults, options)
        if ($.isFunction(options.url)) {
            options.url = options.url()
        }

Thank you for your feedback.

qiangxue commented 10 years ago

@tonydspaniard Could you take a look at this? Thanks.

tonydspaniard commented 10 years ago

@qiangxue sure

tonydspaniard commented 10 years ago

@pjacquemin You are right... Looking carefully to the code of pjax, with extra attention, I have to admit that is kind of a mess, the code should be refactored. The options to handle click events do have that option set to true: https://github.com/yiisoft/jquery-pjax/blob/master/jquery.pjax.js#L810

But then, on other sections of the code: https://github.com/yiisoft/jquery-pjax/blob/master/jquery.pjax.js#L165 It loads from default options and they do not have it set: https://github.com/yiisoft/jquery-pjax/blob/master/jquery.pjax.js#L810, so even this section: https://github.com/yiisoft/jquery-pjax/blob/master/jquery.pjax.js#L276 cannot be reached either nor the reload: https://github.com/yiisoft/jquery-pjax/blob/master/jquery.pjax.js#L276

We should add that option to defaults. Otherwise, the user should pass the option history: true, when calling reload method.

I will work on a PR for a quick fix, on the meantime @pjacquemin mind checking if my findings are correct and use: $.pjax.reload({container:'#translationGrid', history: 'true'});

Whenever I have time, will work on a refactored version were code is not so messy.