r91772 MediaWiki - Code Review archive

Is the shared.css change in core intended?

Yes, they are general classes that can be used anywhere (like the float classes), but it appears that I did a stupid copy-paste error, I'll have to change the second 'left' to 'right'.

My point is that I don't see them referenced anywhere else in the commit (which seems to have been about Collection), so I'm not sure why it was included here.

I made use of mw-align-end in Collection in this commit. I should have perhaps mentioned it in the summary.

This updates $wgCollectionVersion without updating requiredVersion in collection.js, so all the JavaScript front-end code aborts...

Updated on trunk in r95337. Resetting this rev from fixme to new.

I didn't understand the mw-align-start/end and mw-float-start/end at first, but SPQRobin explained it to me. Repeating here in my own words for later reference.

mw-float-start/end allows to float an element to the side where either the text starts, or to the opposite side.

For example, if a diagram should be floated to the right of the page in an LTR context, then it should float to the left of the page in an RTL context. On such element you could use mw-float-end.

That's all nice. But from a front-end perspective it's not very semantic in my opinion and it doesn't solve anything.

Consider the following example:

--- CSS
.mw-float-left {
  float: left; /* flipped by ResourceLoader */
}
.mw-float-end {
  float: right; /* flipped by ResourceLoader */
}

.infobox {
  border: 1px solid grey;
  ...
}

--- HTML
<div class="infobox mw-float-end">
...
</div>

I would rather have us do the following, and therewith separate presentation from content.

-- CSS
.infobox {
  float: right; /* flipped by ResourceLoader as needed */
  border: 1px solid grey;
  ...
}

--- HTML
 ... 
</pe>

To use an extreme example: Suppose there's a class for "error" that is declared like .error { color: red;  } and another called "love": .love { color: red; }, we wouldn't go and use a class "red-text" and use <div class="red-text"> for both....

That's the opposite of what CSS is meant to be used for. Marking fixme.  Use meaningful classes for elements that need special presentation (ie. "mw-collection-label-download").

Should be OK in r96406.