Talk:Theme changes in 2.0

Jump to: navigation, search

I think this is a great system. Some comments on specific issues:

1. Browser-specific stylesheets - is it time to move away from these?

The OU system for this is a lot better. We add special classes to body (in addition to whatever's normally there), so you have

<body class="ie ie7">

or

<body class="gecko gecko19">

Classes indicate the browser engine and version so that we can specify a generic engine if a problem affects all versions, or a specific version.

This means you can put the IE exceptions right next to the standard rules and they can be included in all stylesheets for any module. So you do

#mypage .whatever {
  /** normal rules */
}

.ie6#mypage .whatever {
  /** broken rules */
}

Note that we don't just need exceptions for IE. Over time we've needed rare (single) exceptions for Firefox (hence 'gecko' stuff above; the issue was actually only in gecko18 I think) and WebKit too.

IMO this system should be adopted in core Moodle. It's very simple and is far preferable to either CSS hacks (appalling) or separate stylesheets (unmaintainable).

2. Debugging mode

Maybe developer debug mode should send a special revision number (rev=debug) in order to regenerate the css every time, and send it without the expires header etc. That would probably make it pretty slow though.

Alternatively, this could be an additional option in the debugging page so that you can turn on 'CSS debugging' which would sent the rev=debug on every page, and turn it off when debugging other stuff that isn't css.

having to manually update the number every single time you change css would be a huge annoyance when debugging css problems imo so I think one of these solutions is better.

3. Removing comments

Removing comments would be great.

It's not only a performance issue, but:

a. Because of the performance concern developers (including me) tend to not include comments at all, this is really bad for maintainability.

b. If a student looks into our code and it says something 'unprofessional' like 'only needed because IE sucks' they might in theory complain - yes I have genuinely had a student complaint for using the word 'sucks' in public - so this is another discouragement for developers to comment properly.

4. Performance

If these are served for lifetime (ie expires 10 years in future) as they should be, I think this would actually improve performance for most sites (that see a lot of repeat visits), even if it takes 50x longer to serve the css itself...

That's it, everything else is awesome. :)

sam marshall


2. from sam's suggestions is already done. We should make sure it is not lost in any changes.

I don't see any point in multiple parent themes. Too complicated. Theme + parent + standard is sufficient. I guess once you are at two parents like that, it is no harder to code a general array, it is just much easier for theme developers to screw up. Oh, I guess there is not harm in the extra flexibility.

No problem that this does not include ->requires->css. We should comment that to make it clear that you rarely need to use it, but keep it as an emergency fallback for people who are doing random customisations.

I proposed getting rid of meta.php before, but martin vetoed it. Somewhere in the themes forum is a post form me where I list all the ways it is currently used in core and contrib. I still think we should get rid of it.

Moodle 1.9 themes still work in Moodle 2.0. I would like that to continue to be true. I think it is still possible. Look at the code in theme_config class for handling legacy config.php files, and the code in deprecatedlib.php that implements the old function for serving CSS.

--Tim Hunt 17:00, 21 August 2009 (UTC)



2. yes, I considered this too - I called this theme designer mode. I am not a big friend to using DEBUG_DEVELOPER for this, that is why I proposed it as separate option.

Why not multiple parents when it is so easy to implement?

I do not understand the "emergency need" for ->requires->css() - the /local/ plug-ins are the correct place for emergency hacks, not the core. You just create new dir in /local/something/styles.css and it will be included automatically on all pages. The problem with custom CSS is that it can not fit into the one huge stylesheet - you can not load it before it because YUI reset will alter it and also after because it would not by style-able via themes which is not an option imo.

I wanted to intentionally disable all old themes, I agree it should be as easy to upgrade as possible, but I still think that admins should be required to take some actions manually or else they will keep complaining that themes are "broken" in 2.0 and nobody told them they should update them. I think the ideal solution would be to allow easy downloads of themes via the Moodle UI instead of full backwards compatibility :-)

Petr Škoda (škoďák) 19:34, 21 August 2009 (UTC)

I think the thing is that for a lot of less experienced admins, for example, the teacher in a school who does it in their same time, they may have tinkered with their theme in the past (or the person before them may have) and they may not remember what was done. Tinkering with the theme, for example changing the logo or a few colours, may be one of the most For them, we want the upgrade to be as simple as possible. If we cannot keep their old theme working, is there any way we can automate the upgrade? Or do we just encourage them to use one of Patrick Malley's nice new themes, and show them how easy it is to add their own logo to one?--Tim Hunt 20:09, 21 August 2009 (UTC)

I like the sound of these proposals generally.

1) Themes in dataroot: I assume the intention is to completely remove any executable code in these themes?

2) Breaking backwards compatibility: This is the one close to my heart, I just did a quick find and it'll mean we have to update 1003 custom themes (as today stands) which will obviously be a big barrier to Moodle 2.0 upgrade. Having said that I don't necessarily think its a bad idea. Having 'limited' backwards compatibility where by the theme 'just' works might be of limited value (i'm sure we'd need to convert all themes eventually anyway). We need to come up with clear reasons why we need to break backwards compatibility to satisfy less tolerant people than me though ;-)

3) Removing $CFG->themeww/themedir. From anecdotal evidence I think I might be the only person in the world using this feature and I think there are probably better ways to achieve what we are doing, so I am not going to argue very hard to keep it..

I wrote some guidelines to give theme authors a few years ago, which sort of documents the stupid things we get in submitted themes (overriding all styles, uncessarily altering header/footer etc). I put these in Moodle Docs, might be interesting to you or not: CLEO_Moodle_Theme_Guidelines.

In most cases, the 'flexibilty/power' we give theme authors at the moment leads to more breakage due to use of unnecessary modifications in something which could be achieved in a 10 line css file..

--Dan Poltawski 09:45, 22 August 2009 (UTC)


Caching of 'minified' unified stylesheet?

Shortly after Tim did his javascript cleanup, I did a hacky little test of minifying all included javascript and caching it by md5 hash. I didn't get very far, but one thing I did notice was that the minification process was surprisingly slow. Serving the single stylesheet might suffer from this problem also if we do additional processing. Do we need caching of the compiled stylesheets? --Dan Poltawski 19:29, 25 August 2009 (UTC)

Yes, we definitely need caching. I am pretty sure that is in the proposal.--Tim Hunt 21:18, 25 August 2009 (UTC)

Apologies.. just found 'storing of final CSS files in dataroot - caches need to be deleted after each theme revision change; this could significantly reduce server load' --Dan Poltawski 21:56, 25 August 2009 (UTC)

Suggestions for improvement of language (not content):

Replace: It should be easy to upgrade simple older themes. I think it is better if old themes are completely disabled with reasonable fallback mechanism. Why should we display broken themes during upgrade? Imagine users or teachers can select "broken" themes in the course edit page or a profile, I think it is more professional to display only new and upgraded themes in selectors instead of pretending old themes will work 100%. We have to make sure old themes will not cause fatal white screen errors.

With: It should be easy to upgrade simple older themes. It would be better if old themes were completely disabled with a reasonable fallback mechanism. We should not display broken themes during upgrade; users or teachers should not be able to select "broken" themes in the course edit page or profile. It is more professional to display only new and upgraded themes in the selectors. We must make sure old themes will not cause fatal white screen errors.