⚓ T213336 Regression: Mobile gallery loading spinner appears on left instead of centered

Subject	Repo	Branch	Lines +/-
Aligning ImageOverlay prev/next buttons with loading spinner.	mediawiki/extensions/MobileFrontend	master	+2 -6
Align ImageOverlay prev/next buttons with top of viewport.	mediawiki/skins/MinervaNeue	master	+0 -4
Add MediaViewer-specific selector to loading-spinner CSS	mediawiki/extensions/MobileFrontend	master	+2 -1

• Niedzielski created this task.Jan 9 2019, 8:25 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJan 9 2019, 8:25 PM

• Niedzielski added a project: Regression.Jan 9 2019, 8:25 PM

Jdlrobson moved this task from Incoming to Needs Prioritization on the Web-Team-Backlog-Archived board.Jan 9 2019, 9:15 PM

I believe this is a regression, so @jan would you mind investigating the patch that introduced this problem (using git blame) and add a developer note? I think if this proves to be a regression that should be enough for an estimation.

Jdlrobson triaged this task as High priority.Jan 11 2019, 12:53 AM

Jdlrobson lowered the priority of this task from High to Medium.

When I briefly looked into this, I thought it was https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/MobileFrontend/+/472368/ that introduced the regression. I couldn't reproduce before that commit

Got it!

ovasileva set the point value for this task to 1.Jan 16 2019, 5:10 PM

Jdlrobson moved this task from Upcoming to Readers-Web-Kanbanana-Board-2018-19-Q3 on the Web-Team-Backlog-Archived board.Jan 18 2019, 12:50 AM

Jdlrobson edited projects, added Web-Team-Backlog-Archived (Readers-Web-Kanbanana-Board-2018-19-Q3); removed Web-Team-Backlog-Archived.

Change 485734 had a related patch set uploaded (by Jdrewniak; owner: Jdrewniak):
[mediawiki/extensions/MobileFrontend@master] Add MediaViewer-specific sekector to loading-spinner CSS

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

gerritbot added a project: Patch-For-Review.Jan 22 2019, 4:33 AM

Jdrewniak moved this task from To Do to Needs Code Review on the Web-Team-Backlog-Archived (Readers-Web-Kanbanana-Board-2018-19-Q3) board.Jan 22 2019, 4:33 AM

Change 485734 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Add MediaViewer-specific selector to loading-spinner CSS

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

ReleaseTaggerBot added a project: MW-1.33-notes (1.33.0-wmf.14; 2019-01-22).Jan 22 2019, 7:01 PM

Jdlrobson moved this task from Needs Code Review to Needs QA on the Web-Team-Backlog-Archived (Readers-Web-Kanbanana-Board-2018-19-Q3) board.Jan 22 2019, 7:45 PM

Edward - you should have everything you need in QA steps. Let me know if that's not the case. This is an interesting kind of bug we hit often- UI regressions. We'd love to automate the testing of these. (bookmark T107588 for a rainy day on that subject:-))

I tested this following the QA steps (very helpful @Jdlrobson! ). The spinner always appeared in the center horizontally. However, it appears to be off-centered vertically. I have attached a screen video. I put some crude red lines to show more clearly.

Change 487869 had a related patch set uploaded (by Jdrewniak; owner: Jdrewniak):
[mediawiki/extensions/MobileFrontend@master] Aligning ImageOverlay prev/next buttons with flexbox.

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

Change 487870 had a related patch set uploaded (by Jdrewniak; owner: Jdrewniak):
[mediawiki/skins/MinervaNeue@master] Align ImageOverlay prev/next buttons with top of viewport.

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

thanks for spotting that @Edtadros!

In the above patches I've aligned the prev/next buttons to the spinner (not the other way around).
@alexhollender I found the prev/next buttons were moved down the page by 3.35em on account of a @headerHeight value here. Since I didn't see a header anywhere, I removed that so they align to top: 0;

This moves the prev/next buttons up a bit, closer to the vertical centre of the page.


before	after

The alignment to the close button could use some work, but modifying that is pretty difficult so I left it as is.

Jdrewniak claimed this task.Feb 4 2019, 6:10 PM

Jdrewniak moved this task from Needs More Work to Needs Code Review on the Web-Team-Backlog-Archived (Readers-Web-Kanbanana-Board-2018-19-Q3) board.

• alexhollender_WMF awarded a token.Feb 5 2019, 5:40 PM

Jdlrobson moved this task from Needs Code Review to Needs More Work on the Web-Team-Backlog-Archived (Readers-Web-Kanbanana-Board-2018-19-Q3) board.Feb 5 2019, 8:27 PM

Jdrewniak moved this task from Needs More Work to Needs Code Review on the Web-Team-Backlog-Archived (Readers-Web-Kanbanana-Board-2018-19-Q3) board.Feb 6 2019, 6:59 PM

Change 487869 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Aligning ImageOverlay prev/next buttons with loading spinner.

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

Change 487870 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Align ImageOverlay prev/next buttons with top of viewport.

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

Jdlrobson moved this task from Needs Code Review to Needs QA on the Web-Team-Backlog-Archived (Readers-Web-Kanbanana-Board-2018-19-Q3) board.Feb 6 2019, 7:46 PM

Over to Edward for a second round of QA (same steps apply) but probably best to wait for another 10 minutes to ensure the change is live (Minerva one has not yet been deployed).

ReleaseTaggerBot edited projects, added MW-1.33-notes (1.33.0-wmf.17; 2019-02-12); removed MW-1.33-notes (1.33.0-wmf.14; 2019-01-22).Feb 6 2019, 8:00 PM