Moodle

Alrighty, I have attached `MDL-21862.theme.20100319.01.patch` which is a patch that gets base theme up and running as well as a new standard theme that is standard+standardwhite from Moodle 1.9!

The patch is huge but fret not 90% of it is just reorganizing +optimizing CSS.

Major changes include:

• Base theme consists of styles that are absolutely required, defines basic layouts and does nothing special.
• Standard theme consists of the `look`, extends base, doesnt define any additional layouts, and enables the dock.
• All pluggin CSS has been written as above and required styles have been moved back into plugins as styles.css files.
• Tidied up all CSS removing duplicate, naturally overridden, and frivolous styles.
• Removed CSS that is no longer used anywhere in Moodle 2.0.
• Reformatted CSS files to single line style definitions, this can be changed if there are good reasons + support for changing it but it certainly makes it easy to see what is going on (for myself at least). I did run this past Martin and he was in agreement.
• CSS is now split based on what it relates to (comments, calendar, messages...) and is split into seperate files if there is enough CSS to justify it.

Petr, thank you in advance for taking the time to look at this...... hopefully it doesn't cause you as many nightmares as it caused me.

Ohhh I nearly forgot to mention.
This is not 100% complete, there is still tweaking and such that will need to be done due to changes I have not had time to fix yet due to things such as YUI resets + changing classes.
I would greatly like to get this commit then work on this in a progressive manner. That way I can also easily delegate plugin tweaking to maintainers or interested parties.
I am open to this however....

hi, just a list of issues we need to discuss:
1/ page /course/index.php has is "#course .course .course-1" - this is a huge problem because we can not use the class and ID "course" anywhere else - this is due to the automatic class adding that is based on the script path
2/ the ".forumpost" breaks all conventions, we need to list also threads and other things - the places where we might want to use these classes are - mod/forum/, /user/, /my/*, portfolio export, some 3rd party contrib plugins; we have exactly the same problems in data module, glossary...

Possible solutions:
1/ add page prefix to IDs - such as #page-mod-forum-index, #page-mod-forum-view + remove the classes mirroring the paths completely
2/ frankenstyle component classes + manually specified classes for pages like groups and tags

Hiya,

Yeah I can see that #course and #user can be a problem for ids and I like the idea of the prefix. #page-mod-forum-index sounds good to me. +1 for that.

About the classes, I still very strongly believe they should be consistent with the ids and not use inconsistent magic names. I agree with you that .user .course etc could be problematic so we should uses prefixes here too. +1 for that too.

So on the page mod/forum/index.php it would have:

#page-mod-forum-index .path-mod-forum .path-mod

...which looks very sane.

Note if some forum content is shown in another page (eg #path-my-index) then the forum content should have .path-mod-forum wrapped around it to give the content a clear context.

All this sounds very good to me now.

Are there any remaining problems, Petr? If not, I'd like Sam to finish this off ASAP.

=== #page-mod-forum-index ===
great!

=== .path-course .path-blog ===
Very useful for core subsystems because you may address anything in /course/* , /blog/* easily.

=== plugins ===
I do not like the wrapping of forum stuff in .path-mod-forum because we do not want to say this comes from this directory, but we really want to say this is part of forum. In case of chat we have the problem with subdirectories, it could be solved by adding all parent paths to each page.

All plugins are defined in get_plugin_types() - that is the only place we need to modify when you are adding new type o plugin - it returns array (plugintype=>plugindirectory), the $component name is constructed as $plugintype.'_'.$pluginname. The rest of functions in moodle accept either $component or $plugintype + $pluginname. There is no place that would accept the path as defined in the CSS class, everything is now using the standardised plugin names (get_string(), install, upgrade, db tables, ... ... ... ... ...). Sometimes unfortunately the plugin name is very different from the actual location, the main reason is that we have a limit on the length of DB tables dues to oracle legacy.

I do not think theme designers actually care how we call the classes - they can see them there without any problems. The problem is that the PHP developers write the first CSS and have to wrap the divs withs classes around the code - so the argument that it is confusing is invalid because all developers have to use get_string() and friends which require component name.

So still ".mod_forum" or better ".plugin-mod_forum" makes much more sense to me.

Great that we are in agreement about prefixing with page (hoorah).

Petr out of curiosity you are proposing that we use .path-subsystems and .plugin-mod_name ?? or would it be that subsystems got the .page-dirpath and plugins only got the .plugin-mod_name ??

I would really like to get all of this in and commit as soon as possible, if you were envisaging both then I would think we have a solution that meets everyone's needs

Sorry, I'm trying hard to be fair and think carefully about both sides of this, but the component name idea seems to be more for the convenience of the developers and not the themers.

I really believe we need to change this balance to improve the themes in Moodle, even if it's very slightly more work for the initial developer. They too can always look at the classes in the auto-generated BODY tags if they get confused (I don't think they will be). Themers don't need to be constrained by Oracle.

The path is a very good identifier because it is unique, consistent, and guessable. There will never be two installed modules in mod/forum for example. Generating the classes is 100% automatic. And if you see .path-mod-forum being used in My Moodle it tells you exactly where the HTML was generated. There are a lot of wins here.

This has become a paint shed issue (because really, the final differences are mostly minimal) and is wasting a lot of time now, so unfortunately I'll have to make an executive decision on this one (as per MDL-21759 though I'm glad we've now added the 'path-' and 'page-' prefixes).

Nothing personal Petr, of course! There is so much to do and time is short.

Sam can you please go ahead and implement all this?

Hey guys,

I have attached a patch that makes the requested modifications
I'll look to get this commit tomorrow, so if anyone feels like reviewing it for me I'd be most appreciative.

Cheers
Sam

Just attached a new patch MDL-21862.theme.20100324.03.patch

Fixes include:

• Altered install_print_header to use the new style sheets of base and standard theme
• Added upgrade code to change theme entries for standardold to standard theme
• Fixed spelling mistakes

Some comments:

1) The pagelib code generating the classes makes one too many classes. Eg on mod/forum/view.php it generates:

id="page-mod-forum-view" class=" path-mod path-mod-forum path-mod-forum-view ...."

path-mod-forum-view is unnecessary and probably confusing.

2) More soon

OK, I've been playing with this.

There's still a lot of small theme issues all over the place that I'm sure you're aware of already, but I think basically this patch is sound enough for HEAD, and you can keep working on the small things there.

+10 to check in!

And its in !!
Now on to the polishing

sorry for coming a bit late into this discussion but I just added an objection into http://docs.moodle.org/en/Development_talk:Themes_2.0

Basically I think page-mod-forum-view should be CSS class and page-mod-forum-view-42 is the CSS id. more at the talk page.

I do see your argument, David, but there are other ways to get the same effect already.

Sam actually included .page-mod-forum-view originally but I vetoed that as it's redundant and would confuse themers.

The problem with adding the id param in the page class is that it's not used consistently everywhere and would require all the pages telling the themes how to name it, which is complicated and error-prone. The current system is consistent and automatic and thus reliable.

If you need to address a particular activity there is the cmid class (eg cmid-42), as well as course, category and context. By combining two classes you can get any combination needed.

No More Refactoring.

I have discovered some nasty CSS selectors in mod/quiz/styles.css, ex: .categoryinfo {padding: 0.3em;}
please review it once again, thanks

Is all done