Page MenuHomePhabricator
Create Task
Maniphest T190469

Top alignment for Page Issues Modal
Closed, ResolvedPublic2 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

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

Related Objects

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
Nirzar added a comment.Mar 22 2018, 10:14 PM

@alexhollender to followup on these

ovasileva triaged this task as Normal priority.Mar 23 2018, 11:47 AM
ovasileva edited projects, added Readers-Web-Backlog (Design); removed Readers-Web-Backlog.
alexhollender added a comment.Mar 28 2018, 1:30 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
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
alexhollender added a comment.Mar 28 2018, 4:23 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.

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

The empty p is probably the problem here...

gerritbot added a subscriber: gerritbot.Mar 28 2018, 6:52 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

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.
gerritbot added a comment.Mar 29 2018, 3:26 PM

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

alexhollender added a comment.Mar 29 2018, 5:47 PM

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)

gerritbot added a comment.Mar 29 2018, 11:24 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

Jdlrobson added a subscriber: pmiazga.EditedMar 29 2018, 11:26 PM

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.

gerritbot added a comment.Apr 2 2018, 5:25 PM

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.
Jdlrobson added a comment.Apr 2 2018, 7:54 PM

Staging has been updated with latest master branch.

ABorbaWMF added a comment.Apr 3 2018, 5:11 PM

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

Android/firefox

ios/safari

alexhollender added a comment.Apr 3 2018, 5:47 PM

Talked with Nirzar. It's okay to ignore Firefox here. Thanks for catching that @ABorbaWMF.

ovasileva closed this task as Resolved.Apr 4 2018, 10:29 AM

Looks good then - thanks all!

Content licensed under Creative Commons Attribution-ShareAlike 3.0 (CC-BY-SA) unless otherwise noted; code licensed under GNU General Public License (GPL) or 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