Page MenuHomePhabricator

Add visualhide css from common.css
Closed, ResolvedPublic

Description

See parent ticket for details. This is a subtask to ensure the fix is incorporated and tested on iOS as well.

Details

Related Gerrit Patches:
mediawiki/extensions/MobileApp : masterAdd visualhide class from Common.css

Event Timeline

RHo added a subscriber: RHo.Oct 19 2016, 11:39 PM

Just did a quick check and pulling in the entire common.css on iOS messes up the layout of the article a bit (particularly display of the info box), so suggest that for iOS at least, we only pull in the .visualhide class into our existing styleoverrides.css file.
Quick screencast:
https://youtu.be/OO4rqDLdbmk

Mhurd added a subscriber: Mhurd.EditedOct 20 2016, 12:00 AM

@RHo We have a process for pulling upstream css and iirc we don't pull wholesale as you tried. I can give it a go after I finish the welcome ticket...

RHo added a comment.Oct 20 2016, 12:06 AM

No worries @Mhurd – I just had a quick go at just adding the visualhide class on this PR:
https://github.com/wikimedia/wikipedia-ios/pull/962

Mhurd added a comment.EditedOct 20 2016, 12:09 AM

@RHo Unfortunately that PR won't work because of the way we build our css files. That's actually the output file built when we run Grunt to process our less/css. For the visual hide bits we'll most likely want to have that added upstream so we can run it through the existing process that brings in all our other upstream css.

I assume this can be added it in such a way that both apps can inherit the change :)

RHo added a comment.Oct 20 2016, 12:22 AM

@Mhurd – derp yes I completely blanked on the processing, did it once before so can go through the process again tomorrow (unless you get to it first :p)

I assume this can be added it in such a way that both apps can inherit the change :)

@RHo per Dbrant's comment we'll probably want to check about adding it upstream before we do anything...

RHo added a comment.Oct 20 2016, 7:06 PM

hi @Mhurd – per our chat offline just PR updated with addition to the correct file (misc.less instead of styleoverrides.css) ready for grunting :)

https://github.com/wikimedia/wikipedia-ios/pull/962

Mhurd added a comment.Oct 20 2016, 7:09 PM

@RHo I ran it and pushed. Should be able to pull and test now.

@RHo

Hey I met with @Dbrant and he or one of the android guys are going to add this upstream then I'll sync the iOS app's css again.

Change 317293 had a related patch set uploaded (by Dbrant):
Add visualhide class from Common.css

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

Change 317293 merged by jenkins-bot:
Add visualhide class from Common.css

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

Mhurd added a comment.Oct 24 2016, 6:26 PM

As soon as this gets deployed I'll re-sync iOS css...

Mhurd added a comment.Oct 27 2016, 1:09 AM

@Dbrant hmm I tried re-syncing our CSS and I don't see the addition...

Hey @Mhurd, I think this'll roll out to enwiki (where it looks like you're pulling down from) during today's MediaWiki train deployment. Mind giving it another try after 2pm PDT?

Mhurd added a comment.Oct 27 2016, 8:34 PM

@Mholloway Thanks! It's coming through now :)

== Testing criteria ==

  • On either iPhone or iPad, load the "Top quark" article and ensure the "spin 1/2" fraction in the first paragraph looks correct:

Correct "spin 1/2" fraction:

Incorrect "spin 1/2" fraction:

@Nicholas.tsg Could you add a test case for this? Thanks!

cmadeo added a subscriber: cmadeo.

Fractions look good on phone and tablet.

JMinor closed this task as Resolved.Nov 7 2016, 8:48 PM