Details
-
Type:
Improvement
-
Status:
Closed
-
Priority:
Minor
-
Resolution: Won't Fix
-
Affects Version/s: 2.0
-
Fix Version/s: None
-
Component/s: General
-
Labels:None
-
Affected Branches:MOODLE_20_STABLE
Description
Currently in Moodle every page gets a id and class added to the <body> element that represents the structure leading to the page being called:
e.g. mod-chat
grade-report-grader
After talking to Petr we think this should be changed to use hyphens rather than underscores for consistency, underscores being the preferred character.
The only foreseeable problem with this is that the chat module has directories using underscores (which I've been told is a no-no anyway) so these would need to be renamed at the same type.
If there are no arguments to doing this I'll look to tackle it while I work with all of the CSS for the base theme.
Issue Links
| This issue has been marked as being related by: | ||||
| MDL-21862 | Seperate existing standardold theme into base + other theme |
|
|
|
Activity
this should be changed to use hyphens rather than underscores
??? You mean the opposite, yup? i.e. changing to underscores ?
Apart from the chat module we also have a bunch of files containing underscores (I really never hear it being a "no-no"), more noticeably, all the blocks, all the xmldb editor pages, some (old) themes and some question formats:
find . -name "*_*" -type d -and -not -path "*_utf8*"
Also, I remember some code from 2.0 blocks using hypens to determine where one block must be displayed and, potentially, different javascript code can be also using that here and there in one "harcoded "way.
So, not really sure if it's a good idea or so "critical" in order to be changed, more if think underscored never have been forbidden before now (nor in core nor in contrib).
So I feel inclined to say -1...ciao ![]()
this should be changed to use hyphens rather than underscores??? You mean the opposite, yup? i.e. changing to underscores ? Apart from the chat module we also have a bunch of files containing underscores (I really never hear it being a "no-no"), more noticeably, all the blocks, all the xmldb editor pages, some (old) themes and some question formats:
find . -name "*_*" -type d -and -not -path "*_utf8*"
-1000000 from me.
Some of these class names have been there for years. We only added some more in Moodle 2.0.
Therefore changing them now would break lots of people's CSS.
Yes, they will have to change their CSS a bit for Moodle 2.0, but mostly move it into new files. Gratuitouly breaking all their CSS selectors is pointless.
Note that these are page-types, like mod-quiz-view. They are not in the same name-space as component names like mod_quiz.
1/ we can keep both the mod-quiz-view for mod/quiz/view.php and one mod_quiz for all mod/quiz/*, the problem is that people started putting "" everywhere in css which looks clearly wrong to me; so page types would still use the "" but the rest should imo use underscores.
2/ We use underscores to separate parts in component names, I would prefer to keep them only for that. We must not allow "_" in module names, because it would break our
code completely.
3/ "_" in block names encourage long names also, this is a problem because then there are problems with db table names.
looking at the classes, the current forum has: body #mod-forum-view .mod-forum
and I really think it should be: body #mod-forum-view .mod_forum
because it really is the component "mod_forum". I always had problems distinguishing the page address "#mod-forum-view" from the component class ".mod-forum".
I would like to remind you we already do this for blocks - each block is encapsulated in class, ex: div #inst4 .block_global_navigation_tree sideblock
In any case we have the unique chance to fix any mess and inconsistencies in CSS now, we will not be able to break BC again anytime soon...
another example is: body #course-report-outline-index .course-report-outline
it should be instead: body #course-report-outline-index .coursereport_outline
so +1000002 from me to include the full component name as a class everywhere and keep the current "page address" with dashes, and no I do not care at all abut BC in custom CSS because this is a bug for me and now is the only time to fix it ![]()
My -1000 for this proposal.
Leave it as it is: where hyphens to replace all slashes in paths, and underscores to remain where they already exist in directory names (like block_global_navigation_tree).
This is PERFECTLY consistent and predictable already so I can't see any justification for changing it.
Theme authors know nothing about Moodle module internals and shouldn't have to. But they can see paths in the URL.
Mine is bigger:
+9999999999999999999999999999999
Why the hell do you all use those big numbers to vote? I hate to take the calculator to see how this is going! Grrr!
Ciao ![]()
When I use some "widget" in my module I have to known on what pages exactly it is used and then make a list of #mod-book-view, #mod-book-export, etc. If we had a simple standardised "component class" I could simply use CSS class ".mod_book". This is exactly what we have for blocks already, each block instance is inside a div with "component class".
In themes we can not simply define rule that is applied to output of one plugin and that is a bug imo. Workaround is to manually enclose the output of each plugin with custom div with class, but why should the core not do this?
real world example:
1/ in forum we have
mod/forum/view.php #mod-forum-view and .mod-forum
mod/forum/index.php #mod-forum-index and .mod-forum
2/ the unread class may be theoretically used on several pages, but unfortunately it is defined as:
#mod-forum-index .unread {
background: #9EBEFF;
}
#mod-forum-view .unread{
background: #9EBEFF;
}
why is that? We want to highlight any unread messages in all forum output. So I would personally expect to do just:
.mod_forum .unread{
background: #9EBEFF;
}
Now imagine we want to highlight the unread posts in user profile or dashboard. How do you do that?
At present you would to invent new classes or use mod-forum and add a new CSS.
There is also the forward compatibility issue - if we use the hardcoded page ids in css it breaks after improvements and refactoring, imagine you want to add unread count into some new page, at present developer would just add new #mod-forum-something. The problem is that all themes derived from standard will get the colour from it, instead of the already overridden colour from #mod-forum-view - this is insane!
Now let's have a look at the CSS used in glossary, the selectors either use the #mod-glossary-xxx or invent custom classes. Hmm, is this optimal? I guess not, because very often developers were creating colliding class names. This was not a huge problem because the plugin stylesheets were loaded manually, but we are now loading all CSS in one huge files and this started to matter a lot.
If you do not believe me have a look at the css for glossary:
.entryboxheader {
border-color: #BBBBBB;
}
.entrybox {
border-color: #BBBBBB;
}
.entry {
}
It has to use something like:
.mod_glossary .entry {
}
Now let's talk about backwards compatibility, I proposed to replace .mod-quiz with .mod_quiz, how often did we use this class selector in CSS (1.9 official distro)?
- search for ".mod-" - 20 hits
- search for "#mod-" - 240 hits
This tells us that our CSS is not component based, but instead page url based and the proposed change would have very little backwards compatibility problems. Unfortunately the current CSS selectors in theme CSS are so vague or ambiguous that they need to be fixed and that would break most of the themes anyway, but in the long term they could be a lot more stable.
Martin says that themers do not know anything about our plugins, and that the only thing they should care about are page URLs. That seems wrong because imagine you want to change the colour of unread messages in forum - you are not going to find out which pages inside mod/forum are using it and which other parts of moodle are actually printing the unread counts (profile, dashboard) - it can be any function inside mod/forum/lib.php that can be called pretty much from any part of Moodle. Instead in 2.0 we could refactor all plugin CSS and start using .mod_forum selector for everything, then designers would just say:
1/ ok it is forum stuff, so it must have .mod_forum prefix (only if I want to make one page in forum module different I would use the #mod-forum-view selector)
2/ find the class I want to override
3/ search the default theme for examples
4/ put overrides into your theme
5/ the change would appear on all new forum pages added in the future too
Hmm, but we also may have unread tracking in other modules, are we going to make lists of all pages again like in forum, or invent .somethingelseunread classes all over the place when we could simple use component class and standardised .unread class for all the modules?
===============================================================================
I hope I have proved that most of the page id selectors (#mod-forum-view) should be replaced by something else, because they are actually bad for theme developers. One candidate is current .mod-forum style (used very little), the second one is new .mod_forum class style I proposed while discussing some chat issues with Sam (reason is to keep consistency with the rest of plugins infrastructure and also to make it easier to explain and use).
- search for ".mod-" - 20 hits
- search for "#mod-" - 240 hits This tells us that our CSS is not component based, but instead page url based and the proposed change would have very little backwards compatibility problems. Unfortunately the current CSS selectors in theme CSS are so vague or ambiguous that they need to be fixed and that would break most of the themes anyway, but in the long term they could be a lot more stable.
eh, forgot to describe the actual problem in mod/chat - the actual UI files are stored in subfolder, that means there is not ".mod-chat" class there, instead there is only ".mod-chat-gui_ajax" and this means you can not define shared CSS for all chat UIs - repeating the class or id selector looks wrong.
Please, could all those people with huge negative numbers explain how to deal with these issues I outlined?
1/ forum unread class + unread in other modules
2/ glossary entry colliding class
3/ chat UI subdirectories
My +1 for the style proposed by Sam & Petr. Page-based selectors (#mod-workshop-view) should be used in very rare cases only. We should encourage module-scope selectors almost everywhere. And then, using .mod_workshop is very intuitive. Those page-based selectors are tricky because you actually never know if you have customized all places or not.
I have used this style in workshop, actually. My mod/workshop/styles.css uses exclusively selectors like
.mod-workshop .submission .title .mod-workshop .userplan
etc. The point is that widget like submission (and its title) can appear at many pages in workshop (view.php, assess.php, submission.php etc). But there should be a single selector to access it. If a customized theme overrides the standard workshop theme and a new script publish.php is added, submissions displayed at this new script will automatically use the custom theme. If I used explicit list of pages (#mod-workshop-view etc.), the custom theme designer would not be happy.
.mod-workshop .submission .title .mod-workshop .userplan
thanks David, your new CSS in /mod/workshop/styles.css is the best example how the stylesheets should look in Moodle 2.0 ![]()
Uhm,
I agree we should be using more and more "page class" selectors in CSS (20 in your count) instead of "page id" ones (240). 100%
That's the way to go in your 1), 2) and 3) problems above, to get selectors working along all the module, independently of the page (index/view/whatever). 100%
What I'm not getting (hence my -1) is the need to transform current "hyphened" class/id selectors by their "underscored" counterparts. More when the underscores clearly conflict with folder/file names and hyphens don't. And more where, potentially (note it's potentially), changing that can break js/themes far more than continue using hyphens.
So, I don't see the need for the change (hyphens=>underscores). I agree we must invert those 20/240 uses completely, as said. But I don't get which is the key for the proposed names change/how it would be better.
Ciao ![]()
to Eloy:
1/ current .mod-forum is derived from the actual directory and file name === it returns useless info for mod/chat subdirectories and also any plugins that are not at the to of site directory (such as .course-report-something)
2/ we reference plugins by component names EVERYWHERE in Moodle 2.0, that was the fundamental part of my major plugin refactoring and improvements; the CSS class name is one of the last missing bits - we can either "fix" the .mod-forum or get rid of it completely and replace it by new code that looks at the list of known plugins and automatically set's up the correct ".mod_forum" class for each forum page. We will have to put some divs with this class into several other areas of moodle such as profile, dashboard, gradebook plugins - it would be imho bad to have to use the old ".mod-forum" in code like this:
function forum_render_funny_stuff_for_dashboard(...) {
//echo '<div class="mod-forum">'; // inconsistent
echo '<div class="mod_forum">'; // mostly consistent
...
echo '</div>';
}
when everywhere else we reference everything belonging to forum as legacy "forum" or new "mod_forum". Everything else except mod/ has to use the prefix with plugin type, exceptions are not allowed.
function coursereport_online_some_funny_output_outside_of_report_page(...) {
//echo '<div class="course-report-online">'; // ugly & totally inconsistent
echo '<div class="coursereport_online">'; // fully consistent
...
echo '</div>';
}
function forum_render_funny_stuff_for_dashboard(...) {
//echo '<div class="mod-forum">'; // inconsistent
echo '<div class="mod_forum">'; // mostly consistent
...
echo '</div>';
}
function coursereport_online_some_funny_output_outside_of_report_page(...) {
//echo '<div class="course-report-online">'; // ugly & totally inconsistent
echo '<div class="coursereport_online">'; // fully consistent
...
echo '</div>';
}
to Eloy:
3/ blocks are already doing these .block_blockname classes on the course page; if you like the .mod-forum we should rename it to .blocks-blockname or else it is an inconsistency (==bug)
David,
I think nobody is discussing that selectors based in page class are better (apply to the whole module) than page id ones. As commented, +1 for using them all the time (but exceptions that surely will exist, for example, in glossary/print I'd want to have different selectors than the ones in the rest of the module.
Petr,
I see (and think it's highly positive) all the improvements/normalization done in all the plugins for 2.0. Really cool. What I don't see yet is why you see:
class="course-report-online"
ugly and totally inconsistent and
class="coursereport_online"
fully consistent.
Not being a developer I'd say that the first one is clearly easier to remember as far as it's the exact base path of the plugin and not something (the component name) that only developers (and not all, hehe) use to know.
In any case, it's true that, if we are already breaking all old themes completely AND if there isn't any possible conflict in names because the use of underscores and the names of the components this could be a good moment to do the change. After all, I'm sure that theme designers will use CSS inspectors so they will see one page being class="mod_forum" and not class="mod-forum", they are supposed to be specialists on that. And we (you) already are changing all the core CSS so there isn't a big difference from the POV of our amount of work, is it?
Ciao ![]()
Eh!
I'm the only that voted with a minimal -1 (that in fact was going to be only -0.5 originally). So you don't have to convince me!
Other ppl has voted way stronger than me! :-P
you do get_string('xx', 'coursereport_outline'); all functions start with coursereport_outline_, install and upgrade scripts use coursereport_outline, capabilities are coursereport/outline:xxx, etc.
the only place where you get "course-report-online" is the CSS - this is my definition of "inconsistent"
I have edited the issue title because we all agree that the #mod-forum-view should be kept as is because it is designed to reflect only the url of page
Wow alot of feedback here for one day!
I REALLY like where this has gone, I certainly see the need to keep -'s for url reflection and would REALLY love to have the body class reflect the plugin so much so that I have been working with a patch when converting CSS in anticipation... so +1 from me for this change.
Just catching up here.
The arguments above about class vs id are not really arguments. Everyone agrees we need to use selectors with widest possible scope for the job and if themes aren't doing that well enough then that's because we never made that clear enough. Hopefully something we will do better in the new base/core themes.
I never liked the frankenstyle plugin-naming scheme (coursereport_outline) ... it started as an ugly hack to get around our own limitations in capability names and then took off like a virus all over the code. I really don't think we should inflict it on themers. For themers it would be much more ugly to have hyphens on body ids and underscores in body classes, and then hyphens again in non-body classes!
Some solutions for the issues:
1) Hyphens vs underscores. Keeping the hyphens everywhere will make things easier for all the many thousands of themers who will be cutting and pasting and rebuilding their themes for 2.0, who will be already cursing and damning those arrogant Moodle developers for making their life harder.
2) The mod-chat-gui_ajax issue. I know $CFG->pagepath is deprecated and even $PAGE->set_pagetype isn't ideal (because class is always one word shorter than the id), even though we do need to have that functionality to specify a virtual path for some exceptions. I think the best answer is to make initialise_standard_body_classes() in pagelib.php always produce standard classes for all the parent paths back to the root:
.mod-chat-gui_ajax .mod-chat .mod
.course-report-overview .course-report .course
This will work completely consistently across all of Moodle, whether the pages are in a plugin or not. It has backward compatibility while adding more functionality.
3) The dashboard widget issue. I see nothing wrong with a forum widget using .mod-forum in the CSS if it has to. It won't happen very often so I think PHP developers can cope with the occasional mental strain of writing mod-forum instead of mod_forum. ![]()
My +1 to put all of this effort into creating a stable and well documented theme engine so that theme designers like me can get to work on 2.0 before April. Nobody will care about how well the body classes match the underlying syntax in the PHP if we're still installing with the Standard theme.
I realize that my position biases me a bit, but I think that these 20 new themes are going to be one of the biggest new features for users in the 2.0 release. They matter and I'm not going to code crap to save time. That will kill me in tickets on the backend. Therefore, I need time - an inordinate amount of time since I don't even yet fully know what I have myself into.
Please stop changing things that will break more things than they will fix just because it seems like a good time to change them.
Added some watchers