Moodle
  1. Moodle
  2. MDL-21862

Seperate existing standardold theme into base + other theme

    Details

    • Type: Task Task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0
    • Fix Version/s: 2.0
    • Component/s: Themes
    • Labels:
      None
    • Affected Branches:
      MOODLE_20_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE
    • Rank:
      15736

      Description

      Currently in HEAD there is the standardold theme that needs to be split into two parts.

      The first base that contains CSS information that is required for functionality or core layout but does not contain any set style or frilly bits, and then the second half (the style + frilly bits) need to be split into a new theme that will look like standard white.

      At the same time plugin CSS can be moved back into the plugins and a general tidyup of the whole CSS can happen..... as much as time permits anyway.

      1. MDL-21862.theme.20100319.01.patch
        255 kB
        Sam Hemelryk
      2. MDL-21862.theme.20100323.02.patch
        262 kB
        Sam Hemelryk
      3. MDL-21862.theme.20100324.03.patch
        268 kB
        Sam Hemelryk

        Issue Links

          Activity

          Hide
          Sam Hemelryk added a comment -

          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.

          Show
          Sam Hemelryk added a comment - 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.
          Hide
          Sam Hemelryk added a comment -

          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....

          Show
          Sam Hemelryk added a comment - 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....
          Hide
          Petr Škoda added a comment - - edited

          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

          Show
          Petr Škoda added a comment - - edited 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
          Hide
          Martin Dougiamas added a comment -

          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.

          Show
          Martin Dougiamas added a comment - 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.
          Hide
          Petr Škoda added a comment -

          === #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.

          Show
          Petr Škoda added a comment - === #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.
          Hide
          Sam Hemelryk added a comment -

          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

          Show
          Sam Hemelryk added a comment - 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
          Hide
          Martin Dougiamas added a comment -

          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?

          Show
          Martin Dougiamas added a comment - 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?
          Hide
          Sam Hemelryk added a comment -

          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

          Show
          Sam Hemelryk added a comment - 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
          Hide
          Sam Hemelryk added a comment -

          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
          Show
          Sam Hemelryk added a comment - 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
          Hide
          Martin Dougiamas added a comment -

          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

          Show
          Martin Dougiamas added a comment - 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
          Hide
          Martin Dougiamas added a comment -

          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!

          Show
          Martin Dougiamas added a comment - 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!
          Hide
          Sam Hemelryk added a comment -

          And its in !!
          Now on to the polishing

          Show
          Sam Hemelryk added a comment - And its in !! Now on to the polishing
          Hide
          David Mudrak added a comment -

          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.

          Show
          David Mudrak added a comment - 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.
          Hide
          Martin Dougiamas added a comment - - edited

          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.

          Show
          Martin Dougiamas added a comment - - edited 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.
          Hide
          Petr Škoda added a comment -

          I have discovered some nasty CSS selectors in mod/quiz/styles.css, ex: .categoryinfo

          {padding: 0.3em;}

          please review it once again, thanks

          Show
          Petr Škoda added a comment - I have discovered some nasty CSS selectors in mod/quiz/styles.css, ex: .categoryinfo {padding: 0.3em;} please review it once again, thanks
          Hide
          Sam Hemelryk added a comment -

          Is all done

          Show
          Sam Hemelryk added a comment - Is all done

            People

            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: