Page MenuHomePhabricator

Notification count appears misaligned in all mobile web
Closed, ResolvedPublic8 Estimated Story Points

Description

Due to new header changes, notification count appears misaligned on ALL wikis.

Screenshot_20170223-184245(1).png (1,080ร—1,920 px, 556 KB)

Event Timeline

Restricted Application added a subscriber: Aklapper. ยท View Herald TranscriptFeb 24 2017, 12:36 PM

This is what I see:

Screen Shot 2017-02-24 at 7.58.33 AM.png (1,948ร—198 px, 31 KB)

Can you provide more info about the browser? We should probably test it in different browsers and identify the ones that are affected. Needs reproduction?

Android 6.0.1
Chrome 56.0.2924.87

Screenshot_20170224-133108.png (1,080ร—1,920 px, 356 KB)

Firefox seems to strangely not have the new header, same for Opera Mini.

@bmansurov - try it on your phone, it looks aligned for me on desktop

Change 339629 had a related patch set uploaded (by Bmansurov):
Fix notification count vertical alignment

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

@bmansurov
Chrome: new header + notifications appearing

Screenshot_20170224-133108.png (1,080ร—1,920 px, 356 KB)

Firefox: new header not appearing
Screenshot_20170224-140938.png (1,080ร—1,920 px, 473 KB)

Opera mini beta mode - non-beta header appears
expected behavior: beta mode header should appear

Screenshot_20170224-145618.png (1,080ร—1,920 px, 282 KB)

Opera mini regular - new header not present
expected behavior: new header should appear (without branding)
Screenshot_20170224-145712.png (1,080ร—1,920 px, 307 KB)

@Nirzar - it seems I was logged in to firefox as two different users. For the new header, is the empty bell icon expected behavior for user with no notifications?

MBinder_WMF set the point value for this task to 8.Feb 27 2017, 6:26 PM

Change 340308 had a related patch set uploaded (by Jhernandez; owner: Jhernandez):
Fix vertical alignment of notifications label

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

I'm sorry to be a pain but I've also spent time and the best solution I could find that wouldn't change the HTML is center using flexbox. I've submitted https://gerrit.wikimedia.org/r/340308 as an alternative to Baha's patch.

Browserscreenshot
Chrome mobile
chrome mobile.jpg (1,080ร—720 px, 163 KB)
Chrome
chrome.png (391ร—633 px, 54 KB)
Firefox 51
firefox.png (323ร—546 px, 42 KB)
Safari 9
safari-9.png (501ร—625 px, 46 KB)
Safari 8
safari-8.png (318ร—425 px, 38 KB)

QA on Baha's take https://gerrit.wikimedia.org/r/#/c/339629

Summary: Old iOS <= 8 aligns properly, Chrome mobile misaligns a bit to the top

BrowserScreenshot
chrome mobile
bman-chrome-mobile.jpg (1,080ร—720 px, 166 KB)
chrome 56
bman-chrome-56.png (434ร—648 px, 51 KB)
firefox 51
bman-firefox-51.png (425ร—548 px, 41 KB)
iOS8 safari 8
bman-safari-8.png (319ร—456 px, 34 KB)
safari 9
bman-safari-9.png (502ร—532 px, 42 KB)

T158957#3061586 and T158957#3060917 contain local QA on several browsers and the different tradeoffs on the solutions. I don't mind which one to choose right now, but we should do something.

Looks pretty good on Staging. Here are the screenshots:

ScreenshotsAndroid Device OS - Browser VersioniOS Device OS - Browser Version
Nexus 6P Android 7 - Firefox 50iPhone 6 iOS 8.1 - Safari 8.0
SearchNotiAlign-Nexus6P_A7-FF50.png (720ร—1,304 px, 170 KB)
SearchNotiAlign-iphone6_ios8.1-Saf8.0.png (376ร—667 px, 72 KB)
Galaxy Note 3 Android 4.4 - Firefox 50iPhone 6S+ iOS 9.0 - Chrome 47
SearchNotiAlign-GalaxyNote3_A4.4-FF50.png (540ร—984 px, 84 KB)
SearchNotiAlign-iphone6S+_ios9.0-CH47.png (540ร—960 px, 123 KB)
Nexus 9 Tablet Android 6.0 - Chrome 55iPad Air 2 iOS 9.0 - Safari 9.0
SearchNotiAlign-Nexus9Tab_A6.0-Chrome55.png (768ร—1,048 px, 124 KB)
SearchNotiAlign-ipadair2_ios9.0-Saf9.0.png (768ร—1,024 px, 112 KB)
Galaxy S6 Android 5.0 - Android Browser 5.0iPhone 5s iOS 7.0 - Safari 7.0
SearchNotiAlign-GalaxyS6_A5.0-Android5.0.png (720ร—1,304 px, 166 KB)
SearchNotiAlign-iphone5S_ios7.0-Saf7.0.png (640ร—1,136 px, 165 KB)
Nexus 6 Android 5 - Maxthon 4.5iPhone 6s iOS 9.0 - Chrome 47
SearchNotiAlign-Nexus6_A5.0-Max4.5.png (720ร—1,304 px, 167 KB)
SearchNotiAlign-iphone6S_ios9.0-CH47.png (376ร—667 px, 70 KB)

@ABorbaWMF, @Nirzar - the iPhone 5s iOS 7.0 - Safari 7.0 combination still seems a bit misaligned to me.

Summary of tradeoffs:

  • Table-cell approach (Baha): All well aligned but Chrome mobile (aligned but a bit misplaced to the top)
  • Flexbox approach: All well aligned but iOS <= 8 (aligned but a bit misplaced to the bottom)

The patch deployed is the Flexbox approach, which is why you see old iOS a bit misaligned.

@ABorbaWMF, @Nirzar - the iPhone 5s iOS 7.0 - Safari 7.0 combination still seems a bit misaligned to me.

True, it's 1-2px down.

Change 339629 abandoned by Bmansurov:
Fix notification count vertical alignment

Reason:
See an alternative approach: I3aa1be.

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

Change 340308 merged by jenkins-bot:
Fix vertical alignment of notifications label

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

Summary of tradeoffs:

  • Table-cell approach (Baha): All well aligned but Chrome mobile (aligned but a bit misplaced to the top)
  • Flexbox approach: All well aligned but iOS <= 8 (aligned but a bit misplaced to the bottom)

The patch deployed is the Flexbox approach, which is why you see old iOS a bit misaligned.

For the record, the Flexbox approach was chosen.

Based on the results from staging, signing this off.