Page MenuHomePhabricator
Create Task
Maniphest T232011

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

Assigned To
ovasileva
Authored By
Etonkovidova
Sep 4 2019, 5:08 PM
Tags
Referenced Files
F30464221: T232011a.png
Sep 25 2019, 1:14 PM
F30464222: T232011b.png
Sep 25 2019, 1:14 PM
F30464220: T232011c.png
Sep 25 2019, 1:14 PM
F30464216: T232011c.gif
Sep 25 2019, 1:14 PM
F30381499: IMG_97F63168B288-1.jpeg
Sep 17 2019, 5:57 PM
F30222906: IMG_8254.PNG
Sep 4 2019, 5:08 PM
F30222908: IMG_8255.PNG
Sep 4 2019, 5:08 PM
Subscribers
Aklapper
• alexhollender_WMF
DannyS712
Edtadros
Etonkovidova
Jdlrobson
Jdrewniak
View All 10 Subscribers

Description

testwiki wmf.21 - the notification icons overflows:

IMG_8254.PNG (1×640 px, 115 KB)

Compare with wmf.20:

IMG_8255.PNG (1×640 px, 250 KB)

QA Results

*Please see Misc Observations T232011#5522428

ACStatusDetails
1✅T232011#5522428
2✅T232011#5522428

Details

SubjectRepoBranchLines +/-
Adjust notification icon to be the same size as the other iconsmediawiki/skins/MinervaNeuemaster+22 -25
Minerva unseen notification count should use icon classmediawiki/skins/MinervaNeuemaster+14 -11
Customize query in gerrit

Related Objects
Search...

Event Timeline

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

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

kostajh added a project: Notifications.Sep 5 2019, 2:38 PM
Jdlrobson mentioned this in T230232: Site header doesn't fit on screen in advanced contributors mode.Sep 5 2019, 4:43 PM
Jdlrobson added a parent task: T229440: [EPIC] Refactor icon sizing in MF/Minerva to achieve consistency.
Jdlrobson added a project: Web-Team-Backlog.
Jdlrobson subscribed.

@kostajh we broke it, we'll fix it (and are on it!) :)

Jdlrobson assigned this task to ovasileva.Sep 5 2019, 8:04 PM
Jdlrobson moved this task from Incoming to Needs Prioritization on the Web-Team-Backlog board.
ovasileva triaged this task as Medium priority.EditedSep 6 2019, 12:02 PM
ovasileva edited projects, added Web-Team-Backlog (Readers-Web-Kanbanana-2019-20-Q1); removed Web-Team-Backlog.

@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 subscribed.
Jdlrobson moved this task from Needs Analysis to Upcoming on the Web-Team-Backlog (Readers-Web-Kanbanana-2019-20-Q1) board.Sep 9 2019, 5:13 PM

Yes it will hopefully be fixed as part of T229440

ovasileva added a subscriber: • alexhollender_WMF.Sep 10 2019, 4:32 PM

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

• alexhollender_WMF added a comment.Sep 10 2019, 4:34 PM
In T232011#5479836, @ovasileva wrote:

@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.

gerritbot added a comment.Sep 10 2019, 6:22 PM

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

gerritbot added a project: Patch-For-Review.Sep 10 2019, 6:22 PM
Jdlrobson moved this task from Upcoming to Needs Code Review on the Web-Team-Backlog (Readers-Web-Kanbanana-2019-20-Q1) board.Sep 10 2019, 6:22 PM
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.

gerritbot added a comment.Sep 13 2019, 7:21 PM

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

gerritbot added a comment.Sep 13 2019, 7:21 PM

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

Reason:
possible misunderstanding

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

gerritbot added a comment.Sep 13 2019, 8:21 PM

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

Reason:
trying different approach

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

gerritbot added a comment.Sep 13 2019, 8:30 PM

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

gerritbot added a comment.Sep 13 2019, 9:53 PM

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

Restricted Repository Identity mentioned this in rSKIN482006e30e7e: Update git submodules.Sep 13 2019, 9:55 PM
ReleaseTaggerBot added a project: MW-1.34-notes (1.34.0-wmf.23; 2019-09-17).Sep 13 2019, 10:00 PM
Jdlrobson assigned this task to • alexhollender_WMF.Sep 16 2019, 6:00 PM
Jdlrobson moved this task from Needs Code Review to Needs Design Review on the Web-Team-Backlog (Readers-Web-Kanbanana-2019-20-Q1) board.
Jdlrobson moved this task from Needs Design Review to Needs More Work on the Web-Team-Backlog (Readers-Web-Kanbanana-2019-20-Q1) board.Sep 16 2019, 6:31 PM
Jdlrobson moved this task from Needs More Work to Needs Design Review on the Web-Team-Backlog (Readers-Web-Kanbanana-2019-20-Q1) board.Sep 16 2019, 8:10 PM
• alexhollender_WMF reassigned this task from • alexhollender_WMF to Edtadros.Sep 17 2019, 5:57 PM
• alexhollender_WMF moved this task from Needs Design Review to Needs QA on the Web-Team-Backlog (Readers-Web-Kanbanana-2019-20-Q1) board.

This has been fixed

IMG_97F63168B288-1.jpeg (1×750 px, 397 KB)

Edtadros added a comment.Sep 18 2019, 11:09 PM

@alexhollender which environment do you want this QAd?

Edtadros reassigned this task from Edtadros to ovasileva.Sep 25 2019, 1:14 PM
Edtadros moved this task from Needs QA to Ready for Signoff on the Web-Team-Backlog (Readers-Web-Kanbanana-2019-20-Q1) board.
Edtadros subscribed.

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

T232011a.png (2×1 px, 376 KB)

✅ AC2: Notification Icon Read

T232011c.png (2×1 px, 375 KB)

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).

T232011b.png (2×1 px, 376 KB)

  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:

T232011c.gif (808×372 px, 1 MB)

Edtadros updated the task description. (Show Details)Sep 25 2019, 1:18 PM
ovasileva added a comment.Sep 30 2019, 10:30 AM
  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).

T232011b.png (2×1 px, 376 KB)

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:

T232011c.gif (808×372 px, 1 MB)

MBinder_WMF edited projects, added Web-Team-Backlog (Kanbanana-2019-20-Q2); removed Web-Team-Backlog (Readers-Web-Kanbanana-2019-20-Q1).Sep 30 2019, 5:20 PM
MBinder_WMF moved this task from Needs Analysis to Ready for Signoff on the Web-Team-Backlog (Kanbanana-2019-20-Q2) board.Sep 30 2019, 5:28 PM
ovasileva closed this task as Resolved.Oct 1 2019, 11:13 AM
ovasileva mentioned this in T234319: [Bug] Notification number inconsistent on mobile.

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

DannyS712 removed a project: Patch-For-Review.Nov 18 2019, 11:37 AM
DannyS712 subscribed.

[batch] remove patch for review tag from resolved tasks

Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits