Manual talk:Coding conventions/CSS

Jump to navigation Jump to search

About this board

Previous discussion was archived at Manual talk:Coding conventions/CSS/Archive 1 on 2016-05-25.

CSS/LESS variable naming guidelines request for comments

9
Volker E. (WMF) (talkcontribs)

We're still in need of documenting variable naming guidelines similar to PHP variables or JavaScript variables. Following up archived discussion at https://www.mediawiki.org/wiki/Manual_talk:Coding_conventions/CSS/Archive_1#Less_and_future_CSS_variable_naming_convention it's now time to settle on a guiding convention. The full property name variant has seen the most supportive feedback due to reduced cognitive load and is therefore the selected proposal.

LESS exampleː

/* Colors */
@background-color-base: #fff;
@background-color-error: #fff36f;
@color-base: #000;
@color-error: #c00;
@color-progressive: #347bff;

/* Sizes & Boxes */
@size-selector: 42rem;
@size-small: 0.1rem;
@border-error: @size-small solid @color-error;

/* Fonts */
@font-family-heading: "Linux Libertine", Georgia, Times, serif;
@font-family-flow: "Helvetica Neue", Helvetica, Arial, sans-serif;

/* Transitions */
@transition-base: 0.2s ease-in;

/* CSS Variables example */
--background-color-error: #fff36f;
--color-error: #c00;


/* Applied (Less): */
.mw-selector {
	background-image: url( selector.svg );
	background-repeat: no-repeat;
	color: @color-error;
	display: block;
	position: absolute;
	top: 0;
	left: 0;
	width: @size-selector;
	height: @size-selector;
	font-family: @font-family-flow;
}
/* Applied (mixed): */
.mw-error {
	background-color: @background-color-error;
	background-color: var( --background-color-error );
	margin: @size-small 0;
	border: @border-error;
	transition: opacity @transition-base;
}

Variables are (with the very exception of calculations and size values) almost always belonging to a certain CSS property, therefore starting with repeating the property clarifies and emphasizes the content of the variable.

ESanders (WMF) (talkcontribs)

This seems like a good first step. +1

JDrewniak (WMF) (talkcontribs)

I think the full variable names aid in comprehension and readability, and are thus more maintainable. +1

EGardner (WMF) (talkcontribs)

+1, having well-thought-out and documented conventions here would be an improvement.

Volker E. (WMF) (talkcontribs)

Going ahead after today's Front-end Standards Developer Group Meeting with general support and agreement on the proposal. Adding the guidelines to the main article. Additionally, enforcing such guidelines with help of a stylelint plugin might be possible, but time-consuming to build. Only close project is part of a SonarQube plugin. Propose to keep it a soft guideline, with explanation on the article and main Product projects supporting it, a.o. WikimediaUI Base, OOUI, Vector, MobileFrontend/Minerva Neue.

Volker E. (WMF) (talkcontribs)
Aron Manning (talkcontribs)

The convetion has an example with "font-family-sans-serif":
@font-family-sans-serif: "Helvetica Neue", Helvetica, Arial, sans-serif;
WikimediaUI Base has "font-family-sans":
@font-family-sans: 'Helvetica Neue', 'Helvetica', 'Nimbus Sans L', 'Arial', 'Liberation Sans', sans-serif;

Which one is normative?

Aron Manning (talkcontribs)
Volker E. (WMF) (talkcontribs)
Reply to "CSS/LESS variable naming guidelines request for comments"
Manbu (talkcontribs)

In the github mediawiki-vendor- master is now a less 1.80 .

But at github is https://github.com/wikimedia/less.php/archive/master.zip (1.8.1 with less 2.53), which is php 7.3 ready

require_once "$IP/vendor/wikimedia/less.php/lib/Less/Autoloader.php";

Less_Autoloader::register();

Volker E. (WMF) (talkcontribs)

@Manbu Sometimes mediawiki-vendor- lacks behind. We're continuously updating, but there's no promise that those two are on same version level at a given point in time.

"The filename of an import statement should omit the .less file extension."

3
Summary by Volker E. (WMF)

Updated all import statements and settled on “has to” on the conventions description.

Jdlrobson (talkcontribs)

Omitting the '.less' file extension causes problems in environments where you are importing different types of files. Currently the code

@import 'foo';

in a folder with 'foo.css' and 'foo.less' will import the latter (foo.less). In Webpack or any JS bundler, it will throw an error as it will assume it is a package with an index.css or index.js file meaning the code cannot be used.

I would thus propose we change this convention. In the web team, we've been using code outside ResourceLoader, for example to create this storybook instance: https://doc.wikimedia.org/MobileFrontend/master/js/ui/?path=/story/*

To workaround this issue in the meantime we have to create extension-less files like this one: https://github.com/wikimedia/mediawiki-extensions-MobileFrontend/blob/master/.storybook/resolve-less-imports/mediawiki.ui/variables I would rather not do that.

Jdlrobson (talkcontribs)
Volker E. (WMF) (talkcontribs)

@Jdlrobson ”Should” or “has to”. My understanding from our last group's conversation was “has to”?ǃ

Indentation failure with stylelint on ZeroBanner/modules/redux.less

1
Summary by Volker E. (WMF)

No new activity for a while. stylelint in production in a huge range of Wikimedia deployed products without this specific issue.

Umherirrender (talkcontribs)

Stylelint always gives an error

on this file: https://gerrit.wikimedia.org/r/#/c/348688/8/modules/redux.less

Running "stylelint:all" (stylelint) task

>> modules/redux.less

>>  2:2   ✖  Expected indentation of 1 tab     indentation                 

>>  2:32  ✖  Expected indentation of 1 tab     indentation                 

>>  4:27  ✖  Expected indentation of 1 tab     indentation                 

Would be nice to know why and how to fix it. The "stylelint-disable" comment for the whole file is not the best option.

Thanks.

Volker E. (WMF) (talkcontribs)

Copying over from Manual talk:Coding conventions/CSS/Archive 1#CSS/Less property order proposal as it's still an unresolved topic:

It would be useful to have a coding convention for the order of the declarations in a declaration block. A consisted order improves the gzip compression and makes it easier to find or insert declarations.

The intuitive order is the alphabetical order. A alphabetical order has the disadvantage that it may disrupt declarations that belongs together, especially when there are blocks of comments.

An other possibility would be the order in the CSS specification. But CSS 2 is the last complete specification.

Is it desired to have an order for declarations? --Fomafix (talk) 11:46, 21 December 2015 (UTC)

https://css-tricks.com/poll-results-how-do-you-order-your-css-properties/ -— Isarra 20:22, 5 January 2016 (UTC)
I'm in favor of adding a CSS guideline for property order, especially when looking at the growing number of possible CSS properties. Here's one example where missing property order has caused unnecessary property duplication (background-size: https://phabricator.wikimedia.org/diffusion/MW/browse/wmf%252F1.27.0-wmf.9/resources/src/mediawiki.ui/components/checkbox.less;2cb36350c88ca6cd214ccdf996bdcba33eac94f9$90
This is an outcome we definitely don't want to see.
The outcomes of the poll results at css-tricks above (45% in favor of grouped by type) are interesting and also the summaries by the author:

I think this is a bigger deal in teams. There has to be a standard otherwise the CSS project-wide looks sloppy. I know that inconsistent styles would wear on my conscience and I'd spend time fixing trivial formatting stuff rather than doing actual work.

Cognitive load factors into this. If you can always count on certain properties being in the same place, you can understand the CSS a bit faster (less scanning). Again, a bigger deal when on a team and you are looking at code you are slightly less familiar with because you didn't write it.

I couldn't agree more with those thoughts (emphasizing done by myself).
Therefore I'd like to come up with a new proposal for a guideline on CSS property order:

CSS/LESS property order proposal

.mw-selector {
	/* ::: Generated Content ::: */
	content: '';
	/* ::: Background/List Style, Color, Filter & Opacity ::: */
	background-image: url( selector.svg ); //or:
	.oo-ui-background-image-svg( /icons/circle-constructive' );
	background-origin: border-box;
	background-position: center center;
	background-repeat: no-repeat;
	background-size: 0 0;
	color: @color-error;
	list-style: square;
	filter: blur( 1px );
	opacity: 0.2;
	/* ::: Display (& Flex Box properties) ::: */
	display: block;
	clip: rect( 0, 0, 0, 0 );
	overflow: auto;
	visibility: visible;
	/* ::: Positioning ::: */
	// Float Model
	float: none;
	clear: both;
	// Positioning Model
	position: absolute;
	top: 0;
	left: 0;
	bottom: 0;
	right: 0;
	/* ::: Box Model (from outside to inside) ::: */
	box-sizing: border-box;
	min-width: auto;
	width: @size-selector;
	max-width: 100%;
	height: @size-selector;
	margin: 4em 2em;
	border: @border-error;
	border-radius: @border-radius-global;
	padding: 0;
	box-shadow: @box-shadow-menu;
	outline: 0;
	/* ::: Typography incl. Print properties ::: */
	direction: ltr;
	hyphens: none;
	font-family: @font-family-flow-text;
	font-size: 1em;
	font-weight: bold;
	line-height: 1;
	text-align: left;
	text-decoration: none;
	text-overflow: ellipsis;
	text-shadow: 0 0 1px #fff;
	text-transform: uppercase;
	vertical-align: baseline;
	white-space: nowrap;
	/* ::: Animations & Transitions ::: */
	animation: orbit 3s infinite linear;
	transition: opacity @transition-global;
	/* ::: Others ::: */
	cursor: default;
	zoom: 1;
	/* ::: Stacking Context ::: */
	z-index: 1;
}
  1. Generated content comes first, as content gets priority,
  2. Backgrounds (in alphabetical order of sub properties), colors, list-styles as similar to background-images, filter and opacity properties are next, as they are together with box sizing the most used and most altered properties and therefore should be on top of the list
  3. Display is next, as it is also a often used and strong property
  4. Box Model in order of outside in, so outline > margin > padding > border and min- & max- values before the fixed property value.
  5. Typographic values in alphabetical sub order
  6. Animations and transitions are last, as they are extending the scope of the styles applied to the element

--VEckl (WMF) (talk) 03:48, 10 January 2016 (UTC)

Krinkle (talkcontribs)

@Volker E. (WMF) I agree some consistency would be useful here. I'd like to lean towards flexibility in this case, though. While consistency matters (to easy quick scanning and reviewing of code), it is also important for code to intuitively communicate its intended purpose. Forcing a very strict order of statements seems to be like it would be 1: impossible to remember, 2: lead to counter-intuitive examples.

Perhaps we can start with something simple and work our way up. One starting point could be to enforce flexible grouping of properties. This is supported by the stylelint-order rules. For example, we could define a handful of groups (5 or 6 at most), but keep both the order within the group and the order of groups flexible at first. We'd only enforce that the related properties are declared together.

From there we can progressively enforce more patterns as we go. For example, we could already require that position goes before any of top/right/bottom/left.

TheDJ (talkcontribs)

I've never been too bothered by this. The amount of duplication inside rules, pales compared to duplication in the hierarchy.

Volker E. (WMF) (talkcontribs)

@TheDJ Not sure, what you're referring to exactly with “duplication”? For property duplication, stylelint has provided us already with a great leap forward, taming most of those in-development code shortcomings.

Reply to "Order of declarations"

Obsessive code formatting on comments

5
Volker E. (WMF) (talkcontribs)
Quiddity (WMF) (talkcontribs)
Volker E. (WMF) (talkcontribs)

@Quiddity (WMF) Was this from you or from another user? From my POV a solution here should be in alignment with other comment styles. I think, when we could come up with one clear hierarchy markup and align the most used files, rest will get aligned by itself. In contrast to majority of other parts in CSS our current (great!) grunt-stylelint solution doesn't cover comments. So we'd need to rely on manual activity.

Quiddity (WMF) (talkcontribs)

From someone else, but I forget where or who. (I think they had logged-off, hence I didn't ask them to post it themselves, or if I could attribute it).

I also found this article (whilst searching for syntaxhighlight references), where the first section is entirely about code-comments, and its advice is the complete opposite of the comment I pasted above: https://medium.com/@MrJamesFisher/your-syntax-highlighter-is-wrong-6f83add748c9

I don't know enough about other code comment styles, to be able to give useful input myself.

Do we use minification of some sort, for the CSS that is sent to the browser? That might be relevant, when deciding between large comment blocks vs inline-micro-comments.

Volker E. (WMF) (talkcontribs)

@Quiddity (WMF) Code comment length is not an issue, comments are not sent over the wire in majority of cases.

Reply to "Obsessive code formatting on comments"
There are no older topics