Page MenuHomePhabricator
Create Task
Maniphest T143533

Not enough padding for title if Wikidata description is not present
Closed, ResolvedPublic1 Estimated Story Points
Actions

Assigned To
Jdlrobson
Authored By
• Nirzar
Aug 22 2016, 1:57 AM
Tags
Referenced Files
F4561391: Wikidata description no page issues.PNG
Oct 5 2016, 9:03 PM
F4561390: No Wikidata description no page issues.PNG
Oct 5 2016, 9:03 PM
F4561393: Wikidata description page issues.PNG
Oct 5 2016, 9:03 PM
F4561392: No Wikidata description page issues.PNG
Oct 5 2016, 9:03 PM
F4549217: Screen Shot 2016-10-03 at 11.03.37.png
Oct 3 2016, 10:12 AM
F4549216: Screen Shot 2016-10-03 at 10.57.03.png
Oct 3 2016, 10:12 AM
F4549215: Screen Shot 2016-10-03 at 10.56.22.png
Oct 3 2016, 10:12 AM
F4383346: pasted_file
Aug 22 2016, 1:57 AM
Subscribers
Aklapper
• bmansurov
gerritbot
Nicholas.tsg
• Nirzar
ovasileva
phuedx

Description

pasted_file (152×375 px, 17 KB)

The title is almost sticking to the page actions. increase the padding by 0.5em

from:

.feature-page-action-bar-v2 .heading-holder  {
remove - padding-bottom: 3.1em;
add + padding-bottom: 3.6em;

}

Test Plan

Visit pages pages with and without Wikidata descriptions and "page issues" using a Grade A browser, e.g. Safari 9 on an iPhone 6, and observe that the whitespace above the page actions bar is consistent.

Details

SubjectRepoBranchLines +/-
Move positioning logic from tagline to container elementmediawiki/extensions/MobileFrontendmaster+2 -2
Customize query in gerrit

Related Objects

Event Timeline

• Nirzar created this task.Aug 22 2016, 1:57 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 22 2016, 1:57 AM
• Nirzar added a project: Design.Aug 22 2016, 2:07 AM
Jdlrobson triaged this task as Medium priority.Aug 25 2016, 8:42 PM
Jdlrobson moved this task from Incoming to Triaged but Future on the Web-Team-Backlog board.
dr0ptp4kt moved this task from Triaged but Future to Product Owner Backlog on the Web-Team-Backlog board.Aug 25 2016, 8:42 PM
ovasileva added a project: Reading-Web-Sprint-82-Xpect-Rspec.Sep 15 2016, 4:49 PM
• Jhernandez moved this task from Product Owner Backlog to 2016-17 Q2 on the Web-Team-Backlog board.Sep 15 2016, 4:49 PM
MBinder_WMF set the point value for this task to 1.Sep 19 2016, 4:52 PM
MBinder_WMF set the point value for this task to 1.
MBinder_WMF moved this task from Needs Analysis to To Do on the Reading-Web-Sprint-82-Xpect-Rspec board.
Jdlrobson claimed this task.Sep 26 2016, 11:01 PM
gerritbot subscribed.Sep 26 2016, 11:11 PM

Change 312950 had a related patch set uploaded (by Jdlrobson):
Always render tagline container

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

gerritbot added a project: Patch-For-Review.Sep 26 2016, 11:11 PM
Jdlrobson moved this task from To Do to Code Review on the Reading-Web-Sprint-82-Xpect-Rspec board.Sep 26 2016, 11:12 PM
phuedx moved this task from Code Review to Needs More Work on the Reading-Web-Sprint-82-Xpect-Rspec board.Sep 27 2016, 12:57 PM
phuedx updated the task description. (Show Details)Sep 27 2016, 1:11 PM
Jdlrobson moved this task from Needs More Work to Doing on the Reading-Web-Sprint-82-Xpect-Rspec board.Sep 27 2016, 3:17 PM
Jdlrobson added a subscriber: phuedx.Sep 27 2016, 6:09 PM

Thanks for the test plan @phuedx
I have updated the patch.

Jdlrobson moved this task from Doing to Code Review on the Reading-Web-Sprint-82-Xpect-Rspec board.Sep 27 2016, 6:10 PM
Jdlrobson added a subscriber: ovasileva.Sep 27 2016, 9:22 PM

@ovasileva during code review we hit some problems relating to old code that supports the old page actions bar. Given we shipped this 2 months ago is there any reason to continue supporting the old design without the language icon or can I begin work on one bullet point in T130849 to make this task easier?

phuedx changed the task status from Open to Stalled.Sep 28 2016, 7:31 AM

Per T143533#2672128.

ovasileva added a comment.Sep 28 2016, 10:36 AM

@Jdlrobson - go for it

Jdlrobson moved this task from Code Review to Doing on the Reading-Web-Sprint-82-Xpect-Rspec board.Sep 28 2016, 4:01 PM
Jdlrobson moved this task from Doing to Code Review on the Reading-Web-Sprint-82-Xpect-Rspec board.Sep 28 2016, 6:08 PM
Jdlrobson changed the task status from Stalled to Open.Sep 29 2016, 7:41 PM

This patch should not be stalled now.

phuedx added a comment.Sep 30 2016, 4:44 PM

@Jdlrobson: Am I right in thinking that rEMFR25eb6303798a: Move positioning logic from tagline to container element shouldn't be merged until rEMFRea8c58577cd6: Remove MinervaUsePageActionBarV2 config variable is?

Jdlrobson added a comment.Sep 30 2016, 4:48 PM

They are chained that way purposely yes. The patch can be +2ed but will not merge until "Remove MinervaUsePageActionBarV2 config variable " is also merged.

phuedx added a comment.Oct 3 2016, 10:12 AM

After manually rebasing rEMFRc95511653e30: Move positioning logic from tagline to container element, I tested it locally and it LGTM. Given that the change is a tweak of a padding and a margin value, I feel that it's reasonable to restrict browser testing to my browser of choice, Google Chrome (53.0.2785.116) on OS X El Capitan (10.11.6).

@Nirzar: The following are screenshots that I took while testing @Jdlrobson's change on my dev wiki:

Screen Shot 2016-10-03 at 10.56.22.png (222×340 px, 21 KB)

Screen Shot 2016-10-03 at 10.57.03.png (236×336 px, 23 KB)

Screen Shot 2016-10-03 at 11.03.37.png (261×334 px, 27 KB)

IMO our treatment of page issues needs some design attention but that's way out of the scope of this task.

phuedx added a subscriber: • bmansurov.Oct 3 2016, 10:14 AM

@Jdlrobson, @bmansurov: I've left a comment on rEMFR12693bbe42bb: feature-page-action-bar-v2 class is no longer necessary, which might help move the conversation along.

phuedx moved this task from Code Review to Needs More Work on the Reading-Web-Sprint-82-Xpect-Rspec board.Oct 3 2016, 10:16 AM
• Nirzar added a comment.Oct 3 2016, 4:25 PM

Looks good.

IMO our treatment of page issues needs some design attention but that's way out of the scope of this task.

https://phabricator.wikimedia.org/T143535 > on "designing right now" column :)

Jdlrobson added a comment.Oct 3 2016, 6:43 PM

I've replied and suggested an approach to get this done.

Jdlrobson moved this task from Needs More Work to Code Review on the Reading-Web-Sprint-82-Xpect-Rspec board.Oct 3 2016, 6:43 PM
gerritbot added a comment.Oct 3 2016, 9:12 PM

Change 312950 merged by jenkins-bot:
Move positioning logic from tagline to container element

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

• bmansurov moved this task from Code Review to Needs QA on the Reading-Web-Sprint-82-Xpect-Rspec board.Oct 3 2016, 9:28 PM
ReleaseTaggerBot added a project: MW-1.28-release (WMF-deploy-2016-10-04_(1.28.0-wmf.21)).Oct 3 2016, 10:00 PM
Nicholas.tsg subscribed.Oct 5 2016, 9:03 PM

Testing on iPhone 6 (iOS 9.3.2), I took screencaps of how the above Test Plan links looked using my testing device. Padding appears consistent in all examples so this is fixed.

Wikidata description page issues.PNG (2×1 px, 1 MB)

No Wikidata description page issues.PNG (2×1 px, 203 KB)

Wikidata description no page issues.PNG (2×1 px, 189 KB)

No Wikidata description no page issues.PNG (2×1 px, 185 KB)

Nicholas.tsg moved this task from Needs QA to Ready for Signoff on the Reading-Web-Sprint-82-Xpect-Rspec board.Oct 5 2016, 9:04 PM
ovasileva closed this task as Resolved.Oct 6 2016, 1:07 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and 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 · Credits