Page MenuHomePhabricator

QuickSurveys get placed incorrectly in mobile skin
Closed, ResolvedPublic

Description

As a side effect of T115147 which makes mobile skin HTML consistent with desktop HTML the quick surveys on mobile are inserting in the wrong place.

The HTML changes are reflected in https://gerrit.wikimedia.org/r/261218 - the contents of #bodyContent are now wrapped in an element <div id="mw-content-text" lang="en" dir="ltr" class="mw-content-ltr">

There are no surveys currently running but we should get this fixed before the next survey run.

Unit tests will need updating.

Event Timeline

Jdlrobson raised the priority of this task from to Needs Triage.
Jdlrobson updated the task description. (Show Details)
Jdlrobson added a project: QuickSurveys.
Jdlrobson added a subscriber: Jdlrobson.
Restricted Application added subscribers: StudiesWorld, Aklapper. · View Herald TranscriptDec 28 2015, 5:07 PM
Jdlrobson triaged this task as High priority.Dec 28 2015, 6:09 PM
Jdlrobson renamed this task from QuickSurveys get placed incorrectly in mobile to QuickSurveys get placed incorrectly in mobile skin.Dec 28 2015, 7:47 PM
Jdlrobson updated the task description. (Show Details)
Jdlrobson set Security to None.
Jdlrobson moved this task from Needs Analysis to To Do on the Reading-Web-Sprint-63-Ellip… board.

Change 261218 had a related patch set uploaded (by Jhobs):
Update QuickSurveys to new structure of Minerva

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

jhobs moved this task from To Do to Code Review on the Reading-Web-Sprint-63-Ellip… board.

Change 261218 merged by jenkins-bot:
Update QuickSurveys to new structure of Minerva

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

Have asked @jhobs to clean up the tests to make them more readable to mortals before signing off.

12:06 PM <jdlrobson> e.g. assert.ok( this.isPanelElement( $locationMinervaTablet.find( '> div > div' ).children().eq( 1 ) ), 'Check on mobile page it is inserted in correct place (before infobox)' )
12:06 PM <jdlrobson> so apparently $locationMinervaTablet.find( '> div > div' ).children().eq( 1 ) means before infobox
12:06 PM <jdlrobson> but if it say $locationMinervaTablet.find( '.infobox' ).prev() wouldn't that make a lot more sense?

Oh I didn't know you wanted that tied to this task. Patch is already up, so I'll go update the commit message to link it here.

Change 261288 had a related patch set uploaded (by Jhobs):
[Hygiene] Update qunit tests for clarity

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

Jdlrobson closed this task as Resolved.Dec 29 2015, 12:06 AM

and merged :)

@jhobs new release needed for sign off
As shown on http://en.m.wikipedia.beta.wmflabs.org/wiki/Barack%20Obama?quicksurvey=true

survey loads above infobox in mobile.
With new release it should load below first paragraph.

Change 261288 merged by jenkins-bot:
[Hygiene] Update qunit tests for clarity

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

phuedx added a subscriber: phuedx.Jan 20 2016, 11:11 AM

261288 will be released as part of 265239.