[#1003] - 30599 bc break tabs
- Closed
- 20 Apr 2013
- Medium
- Build: master
- # 1003
- Diff
- shumisha:30599_BC_break_tabs
- Foreign ID 30599
User tests: Successful: Unsuccessful:
This is a propsed fix for http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=30599
As the API was modified for JHtmlBootstrap::startPane and ::addPanel, the change reverts this back, and introduces new methods: startTabset, endTabSet, addTab and endTab. This new methods implements all the changes that were introduced recently in J 3.1, w/o interfering with pre-existing methods.
Where needed throughout the CMS, calls to JHtmlBootstrap::startPane have been changed to JHtmlBootstrap::startTabSet. Same for endPane, addPanel and endPanel.
Note that tabs html and javascript is now output through JLayout, so the tabs html and javascript can be overriden in templates.
I fixed the styles and self vs class name issue
As for the JLayout:
- they decouple API for creating tabs from the underlying renderer (bootstrap) in that case. The suggested plan is to have suitable output from the cms libraries independant of the currently used "renderer". Today it's bootstrap, tomorrow can be something else. That's already doable with this PR, as the layouts rendering the tabs can be overriden in the template. But with this structure, using Bootstrap or another renderer is a matter of selecting in a drop down list in the backend. This explains also the folder structure, as bootstrap is only a "renderer" amongst other possible ones, hence the need for its own subfolder inside libraries/cms/html.
Rgds
Since the layouts are overrideable, we could use a more generic name. This is a bad example, but something simple like /layouts/libraries/tabs/ that would explain the layouts in this folder render tabs and it doesn't feel like the folder is explicitly tied to a framework.
Actually, it´s rather the opposite. What has now become generic is the php startTabSet, addTab.
The layouts are bootsap-dependant and that's why you need a sub folder for them. Having a "tabs" sub folder directly under /libraries seems to restrictive to me, considering that other kind of items than "html" will be using jlayout in the future.
The PHP code is generic, yes, but the layout names to me imply that the implementation (whether the default or if I override them in my template) is supposed to use Bootstrap. If the layouts can be overridden, even though the default rendering is of a Bootstrap flavor, I would name the folder something more generic.
The naming isn't something to hold merging the code over, there's legitimate issues that need to be fixed in here. I'm just thinking out loud about it and trying to make sure I fully understand the logic behind the choices.
Actually again, no, that´s not how it´s becoming "generic". The php code is indeed becoming generic, and has (will have) an option such as $renderer="bootstrap" or $renderer="nextbigthing". And then it can calculate the path to the layout
$layout="libraries.cms.html." . $renderer . ".startpane";
And then:
Echo ....render( $layout, $data);
The calling php don't depend on which renderer is used, only it uses the renderer names to finally ufigure out which exact layout to use.
Sorry, am on tablet, typing not easy.
Will be back tomorrow afternoon now. Feel free to do as you see fit.
Yannick
OK, that makes a bit more sense actually.
Please test and comment on
http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=30599
Fixed in master. Thanks!
Maybe I missed the reason for this somewhere, but why are these changes being added into JLayout files? Are they intended to be overrideable by users? Also, could you consider a different file path for these files, they feel excessively long for no reason other than to match the libraries path (which there's no need to).