Page MenuHomePhabricator

Echo broken in conjunction with Modern skin
Closed, ResolvedPublic

Description

Echo is totally inaccessible from the Modern skin.

Primary cause is that modern sets p-personal to 'overflow:hidden', hiding the overlay which is outside of it's parent box. I'm not really sure WHY the skin has overflow hidden, it doesn't seem required on first look.

After fixing that, it is still shown partially outside of your screen, because p-personal is left aligned on modern (and cologne blue). The positioning of Echo does not account for this in any way (it uses left: -200px;). Additionally Modern sets an explicit height for <ul> elements (regardless of being direct descent dents) which makes the notifications area very small.


Version: master
Severity: critical

Details

Reference
bz47932

Event Timeline

bzimport raised the priority of this task from to Normal.Nov 22 2014, 1:27 AM
bzimport added a project: Notifications.
bzimport set Reference to bz47932.
bzimport added a subscriber: Unknown Object (MLST).
TheDJ created this task.May 1 2013, 2:34 PM
TheDJ added a comment.May 1 2013, 2:35 PM

Translated, please for now disable Echo for people with skins that aren't explicitly supported :D

TheDJ added a comment.May 1 2013, 3:27 PM

.skin-modern #p-personal {

overflow: visible;

}

.skin-modern .mw-echo-overlay {

left: -10px;
top: 31px;

}

.skin-modern .mw-echo-overlay-pokey {

left: 4px;

}

.skin-modern #pt-notifications ul,
.skin-modern #pt-notifications li {

height: auto;

}

.skin-modern #p-personal #mw-echo-overlay-link {

padding: 15px 15px 15px 60px;

}
.skin-modern #p-personal #mw-echo-overlay-pref-link {

padding: 15px 15px 15px 35px;

}

.skin-modern #p-personal .mw-echo-overlay a {

padding: 0;
color:#003366;

}
.skin-modern #p-personal .mw-echo-overlay a:visited {

padding: 0;
color:#5a3696;

}
.skin-modern #p-personal .mw-echo-overlay a:hover {

padding: 0;
text-decoration: inherit;

}

TheDJ added a comment.May 1 2013, 4:03 PM

Another issue, the preferences accessor on https://en.wikipedia.org/wiki/Special:Notifications?useskin=modern is broken because it is using H1 positioning, which is inconsistent.

TheDJ added a comment.May 1 2013, 10:22 PM

i'm choosing to deploy this as a local change at en.wp for now.

Yeah, I don't think we should have those links in the h1 header, personally. This will also likely cause problems in future skins.

TheDJ added a comment.May 2 2013, 9:34 AM

@Ryan, and where you do, give sane and simple defaults, and then make skin specific 'fluffy' decorations, if need be.

TheDJ added a comment.May 3 2013, 12:40 PM

My changes don't work for IE users apparently, if someone could invest some time in that, would be awesome, because these users have a properly broken interface right now, with now way to switch back to the working Orang bar.

https://en.wikipedia.org/w/index.php?title=MediaWiki%3AModern.css&diff=553111284&oldid=523652236

(In reply to comment #7)

My changes don't work for IE users apparently

DJ: Could you provide a screenshot?

rkaldari: Is anybody working on this, or what's the priority of this bug?

TheDJ added a comment.May 17 2013, 6:17 PM

@Andre, no, i don't own Windows machines. You can just look at mediawiki.org using modern skin (which wouldn't use my en.wp specific changes) and you'd see the problem as well.

(Yes another case where putting the notifications into the personal toolbar cause issues; see bug 47993.)

Here a link, to make it dead easy to verify that Notifications is totally broken on Modern: https://www.mediawiki.org/wiki/MediaWiki?useskin=modern

Related URL: https://gerrit.wikimedia.org/r/66367 (Gerrit Change I633a93a78f5a78d0642a3a059fa6f7208f99cec4)

TheDJ added a comment.Jun 2 2013, 8:08 PM

Now someone can actually take the code from git and improve it to run on IE10.

Bug summary: icw --> in conjunction with

Finally had some time to work on the non-default skin support. Sorry it's taken so long.

@Derk-Jan: Thanks for your work on this. I've taken your patch as a starting point and expanded it. I've also refactored some of the logic for Special:Notifications so that it has sane defaults (as you suggested), and only overrides the styling specifically for monobook and vector. This also has the added benefit of making more of the interface work with Javascript turned off.

I don't have IE10, but I tested it in the various skins in IE8 and it seems to work fine.

Thanks to Kaldari's fine work, Modern skins can now get notifications, and the badge and flyout display as intended.

Please let us know if it works for you. For now, I will mark that bug as resolved, as it seems to work for both Kaldari and I.