[#2910] - Improved if/else structure related to sidebar
- Closed
- 6 Feb 2014
- Medium
- Build: staging
- # 2910
- Diff
- nonumber:patch-4
- Foreign ID 33200
- Success The Travis CI build passed Details
User tests: Successful: Unsuccessful:
http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=33200
This PR improves the if/else structure around the sidebar div in different admin views.
This is to make it the same across the different views. And also to prevent IDEs from messing up indentation.
Currently the if/else is around the starting div, which causes the code to have 2 starting divs, but just one closing div.
This causes formatting and error checking issues.
Update
After discussion, the entire if structures have been removed as they are useless
Agree that this is not covered by the codestyle initiative as it changes actual code.
The change looks good to me and it's good to have a consistent code for this over all files. But let us use the standard process here.
Going to change the title of the PR so it's clear it's not a codestyle PR.
Test
- Admin banners view
- Admin banners clients view
- Admin banners tracks view
- Ammin cache view
- Admin cache purge view
- Admin categories view
- Admin checking view
- Admin installer update view
- Admin templates view
Success!
Personally, I've always wondered why we have that if conditional. There isn't anything in a view class that could cause $this->sidebar to be unset between when it is compiled together and when it is rendered. I don't even use it in my own layouts and everything seems fine, maybe we could ditch them too.
I think it was to prevent copy&paste errors but I agree that it will be cleaner to remove it and missing/empty sidebar problems will be easy to detect if somebody uses copy&paste
So 
to use directly:
<div id="j-sidebar-container" class="span2">
<?php echo $this->sidebar; ?>
</div>
<div id="j-main-container" class="span10">
Ok, all those useless ifs are removed as suggested.
@test success
Test
Works well on
- option=com_banners&view=banners
- option=com_banners&view=clients
- option=com_banners&view=tracks
- option=com_cache&view=cache
- option=com_cache&view=purge
- option=com_categories&view=categories
- option=com_checkin&view=checkin
- option=com_installer&view=update
- option=com_templates&view=templates
Success!
Missing (not yet done) are other views from com_installer and com_templates
- option=com_installer&view=database
- option=com_installer&view=discover
- option=com_installer&view=install
- option=com_installer&view=languages
- option=com_installer&view=manage
- option=com_installer&view=warnings
- option=com_templates&view=styles
| Status | New | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2014-02-06 08:09:47 |
Merged after correcting legacy wrong indentation for cache and purge default.php
Please create a tracker on joomlacode. This has to be tested