Changes needed:
Currently: appears at bottom of lead section
Expected on desktop: survey appears floated to right above the image of the cat
Expected on mobile: survey appears above the image of the cat full width
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Resolved | • bmansurov | T113663 Do end of quarter release for QuickSurveys | |||
| Resolved | Jdlrobson | T113651 One last tweak to survey placement.. |
Change 240921 had a related patch set uploaded (by Robmoen):
Final tweaks to QuickSurvey panel placement.
I'll pick up the card since Rob is out today. I hope that's OK since we want to get this merged before today's deployment.
a few suggested improvements on the patch
also would be great if you could +2 https://gerrit.wikimedia.org/r/241900
Two issues came up in testing:
http://en.m.wikipedia.beta.wmflabs.org/wiki/Quick_survey_test_page_stub?quicksurvey=true the survey shows at bottom.
Expected: after first paragraph.
http://en.wikipedia.beta.wmflabs.org/wiki/Quick_survey_test_page_without_lead_section?quicksurvey=true
Expected: in lead section consistent with mobile.
(Also if someone could merge https://gerrit.wikimedia.org/r/242370 that would help with testing)
This is a caching issue. For example, http://en.m.wikipedia.beta.wmflabs.org/wiki/Quick_survey_test_page_stub?quicksurvey=true&x works.
http://en.wikipedia.beta.wmflabs.org/wiki/Quick_survey_test_page_without_lead_section?quicksurvey=true
Expected: in lead section consistent with mobile.
This is also related to old HTML. The new html should not contain empty paragraphs in this article as the name suggests the article does not have a lead section. Re-running browser tests should fix this problem.
Not true. I purged the HTML using action=purge so now I have new HTML and the problem is still there.
With &x I still see the same - the survey is at the bottom of the page rather than after the first paragraph..
http://en.wikipedia.beta.wmflabs.org/wiki/Quick_survey_test_page_without_lead_section?quicksurvey=true
Expected: in lead section consistent with mobile.This is also related to old HTML. The new html should not contain empty paragraphs in this article as the name suggests the article does not have a lead section. Re-running browser tests should fix this problem.
Also not true.
http://en.wikipedia.beta.wmflabs.org/wiki/Quick_survey_test_page_without_lead_section?quicksurvey=true
Change 242782 had a related patch set uploaded (by Bmansurov):
Insert the panel after the lead section if it exists
@Jdlrobson, you were right. I could replicate both problems after a hard refresh. I've submitted a new patch, which takes care of those problems.
It looks like you've worked out the issues that are causing these 2 problems but I think you've overcomplicated your solution. I'd expect this fix to be no more than 5 lines added.
Change 242940 had a related patch set uploaded (by Jdlrobson):
Tweak positioning of Quick Survey
Whilst in this headspace @bmansurov I submitted an alternative patch which seems to fix the problems ^
@Jdlrobson, I've left some comments on your patch. I'm assigning this to you since your patch looks cleaner.
Change 244583 had a related patch set uploaded (by Jdlrobson):
Fix placement of quick survey
I captured the bug @phuedx found in T115058 - I don't think it's a big blocker.
I rewrote the patch with QUnit tests (yay!) so I will be very surprised if you find any issues in https://gerrit.wikimedia.org/r/244583
Please be sure to review its parent commit which just adds tests for the status quo before fixing the edge cases
242940 is C:-1. @Jdlrobson: If you'd like to talk about/pair on this then that'd be great.
Change 242782 abandoned by Bmansurov:
Insert the panel after the lead section if it exists
Reason:
See https://gerrit.wikimedia.org/r/#/c/244583/
Can someone merge https://gerrit.wikimedia.org/r/248986 so I can sign this off? @bmansurov @Jhernandez given that Sam is away that falls to one of you...