Maniphest T190469

Top alignment for Page Issues Modal
Closed, ResolvedPublic2 Estimate Story Points
Actions

Description

T187916: Improve Page Issue modal
After we finished work on modal, we have 1 minor follow up tweak around this work

The top alignment for text and the icon

Note: modal name should be "Page issues" (as is currently)

Developer notes

Can be seen here: https://en.m.wikipedia.org/wiki/Offset_printing

While tempting to add a top: -2px we should investigate the route cause - the HTML elements themselves are aligned but the paragraph o text gets top padding from somewhere. We should not incur tech debt as part of this change. We may need to look at the implementation of mw-ui-icon

Testing Criteria

visit http://reading-web-staging.wmflabs.org/wiki/Infrastructure?useformat=mobile#/issues
ensure that the top of the icon (orange circle) is aligned with the top of the text ("This article...")
check on multiple browsers (mobile & desktop)
repeat steps 1–3 with http://reading-web-staging.wmflabs.org/wiki/Rowing_(sport)?useformat=mobile#/issues

Details

Related Gerrit Patches:

mediawiki/extensions/MobileFrontend : master	Issue line height makes icon and text vertically aligned
mediawiki/extensions/MobileFrontend : master	Issue line height makes icon and text vertically aligned

Related Objects

Mentioned Here: T187916: Improve Page Issue modal

Event Timeline

• Nirzar created this task.Mar 22 2018, 10:14 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 22 2018, 10:14 PM

@alexhollender to followup on these

ovasileva triaged this task as Medium priority.Mar 23 2018, 11:47 AM

ovasileva edited projects, added Readers-Web-Backlog (Design); removed Readers-Web-Backlog.

• Nirzar moved this task from Needs analysis to Designing/discussing right now on the Readers-Web-Backlog (Design) board.Mar 23 2018, 5:49 PM

alexhollender moved this task from Design to Triaged but Future on the Readers-Web-Backlog board.Mar 28 2018, 1:15 PM

alexhollender edited projects, added Readers-Web-Backlog; removed Readers-Web-Backlog (Design).

ovasileva moved this task from Triaged but Future to Upcoming on the Readers-Web-Backlog board.Mar 28 2018, 1:25 PM

I'm seeing "Page issues" as the modal title

Jdlrobson renamed this task from follow up fixes for Page Issues Modal to Top alignment for Page Issues Modal.Mar 28 2018, 1:41 PM

Jdlrobson updated the task description. (Show Details)

ovasileva updated the task description. (Show Details)Mar 28 2018, 1:59 PM

alexhollender updated the task description. (Show Details)Mar 28 2018, 4:09 PM

ovasileva set the point value for this task to 2.Mar 28 2018, 4:14 PM

ovasileva added a project: Readers-Web-Kanbanana-Board-Old.

ovasileva moved this task from Upcoming to 2017-18 Q3 on the Readers-Web-Backlog board.

Jdlrobson updated the task description. (Show Details)Mar 28 2018, 4:14 PM

You can use the ::first-line selector and specify a line-height of 1 to achieve this. Not sure if that method is allowed.

The empty p is probably the problem here...

Jdlrobson moved this task from Doing to To Do on the Readers-Web-Kanbanana-Board-Old board.Mar 28 2018, 6:49 PM

Change 422464 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] Issue line height makes icon and text vertically aligned

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

gerritbot added a project: Patch-For-Review.Mar 28 2018, 6:52 PM

Jdlrobson removed Jdlrobson as the assignee of this task.Mar 28 2018, 6:52 PM

Jdlrobson moved this task from To Do to Needs Code Review on the Readers-Web-Kanbanana-Board-Old board.

Change 422464 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Issue line height makes icon and text vertically aligned

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

ReleaseTaggerBot added a project: MW-1.31-release-notes (WMF-deploy-2018-04-03 (1.31.0-wmf.28)).Mar 29 2018, 4:00 PM

I've updated https://reading-web-staging.wmflabs.org/ so you can test this there.

This achieves the top alignment but appears to have the unintended consequence of the line-height being 1 for all the text in the modal

(related to your comment above: when I delete that empty <p> tag from the HTML the problem persists)

alexhollender reassigned this task from alexhollender to Jdlrobson.Mar 29 2018, 5:47 PM

alexhollender moved this task from Needs Design Review to Needs More Work on the Readers-Web-Kanbanana-Board-Old board.

Jdlrobson moved this task from Needs More Work to Doing on the Readers-Web-Kanbanana-Board-Old board.Mar 29 2018, 11:13 PM

Jdlrobson moved this task from Doing to Needs Code Review on the Readers-Web-Kanbanana-Board-Old board.Mar 29 2018, 11:23 PM

Change 423066 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] Issue line height makes icon and text vertically aligned

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

I've gone with the first-line technique.I cannot find a better solution and the time investment I'm making fighting for one is not worth it.

It's on staging for design review (but also needs code review!).

@pmiazga @alexhollender we'll need to merge this before Tuesday AM UTC to make sure it doesn't go out in current form so this is time sensitive.

Looks good from a design perspective (feel free to skip design QA if no code changes).

Tested on:
http://reading-web-staging.wmflabs.org/wiki/Infrastructure?useformat=mobile#/issues
http://reading-web-staging.wmflabs.org/wiki/Rowing_(sport)?useformat=mobile#/issues

@Niedzielski to review.

Niedzielski reassigned this task from Jdlrobson to alexhollender.Apr 2 2018, 5:17 PM

Niedzielski moved this task from Needs Code Review to Needs Design Review on the Readers-Web-Kanbanana-Board-Old board.

Change 423066 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Issue line height makes icon and text vertically aligned

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

alexhollender reassigned this task from alexhollender to ABorbaWMF.Apr 2 2018, 6:25 PM

alexhollender updated the task description. (Show Details)

alexhollender moved this task from Needs Design Review to Needs QA on the Readers-Web-Kanbanana-Board-Old board.

Staging has been updated with latest master branch.

Looks pretty good on staging across a selection of browsers, except for Firefox. It looks a little high to me, but not terribly so. @alexhollender, can you take a look at firefox and if it's good to you, we can move to PM signoff.

android/chrome - android/android - android/opera