Page MenuHomePhabricator

[regression] Echo badges positioned incorrectly inside the Refreshed header and Refreshed user dropdown menu
Closed, ResolvedPublic

Description

The echo badges should only be appearing directly in the header, but only one of them (the alerts badge) does while the other (the notice badge) shows inside the user dropdown menu.

http://typography.shoutwiki.com/wiki/Special:RecentChanges

Related Objects

Event Timeline

SamanthaNguyen added a subscriber: MtMNC.
SamanthaNguyen moved this task from Backlog to Bugs on the Refreshed board.Apr 27 2017, 12:14 AM
SamanthaNguyen renamed this task from Echo badges positioned incorrectly inside the Refreshed header and Refreshed user dropdown menu to [regression] Echo badges positioned incorrectly inside the Refreshed header and Refreshed user dropdown menu.May 14 2017, 10:49 PM
SamanthaNguyen moved this task from Unsorted to Needs refactor on the Technical-Debt board.EditedMay 14 2017, 11:02 PM

So basically this is happening because before when Echo first started out as an extension, it had stored notifications in one category. Later then they were resorted into 2 categories: "Alerts" and "Notices" in July 2016 (see https://www.mediawiki.org/wiki/Notifications ) - wow, almost a year! Some of the echo hacks will need slight rewriting.

And not only are they visually in different places, they're in different places of the DOM too.

MacFan4000 added a comment.EditedMay 15 2017, 2:53 PM

I was able to get to look okay by changing the element in https://github.com/wikimedia/mediawiki-skins-Refreshed/blob/master/refreshed/refreshed.js#L263 to #pt-notifications-notice, and adding the following to MediaWiki:Refreshed.css
#pt-notifications-alert {
float: right
}

#pt-notifications-notice {
float: right
}

#pt-notifications-personaltools {
display: none;
}

The only problem is that the number of notifications is still not aligned properly.

MacFan4000 added a comment.EditedMay 16 2017, 1:10 PM

With the latest update without any modifications it's moving the use tools drop down out of place. With modifications to the csss it looks similar to before except that the icons are higher up. Clicking on brings them down some. If you would like to see for yourself, my wiki can be accessed at https://testwiki.wiki

Can you provide a screenshot please? This makes it's easier for people to
understand what's being referred to.

MacFan4000 added a comment.EditedMay 16 2017, 10:27 PM

With CSS mods:
((When a page is first loaded)


(When a badge is clicked)

(Without any mods in MediaWiki:Refreshed.css)

Change 354069 had a related patch set uploaded (by LegoFan4000; owner: LegoFan4000):
[mediawiki/skins/Refreshed@master] hange echo element to #pt-ntifications-notice as #pt-notifications-message is no longer used. (T163616)

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

Change 354069 abandoned by LegoFan4000:
Change echo element to #pt-ntifications-notice as #pt-notifications-message is no longer used. (T163616)

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

MacFan4000 added a comment.EditedMay 17 2017, 9:58 PM

https://gerrit.wikimedia.org/r/#/c/354123/ will move the notice badge out of the user drop down. Other than that alignment work is needed. (Note that in all screenshots I provided I had made the linked change)

Change 354123 had a related patch set uploaded (by LegoFan4000; owner: LegoFan4000):
[mediawiki/skins/Refreshed@master] Change #pt-notifications-message to #pt-nptifications-ntice as #pt-notifications-message is outdated

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

Change 354123 abandoned by LegoFan4000:
Change #pt-notifications-message to #pt-nptifications-ntice as #pt-notifications-message is outdated

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

I figured it out! Screenshots are below.




Change 354241 had a related patch set uploaded (by LegoFan4000; owner: LegoFan4000):
[mediawiki/skins/Refreshed@master] Fixes for Echo

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

Change 354123 restored by LegoFan4000:
Change #pt-notifications-message to #pt-nptifications-ntice as #pt-notifications-message is outdated

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

Restricted Application added a subscriber: TerraCodes. · View Herald TranscriptMay 19 2017, 3:08 PM
MacFan4000 added a comment.EditedMay 21 2017, 10:41 PM

@SamanthaNguyen Could you please look over my changes?

Change 357017 had a related patch set uploaded (by SamanthaNguyen; owner: SamanthaNguyen):
[mediawiki/skins/Refreshed@master] Position echo notifications badge correctly in Refreshed header

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

SamanthaNguyen added a comment.EditedJun 3 2017, 6:07 PM

@MacFan4000: Sorry, the patches you submitted were split into two but also descendant patches of a parent patch that was abandoned (plus it needed some revising) and this needed fixing quickly (but unfortunately I was busy at the time), so I ended up having to submit my own patch. I was going to self-assign this task and raise it to high-priority because of SW deployment but I didn't do that and that was my fault (I also wrongly added the good first bug task, since this is a bit more complex). Thank you for taking the time to make some patches - I added your name into the author array of the skin.json file to give you credit. Thank you for understanding!

Change 354123 abandoned by SamanthaNguyen:
Change #pt-notifications-message to #pt-nptifications-notice as #pt-notifications-message is outdated

Reason:
see I15de28e383ad9e726236089cf859410cd6fb1ce4

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

Change 354241 abandoned by SamanthaNguyen:
Fixes for Echo

Reason:
see I15de28e383ad9e726236089cf859410cd6fb1ce4

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

Change 357017 merged by jenkins-bot:
[mediawiki/skins/Refreshed@master] Position echo notifications badge correctly in Refreshed header

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

SamanthaNguyen closed this task as Resolved.Jun 3 2017, 9:08 PM
SamanthaNguyen claimed this task.
SamanthaNguyen triaged this task as High priority.
SamanthaNguyen moved this task from Incoming to Done on the Design board.
SamanthaNguyen removed a project: Patch-For-Review.
SamanthaNguyen removed a subscriber: gerritbot.
MacFan4000 reopened this task as Open.Jun 28 2017, 10:56 AM

The Notification counters don't appear correctly and the new talk page message indicator still shows up in the user drop down.

SamanthaNguyen closed this task as Invalid.Jul 24 2017, 8:52 PM

Saying things like "it doesn't work"/"it's not working", "it doesn't appear correctly", "it's broken", etc is extremely vague and doesn't mean anything.

Furthermore, If you don't give reproduction steps, and if you don't give other information such as software versions (including for MediaWiki, skins, and extensions), screenshots, or browser info, I will not be able to help you and will close the task as invalid. You need to be running the latest stable version of anything, so stable MediaWiki version (not alpha version), master branch (latest version) of the Refreshed skin (currently this patch is the latest), etc.

In any case, you need to read https://www.mediawiki.org/wiki/How_to_report_a_bug before proceeding.

Until this is done, this task will be closed.

Oznogon2 added a subscriber: Oznogon2.EditedAug 16 2017, 2:49 AM

@SamanthaNguyen I can reproduce this issue described by @MacFan4000 by following the steps to reproduce in https://phabricator.wikimedia.org/T170773#3526393 and viewing the page at medium and large media query widths.


Expected behavior

Unknown, because the expected behavior is not documented.

My own expectation is similar behavior as Vector, with the both the icons and the new message notification in the top bar. If this expectation is incorrect, this issue should be addressed by noting differences in Echo's expected behavior in the documentation.


Actual behavior

The notifications icon is not visible (Refreshed REL1_29 3.1.7, Echo REL1_29 ea91b37 or Echo master 551f252) or too dark to be visible (Refreshed master 3.3.4, Echo REL1_29 ea91b37 or Echo master 551f252).

The new messages indicator is not visible in the top bar. It can be seen only by opening the user menu (Refreshed, both 3.1.7 and 3.3.4, and Echo REL1_29 ea91b37 or Echo master 551f252). In 3.1.7, the new messages indicator also has a non-interactive, empty user menu list item above it.


This affects:

  • Windows 7, Firefox 56.0b2 64-bit
  • Windows 7, Chrome 60.0.3112.90 64-bit
  • macOS Sierra 10.12.5, Chrome 59.0.3071.115 64-bit
  • macOS Sierra 10.12.5, Safari 10.1.1 (12603.2.4)

The versions of MediaWiki, Refreshed, and Echo that I tested and confirmed are affected are:

The Echo extension does not provide version numbers, so it is not possible to comply with the bug report requirement for an extension version number.

Oznogon2 reopened this task as Open.EditedAug 16 2017, 2:56 AM

I have test instances of

These will not be available after August 22, or if this or T170773 are closed.

The patch set that I uploaded seemed to fix this too.