Page MenuHomePhabricator

Echo makes the border of h1#firstHeading too short
Closed, ResolvedPublic

Description

Since https://gerrit.wikimedia.org/r/#/c/231200/ the border of h1#firstHeading started not taking the whole width of the screen.

This is already seen in translatewiki, for example at https://translatewiki.net/wiki/User:Amire80 (you need to be logged in, otherwise Echo is not loaded).

I can also reproduce it on my local wiki.

Event Timeline

Amire80 raised the priority of this task from to High.
Amire80 updated the task description. (Show Details)
Amire80 added a project: Notifications.
Restricted Application added a subscriber: Aklapper. · View Herald Transcript

This probably has something to do with the loading of modules/nojs/mw.echo.special.less ; the "nojs" part in the path is suspicious.

Actually, the 'nojs' part is just saying that this is the "base" styles that are shared between the notifications in the Special:Notifications page and the ones in the popup. The popup is js-specific, so its module and its styles load later and overrides to notifications styles are there. The base styling, however, is in the base one, which is the 'nojs' that is loaded before the JS had any chance to load.

If it is confusing and makes no sense, I can change its naming, but it seemed to be consistent with the idea of nojs system (echo can load without js, its badge just leads to Special:Notifications page) and on top of it a js layer that makes things interactive and pretty.

As for the bug, I'll take a closer look, it's probably because of the changes in structure with the new OOUI system of the flyout.

The "nojs" raises some suspicions, but is probably not the culprit. The weird part is why is max-width set on #firstHeading in the first place. It was done more or less in https://gerrit.wikimedia.org/r/#/c/77569/ by @matmarex. The comment "Custom header styling for Vector and Monobook skins" doesn't say much; maybe that LESS block could be simply removed? (But I am probably very shallow here.)

Change 236504 had a related patch set uploaded (by Mooeypoo):
Specify firstHeading rule for .mw-special-Notifications only

https://gerrit.wikimedia.org/r/236504

This rule resided in a css file that only loaded in Special:Notifications, along with the styles that were defined for the notifications, which resulted in a lot of duplication.

When we rewrote echo to be ooui, we've removed a lot of duplication by making the styles be unified -- a "base" style that works on Special:Notifications and on the ooui notifications together (but always on Special:Notifications in js/no-js mode, which is why it's called nojs) and an overlaying style for whatever adjustments are done for the ooui popup.

However, since this style now touches the popup too, it became way too broad.... which meant that this rule affected all pages. I've made the rule more restricted so it would only work on the Special:Notifications page.

We should at some point redesign Special:Notifications anyways, at which point we will probably rethink whether or not to limit the size of the heading there at all -- but for now, this solution at leasts makes sure we're not changing any styles anywhere outside Special:Notifications and the popup itself.

Thanks for catching this!

Change 236504 merged by jenkins-bot:
Specify firstHeading rule for .mw-special-Notifications only

https://gerrit.wikimedia.org/r/236504

matmarex assigned this task to Mooeypoo.
matmarex removed a project: Patch-For-Review.