yiisoft / yii

Yii PHP Framework 1.1.x
http://www.yiiframework.com
BSD 3-Clause "New" or "Revised" License
4.85k stars 2.28k forks source link

Commit 89be0cb01c2911ee8a2e750af8c3425f917c9124 breaks CgridView #964

Closed tudorilisoi closed 12 years ago

tudorilisoi commented 12 years ago

This commit introduces the following lines in jquery.yiigridview.js

                        // We're dealing with arbitary non-sorting link
                        if (!$(this).hasClass('sort-link'))
                            return true;

This effectively restricts ajax grid updates to elements specified in updateSelector ONLY if they also have a .sort-link CSS class. For example, the navigation links (CSS selector '.pager a') will not be handled by ajax, although you specify ajaxUpdate=true in widget options

The fix is to remove these 3 lines.

Original issue: #803

resurtm commented 12 years ago

I'm working on better, non-reverting solution.

tudorilisoi commented 12 years ago

Okay. This was a show-stopper for me, so I fixed it in my fork.

tudorilisoi commented 12 years ago

You can also notice how confused I am about branching and submitting pull requests ;)

resurtm commented 12 years ago

@tudorilisoi I've commented this two Javascript lines with .sort-link checking (within jquery.yiigridview.js file), then added updateSelector property and grid update still not being updated via AJAX. Application assets and browser cache were cleaned.

In other words AJAX update for pager and sorter with specified updateSelector was not working before commit 89be0cb01c2911ee8a2e750af8c3425f917c9124.

I'm properly understood the bug?

My testing code:

<?php

class GridViewController extends Controller
{
    public function actionIndex()
    {
        $dataProvider=new CArrayDataProvider(array(
            array('id'=>2,'title'=>'test2'),
            array('id'=>3,'title'=>'test3'),
            array('id'=>4,'title'=>'test4'),
            array('id'=>5,'title'=>'test5'),
            array('id'=>6,'title'=>'test6'),
            array('id'=>22,'title'=>'2test2'),
            array('id'=>23,'title'=>'2test3'),
            array('id'=>24,'title'=>'2test4'),
            array('id'=>25,'title'=>'2test5'),
            array('id'=>26,'title'=>'2test6'),
            array('id'=>2,'title'=>'test2'),
            array('id'=>3,'title'=>'test3'),
            array('id'=>4,'title'=>'test4'),
            array('id'=>5,'title'=>'test5'),
            array('id'=>6,'title'=>'test6'),
            array('id'=>22,'title'=>'2test2'),
            array('id'=>23,'title'=>'2test3'),
            array('id'=>24,'title'=>'2test4'),
            array('id'=>25,'title'=>'2test5'),
            array('id'=>26,'title'=>'2test6'),
        ),array(
            'sort'=>array(
                'attributes'=>array(
                    'id'=>array(
                        'asc'=>'id ASC',
                        'desc'=>'id DESC',
                    ),
                ),
            ),
        ));

        $this->render('index',array(
            'dataProvider'=>$dataProvider,
        ));
    }
}
<?php /* @var $this GridViewController */ ?>

<?php echo CHtml::link('Update', array('gridView/index', 'sort'=>'id.desc'), array('class'=>'grid-view-update-link')); ?>

<?php $this->widget('zii.widgets.grid.CGridView',array(
    'updateSelector'=>'.grid-view-update-link',
    'dataProvider'=>$dataProvider,
    'ajaxUpdate'=>true,
    'columns'=>array(
        array(
            'name'=>'id',
        ),

        array(
            'class'=>'CDataColumn',
            'header'=>CHtml::link('Yii Framework','http://yiiframework.com/'),
            'name'=>'title',
        ),

        array(
            'class'=>'CCheckBoxColumn',
            'header'=>CHtml::link('Yii Framework','http://yiiframework.com/'),
        ),

        array(
            'class'=>'CLinkColumn',
            'header'=>CHtml::link('Yii Framework','http://yiiframework.com/'),
        ),

        array(
            'class'=>'CButtonColumn',
            'header'=>CHtml::link('Yii Framework','http://yiiframework.com/'),
            'viewButtonUrl'=>'"#"',
            'updateButtonUrl'=>'"#"',
            'deleteButtonUrl'=>'"#"',
        ),
    ),
)); ?>
resurtm commented 12 years ago

@tudorilisoi Could you please create two demo applications? One with and another without bug. :)

tudorilisoi commented 12 years ago

you may set ajaxUpdate to true in widget options, and remove the updateSelector option, this will auto-configure the selector as

'#id .pager a, '#id .grid thead th a',

, where id is the grid widget Html id. I can guarantee it works

<?php $this->widget('zii.widgets.grid.CGridView',array(
// this is not needed unless you want to customize the grid
//    'updateSelector'=>'.grid-view-update-link',
    'dataProvider'=>$dataProvider,
    'ajaxUpdate'=>true,
    'columns'=>array(
        array(
            'name'=>'id',
        ),

        array(
            'class'=>'CDataColumn',
            'header'=>CHtml::link('Yii Framework','http://yiiframework.com/'),
            'name'=>'title',
        ),

        array(
            'class'=>'CCheckBoxColumn',
            'header'=>CHtml::link('Yii Framework','http://yiiframework.com/'),
        ),

        array(
            'class'=>'CLinkColumn',
            'header'=>CHtml::link('Yii Framework','http://yiiframework.com/'),
        ),

        array(
            'class'=>'CButtonColumn',
            'header'=>CHtml::link('Yii Framework','http://yiiframework.com/'),
            'viewButtonUrl'=>'"#"',
            'updateButtonUrl'=>'"#"',
            'deleteButtonUrl'=>'"#"',
        ),
    ),
)); ?>
resurtm commented 12 years ago

@tudorilisoi Okay, i've done exactly what you said.

When there is line if (!$(this).hasClass('sort-link')) return true; pager is not AJAX powered, but sorting is okay. When i've removed this line, both pager and sort started to work via AJAX. This is the problem? :)

resurtm commented 12 years ago

Please, do not revert my commits now — we always have time to do it before 1.1.11 release. Those who use the development version of Yii should be ready for such regressions.

resurtm commented 12 years ago

Briefly (note): PR #971 offers solution through introduction new special class for CHtml::link() inside CDataColumn::$header.

tudorilisoi commented 12 years ago

Yes, this is the problem. The grid works fine without these lines. I believe the updateSelector property is sufficient for anyone who might need to specify additional elements that should trigger the grid ajax update.