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ร—1 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 (198ร—1 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ร—1 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ร—1 px, 356 KB)

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

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

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

Opera mini regular - new header not present
expected behavior: new header should appear (without branding)
Screenshot_20170224-145712.png (1ร—1 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 (720ร—1 px, 163 KB)
Chrome
chrome.png (633ร—391 px, 54 KB)
Firefox 51
firefox.png (546ร—323 px, 42 KB)
Safari 9
safari-9.png (625ร—501 px, 46 KB)
Safari 8
safari-8.png (425ร—318 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 (720ร—1 px, 166 KB)
chrome 56
bman-chrome-56.png (648ร—434 px, 51 KB)
firefox 51
bman-firefox-51.png (548ร—425 px, 41 KB)
iOS8 safari 8
bman-safari-8.png (456ร—319 px, 34 KB)
safari 9
bman-safari-9.png (532ร—502 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 (1ร—720 px, 170 KB)
SearchNotiAlign-iphone6_ios8.1-Saf8.0.png (667ร—376 px, 72 KB)
Galaxy Note 3 Android 4.4 - Firefox 50iPhone 6S+ iOS 9.0 - Chrome 47
SearchNotiAlign-GalaxyNote3_A4.4-FF50.png (984ร—540 px, 84 KB)
SearchNotiAlign-iphone6S+_ios9.0-CH47.png (960ร—540 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 (1ร—768 px, 124 KB)
SearchNotiAlign-ipadair2_ios9.0-Saf9.0.png (1ร—768 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 (1ร—720 px, 166 KB)
SearchNotiAlign-iphone5S_ios7.0-Saf7.0.png (1ร—640 px, 165 KB)
Nexus 6 Android 5 - Maxthon 4.5iPhone 6s iOS 9.0 - Chrome 47
SearchNotiAlign-Nexus6_A5.0-Max4.5.png (1ร—720 px, 167 KB)
SearchNotiAlign-iphone6S_ios9.0-CH47.png (667ร—376 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: I3aa1beb9b12d99f4b08dd7f98c4455156a0dc458.

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.