| Izno | |
| Apr 15 2021, 7:14 PM |
| F34440934: image.png | |
| May 14 2021, 11:33 PM |
| F34453339: Screen Shot 2021-05-13 at 5.07.41 PM.png | |
| May 14 2021, 1:10 AM |
| F34453328: Screen Shot 2021-05-13 at 4.46.59 PM.png | |
| May 14 2021, 1:10 AM |
| F34298590: image.png | |
| Apr 15 2021, 7:14 PM |
Both Modern, CologneBlue, Timeless are displaying text that is much smaller than it was pre-legacy split. Monobook, Vector and Minerva are not impacted as they already load the content styles package.
Timeless's heading is also aligned center instead of left.
The table of contents is 95% the font-size of the article.
This looks to be applied consistently across all skins so while something definitely changed, I'm not sure if I would call this a bug/regression.
Modern Article 13px, TOC 12.35px
Vector: Article 14px, TOC 13.3px
Timeless: Article 15.2px, TOC: 14.44px
I defer to Isarra for Timeless. Not sure what to do with Modern.
Also have T280292: Timeless thumbnails do not appear as they used to.
Was Timeless just so broken before or is the split causing CSS to load in a wrong/different order now?
Timeless was not using core styles at all for a lot of this stuff... hopefully it's just specific features we need to (explicitly?) disable here, because if not, overriding this could get very annoying. But if it is, then maybe we can also disable some other things that we were overriding previously? Make things a bit neater overall?
I'll look into it when I'm awake and not drunk. Right now I just tried to wget some icons from one server to another and took way too long to realise I was actually on the server they were on wgetting them over themselves...
The problem here appears to be that various things have been added to the 'legacy' feature that were previously part of 'interface' or 'elements', or some other feature that was not previously used. This is a breaking change for all skins that were previously only using legacy and not these other features.
Okay, this might be the only one of these skin features issues that can actually be fixed in Timeless without just totally reimplementing everything... and it's like the least impactful anyway. And some of the others even made it into 1.36, whereas this doesn't appear to have.
You know, when I said in my end grant report that timeless was probably stable enough that it shouldn't require further funding for maintenance going further, I kind of assumed we wouldn't be winding up in situations like all the core styles suddenly becoming unusable...
Change 683777 had a related patch set uploaded (by Isarra; author: Isarra):
[mediawiki/skins/Timeless@master] Support mw 1.36/1.37: Disable all skin features except for a couple in favour of mw 1.34 shared.css to avoid weird inconsistent visual styles
Change 683777 merged by jenkins-bot:
[mediawiki/skins/Timeless@master] Support mw 1.36/1.37: Disable all skin features except for a couple in favour of mw 1.34 shared.css to avoid weird inconsistent visual styles
I think this one can be perhaps me fixed by adding toc: false to your ResourceLoaderSkinModule feature definition. I will look a bit closer later in the week.
So the issue here was that in https://gerrit.wikimedia.org/r/c/mediawiki/core/+/647333 we merged styles inside the content feature into the default-on toc feature.
Timeless and Modern do not use the content feature, so as a result of this change got styles it didn't previously, gaining several rules in https://gerrit.wikimedia.org/r/c/mediawiki/core/+/647333/58/resources/src/mediawiki.skinning/content.less.
The issue here is therefore not with the legacy module, but the content module spilling into the default table of content styles.
When I remove these styles as I do in https://gerrit.wikimedia.org/r/c/mediawiki/core/+/690779 the module provides something pretty useless that looks like this which I hope you agree is not an acceptable default styles for the table of contents for skins:
I don't think that should be the default and I think creates a lot of technical debt that is far worse than the alignment issue you raise above. If we take this approach we have a larger piece of technical debt to think through, and a non-helpful default.
An alternative would be to simply remove the alignment and font-size rules and add those in Vector and Monobook.
Without those rules a skin using the feature gets this:
I think this is an acceptable default state for all skins (for now).
The alignment issue and font-size issue seem pretty minor to me. Now we have a central definition for table of contents, we can work towards more sensible minimal defaults in preparation for the 1.37 release. Perhaps our time is better spent there, given Timeless has patched this already. That's kind of what option 2 is getting at, but with more time to actually do this right. Like you correctly say on https://gerrit.wikimedia.org/r/c/mediawiki/core/+/685614 "layout and functionality features shouldn't be providing 'designs' at all. Visual styles such as borders MUST be separate or modern skins like Timeless and Minerva will never be able to effectively utilise any of these. "
I'd love to work towards a default that makes sense for all skins and move out Vector/Monobook-specific styles to those skins, but we need a longer runway for that.
Thoughts on how to go about this? Any other possible options?
Change 691255 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):
[mediawiki/skins/MonoBook@master] Move table of contents font-size and alignment out of core
Change 691256 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):
[mediawiki/skins/Vector@master] Move table of contents font-size and alignment out of core
Change 691257 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):
[mediawiki/core@master] Remove Monobook/Vector specific toc styles
Perfectly readable, and very broken:
@Isarra which skin is this? I'd like to test the patches on this one to see if it goes far enough.
Change 690779 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):
[mediawiki/core@master] Conditionally load table of content styles for skins using `content` and `legacy`
Change 691257 abandoned by Jdlrobson:
[mediawiki/core@master] Remove Monobook/Vector specific toc styles
Reason:
See https://gerrit.wikimedia.org/r/c/mediawiki/core/ /690779
Change 691256 abandoned by Jdlrobson:
[mediawiki/skins/Vector@master] Move table of contents font-size and alignment out of core
Reason:
See https://gerrit.wikimedia.org/r/c/mediawiki/core/ /690779
Change 691255 abandoned by Jdlrobson:
[mediawiki/skins/MonoBook@master] Move table of contents font-size and alignment out of core
Reason:
See https://gerrit.wikimedia.org/r/c/mediawiki/core/ /690779
Change 690779 abandoned by Jdlrobson:
[mediawiki/core@master] Conditionally load table of content styles for skins using `content` and `legacy`
Reason:
I have understood your desire to have a separation between styles and layout, and I totally agree, but as I've tried to explain that's not the goal right now. Right now the goal has always been to split up the legacy stylesheet into usable units, so that skins do not have to load CSS that we know is rotten and can start thinking about these components in isolation.
If you don't care, there's no point in pursuing this patch further. The styles for table of contents are acceptable for now and will be revised for the next release.
I'll make sure to include you on the ticket to clean up the table of contents styles as you obviously have lots of thoughts and good ideas on the subject.
This has been resolved for Timeless. The goal of this project was to get all the table of contents styles in one place. CologneBlue and Modern will benefit from the next phase: T283836
Perfectly readable, and very broken:
@Isarra which skin is this? I'd like to test the patches on this one to see if it goes far enough.
GreyStuff. As I have not yet removed or overridden the feature, the ToC remains impacted by all unwanted changes.