Page MenuHomePhabricator
Paste P8805

RFC meeting 2019-07-24: Merge Extension:Theme into core (T122924)
ActivePublic

Authored by daniel on Jul 25 2019, 9:47 AM.
Tags
None
Referenced Files
F29868982: raw.txt
Jul 25 2019, 9:47 AM
<Krinkle> #startmeeting TechCom RFC meeting
<wm-labs-meetbot> Meeting started Wed Jul 24 21:10:19 2019 UTC and is due to finish in 60 minutes. The chair is Krinkle. Information about MeetBot at http://wiki.debian.org/MeetBot.
<wm-labs-meetbot> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
<wm-labs-meetbot> The meeting name has been set to 'techcom_rfc_meeting'
<Krinkle> #topic Merge Extension:Theme into core
<Krinkle> #link https://phabricator.wikimedia.org/T122924
<duesen_> thanks Krinkle :)
<Krinkle> yw
<duesen_> Isarra: do you want to give a brief summary of what this is about?
<Isarra> Right!
<Isarra> So basically we want to add built-in functionality to mw skins to also create variants of the skin.
<Isarra> Like a night mode or different fonts and whatnot. Most of which would be done by changing variables that set these throughout the skin's less files.
<Isarra> And being able to do it this way would make a bunch of wishlist items and accessibility features and a lot of third-party reskinning/branding a lot easier and especially more stable and less likely to break over time.
<duesen_> Isarra: would these variables be set in the config? or in user preferences?
<Isarra> The extension currently doesn't have that support, but doing that in an extension kind of... isn't worth it, mostly because as far as we can tell there isn't any way to make it actually support skinstyles and whatnot and work with *other* extensions, without editing RL and stuff.
<Krinkle> Would these variants be registered with a name in e.g. LocalSettings or a skin-extension-extension?
<ashley> right now themes are registered in either the Theme extension (for MonoBook & Vector) or in skins' skin.json file (e.g. Bouquet, Gamepress, ...)
<Isarra> Basically you register it in a skin or extension.json.
<Krinkle> so the number of variants is finite and controlled by software (not sysops or users)
<Isarra> For a skin. At some point we might want to also add the ability to register them as standalone packages, but that's way off.
<Isarra> Well, basically.
<Isarra> Mostly we just want it to actually do enough to be worth, uh, using before we start adding more configurability.
<duesen_> So, the skin defined three variants (say day, night, and high-contrast). Each variant (or theme?) sets certain variables to certain values. This is all controleld by the skin code.
<Isarra> Right now in order to even do a variables-based theme it requires some messy stuff in the skin itself, and we still can't use the variables in anything for other extension compatibility.
<Krinkle> Does the extension currently work or are there primitives that core needs to make this work? Sounds like we need some RL support for skin variants and/or something in MW itself to have skin IDs be mappable to a skin and a variant etc.
<duesen_> did i get that right?
<RoanKattouw> So my understanding of how this would work is, and correct me if I'm wrong: a theme is defined in a $wg config var, either in LocalSettings or extension.json. A theme extends a base skin by adding 1) its own CSS/LESS and 2) overrides for the values of LESS variables used in the main skin's styles
<Isarra> Yeah, the themes are registered in skin.json (or another registration place, theoretically you could also register it in localsettings or whatever), and the modules associated with them are files... wherever that registration points at.
<RoanKattouw> (sorry, the discussion advanced a bit while I was typing that)
<Isarra> Krinkle: Yeah, the extension works, it's just without what you're describing it's kind of... dumb currently. That's basically the heart of the skinstyles issue.
<Isarra> ashley: Can you answer Roan's question?
<Krinkle> Isarra: Interesting. Can you summarise how it works currently?
<RoanKattouw> Is the extension currently able to override variables in core skins? If yes, how does it accomplish that?
<Isarra> (Mostly I'm not sure how to link to an actual example...)
<Isarra> Currently the extension registers RL modules and theme names with which they're associated and adds settings and preference options for users to select between them.
<Isarra> It can't override existing variables, because it's just registering new modules entirely.
<ashley> RoanKattouw: yes, there's $wgThemeModules['<skin name>'] to which skins can append their themes and then the corresponding RL modules (e.g. skins.coolskin.night) are added to the output; for a live demo, install Extension:Theme and one of the four skins I mentioned earlier (Gamepress, Bouquet, MonoBook, Vector) and try it out!
<Isarra> And we have no way to make this interact in any meaningful way with other modules, either.
<Isarra> They just... well, load after each other.
<RoanKattouw> OK, so right now it's only #1 from my summary, and not (yet) #2
<quiddity> (IIUC) http://www.shoutwiki.com/w/index.php?title=Main_Page&useskin=vector&usetheme=dark = Vector + https://www.mediawiki.org/wiki/Skin:Vector-DarkCSS
<Isarra> That, yeah.
<ashley> yup! we'd like to make #2 happen as well in the future, obviously
<Isarra> I'd kinda like it now. >.>
<RoanKattouw> Purely from a ResourceLoader perspective, I'm not at all worried about #1, because it's just about adding another module and figuring out whether to load it
<ashley> quiddity: well actually literally "just" Vector + Extension:Theme; I've no clue what "Vector-DarkCSS" is (but let me see, /me clicks)
<Krinkle> Aha, so it bypasses the skin fragmentation issue by registering it as a separate module and loading that. That indeed works for adding CSS but not for influencing the original.
<RoanKattouw> It's #2 that caught my eye, that one will be more complicated in terms of RL caching
<Krinkle> What is the desired behaviour with skinStyles from extensions perspective? E.g. if an extension provides skinStyles for 'vector' should it also apply to 'vector#theme=dark'?
<RoanKattouw> Interesting question
<RoanKattouw> I'm just spitballing here, but here's a sketch that would address both the question Krinkle just asked, and provide a path for #2
<Isarra> I'd expect so, under normal circumstances.
<ashley> sure, since it's still Vector; but there could be a dark-specific override or somesuch, like how skinStyles in general currently work
* halfak heißt jetzt halAFK
<Isarra> If you're just trying to make it lay out properly in vector, the theme shouldn't matter if it'd dark or bright pink.
<Isarra> If you're setting actual colours or stuff, though, that would be a point where the dark theme would need to add its own +whatever (or replacement) to the registered skinstyle for that to set it.
<RoanKattouw> We come up with some separator symbol (maybe not # because that's hard in URLs; for the sake of argument let's say '|'), and we use ?useskin=vector for unthemed vector, and ?useskin=vector|dark for themed vector
<Isarra> Ideally, though, it wouldn't matter what theme - it'd be a generic skin-wide style for the extension that just sets the background to @background or whatever, which all the themes set to their usual thing regardless.
<Isarra> Currently it's ?useskin=vector&theme=dark
<RoanKattouw> Then vector|dark gets the skinStyles for vector|dark if they exist, or those for vector if they don't (we already have a fallback path from the skin name to 'default')
<RoanKattouw> In ResourceLoader, we would have to add anything special, because for purposes of varying the cache, vector|dark would be a "different skin" from vector
<Isarra> Well, as I'm seeing it, extensions should not be worrying about this.
<Krinkle> Isarra: Right, so the question of extension interop would be deferred to variables (that is, there is no need for either Theme nor Echo to deal with vector-dark-echo styles.) The proposal is for Echo to add vector/skinStyles, and for the dark mode theme to provide variables that both Vector and Echo/skinStyles/vector will use as needed.
<Krinkle> Does that sound right?
<RoanKattouw> Hmm right, we might be able to avoid skinStyles needing to target themes altogether
<Isarra> RoanKattouw: If you're trying to style things for particular colours or non-generic layouts, you're going to run into so many problems even without themes.
<Isarra> Skin-side, you just set the variable and reuse it for evrything, including extension compatibility.
<Isarra> Yeah.
<Krinkle> Yeah, but I wouldn't mind allowing specific targeting as well. It's either wgRLskinStyles[vector] or wgThemeStyles[skin][theme]. Either would work.
<RoanKattouw> Fair enough. I can see us not needing to support that, or even deciding to affirmatively not support it
<Krinkle> the latter being an extra module.
* awight hat die Verbindung getrennt (Quit: leaving).
<RoanKattouw> The variable injection stuff would require adding 'theme' as a parameter that RL output varies on, but otherwise wouldn't be very difficult I think
<RoanKattouw> Krinkle: You said there was a related thing to do with LESS variables that you and Volker were working on, can you elaborate on that?
<Isarra> Did you see the example how I'd like to define it on #2 here? https://phabricator.wikimedia.org/T122924
<Isarra> Basically I'd like to be able to specify target themes from any direction, but it'd usually be the skin, ues.
<Isarra> yes
* stuwest hat die Verbindung getrennt (Read error: Connection reset by peer).
<Krinkle> so the 2 things that stand out as needing core support are 1) registering of themes. This one might not be needed, for example, if the existing 'useskin' and 'skinStyles' suffice. The Theme feature would just register these as separate skins internally. We do need to think about Special:Preferences. E.g. could be a single dropdown still like [Vector, Vector (dark mode)] etc. but might make sense to allow making that nicer and more dedicated.
<Krinkle> 2) injecting of variables
<jdlrobson> Could a theme be registered using skinStyles with a special name e.g. 'vector-dark' => dark.less ?
<Krinkle> the injecting of variables is something I think would be better implemented as its own thing rather than tacked onto this.
<Krinkle> Covered mainly by https://phabricator.wikimedia.org/T112747
<Krinkle> Where the use case is more primitive as well
<Krinkle> Because the issue of extension-provided interfaces varying by skins exists as well, separate from skin themes.
<Isarra> That's... not even remotely feasible at this stage.
<RoanKattouw> Looks like the meat of that task is this comment? https://phabricator.wikimedia.org/T112747#4093591
<Isarra> We're still at a point in mw where skins are doing good to reuse ANY variables anywhere, let alone in any consistent way...
<Isarra> Am I misunderstanding, or are you suggesting we should have a standard internal pile of variables for all skins/extensions/whatever to reuse/set/override?
<Krinkle> yes and no.
<RoanKattouw> Reading over that proposal, it sounds like roughly the same thing that the Theme authors want to do, but one level up: allowing skins to set variables for use by skin-agnostic styles
<quiddity> Krinkle, Special:Preferences UI currently looks like this: https://i.postimg.cc/9Mg1yTg8/image.png
<Krinkle> Yes, I think it would be incredibly powerful if there is a subset of variables that multiple skins provide so that an extension could implement its layout once, using the standard set of variables.
<RoanKattouw> (Whereas Theme would allow themes to set variables for use by skin-specific-but-theme-agnostic styles)
<Isarra> Right now I just want to add a standard RLvariablemodule to quickly grab different registered theme variables files and include them. That's it, but it would still make everything so much easier here.
<Krinkle> Of course it can still provide custom styles via skinStyles, but the bottom line would be much better off.
<Krinkle> But the mechanism from T112747 is not limited to the standard vars. Skins can provide any number of variables that extensions would be able to read in their Less files.
<stashbot> T112747: Devise a generic way for theme-agnostic stylesheets to adapt to the current theme - https://phabricator.wikimedia.org/T112747
<Isarra> As nice as it would be to make standard ones, that's such a bigger problem and... well, once done, we could probably just reuse that regardless?
Krinkle kaldari KateChapman Keegan Kemayo kmuthu_ kostajh kristenlans kzeta
<Krinkle> Of course, if those vars are only implemented by a certain skin, the extension would do good to only access them from skinStyles[x] as they would be undefined otherewise.
<RoanKattouw> Maybe the focus of our discussion of T112747 should be the infrastructure: it proposes a way to allow skins to inject variables into generic styles, and we could use that same mechanism to let themes inject variables into skin styles
Krinkle kaldari KateChapman Keegan Kemayo kmuthu_ kostajh kristenlans kzeta
<duesen_> Isarra, Krinkle: can multiple styles be applied at the same time? E.g. "night mode" and "large fonts"?
<Isarra> That is certainly the plan.
<duesen_> That complicates the preferences UI. A lot.
<Isarra> We wouldn't want people to apply multiple colour ones (night/white), but any or all accessibility type ones would need to be applicable together with whatever.
<Isarra> Yes.
<RoanKattouw> The issue of generic cross-skin variables is a legitimate discussion, but not super relevant to the infrastructure to enable it (at this stage) IMO
<Krinkle> From the perspective of T_112747, that would be fine. The skin layer would, in case of a Theme-supported skin, inject multiple sets of vars. the Infra won't mind.
<duesen_> Isarra: that restriction complicatdes it even more :)
<Isarra> But for that one we'd probably just need to scoot that into its own thing.
<Krinkle> But it does mean that plain preferences as-is right now wouldn't suffice.
<Krinkle> So probably Theme would provide its own preference row underneath skin that would select the variant(s)?
<Isarra> Well, if it actually catches on, we can make a separate interface for accessibility settings and standardise that one across skins.
<quiddity> My dream UI (incl. for anon users) is something like this https://phabricator.wikimedia.org/M17
<Krinkle> It also means fragmenteation becomes a significant issue.
<Krinkle> E.g. day+night wouldn't make sense.
<RoanKattouw> Right. So maybe we can work on the infrastructure for injecting variables, and worry about the precise details of how it would be used in what place later, since it seems to be clear that we have multiple projects that want to use it
<Isarra> Yeah.
<Isarra> Er.
<RoanKattouw> Sorry, mistimed comment again
<Krinkle> Perhaps that can be deferred to theme creators having the variants they want? A tiny bit of duplication in exchange for not allowing arbirrary combinations?
<RoanKattouw> Pretend I sent that before we started about multi-theme combos
<Isarra> Yeah, that is something we absolutely DO NOT want to get into, here.
<duesen_> we are 2/3 through the meeting
<duesen_> any notable points to makr with #info?
<Isarra> I'm sorry, but the standardisation of variables across all of mediwaiki is just way too large of a problem for us to tackle.
<duesen_> please use the #info tag liberally :)
<RoanKattouw> OK I'm going to sum some things up with #info
<Krinkle> Isarra: can you elaborate why that would be a large problem? It's purely opt-in. Patches are ready to land in Gerrit.
<RoanKattouw> #info A "theme" is a skin plus some modifications. Those modifications are 1) additional style files, 2) variable overrides. The current Extension:Theme code supports #1 but not #2
<Krinkle> Main use case for it is so that an extension that uses OOUI only has to define its styles once, instead of N times for each skin/ooui-theme (in addition to non-ooui skins).
<Isarra> We... have no standardisation of it. To my knowledge, nobody's even looked into what common colours and affordances and whatnot we actually need to be defining things.
<RoanKattouw> #info There's a proposal at https://phabricator.wikimedia.org/T112747#4093591 that outlines how variable overrides/injection could work. It was proposed for a semi-unrelated use case, but such infrastucture would be useful here too
<Krinkle> Isarra: sure, we can start with 0 core vars as well. What you want is the mechanism which this provides.
<Isarra> The closest thing we have to 'consistent interfaces' in mw is... toccolors.
<Isarra> No!
<Isarra> To come up with any mechanism that makes sense, we have to start by figuring out what we even need it for!
<Krinkle> so you don't want to be able to inject variables from a skin or an extension into the Less engine that other extensions' less files can then use?
<Isarra> We are so far out from that that... that just terrifies me.
<RoanKattouw> #info The vision for T112747 includes standardizing a bunch of variables in MW core, but we don't need to go that far to implement a variable injection mechanism or for it to be useful
<stashbot> T112747: Devise a generic way for theme-agnostic stylesheets to adapt to the current theme - https://phabricator.wikimedia.org/T112747
<Krinkle> It is my understanding that this variables feature is a central part of the RFC. If it is an afterthought for future expansion, then I'm okay with dropping that topic.
<Isarra> I just want to be able to specify set(s) of variables, and then include any applicable ones programatically based on user/site/skin settings.
<Isarra> And have them work with skinstyles.
<Krinkle> That's 99% of T_112747. Let's work on that together?
<Isarra> Except no it isn't.
<RoanKattouw> #info There was some discussion of whether skinStyles should support themes (through something like 'vector|dark') or not. No full consensus on it, we appear to be somewhere between "in theory we shouldn't need that" and "can't hurt to implement it anyway"
<Isarra> That's not how ooui works.
<Isarra> That... might be how it should work, but... this isn't about ooui.
<Krinkle> Let's not focus on ooui. Can you answer my question of " so you don't want … "?
<duesen_> Is this still about merging the Theme extension into core? Or about creating an interface in core so the Theme extension can work better?
<RoanKattouw> The latter I think
<Isarra> ...
<RoanKattouw> So I think that Isarra and Krinkle are in violent agreement about wanting to have MW core infrastructure for injecting LESS variables into modules, and in violent disagreement about what should (or could feasibly) be accomplished once that feature exists
<duesen_> :D
<Isarra> wat
<Krinkle> Isarra: I think you might be thrown off by the OOUI mention that task. Nothing about that less vars ability is specific to OOUI.
<Krinkle> You want to be able to define variables in a skin or extension, and have other extensions' skinStyles/less files use those, yes?
<Isarra> ...can we get back to the RfC?
<Isarra> Can we merge Theme into core or not?
<Isarra> Because I have absolutely no idea where you're getting this cross-extension thing from.
<Isarra> I'm sorry, but you have just completely lost me.
<duesen_> What problem would be solved by merging the extension into core?
<Krinkle> OK. So you're saying this cross-extension thing is not a requirement for this RFC. thank you.
<Krinkle> So how will dark mode then work for interfaces provided by extensions? Is that not an in-scope problem?
<Isarra> Merging it as-is resolves a few different issues: we establish a standard interface for users to select skin variants, we reduce skin fragmentation and technical debt resulting from folks just forking skins to change their css or including chunks of theme in their skins to avoid having to deploy an extra extension, and we pave the way to actually resolving specific issues in the extension like total lack of support for skinstyles, as this requires
<Isarra> RL to have some comprehension of themes as something besides just modules.
<duesen_> Isarra: the RFC has three parts: 1) merge the theme extension into core, 2) add support for themes to RL and 3) add support for overriding LESS variables from themes.
<duesen_> I totally get (2) and (3), but I don't really see the need for (1).
<Isarra> That last one, I guess, is where you were getting the thing about sharing variables between extensions?
<ashley> it's basically about providing readiness for external consumers
<ashley> similar to how MW supports extensions or skins
<Isarra> Like 90% of skins out there are just forks people made so they could change the css.
<ashley> i.e. if you want to write a MW skin, you don't have to write the entire *architecture* for it ;)
<Isarra> This then results in them breaking, when nothing was changed that should break.
<duesen_> Isarra: i'm afraid merging into core might be blocked on UI issues...
<Isarra> ...why?
<Krinkle> Yes, a well-functionining and core-supported Theme extension seems very useful for that.
<duesen_> Would it make sense to look at the RFC parts in reverse order (and perhaps rename it)?
<Krinkle> Perhaps even bundled in the tarball by default once it has settled in
<duesen_> The "merging into core" bit seems to be the least compelling, so perhaps it shouldn't block the rest
<Isarra> It blocks the rest as the other things aren't really doable without it?
<Ostrzyciel> (as a wildly-appeared guest I would like to confirm that porting our changes in minerva's CSS each release is... tiring and the Theme thing would make it much easier)
<Ostrzyciel> (also Vector)
<Isarra> Like how the hell do we register 'themes' in core when core has no concept of themes?
<Krinkle> I think (2) and (3) very solvable without that.
<Isarra> Are you volunteering to do that, then?
<Krinkle> I've already done (3), and in this meeting we've established that depending on whether multi-theme-selection is a thing, RL mostly is able ot handle it already.
<duesen_> Isarra: nood point about the *concept* of themes
<Krinkle> Needing only a 3-line change described by Roan, maybe.
Krinkle kaldari KateChapman Keegan Kemayo kmuthu_ kostajh kristenlans kzeta
<Krinkle> for (2)
<Krinkle> .
<duesen_> Krinkle: can (2) and (3) be readily done without introducign the *idea* of skins into core?
<Krinkle> s/skins/themes/, yes.
<duesen_> hm, ok....
<Krinkle> and yes, I am volunteering to do (2) as well. I'd likely have to maintain it anyway
<Isarra> So let me get this straight: you don't think we should be including support for dark/light variants of skins out of the box, despite the recurring wishlist items for this?
<duesen_> Cool.
<duesen_> So, how would a skin expose multiple themes, if core doesn't know about themes?
<Isarra> You don't think we should be setting up consistent interfaces for skins to reuse to implement accessibility options? Or new variants?
<duesen_> would it have an optional dependency on the theme extension?
<Krinkle> Isarra: I think that question does not relate to where some php files are defined on someone's harddrive, which is what (1) effectively is.
<Isarra> We shouldn't be supporting third parties with a consistent way to set all this up?
<Isarra> Okay.
<Krinkle> we bundle Gadgets, we can bundle Themes too. I'd love that.
<Isarra> So the problem here is basically that you've never seen what third-parties actually do. Got it.
<duesen_> we are nearly out of time
<duesen_> next steps?
<Isarra> I think I'm going to go kill myself with a shovel.
<Krinkle> I think there are a lot of major problems in figuring out how this will work in practice. having a well-functioning extenion exist for a while in our ecosystem first might make sense to do first, before considering to merge it into core
<Krinkle> which could still be an option in the future.
<duesen_> Isarra: please don't
<Krinkle> but not before several major skins support it, and presumably with (2) and (3) solved, and bundled for a major release or two.
<Isarra> Sorry. That was a very bad joke not meant for this channel.
<ashley> Krinkle: Theme itself has existed since 2009 or so and has been in use at ShoutWiki, although the idea which evolved into the extension dates back to 2007, even
* Jianhui67 hat die Verbindung getrennt (Quit: Connection closed for inactivity).
* raynor (~raynor@wikimedia/PMiazga-WMF) ist beigetreten.
<Isarra> Yeah, we have the extension. We have the patch to merge it. The support for the other two things would also be very simple to add.
<ashley> plenty of skins do support themes (e.g. Bouquet, Gamepress) and there is support in Theme for MonoBook and Vector, two very popular skins
<Isarra> Except if we're not merging it, I have absolutely no idea how to do the other two things, so...
<duesen_> Maybe the next thing is to find that out? It seemed to me like Krinkle has ideas.
<duesen_> Krinkle: would you help Isarra with sorting out how (2) and (3) will work while themes are not a core concept?
<Isarra> Is there any chance he might be able to follow up on them?
<quiddity> Might a next step be to deploy it to beta-cluster or instance or something?
<Isarra> Like, to my knowledge the wmf has NO current investment working on anything even related to this.
<Isarra> So it would likely have to be a volunteer thing. Please correct me if I'm wrong, though...
<Krinkle> The variables issue is not a volunteer thing.
<Isarra> The wider variables issue as described has been sitting in limbo for four years now.
<Krinkle> Point (2) has been reduced to a 3-line patch in this meeting based on the current understanding of what is actually needed to make it work, and without needing multiple themes concurrently active, (2) appears to not be needed at all.
<Isarra> What exactly is that current understanding?
<duesen_> Maybe deploying the theme extension on the wmf cluster would be a good step towards merging it into core.
<duesen_> Merging it would deploy it as well.
<Krinkle> that all the neccecary cache and infra handling can be done by using a special skin ID.
<duesen_> so deploying it first would remove a barrier to morging it in
Krinkle kaldari KateChapman Keegan Kemayo kmuthu_ kostajh kristenlans kzeta
<Isarra> Krinkle: Can you please explain?
<ashley> it would certainly be something, yes, given how often users on and off-WMF request features like dark mode etc.
<duesen_> Ok, we are over time. I'm closing the meeting in like 10 seconds
<Krinkle> I just did, you'll need to ask a more specific question based on what I said.
<duesen_> Please continue the discussion on phab or in #mediawiki
<duesen_> #endmeeting
<Isarra> Yeah, to ask something more specific I'd have to have some context what that... means. Or anything.
<duesen_> heh, looks like to bot coraked again
<Krinkle> I'm going to follow up on (2) and (3) on the task with my understanding of the problem and how we can relatively easily solve it with existing infrastructure that would add little to no maintenance overhead for WMF within core.
<Krinkle> I would then also propose for Theme extension to be bundled first (if that is desirable). this is the first I hear about a potential wmf deploy. that's a separate question entirely.
* Krinkle (sid113303@wikimedia/Krinkle) ist gegangen.
* Ostrzyciel (~Magwac@ostrzyciel.pl) ist gegangen.
<Isarra> So is there any way to do one of these and not have the discussion get completely derailed? It feels like the core of the proposal was just dismissed out of hand, and yet we never even discussed it.