Fix issue #237

[DaSourcerer]

The rows in a CGridView have a visibility property. So do single entries in CDetailView. Can we have the same in CTabView?

[mdomba]

In CGridView there is a visible property for a certain column not for a row...
In CDetailView there is a visible property for an attribute

CTabView displays multiple tabs where only one is visible at a time...

Your "visible" property just decides if to render a tab or not... I don't see the difference if you just check this expression and then decide if to add the content as a tab or not...

[DaSourcerer]

Exactly. CGridView.columns.visible controls if a certain column is rendered or not. CDetailView.attributes.visible controls the visibility of a specific attribute. The idea of this patch has been to bring the same to specific tabs of CTabView. I've seen a number of occasions were this would have been helpful to me and others. The alternative is to perform more or less complex array manipulations on CTabView.tabs, which tends to result in illegible code after some time.

Really no way to get this in?

[mdomba]

I don't see how this is the same... a GridView shows all columns at once like CDetailView shows all attribute at once... and there is the visible attribute that can be evaluated at run-time so to decide if the column/attribute is visible depending on some current record value... note that this cannot be done otherwise... I mean you cannot evaluate that expression before the rendering method...

A tabview does not show all tabs at once only one at the time... in fact only one tab is visible and all other are not visible... so the "visible" property name is not logical...

and as I see it, you can just evaluate the condition and then decide if to render the TAB or not

maybe I'm wrong... please show me a use case for this... but note that as per your implementation the visible property is checked before rendering any tab so it's like I wrote before... you can just check the condition and depending on that decide if to add the tab or not.

[mdomba]

Ops... sorry for the previous comment... When writing I was thinking on the CButtonColumn::buttons visible option, that is evaluated at runtime...

Actually your proposal is good I see how it can help writing simpler code, so I'm reopening this issue.

My only concern is the name of this property as how I wrote before the definition of CTabView is "displays multiple tabs where at any time, only one tab is visible" so the visible property could be confusing to someone as here the "visible"=>false would mean "do not render this tab"

On the other side "visible" would be consistent with other components like CGridView and CDetailView.

I'm for this change but before pulling would like to hear other core devs (@samdark, @qiangxue)

[samdark]

I don't actually think that array manipulations are that complex. Also if is much more explicit.

$tabs = array();
$tabs['tab1'] = array(…);
if($condition)
{
   $tabs['tab2'] = array(…);
}

vs

$tabs = array(
  'tab1' => array(…),
  'tab2' => array(
    'render' => $condition,
    …
  ),
);

[DaSourcerer]

@mdomba Thanks for reconsidering this. That's more than I have hoped for.

@samdark You are indeed right. if is more explicit. But take into account that your example is quite simple. If you've got a CTabView with more than five tabs (possibly with a different set of tabs with different content for selected user groups), things tend to get really, really messy. I've worked around this with CMap::mergeArray(), unset() and other tricks. But in the end, a visibility prop would have been the far better solution.

[mdomba]

@samdark as @DaSourcerer explained without this option and depending on user needs complicate array manipulation could be needed... and the same attribute "visible" is added for a CGridView column where too an if would be enough

[cebe]

You should add a This option is available since version 1.1.11. like it is there for the data attribute.

[mdomba]

Please update your repo to latest version, merge it and push to your request branch. This way we will be able to merge automatically.

As for changelog consider using this line

• Enh Allow to control the visibility of tabs in CTabView #237: The tabs of CTabView now support the property 'visible' (DaSourcerer)

[DaSourcerer]

Here we go. I hope I didn't blow it.

[samdark]

Merged. @mdomba please check if your changes are there.

[mdomba]

all good (y)