Page MenuHomePhabricator

Spike [1 hour]: Find a way to inject the quick survey after lead section on the desktop & mobile experience
Closed, ResolvedPublic

Description

How do we inject the survey in the desktop & mobile experience? Adding JS/CSS modules to RL when viewing article and do it all client side? Using a hook to inject content?

Is there anything stopping us from getting this done?

Any technical challenges?


Answer the questions and explain technically how we're going to do it for desktop & mobile.

Event Timeline

Jhernandez raised the priority of this task from to Medium.
Jhernandez updated the task description. (Show Details)
Jhernandez subscribed.

We'd need to do this with JS and it's a solved problem for mobile. For desktop.. I'm not sure how we get around flashes of unstyled content, but we can certainly append before the first heading in JS.

@Jdlrobson so we would need a initial module we would load always, as small as possible, to bucket the user and then load or not the surveys modules, right? Something like that?

The flash of content would be only the survey appearing before the first heading, right? It's probably a good idea to smooth in the appearance of the banner with some animation.

Jdlrobson set Security to None.
KLans_WMF renamed this task from Spike: Find a way to inject the quick survey after lead section on the desktop & mobile experience to Spike [1 hour]: Find a way to inject the quick survey after lead section on the desktop & mobile experience.Aug 3 2015, 4:20 PM
KLans_WMF updated the task description. (Show Details)

@Jhernandez, yes a small init script would load the rest of the module.

I think FOUC will happen no matter what (even on mobile) if we use JS to inject the survey panel. It makes little sense to do it in PHP because the quick surveys module won't work without JS (as it needs to keep track of answered surveys among other things).

On desktop insert the survey panel after #toc or if #toc doesn't exist then insert it before the first heading.
On mobile insert the survey panel after .toc-mobile or if .toc-mobile doesn't exist, then append it to the lead section using M.getCurrentPage().getLeadSectionElement().

We avoid a flash of unstyled content on the last modified bar, so provided it's top loaded I think this will not be a problem.

Most responsible thing to do is load the logic for whether to show one or not in the top and show an ajax loader in the spot it's going to be loaded and load the JS for the panel in the bottom.

I would rather not depend on the table of contents in either case. In mobile this could lead to a race condition unless we make mobile.toc a dependency - so let's not do that. Adding to lead section and before first heading seem to be the most preferable ideas to me. Why do we need to even consider the TOC?

You're right, we don't have to consider the TOC. We can just use headings. I wonder how we should handle pages with no headings.

var $firstHeading = $( 'h1,h2,h3,h4,h5' ).eq( 0 );
$firstHeading.length === 0 ? $panel.insertBefore( $firstHeading ) : $panel.appendTo( '#content' )

I don't think we should include h1 because it's usually (always?) the page title. And === should be !==, right?

Also on desktop appending to #content will push the panel down if the content is too long. How about inserting the panel after the first paragraph inside #mw-content-text if no heading is present on the page? Same thing on mobile too?

Not always.. = Foo = is perfectly valid wikitext. h1 is outside #content
#mw-content-text doesn't exist on mobile but yeh that's fine for desktop.

h1 is actually inside #content. See Obama's page for example.

So for desktop $( '#bodyContent' ).find( 'h1,h2,h3,h4,h5,h6' ).eq( 0 ) is what you want.
In mobile $( '#content' ).find( 'h1,h2,h3,h4,h5,h6' ).eq( 0 )

We should see this as an opportunity to make the mobile skin more consistent with desktop skins for this:
In mobile skin:
#content_wrapper should be changed to #content
and #content to #bodyContent

OK, here is the summary:

Desktop
Insert the panel after the first heading. If the first heading is an h1, make sure that it's not #firstHeading because that's the page title. If there are no headings on the page, insert the panel after the first p inside #mw-content-text. If that fails, append the panel to #mw-content-text.

Mobile
Append the panel to the lead section, which can be queried like M.getCurrentPage().getLeadSectionElement().

As for FOUC:

Most responsible thing to do is load the logic for whether to show one or not in the top and show an ajax loader in the spot it's going to be loaded and load the JS for the panel in the bottom.

See https://phabricator.wikimedia.org/T107597#1507806
I'd rather we avoided the special casing and made this as simple as possible - there should not be different code paths for mobile/desktop.

Seems like that should be a trivial change (obviously must be carefully done though!)

$ ag -a '#content' resources/ | wc -l
18

That's 18 lines that'll need to be evaluated and changed as necessary.

I couldn't spot any relevant instances of 'content' in the MobileFrontend PHP code, so I dropped includes/ from the above. Y'all should double-check though…

Regardless, I do think that we should make the proposed change.

Yup also good with me. I setup T108075 to reflect the hygiene change.
Baha could you summarise with this in mind to check we are all on the same page?

Once T108075 is done, I think it's pretty straightforward as summarized above. We still need to make sure that the insertion code takes into account various cases such as whether a heading exists, whether it's the page title, etc. So the instructions for Desktop described in T107597#1507812 should apply for both desktop and mobile.

Awesome, thanks folks.

I'm finding myself a little confused here by use of word "after" and "#mw-content-text" in https://phabricator.wikimedia.org/T107597#1507812

Does this summarise it:?

Insert the panel before the first heading found in #bodyContent
If there are no headings on the page, insert the panel before the first p inside #bodyContent
If that fails, append the panel to #bodyContent

Moving back to sign off since I'm implementing this now and am not 100% clear.

You're right, before the first heading, but after the first p because otherwise it would look weird to see the panel before any text.

Close at will @Jdlrobson when it's clear enough, seems to me like it is
fine now.