Details
Major
Description
Add custom corner divs to boxes on Moodle pages.
Create wrapper functions to add the necessary divs within boxes.
Gliffy Diagrams
Attachments
Issue Links
Activity
- All
- Comments
- Work Log
- History
- Activity
- Links Hierarchy
Please watch your trailing whitespace and initialize all variables. Committing patch into cvs...
OK, we really need to revert this ASAP.
We can not start asking all the modules to identify particular boxes for custom_corners support. It's too hard on the API.
Two other solutions could be (in order of preference):
1) Leave things alone (were they really that bad?)
2) Fix print_box functions so that they do this when $THEME->customcorners is on, fix all the calls to them that might not provide good IDs to work with, and then fix the custom_corners theme so that it excludes any extra custom_corner boxes that should really not have corners displayed.
And if this is really too much overhead then,
3) come up with a more generic function like print_text_box or print_content_box ... it would work something like the patch in this bug but would at least be a bit more generic and describe the structure of the data rather than the appearance.
Petr, can you please revert this (solution 1) while we work/test/think about solution 2 and 3?
yes
work in progress attached, pleasereview...
Should be fixed in cvs now, head was a bit broken already I guess.
Tested in FF and IE6 under wine.
Some more info:
- implemented method 3/ above
- We have now print_container_xxx() and print_box_xxx() functions, container is more general than box with no classes by default. All the code needed for themes using custom corners is only in weblib.php.
- all themes were patched to use print_container_start() and _end() while keeping it backwards compatible
- we are tracking open boxes and containers (we need this for ids in custom corners), the bonus is that error(), notice() and redirect() close open boxes and containers
how to use:
- print_box() - the same as before, internally uses containers with box css
- print_container() - general div container for related elements in rectangular area, custom corners themes may draw round corners; in normal themes only one div used
if anybody objects I can revert/rework it easily 
I've just upgraded a site, and i'm getting "Incorrect closing of custom corners - no more open containers" everywhere.. Not looked into it any further and watching the debug messages..
Problem goes when switching to custom corners theme..
Like the rework though.
eh, reopening
On which page are you getting the warning?
Could you please copy/paste it here?
The previous code was ignoring double/missing closing of boxes/containers. There might be some places where this problem appears, I am going to add more debug info into print_footer to catch all these...
... and thanks a lot for the testing!
found and fixed 
http://moodle.cvs.sourceforge.net/moodle/moodle/theme/standard/header.html?r1=1.39&r2=1.40
should there be a clearfix there?
The clearfix is the first parameter in print_container(), right?
Regarding the change to standard/header.html, do we really need them at all?
Custom_corners can continue to use it of course, but I think 'standard' themes should NOT introduce this function here as it breaks compatibility with 1.8.
yes, the thems from 1.9 would not be compatible with 1.8, this could be partially fixed by backporting simplified print_container into 1.8.x
if we remove it from standard themes the footer hacks will throw developer warnings in several places - though it might be better to fix them anyway
Several aspects are involved
_1 backwards compatible themes:
1.9 themes can't ever be really compatible to 1.8 because several CLASS names have changed from 1.8 to 1.9.
+1 to implement the enhancement (compatibility arguments not really count).
_2 consistency:
Petr has done a great job to make the Moodle container handling more consistent. Now several hacks emerge which have been sleeping in the dark and affected advanced themes only. Backporting a simple print_container function to 1.8 would be marvelous.
+1 to fix this hacks and to add the enhancement to the Moodle 1.9 standard themes.
_3 In the standard theme we can show the enhancements created under the hood.
+1 to fix this hacks and to add the enhancement to the Moodle 1.9 standard themes.
_4 Martins argument to change the standard theme structure only when absolute necessary. This position is very wise to keep the Moodle changes as smooth as possible.
-2 to implement the enhancement
After calculating the numbers 3 - 2 I vote +1 to implement the enhancements. If Martin decides to not implement them now consistency needs to be reestablished in 2.0 with all themes using the enhancement.
It would be marvelous if Petr may find a solution that header and footer work without the print_container functions without errors. Then theme designers who do not want to implement this enhancement are not forced to do so.
And again I think in the standard themes we should use the print_container functions.
The admin/lang.php?mode=missing shows an interesting page structure. Please check the screenshot http://screencast.com/t/AhxuZTfCULp.
I am not sure any more if it is wise to replace all boxes with the same custom_corners supporting function. Developers use boxes not only for content containers. Content containers should have custom_corners support. Any other boxes (to collect some elements or whatever) should be built without custom_corners support.
A bit more semantic code would help out IMO. May be we need a semantic option to specify the content type of the box: "layout", "wrapper", "text" for example. And wrapper boxes don't write the custom_corners divs. We may need to analyze the box usage a bit more.
Custom_corners looks like a never ending story
I have committed rewritten autoclosing for error() and notice() - should be more flexible and compatible with old themes - please note that is is impossible to automatically close containers and tables, the problem is that each page may have different structure, error() and now also notice() should be used before the print_header() - this will have to be solved properly in 2.0
going to backport print_container into 18 now...
basic containers backported into 1.8.x, themes from 1.9 work again in latest 1.8.x
reclosing, please reopen or file a new issue if needed
I add changes to weblib and the forum and glossary view pages to get those pages working.
I added changes to weblib and the forum and glossary view pages, the custom_corners and standard theme to get those pages working.
In the view pages I changed the container function call to a simple "echo 'div ...'" to avoid the custom_corners container overhead and page oddity.
In weblib I added "clearfix" handling to the function "print_box_start($classes ...". "clearfix" is handed over to the containing divs. This is a hack, but I didn't know how to handle it without rewriting too much areas."
Petr, please review and close the issue when you think the changes are ok.
I continue to really dislike all the inappropriate PHP, includes etc that has crept into the custom_corners header.html and footer.html. We must get rid of it.
It is impractical to make a theme that is uses custom_corners as a parent (eg I use one on partners.moodle.com) because all this fragile PHP code breaks every time I upgrade.
Martin, I could not agree more.
As a first step I removed the "nocoursepage" hack, which was an attempt to find out if a page has a layout-table. I developed this hack because you said, that you don't not see any chance to implement a layout-table check. This hack followed several exception hacks because Moodle pages are inconsistently created. All those exceptions I could delete too.
Now I added a JavaScript layout-table check which works great. Without the layout-table info custom_corners does not work. There must be a body class "haslayouttabel" or "nolayouttable" no matter how it's added to the page.
When the issue with the "inpopup" handling (MDL-12093) will be better implemented I can take out some more "hacks" in header.html and may be in footer.html. Then header and footer are nearly back to normal.
Discussion seams to be closed.
The wrapper functions are:
/**
*
*/
function print_custom_corners_box($message, $classes='generalbox', $ids='', $return=false) {
$output = print_custom_corners_box_start($classes, $ids, true);
$output .= stripslashes_safe($message);
$output .= print_custom_corners_box_end(true);
if ($return)
{ return $output; } else { echo $output; }}
/**
* Function adds custom_corners to boxes
* Calls print_box_start
*
* @param string $classes, space-separated class names.
* @param string $ids, space-separated id names.
* @param boolean $return, return as string or just print it
*/
function print_custom_corners_box_start($classes='generalbox', $ids='', $return=false) {
global $CFG, $THEME;
$output = print_box_start('ccbox '.$classes, $ids, true);
if (!empty($THEME->customcorners)) { require_once($CFG->dirroot.'/lib/custom_corners_lib.php'); $output .= print_custom_corners_start(true, true); }
if ($return) { return $output; }
else
{ echo $output; }}
/**
* Function adds custom_corners to boxes
* Calls print_box_end
*
* @param boolean $return, return as string or just print it
*/
function print_custom_corners_box_end($return=false) {
global $CFG, $THEME;
if (!empty($THEME->customcorners)) { require_once($CFG->dirroot.'/lib/custom_corners_lib.php'); $output .= print_custom_corners_end(true); }
$output .= print_box_end(true);;
if ($return) { return $output; } else { echo $output; }
}