Moodle in English: Request for comments: CSS Coding Guidelines

Request for comments: CSS Coding Guidelines

by Dan Poltawski - ಶುಕ್ರವಾರ, 21 ನವೆಂಬರ್ 2014, 3:29 PM

Hi Everyone,

Building on the work of Daniel Wahl, Amy Groshek and others, Frédéric Massart has come up with a draft of a CSS coding guidelines document (MDLSITE-1796):

https://docs.moodle.org/dev/CSS_coding_style

This document has been floating around for a number of years now (see General developer forum: Extending Coding Styles) and we've known the need for CSS guidelines to help us move towards building more consistent implementation of styles across the project.

Please can I request you review this draft document and start giving your feedback here on what is contained within it? We are particularly interested to hear things that you think are completely unworkable with Moodle styling or guidelines which you think are missing.

Once we've agreed these guidelines we will then attempt to start enforcing the style on the implementation of new features into Moodle core.

While these rules won't affect what you do in your own themes (unless you choose to observe them), your experience creating themes and how these guidelines might affect them is immensly valuable.

Finally, implementing these guidelines is just one of a few efforts to improve styling/consistency in Moodle core (the Element library is another) - please can I request that in this discussion we stick to discussing this guidelines document only.

thanks!

Dan

Average of ratings: Useful (4)

Re: Request for comments: CSS Coding Guidelines

by Bas Brands - ಶುಕ್ರವಾರ, 21 ನವೆಂಬರ್ 2014, 5:14 PM

Great to see this document being worked on!

Here's a few suggestion / questions:

1. Bootstrap2 uses camelCase for it's variable, Bootstrap3 uses hyphenated names. At some point Bootstrap2 is going to be quite outdated. What would be the ideal usage of less variables for Moodle?

2. When writing css for plugins I always prefix my css selectors with a selector from the body class. For instance

.path-mod-mymodule .generalbox {
    width: 100%;
}

This prevents module styles to affect other Moodle pages. I guess that should be part of the guidelines.

3. I have seen core Moodle and plugins using Bootstrap2 specific css. For instance span3, container-fluid etc. for creating grids. This locks Moodle into using Bootstrap2 only. When switching frameworks this breaks. Could the guidelines give some guidance on using Moodle / Bootstrap styles?

4. The doc says "Use vendor prefixes only when the supported browser in question does not support the unprefixed property."... Another reason to keep the vendor prefixes is to support older versions of the browser, so perhaps keep them for now until there's a law against using old browsers

(I'd vote for it).

5. The doc says "Colours, font sizes, etc... should never be hardcoded, use a variable instead. ". A Moodle docs page showing a list of Moodle variables that can be used would be great for plugin developers, or is that in included in the element library?

I hope this helps and many thanks for the ongoing efforts improving the Moodle UI!

Average of ratings: Useful (1)

Re: Request for comments: CSS Coding Guidelines

by Dan Poltawski - ಶುಕ್ರವಾರ, 21 ನವೆಂಬರ್ 2014, 6:14 PM

Thanks for the feedback Bas! (Note this is not really my area i'm mostly trying to coordinate, but I am going to attempt to reply anyway )

1. Good point. I don't think its wise trying to come up with a rule that considers what future possibilities might be because we've yet to to define them, so I think you could either remove this rule or keep it in and consider changing it in the future. My vote would be to keep it in because its the defacto standard we have right now and consistency is beneficial.

2. Of course this is tricky because often you do want to make it affect other pages - a proposed guideline for when to do that would be interesting to explore.

3. I think thats going to the sort of thing which sits under the element library work (and then get added to this)

4. Wouldn't that be supporting an unsupported browser? Our general approach is to not specifically develop for non-supported browsers, but also not to specifically break them. I think that guideline fits with our general approach - people can probably interpret the need on a case by case basis.

5. Agree that would be useful, though code would stay more up to date than a docs page - perhaps a variables.less?

Average of ratings: -

Re: Request for comments: CSS Coding Guidelines

by David Mudrák - ಶುಕ್ರವಾರ, 21 ನವೆಂಬರ್ 2014, 6:19 PM

This prevents module styles to affect other Moodle pages.

My +10 for that. It's quite common problem in contributed plugins that we check. The selectors, especially in plugins, must be specific to the context of the plugin.

Average of ratings: -

Re: Request for comments: CSS Coding Guidelines

by Tim Hunt - ಶುಕ್ರವಾರ, 21 ನವೆಂಬರ್ 2014, 6:45 PM

The problem with this is that it goes against the idea of the element library.

We should be thinking in terms of building our UI from widgets that could (potentially at least) be reused elsewhere. Therefore, tying the styling rules for a particular widget to a particular page where it appers is probably bad.

On the other hand, CSS from one plugin interfering with unrelated things is a real problem, so we need some sort of name-spacing. I suggest name-spacing using the component name (like mod_quiz) where the widget is defined, might be better.

Average of ratings: Useful (2)

Re: Request for comments: CSS Coding Guidelines

by sam marshall - ಶುಕ್ರವಾರ, 21 ನವೆಂಬರ್ 2014, 9:01 PM

Guidelines look generally good to me.

I agree with Bas's comment that styles specific to a particular module should begin with .path-whatever or #page-whatever. Additionally, in cases where a class name that is 'owned' by one module may be used on different pages (e.g. filters) then the rule obviously won't have this type of scoping, but the class name itself should include the plugin name.

About vendor prefixes:

1. Consistent with my general policy of being totally against aligning anything in code files, except on the standard indent levels: I really don't like the silly indent. It looks stupid (I know it's supposed to be 'pretty'). It also means that if you remove an obsolete prefix, or add a new one, you potentially have to change all the lines. Prefixed properties should definitely be aligned just like normal properties (I agree they should be either immediately preceding or following the normal one, don't care which),

2. Regarding old versions of prefixes, there are some cases where it might be worth keeping these around at least for a bit, just in case there are people stuck with an old browser version (that happens a lot with Safari, we have a wide spread of versions from ancient Macs; I guess Firefox also sometimes offers a long-term support version?). Not saying there should be a policy to support old browser versions though, just maybe not specifically having a policy to delete them immediately if they have already been put in.

--sam

Average of ratings: Useful (1)

Re: Request for comments: CSS Coding Guidelines

by Gareth J Barnard - ಬುಧವಾರ, 26 ನವೆಂಬರ್ 2014, 8:40 AM

Hi Dan,

Some thoughts:

File naming: allow '-' or '_' to break up words. Especially useful for post fixing, like '-rtl'.

Blocks: four spaces overkill? Bootstrap source has two, so long term better.

Units: is there a good guide / reference / evidence that has solid reasoning behind using em instead of other unit types? I did find a good px to em converter once but I think that also relied on the dpi, so what is the best conversion table to use?

Documentation and comments: If using LESS then use inline '//' comments as they are not placed into the CSS output making it smaller, whereas block '/*' style comments are.

Barebones: Will all the core 'style.css' files be refactored to put the colour definitions in 'base' / 'bootstrapbase' as they are currently a pain to override in some cases?

Right-to-left: Should not be mandatory in the way its worded. Essentially now successfully uses 'grunt-css-flip' to provide RTL support for the most part.

Automatic compliance: When I implemented Essential issue #249 to take out RTL rules for 'grunt-css-flip' I noticed lots of the 'to avoid' examples in 'bootstrapbase'. Will core code be refactored to be a guiding example of best practice?

LESS:

Variables: I agree with Bas and camelCase will be outdated despite my preference for such when JS was debated on the developers forum.

Values and properties: Use mixins for vendor prefixing where possible.

General: When creating themes with LESS and using 'bootstrapbase' as a parent, stick to the same file name when overriding but in another folder so that you can keep track of what maps to what.

Bootstrap 3 is an improvement over Bootstrap 2 so therefore should be considered more modern practice in coding style. Bootstrap 2 style should be considered out dated. Moodle style should be framework independent.

Cheers,

Gareth

Average of ratings: -

Re: Request for comments: CSS Coding Guidelines

by Danny Wahl - ಗುರುವಾರ, 27 ನವೆಂಬರ್ 2014, 11:07 AM

Blocks: four spaces overkill? Bootstrap source has two, so long term better.

the PHP coding standard says 4 spaces (not a tab). I know that my text editor has 1 config and I can't change it based on file type, so that's probably the reasoning. And my text editor can 'fix' 2 spaces to '4' automatically.

Average of ratings: -

Re: Request for comments: CSS Coding Guidelines

by Gareth J Barnard - ಗುರುವಾರ, 27 ನವೆಂಬರ್ 2014, 6:15 PM

Hi Danny,

I have no intent of being a flea on a dog's back arguing with you over spaces. Two spaces is just a question, if the PHP guidelines say four then so be it. Perhaps they should change too.

If your editor does not support different attributes per file type then change it to one that does. For tools I prefer the Ada programming language principle of 'you get what you ask for'. This happens with numbers where you can have whatever size of number you want despite the constraints of the underlying architecture. Therefore the tool needs to bend to the requirements of the developer, not the other way around.

Cheers,

Gareth

Average of ratings: -

Re: Request for comments: CSS Coding Guidelines

by Dan Poltawski - ಗುರುವಾರ, 27 ನವೆಂಬರ್ 2014, 9:52 PM

Hi Gareth,

I agree with '-' to break up words (will suggest edits).
Four spaces matches the php style (and existing CSS style) as mentioned by Danny. We won't be changing the style rules for php and I don't see any reason for CSS to be different.
I am not the right person to answer but the css spec says about em: "Equal to the computed value of the 'font-size' property of the element on which it is used."
We use recess --compress in our less compilation in core, so no comments end up in the css (AFAIK)
I am not familiar with css-flip, but in Moodle core I think we want to require that designers give RTL significant consideration and test RTL rather than use the automated tool and forget. (Will be interested to hear RTL users to hear if they find this tool is sufficient in all cases).

Average of ratings: -

Re: Request for comments: CSS Coding Guidelines

by Gareth J Barnard - ಶುಕ್ರವಾರ, 28 ನವೆಂಬರ್ 2014, 12:39 AM

Thank you for your reply Dan, all good discussion .

With '4', I did start off with 'recess':

But have now moved up to 'grunt-contrib-less' (https://www.npmjs.org/package/grunt-contrib-less) as 'recess' is no longer maintained any more: https://www.npmjs.org/package/recess - last update was a year ago.

Average of ratings: -

Re: Request for comments: CSS Coding Guidelines

by Mary Evans - ಶುಕ್ರವಾರ, 28 ನವೆಂಬರ್ 2014, 7:29 AM

Dan ,

Re: Answer No. 3 read this...

http://kyleschaeffer.com/development/css-font-size-em-vs-px-vs-pt-vs/

And also this blog post by Terrll Thompson who is a guru when it comes to accessibility...

http://terrillthompson.com/blog/589

Cheers

Mary

Average of ratings: -

Re: Request for comments: CSS Coding Guidelines

by Mary Evans - ಗುರುವಾರ, 27 ನವೆಂಬರ್ 2014, 8:13 PM

The problem about CSS Guidelines and Moodle is that these should have been in place when Moodle was first conceived, along with all the other Guidelines necessary for a mammoth project as this.

There are many ways of writing CSS, but since we have been advised to use 4 spaces, and I like this as it defines clearly the rules from the selectors. If we continue to write nice CSS in this manner within the LESS files, the fact it gets compiled into minified CSS is no problem. However there are errors in our LESS files which have been highlighted using CSSLINT, but that is another story and so I will not discuss this here, as it is possibly caused by incorrect CSS markup in Bootstrap 2.3.2.

Another item did crop up when using CSSLINT and that was about CSS selectors should use hyphens - and not underscores _ to extend a selector name. In Moodle, however, we have a few of these CSS selector names that use underscores and not hyphens. So perhaps we should think about cleaning those up first?

EXAMPLE: https://github.com/lazydaisy/moodle/blob/master/theme/bootstrapbase/less/moodle/admin.less#L240

Cheers

Mary

Average of ratings: -

Re: Request for comments: CSS Coding Guidelines

by Gareth J Barnard - ಗುರುವಾರ, 27 ನವೆಂಬರ್ 2014, 8:27 PM

RE: "should have been in place when Moodle was first conceived"

Whilst I agree that guidelines from the start are good, its really a matter of 'Chicken and Egg' theory. What comes first? The processes of a project that is starting out with an unknown chance of success. True, standards can be defined from the outset if a project is funded for a specified length of time and deliverable, but with Moodle it has been the evolution of an idea. Therefore I can understand why processes were not defined earlier, after all why expend effort into something when the main element has not succeed yet? And at the start the developers are co-located, so decisions could be made quickly and understood by all without the full blown effort of a document. Also with the main effort needing to be on the success of the project.

But I do think that once a set of guidelines has been defined, then all existing code should be updated to it within a release cycle. Not because of making it conform, but as a structured way of applying the 'knowledge' that has been put into the guidelines to eliminate bad practice that may exist in the code as it stands. Therefore as a preventative measure against bugs and as a 'practice what you preach' for all developers so that you don't repeatedly expend effort correcting them on improvements.

Average of ratings: -

Re: Request for comments: CSS Coding Guidelines

by Mary Evans - ಗುರುವಾರ, 27 ನವೆಂಬರ್ 2014, 8:52 PM

Well 14 years down the line is a hell of a long time!

Average of ratings: -

Re: Request for comments: CSS Coding Guidelines

by Dan Poltawski - ಗುರುವಾರ, 27 ನವೆಂಬರ್ 2014, 9:58 PM

The problem about CSS Guidelines and Moodle is that these should have been in place when Moodle was first conceived, along with all the other Guidelines necessary for a mammoth project as this.

Hindsight is a wonderful thing.

We have experienced the same thing with php code, the benefit of introducing it later is that slowly but surely code changes and gets updated to the new conventions and newcomers know how they should approach things.

Another item did crop up when using CSSLINT and that was about CSS selectors should use hyphens

Yes, actually I was expecting more comments about the selectors section in this thread. Do you think its workable?

Average of ratings: -

Re: Request for comments: CSS Coding Guidelines

by Mary Evans - ಶುಕ್ರವಾರ, 28 ನವೆಂಬರ್ 2014, 6:49 AM

I think it is very workable, fortunatly the body classes in Moodle Themes use hyphens so why not other areas of Moodle CSS selector names?

The thing about CSS selectors is not so much how they are written but more about what they say, as the name needs to portray what role they have in shaping an element.

Also if you notice Bootstrap 3 have opted to go down that route, and moved away from the camelCode for variable names which, unfortunately, we have to put up with using Bootstrap 2.

That said, as with all Guidelines they should not be written in stone as we need to be flexible too. We also need to give better examples of CORRECT and INCORRECT code in the CSS Guidelines that we have now, as reading through them they contradict themselves in a few places.

Just a few personal thoughts...hope they answer your questions?

Mary

Average of ratings: -

Re: Request for comments: CSS Coding Guidelines

by Dan Poltawski - ಶುಕ್ರವಾರ, 28 ನವೆಂಬರ್ 2014, 8:14 AM

Thanks Mary - I agree with all of your comments - there is also a balance to be made always keeping with the existing style.

Do you have examples of where the document is contradicting itself? We should fix all of these ASAP.

Average of ratings: -

Re: Request for comments: CSS Coding Guidelines

by Mary Evans - ಶುಕ್ರವಾರ, 28 ನವೆಂಬರ್ 2014, 10:35 AM

Perhaps it's because I don't like the layout and general look of the Moodle Docs. But that said the Examples for the CORRECT and INCORRECT way of writing CSS markup is at best not really that clear. For example, the correct and incorrect markup might be this:

Correct:
.selector {
    color: #f00;
}

Incorrect:
#selector { color: #f00;}

At first glance this would suggest to me that it is wrong to use an ID selector.

But in actual fact the 'incorrect' markup is in the way all of the CSS is on one line, like we see in Base theme stylesheets.

So the Incorrect example should look like the Correct one, but befor it is corrected.

Correct:
.selector {
    color: #f00;
}

Incorrect:
.selector { color: #f00;}

And if the Examples need to be different then these too should match in some way

Correct:
.top-menu li a {
    color: #f00;
}

Incorrect:
.topMenu li a {
    color: #f00;
}

Average of ratings: -

Re: Request for comments: CSS Coding Guidelines

by Dan Poltawski - ಶುಕ್ರವಾರ, 28 ನವೆಂಬರ್ 2014, 8:24 AM

Hi Everyone,

Unless something major comes up, I intend to put a close on initial feedback for these guidelines a week from now (Friday 5th December 2014) and proceed to move the proposal forward. So if you have been holding of commenting please comment ASAP, or if you know someone who really should be commenting on this proposal - please draw their attention to it!

Of course, we'll always be happy to refine and incrementally improve these guidelines as time goes on - but this will give us the initial basis to start with (and also rules which linting tools will complain about..).

To be honest, I am pleasantly surprised because I thought this might be a more contentious issue and it seems not from the comments so far - all the better to have the consensus standardised!

Average of ratings: -

Re: Moodle in English: Re: Request for comments: CSS Coding Guidelines

by Derek Chirnside - ಶುಕ್ರವಾರ, 28 ನವೆಂಬರ್ 2014, 9:55 AM

This is a pretty positive approach Dan, well done.

However, I do not think our communication channels are well oiled with the millions on this forum. And the habits of some of us are a bit poor.

Maybe as well a Twitter Channel post? Maybe a news item from Helen??

Derek.

Just testing a response via phone.

Average of ratings: -

Re: Moodle in English: Re: Request for comments: CSS Coding Guidelines

by Dan Poltawski - ಶುಕ್ರವಾರ, 28 ನವೆಂಬರ್ 2014, 3:29 PM

Maybe as well a Twitter Channel post? Maybe a news item from Helen??

As this will only have an impact on people contributing CSS code to Moodle I don't think its good to create more noise for most of our community who it has no relevance to on the main news items.

But its been tweeted by @moodledev and has come up on the frontpage via the useful posts filter and @moodleorg as well. Its also been cross posted onto the General developer forum and Integration exposed. These are the best channels we have for people contributing code I think.

Average of ratings: -

Re: Moodle in English: Re: Request for comments: CSS Coding Guidelines

by Gareth J Barnard - ಶುಕ್ರವಾರ, 28 ನವೆಂಬರ್ 2014, 7:23 PM

Hi Dan,

Sorry for the late thoughts, but these occurred to me this morning:

Will comments be in line with PHP, i.e. Capitals to start and full stops at the end. No colons at the end of lines in single line comments.
Line length: 80 characters maximum? Or is that commits? Perhaps its 128 characters. I just use Code Checker like a useful SatNag tool.

Was just thinking about the lovely Code Checker report . Which hopefully will be updated when published .

Cheers,

Gareth

Average of ratings: -

Re: Moodle in English: Re: Request for comments: CSS Coding Guidelines

by Mary Evans - ಶುಕ್ರವಾರ, 28 ನವೆಂಬರ್ 2014, 9:16 PM

Line length for commits is now 72.

Line length for code is 132 from memory.

Average of ratings: Useful (1)

Re: Moodle in English: Re: Request for comments: CSS Coding Guidelines

by Gareth J Barnard - ಭಾನುವಾರ, 30 ನವೆಂಬರ್ 2014, 9:26 PM

Hi Dan,

I have just come across this in '/mod/quiz/styles.css' in M2.8.1:

#page-mod-quiz-view.dir-rtl .generalbox#feedback h3 {
    text-align: center;
}

So should there be something about the place of 'dir-rtl' on body identifiers as there is with classes?

Cheers,

Gareth

Average of ratings: -

Re: Moodle in English: Re: Request for comments: CSS Coding Guidelines

by Mary Evans - ಭಾನುವಾರ, 30 ನವೆಂಬರ್ 2014, 10:35 PM

I suppose there should be, as there will always be someone adding them in the wrong place...lol

A rule of thumb is to look for and read the body id and class selectors in the source code for the page you need to style differently. This way you will see the id comes before the classes, since ID's take presidence over CLASS selectors. So the class selector like .dir-rtl needs to be tagged on at the end of the body ID, but befor other body classes.

Average of ratings: -

Re: Request for comments: CSS Coding Guidelines

by Danny Wahl - ಸೋಮವಾರ, 1 ಡಿಸೆಂಬರ್ 2014, 8:37 AM

Dan,

thanks for all your work on this. I see a few areas that could be defined:

1) media queries

where should they be defined?
what components are 'allowed' to define them?
what rules should a media query define? (change font sizes, or just arrange display)
how should a media query be defined: screen size, px, em, etc...
what break points are 'officially' supported?
is there anything to avoid in a media query?
@print?

2) 'the grid'

Should we mention anything like 'the grid is holy, the grid is sacred. thou shalt not mess with the grid.'? I'm assuming a component author will/should be encouraged to use a grid layout- which is beyond the scope of this proposal, but if a component is not grid-ified and the grid is not playing nice with the component (or vice versa) how do we handle it?

3) utility classes

the section on .ie could be expanded to cover all Moodle utility class selectors so that authors know not to reinvent the wheel- another example that comes to mind is the bootstrap .pull-left and .pull-right as well as the moodle .clearfix

Average of ratings: -

Re: Request for comments: CSS Coding Guidelines

by Mary Evans - ಸೋಮವಾರ, 1 ಡಿಸೆಂಬರ್ 2014, 8:59 PM

Also we should, were possible, quote examples from the W3C

http://www.w3.org/Style/CSS/

Average of ratings: -

Re: Request for comments: CSS Coding Guidelines

by Gareth J Barnard - ಭಾನುವಾರ, 30 ಆಗಸ್ಟ್ 2015, 8:52 PM

With reference to MDL-51229, the LESS needs something on single line comments '//' -> https://docs.moodle.org/dev/CSS_coding_style#LESS - which are valid in LESS but have the effect of not being transposed to the CSS output.

Average of ratings: -

Re: Request for comments: CSS Coding Guidelines

by Mary Evans - ಸೋಮವಾರ, 31 ಆಗಸ್ಟ್ 2015, 8:11 AM

Go on Gareth, I dare you to ADD it...I've just been changing stuff in that page myself. The language is so stuffy...I hate it. It needs some life breathing into it. Some of the examples are absolutely dire! They should be real life ones that we see everytime we open the LESS & CSS files.

Average of ratings: -

Re: Request for comments: CSS Coding Guidelines

by Dan Poltawski - ಬುಧವಾರ, 6 ಜುಲೈ 2016, 4:59 PM

(Apologies for re-awakeing quite an old thread..)

I have made some minor edits to the proposed style doc, re-read your comments here and moving forward with getting this doc formally the Moodle style. See my comments on the tracker about the status.

In addition i have been working on linting our styles using grunt and stylelint in MDL-55058 following this style to keep us consistent moving forward.

Average of ratings: -

Re: Request for comments: CSS Coding Guidelines

by Mary Evans - ಬುಧವಾರ, 6 ಜುಲೈ 2016, 9:56 PM

Hi Dan,

Good job..thanks for that?

Mary

Average of ratings: -

Re: Request for comments: CSS Coding Guidelines

by Mary Evans - ಗುರುವಾರ, 7 ಜುಲೈ 2016, 3:34 AM

Hi Dan,

I have just noticed something that was missed

https://github.com/danpoltawski/moodle/compare/4ce900a6d89d82b80cff56b07b9827db79783ef2...MDL-55058-master#diff-107420693478a76e1b5076693b56f776L413

The 0px in the above link needs to be just 0 no need for px. I am not sure if those can be targeted though?

Cheers

Mary

Average of ratings: -

Re: Request for comments: CSS Coding Guidelines

by Dan Poltawski - ಗುರುವಾರ, 7 ಜುಲೈ 2016, 4:39 AM

Yes, they can

Note this branch is very WIP. At this point i'm just preparing the style changes which can be done with no difference to the less built output.

Average of ratings: -

Re: Request for comments: CSS Coding Guidelines

by Mary Evans - ಗುರುವಾರ, 7 ಜುಲೈ 2016, 5:55 AM

oh goody! I would love to learn how to do that.

And yes I do appreciate this is a WIP so I will watch the trpacker to see its progress.

Thanks

Mary

Average of ratings: -

Re: Request for comments: CSS Coding Guidelines

by Dan Poltawski - ಶುಕ್ರವಾರ, 29 ಜುಲೈ 2016, 4:53 PM

As mentioned in the General Developer forum - the CSS coding style is now approved. Thanks everyone for your comments!

Average of ratings: Useful (2)