Topic on Talk:Requests for comment/Themes in core

I was hoping this would be in Phabricator, but since that doesnt seem to be happening just yet, wanted to record this question.

You define a theme as a "Variant of a skin in terms of colours, fonts, simple layout changes; specific to a given skin"

FWIW "simple layout changes;" doesn't sound like what I understand theming to be. The rest seem fair and something which could be done by changing a few LESS variables. It would good as part of this RFC to define a contract for what a theme can and can't do and scrutinize that.

The Theme approach of shipping different stylesheets doesn't seem like something we should encourage. I would argue that giving users more control of some underlying CSS/LESS variables would be a better approach here.

Layout changes aside, if a skin is making use of LESS variables in a skin stylesheet, rather than rewriting an entire stylesheet like the existing themes do, have you considered adding ResourceLoader support for using different LESS variables? e.g. Vector Deep sea.

My biggest concern with enabling themes in core, is it should have an opt-out. Personally I don't want to worry about different themes of Minerva and I'd want to opt out of this because maintainability. CSS is hard enough to maintain as it is, without having to worry about different stylesheets for every theme. If I do have to worry about them I'd be much comfortable if the theme was defined by tweaking some CSS variables I'd intentionally exposed - for instance it would be powerful to allow Minerva's header/color scheme to look yellow-ish in the case of MediaWiki.org

Man, I'm sorry, I didn't even see this because I was really only watching the phabricator task.

Yeah, we definitely want to move toward using underlying variables, but we can't always use only that (even if it's just two extra rules to fix something weird with the different borders, something it turned out was needed in timeless) - but as long as we keep the side stuff to a minimum, I think we're doing fine.

And yes, adding that proper RL variable support so it doesn't need to be skin-side is the next step, unless... do you have any idea how to actually implement that now? If we can jump right to that now, that is what I want, here, but as far as I understand it there is just no way to do that in an extension to begin with, hence why I want to merge what we have now into core so we can then move onto that. But if we can just do it all at once...