Manual talk:Coding conventions/CSS/Archive 2

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

• "No spaces on the outside of the parameter lists in function invocations, mixin uses and mixin definitions"
• "same as for url( image.png ) in CSS"
• "background-image: url(@url);"

If "outside of the parameter lists" refers to the inside of parentheses (which it seems to), there is a contradiction. When the first quote was added, the spacing in url() was different.

A Gerrit test build reported an error and sent me over to the section about constructed class names, which suggests that for each css class you add a single-line comment followed by an asterisk and the claas name. Whch is precisely what I did yet here I am.

Is it possible that we shouldn't use single-line comments as suggested there? Or maybe that they should be multi-line comments, or only if that's what you're using already?

It looks like the change you're referring to is https://gerrit.wikimedia.org/r/c/mediawiki/extensions/PageForms/+/917159.

@Rand(1,2022) The rule is documented at https://github.com/wikimedia/eslint-plugin-mediawiki/blob/v0.5.0/docs/rules/class-doc.md. I believe the problem is that your comment is detailing the local variable assignment on line 652, instead the actual variable addClass() call on line 653. The warning is emitted from line 653. If you place the comment there instead, I think the linter will start to pass.

Thanks, that sounds like a more plausible explanation. I will give that a try.

In 2015, S added the already-long-recommend practice of avoiding clashes between extensions' styles by prefixing the class names. However, @Krinkle has reverted this, claiming:

The ext- prefix has afaik never been established. It was written here in 2015 and since then adopted in 3 extensions presumably only from reading it here. We generally don't distinguish between something being a core component or an extension component. That's what namespaces are for, e.g. mw-foo the "foo" feature, whether bundled, core or extension is not important.)

Let's discuss?

Basically I do appreciate the strategy of prefixing selectors, especially class names.

We have a huge pile of selector orgins, which are assigned to cause special effects and special decorations:

• MW core
• MW extensions
• Skin and navigation elements, page structure
• Site gadgets
• User scripts
• TemplateStyles within projects
• So-called “global gadgets” and “global templates” and “global modules”
• Dynamically generated classes and ID, like headline IDs etc.
• This local MainPage is telling me: class="ext-discussiontools-replytool-enabled ext-discussiontools-newtopictool-enabled ext-discussiontools-sourcemodetoolbar-enabled ext-discussiontools-topicsubscription-enabled skin-vector mediawiki ltr sitedir-ltr mw-hide-empty-elt ns-0 ns-subject page-MediaWiki rootpage-MediaWiki skin-vector-2022 action-view"

There is no registry for class names visible, neither for MW nor anywhere else.

• By reserving mw- and ext- at least no conflict is expected with those.
• ext-extName- is rather robust since extension names are unique and quite stable, with exception of cite and echo (later CiteThisPage and Notification).
• Since ext-extName- is unique and easily tracked back to the extension, each extension just needs to administrate their own sub-identifiers within their business, not interfering with anybody else, even not checking entire core whether this word has been used. And core does not need to take care whether there is or ever might be an extension with such a name.
• We do need more prefix policies, not less.
• Since there is no central MW core selector registration, and everybody can setup any extension under any name not yet used for extensions, even within MW name conflicts may occur. Looking at Category:All extensions I am not sure that they never ever collide with any core functionality. E.g. arrays authors checkpoint collection comments contributors counter look like regular English words which might easily clash with any purpose of core mw- selectors.
• If ext- policy shall be terminated, the replacement is at least mw- (as modified), not simply omitting any prefix.
• There is also PHP programming on 3rd party out of WMF area.

The existing word is problematic and likely to create work for my team.

The problem with the `mw-` prefix is that when skins use it, it gives the illusion of code something from core that can be relied on. In Vector and Minerva at least we have been trying to move away from this, and exclusively using `mw-` only inside core code. For example `mw-body` is not a class that applies to all skins, but `mw-body-content` is. This creates a lot of confusion when classes only apply to Vector but not Monobook for example.

At very least, could we suggest mw-<extname>- or mw-<skinname> if we insist on the mw- prefix. In skins we have been using the skin name for skin-specific code e.g. vector-body, minerva-body as it makes it easier for gadget developers to understand the context in which the classes apply.

Agreed, mw- is a bad one to use. mw-<extname>- and mw-<skinname>- could work, though ext- and by analogy skin- seems just as good?

Whether something is an extension or core can change, as can its name e.g. Flow/Structured Discussions, Echo/Notifications/core?, Vector (and the short-lived Vector extension), Monobook, phab:T28751, and phab:T31145 off the cuff.

I don't particularly think it's a big deal to use 'ext-/skin-' but those become (small) lies if something moves from to the other.

Particularly regarding skins, whether you put something in mw- or in skin-, you still add a name that might just end up cargo-copied to some other skin for e.g. compatibility with gadgets (not yet a solved issue), or which has the potential for reuse at a later date and now you're stuck with another name that doesn't tell quite the right story.

I agree that system software should carry a prefix regardless, and even given the above I'd probably say I agree with That's what namespaces are for, e.g. mw-foo the "foo" feature, whether bundled, core or extension is not important.) Just know that we'd end up with a bunch of mw-echo and mw-flow prefixed names floating around possibly in core.

It's astonishingly rare that HTML-generating code (i.e., a feature) is moved into or out of core, however.

Neither skins nor extensions are limited to MediaWiki staff, and many many old extensions and skins were not adopted ever, or removed from canonical MW maintenance.

• Therefore a selector specific to a particular skin or extension is not necessarily a mw-ext- or mw-skin-.
• A new skin may be developed as volunteer experiment, perhaps the dark mode (which is impossible for wiki content but may be used for interface area) or a different mobile experience. They might be included into MW support later. Remember Timeless cologneblue nostalgia MySkin Athena. They should not change their selector naming strategy when changing classification.
• The same goes for JavaScript gadgets developed within a Wikipedia, but offered globally by MediaWiki one day since convincing. Remember navigation popups.

It is quite obvious what skin and gadget and extension matters are, and what a slim core even for non-WMF projects is supporting as minimal solution.

• Therefore no mass migration is expected, if there is a long term and clear concept what core business shall be and how to extend this by additional features from any source.

MediaWiki staff

Can you explain what you think this means? There are no "MediaWiki staff". Do you mean "Wikimedia Foundation, Wikimedia Deutschland, Wikimedia Sevrige, and other paid and volunteer engineers working on MediaWiki core and Wikimedia-deployed extensions, libraries, services, tools, and skins"?

I can't understand your comment.

I do mean all people involved into MedaWiki development, but especially narrowing to WMF employees who are making the final decisions rather than volunteers. Deciding which extensions are merged with core or which skins will be maintained for WMF wikis is mostly confirmed by product managers etc.

On the other hand, everybody might develop an experimental extension or skin. Those need selectors, even if not part of default distribution of MediaWiki.

Deciding which extensions are merged with core […] is mostly confirmed by product managers etc.

That is not true. Why do you think that?

I got this impression over many years.

If it is really “not true” please tell me several examples from the last decade where such a decison has been made by volunteers only without any WMF employee involved and finally approving.

> If it is really “not true” please tell me several examples from the last decade where such a decison has been made by volunteers only without any WMF employee involved and finally approving.

That's not what you said? You claimed decisions were made by product managers, which is not true and has never been true. There is no product manager for MediaWiki. The very few scope decisions that have been made have been made by the MediaWiki development community following the normal consensus process.

But clearly you're here for an argument rather than to spread information, so I'll just disengage. Thanks!

I also like the ext- convention, and would like to see this recommendation returned to the manual. It seems that the main objection is that it's not a widespread practice — but I'm not sure it has to be, to still be useful (and it'll only grow in usage, if we let it).

OK, I've re-added it for now; improvements welcome!

Great, thanks! Looks good.

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 );
	visibility: visible;
	/* ::: Positioning ::: */
	// Float Model
	float: none;
	clear: both;
	// Positioning Model
	position: absolute;
	top: @offset-component;
	left: @offset-component;
	bottom: 0;
	right: 0;
	// Stacking Context
	z-index: 1;
	/* ::: 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-base;
	padding: 0;
	box-shadow: @box-shadow-menu;
	outline: 0;
	overflow: auto;
	/* ::: 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-base;
	/* ::: Others ::: */
	cursor: default;
	zoom: 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 property group
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
7. Others are last,

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

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

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

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

Please note, that we've added 'stylelint-order' plugin into WVUI prototypical library with list (slightly adapted in clearer separation of grouping) above.

A more complete & updated property order list is now also in active use in Codex.

Coming from recent patch on modern Vector to set media queries to being em based, we should consider exemplify the ideation and thoughts behind. The original article by Zell from 2016 on comparing em, rem and px: https://zellwk.com/blog/media-query-units/ And 2015 article on why em media queries are root font-size independent: http://blog.w3conversions.com/2015/03/all-your-base-are-belong-to-us-%E2%80%94-or-what-base-size-do-em-based-media-queries-use/ StackOverflow article on clarifying the em root independence stated in the specs: https://stackoverflow.com/a/29723215/1696030

Proposal is to expand the sizing units section by a paragraph accordingly.

This came up on en.wp and I figured I'd write something on it. I came to the following summary of what I think the current status quo is, but it is difficult to write something that is simple and accurate at the same time :D

Browser support

Support for browsers when using CSS is dictated by the overall Browser support matrix. From this guideline several conclusions follow:

• Grade A browsers should be fully supported by the CSS delivered
• Grade C browsers have limited support and limited testing. All content should be readable, but might not always be pretty. Basic functionality of the wiki should be accessible (read, edit, history, delete, move, block and the various lists).
• Browsers older than Grade C are not supported. Consumers should not expect pages to render correctly.
• When CSS is disabled all content should be correctly machine readable.

Practices for CSS browser support

From the principles the following additional practices are followed:

• CSS supported by both Grade A and Grade C browsers is always safe to use.
• CSS supported only by Grade A browsers:
  1. Should have a fallback with CSS that is supported by Grade C browsers
  2. Should be used for features that are NOT available on Grade C browsers (JS based). However, use of `.client-nojs` might be required to provide graceful degradation of the page in this case.
• CSS supported by a subset of Grade A can be used to further enhance, but should never be required to render.
• We remove vendor prefixed statements and browser hacks for browsers that can't realistically receive html any longer.
• Avoid depending on vendor prefixed CSS for the lowest supported browsers. Vendor prefixes are for experimental features and many confusing browser bugs can hide in this experimental fase of support. For your lowest supported browsers you want something that is predictable, especially because older browsers are harder to test with. Use vendor prefixed CSS to progressively enhance Grade A, but not to provide a baseline of support.

Feedback welcome: ping @Krinkle, @Volker E. (WMF), @Jdlrobson ?

Overall this sounds great to me. I think a practical application of Compatibility aimed at implementors has long been needed.

When CSS is disabled all content should be correctly machine readable.

I understand the intention here, but I think this might be redundant and seems easily misinterpreted. For one, I think it could be misinterpreted to mean that Grade C browser support is limited to only issues with CSS, whereas TLS, HTTP, and HTML are highly relevant there as well in terms of affecting what can and can't read. (As well as e.g. graceful skipping of the JS payload, etc.) As such, this sentence, I think, is not meant to say anything about unsupported browsers being able to consume the HTML, as those unsupported browsers may be unable to even connect to the web server at all, or may encounter HTML format that will break the layout in some way.

I also don't think we test or support manually turning off in an otherwise supported Grade A or C browser (maybe with JS still turned on?). There may be other conventions we follow that allow this to somewhat work currently as happy side-effect, but any issues specific to it I would decline without second thought.

Perhaps underlying your point is that we should strive use of semantic, accessible, and portable HTML. Thus in accordance with the Architecture Principles, for our content to be easy to re-use, archive, and render offline or standalone. Thinking about Kiwix, IPFS, Apple Dictionary Lookup, Internet Archive, HTML dumps, etc. It might also be worth to consider at least two text-based browsers in the Grade C matrix for longevity, along with a new code conventions page for HTML?

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.

This seems like a good first step. +1

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

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

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.

Added at https://www.mediawiki.org/wiki/Manual:Coding_conventions/CSS#Variable_naming

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?

Convetion updated: https://www.mediawiki.org/w/index.php?title=Manual:Coding_conventions/CSS&diff=3665722&oldid=3659267
Answering my question: the simpler @font-family-sans is the pattern to follow.

@Aron Manning Updated the convention accordingly, correct.

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();

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

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

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.

updated convention given my patch to include the .less extension was merged https://www.mediawiki.org/w/index.php?title=Manual:Coding_conventions/CSS&type=revision&diff=3441726&oldid=3206312&diffmode=source

Conversation can continue on https://gerrit.wikimedia.org/r/540498 or here if anyone has any problems with that.

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

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

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.