Page MenuHomePhabricator

[regression wmf.21-minor] Notification icon overflows viewport
Closed, ResolvedPublic

Description

testwiki wmf.21 - the notification icons overflows:

Compare with wmf.20:

QA Results

*Please see Misc Observations T232011#5522428

ACStatusDetails
1T232011#5522428
2T232011#5522428

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 4 2019, 5:08 PM
kostajh moved this task from Inbox to Upcoming Work on the Growth-Team board.Sep 5 2019, 2:38 PM
kostajh added a subscriber: kostajh.

We (Growth-Team or Readers-Web-Backlog) should probably fix this soon.

Jdlrobson moved this task from Incoming to Needs Prioritization on the Readers-Web-Backlog board.
ovasileva triaged this task as Normal priority.EditedSep 6 2019, 12:02 PM

@Jdlrobson - do you think this will be fixed with T229440: [EPIC] Refactor icon sizing in MF/Minerva to achieve consistency or should we look at it separately?

ovasileva removed ovasileva as the assignee of this task.Sep 9 2019, 2:14 PM
ovasileva added a subscriber: ovasileva.

@alexhollender - looks like we won't be able to fix this until Sept 19th (next week's train) - is that okay with you?

@alexhollender - looks like we won't be able to fix this until Sept 19th (next week's train) - is that okay with you?

Yes. Thanks for letting me know.

Change 535663 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@master] Minerva unseen notification count should use icon class

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

Jdlrobson added a subscriber: Jdrewniak.

@ovasileva looked at this one with @Jdrewniak today and will require some additional work (the patch above). Might want to check in and estimate tomorrow AM.

Change 535663 abandoned by Jdlrobson:
Minerva unseen notification count should use icon class

Reason:
Jan is gonna write a fix

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

Change 535663 restored by Jdlrobson:
Minerva unseen notification count should use icon class

Reason:
possible misunderstanding

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

Change 535663 abandoned by Jdlrobson:
Minerva unseen notification count should use icon class

Reason:
trying different approach

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

Change 536680 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@master] Adjust notification icon to be the same size as the other icons

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

Change 536680 merged by Jdlrobson:
[mediawiki/skins/MinervaNeue@master] Adjust notification icon to be the same size as the other icons

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

@alexhollender which environment do you want this QAd?

Edtadros reassigned this task from Edtadros to ovasileva.Wed, Sep 25, 1:14 PM
Edtadros added a subscriber: Edtadros.

Test Result

Status: ✅ PASS
OS: macOS Mojave
Browser: Chrome
Device: MBP
Emulated Device: iPhoneX, Galaxy S5, iPad Pro, Google Pixel 2, Galaxy Note 3

Test Artifact(s):

✅ AC1: Notification Icon Unread

✅ AC2: Notification Icon Read

Misc Observations:

  1. The notification count doesn't seem to make sense. It will go from zero to more than 1 if I thank myself for an edit (using a different user).

  1. Clicking All Notifications then hitting the browser back button twice results in a numbered notification icon that is offset and not red. A reload fixes this.

See both illustrated below:

Edtadros updated the task description. (Show Details)Wed, Sep 25, 1:18 PM
  1. The notification count doesn't seem to make sense. It will go from zero to more than 1 if I thank myself for an edit (using a different user).

I think there was already an open bug for this, but I couldn't find it. @Jdlrobson - do you remember?

  1. Clicking All Notifications then hitting the browser back button twice results in a numbered notification icon that is offset and not red. A reload fixes this.

See both illustrated below:

ovasileva closed this task as Resolved.Tue, Oct 1, 11:13 AM

opened T234319 to track bugs with notification count. Resolving this.