added visible option for CButtonColumn #1346

gregmolnar · 2012-09-06T14:29:54Z

pull request for issue #1345

cebe · 2012-09-06T23:18:52Z

Where is the code to hide buttons that have visible=false?

also your code has to be tab indented, by not spaces.

gregmolnar · 2012-09-07T05:54:28Z

I fixed the intendation. My editor was playing with me.
That part already exists: https://github.com/yiisoft/yii/blob/master/framework/zii/widgets/grid/CButtonColumn.php#L311-312

samdark · 2012-09-07T11:34:56Z

CHANGELOG

@@ -18,6 +18,7 @@ Version 1.1.13 work in progress
 - Enh: Fixed the check for ajaxUpdate false value in jquery.yiilistview.js as that never happens (mdomba)
 - Enh: Requirements checker: added check for Oracle database (pdo_oci extension) and MSSQL (pdo_dblib, pdo_sqlsrv and pdo_mssql extensions) (resurtm)
 - Enh: Added CChainedLogFilter class to allow adding multiple filters to a logroute (cebe)
+- Enh: #1345 CButtonColumn's default buttons(view, update, delete) has visible option now (gregmolnar)


Enh #1345 + it should be ordered properly.

samdark · 2012-09-07T11:35:39Z

Aside from the changelog line everything looks fine.

gregmolnar · 2012-09-07T11:46:10Z

I fixed it.

samdark · 2012-09-07T12:17:13Z

CHANGELOG

@@ -15,6 +15,7 @@ Version 1.1.13 work in progress
 - Enh #486: CHttpSession::$gCProbability and CDbHttpSession::$gCProbability are floats now. Minimal possible $gCProbability value has been changed to the ≈0.00000005% (1/2147483647), was integer 1% before, default value left unchanged (1%) (resurtm)
 - Enh #556: CDbColumnSchema::$comment property has been added. It stores comment for the table column, comment retrieving is working for MySQL, PgSQL and Oracle (resurtm)
 - Enh #1289: Added support for column comments for MSSQL (CDbColumnSchema::$comment property) (resurtm)
+- Enh #1345 CButtonColumn's default buttons(view, update, delete) has visible option now (gregmolnar)


Missing : ;)

samdark · 2012-09-07T15:33:44Z

CHANGELOG

@@ -15,6 +15,7 @@ Version 1.1.13 work in progress
 - Enh #486: CHttpSession::$gCProbability and CDbHttpSession::$gCProbability are floats now. Minimal possible $gCProbability value has been changed to the ≈0.00000005% (1/2147483647), was integer 1% before, default value left unchanged (1%) (resurtm)
 - Enh #556: CDbColumnSchema::$comment property has been added. It stores comment for the table column, comment retrieving is working for MySQL, PgSQL and Oracle (resurtm)
 - Enh #1289: Added support for column comments for MSSQL (CDbColumnSchema::$comment property) (resurtm)
+- Enh #1345: CButtonColumn's default buttons(view, update, delete) has visible option now (gregmolnar)


has → have

samdark · 2012-09-07T16:04:30Z

The rest is OK. Ready to merge after spelling error corrected.

added visible option for CButtonColumn

samdark · 2012-09-08T08:55:51Z

Thank you for working on this one.

gregmolnar · 2012-09-08T14:52:53Z

No worries. Thanks for merging.

mdomba · 2012-09-09T21:12:11Z

@samdark why did you merge this PR?

http://www.yiiframework.com/forum/index.php/topic/7931-cbuttoncolumn-visible-property-for-standard-buttons/page__view__findpost__p__91662

samdark · 2012-09-09T21:56:27Z

@mdomba I'm aware that one can use buttons. This PR is adding a bit more consistency with other existing properties configuring default buttons.

mdomba · 2012-09-09T22:48:54Z

But at the same time it's adding 3 new options that are just "shortcuts", and this way adding a bit of complexity...

the other "existing" options cannot be done by the developer with "buttons" as they are setting the functionality of the default buttons.

creocoder · 2012-09-10T00:36:14Z

the other "existing" options cannot be done by the developer with "buttons" as they are setting the functionality of the default buttons.

@mdomba Easy can be done. Fresh project code for example:

            'buttons'=>array(
                'update'=>array(
                    'imageUrl'=>Yii::app()->assetUrl.'/admin/img/icons/pencil.png',
                ),
                'delete'=>array(
                    'imageUrl'=>Yii::app()->assetUrl.'/admin/img/icons/cross.png',
                ),
            ),

mdomba · 2012-09-10T05:21:50Z

you did not understand my point. in your example you are setting a custom icon... but if you dont CButtonColumn has to set a default one for you... and thats why there is a separate property so that it can be more easily overwritten... but visible does not have an default values so I dont see the point in having 3 more properties for that.

gregmolnar · 2012-09-10T11:31:05Z

@mdomba you are right. I haven't tried to set the visible like that. Would it be possible to document that feature somehow? Also I guess this PR can be reverted.

samdark · 2012-09-10T11:50:21Z

Reverted.

gregmolnar · 2012-09-10T11:53:08Z

Any thoughts on adding this to the docs?

samdark · 2012-09-10T11:59:48Z

I think it's already there http://www.yiiframework.com/doc/api/1.1/CButtonColumn#buttons-detail

gregmolnar · 2012-09-10T12:07:56Z

the configuration for additional buttons.

Since it states additional buttons I wouldn't think I can use these options for the update, view, delete buttons.

mdomba · 2012-09-10T12:22:25Z

We could add a new paragraph to that section after the last sentence "Note that in order..."

Something like:

Note that if the buttonID is 'view', 'update' or 'delete' the options will be applied to the default buttons.

gregmolnar · 2012-09-10T12:23:35Z

@mdomba sounds good to me.

rawtaz · 2012-09-10T20:20:59Z

I agree with @gregmolnar that the "additional" is better left out. It doesn't make sense to first say that this option is for additional buttons, and then further down say "oh, but it's also for the default buttons". Better to just say that it is for "the buttons", cuz that's what it is. It's for all buttons, both default and additional ones. It's not weirder than that.

samdark · 2012-09-12T10:02:25Z

Fixed 73a4003

mdomba · 2012-09-12T10:04:40Z

@samdark should we mention the above line ?
if the buttonID is 'view', 'update' or 'delete' the options will be applied to the default buttons.

Some users on the forum wanted to create a custom button and tried to use "view" or "update" and then was wondering why it does not work as a custom button...

samdark · 2012-09-12T10:22:53Z

No. I've removed mentioning of "additional" so now it applies to all buttons.

mdomba · 2012-09-12T10:27:19Z

yes for that part...

but is it clear enough that the buttonID's 'view', 'update' and 'delete' refers to the corresponding default buttons?

gregmolnar · 2012-09-12T10:32:08Z

@mdomba
I think it is not clear enough yet.

On 12 September 2012 12:27, Maurizio Domba notifications@github.com wrote:

yes for that part... but is it clear enough that the buttonID's 'view',
'update' and 'delete' refers to the corresponding default buttons?

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/1346#issuecomment-8488367.

added visible option for CButtonColumn

1dc399c

fixed intendation

05ed97c

ghost assigned cebe Sep 7, 2012

samdark reviewed Sep 7, 2012
View reviewed changes

changelog order

5cbcecc

samdark reviewed Sep 7, 2012
View reviewed changes

missing colon

097dd78

samdark reviewed Sep 7, 2012
View reviewed changes

spelling mistake

0d9fb8d

samdark added a commit that referenced this pull request Sep 8, 2012

Merge pull request #1346 from gregmolnar/1345_CButtonColumn_Enhancement

adebfbb

added visible option for CButtonColumn

samdark merged commit adebfbb into yiisoft:master Sep 8, 2012

samdark added a commit that referenced this pull request Sep 10, 2012

Rolled back #1346

bfd756f

ghost assigned samdark Sep 10, 2012

samdark added a commit that referenced this pull request Sep 20, 2012

Added doc clarification for #1346 #1345

b2b29bc

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

added visible option for CButtonColumn #1346

added visible option for CButtonColumn #1346

gregmolnar commented Sep 6, 2012

cebe commented Sep 6, 2012

gregmolnar commented Sep 7, 2012

samdark Sep 7, 2012

samdark commented Sep 7, 2012

gregmolnar commented Sep 7, 2012

samdark Sep 7, 2012

samdark Sep 7, 2012

samdark commented Sep 7, 2012

samdark commented Sep 8, 2012

gregmolnar commented Sep 8, 2012

mdomba commented Sep 9, 2012

samdark commented Sep 9, 2012

mdomba commented Sep 9, 2012

creocoder commented Sep 10, 2012

mdomba commented Sep 10, 2012

gregmolnar commented Sep 10, 2012

samdark commented Sep 10, 2012

gregmolnar commented Sep 10, 2012

samdark commented Sep 10, 2012

gregmolnar commented Sep 10, 2012

mdomba commented Sep 10, 2012

gregmolnar commented Sep 10, 2012

rawtaz commented Sep 10, 2012

samdark commented Sep 12, 2012

mdomba commented Sep 12, 2012

samdark commented Sep 12, 2012

mdomba commented Sep 12, 2012

gregmolnar commented Sep 12, 2012

added visible option for CButtonColumn #1346

added visible option for CButtonColumn #1346

Conversation

gregmolnar commented Sep 6, 2012

cebe commented Sep 6, 2012

gregmolnar commented Sep 7, 2012

samdark Sep 7, 2012

Choose a reason for hiding this comment

samdark commented Sep 7, 2012

gregmolnar commented Sep 7, 2012

samdark Sep 7, 2012

Choose a reason for hiding this comment

samdark Sep 7, 2012

Choose a reason for hiding this comment

samdark commented Sep 7, 2012

samdark commented Sep 8, 2012

gregmolnar commented Sep 8, 2012

mdomba commented Sep 9, 2012

samdark commented Sep 9, 2012

mdomba commented Sep 9, 2012

creocoder commented Sep 10, 2012

mdomba commented Sep 10, 2012

gregmolnar commented Sep 10, 2012

samdark commented Sep 10, 2012

gregmolnar commented Sep 10, 2012

samdark commented Sep 10, 2012

gregmolnar commented Sep 10, 2012

mdomba commented Sep 10, 2012

gregmolnar commented Sep 10, 2012

rawtaz commented Sep 10, 2012

samdark commented Sep 12, 2012

mdomba commented Sep 12, 2012

samdark commented Sep 12, 2012

mdomba commented Sep 12, 2012

gregmolnar commented Sep 12, 2012