We would like to explore several options for moving infobox below the first paragraph
Create prototoypes for both scenarios where possible - preferably on live data but static pages are okay (feel free to use loot-ui or weekipedia ).
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Resolved | ovasileva | T143139 [EPIC] Move first paragraph <p> above infobox template on small screens | |||
| Resolved | phuedx | T143803 [Spike 4hr]: Explore methods to move first paragraph above infobox |
Change 309117 had a related patch set uploaded (by Bmansurov):
Show the lead section first paragraph before infoboxes
Change 309202 had a related patch set uploaded (by Bmansurov):
WIP: Show the lead section first paragraph before infoboxes
It's worth pointing out hatnotes may appear below the first paragraph with this flex box based approach:
Consider https://trending.wmflabs.org/en/wiki/Sunderland%20A.F.C. where the hatnote appears underneath the lead section (ignore the fact it appears twice - I've been playing with a fix for T143739 in the Mobile content service separately)
There are also various other edge cases that the Android app inside mobile content service is dealing with.
I thus explored this from a slightly different angle.
On the long term I'd like to see MobileFormatter and Mobile Content service converge - ie. do the same things and make it possible for us to use MCS in MobileFrontend.
Given we rely on MobileFormatter for lazy loading images and a host of other things how do people feel about exploring using MobileFormatter to make the lead paragraph, hatnotes and infobox first class citizens? (i.e. mimic the mobile content service approach I proposed here - T145034).
There are probably performance gains to be had here - I notice the API and skin use MobileFormatter in two different places which don't share the same caching and in addition to this it would allow us to control the positioning of the infobox which may be highly useful in tablet mode...
I'm okay with the flex box approach if it's a short term solution, but I think this is a good opportunity to make life easier for ourselves going forward.
Here are the spike results using CSS and MobileFormatter. The MobileFormatter approach is more robust as we only move the first paragraph above the first infobox. The CSS way of doing things makes assumptions about ordering of elements, thus may not be the best solution.
I think @bmansurov's suggestion of using the MobileFormatter makes the most sense. I'm a little concerned about edge cases given the issues I've seen with the implementation in the mobile content service. I think on the long term though we should aim to make the infobox a first class citizen per https://phabricator.wikimedia.org/T143803#2618044
People should chip in with thoughts and a task should be created before this is resolved.
@Dbrant @Mholloway @Niedzielski @bearND @JMinor @Fjalapeno @Mhurd @JoeWalsh adding you here for visibility. If action=mobileview output format changes, we'd need to test. This is just a spike. Do people have the server/client transforms handy from the apps that do this sort of thing today in the apps?
Adam: We didn't impact apps with the lazy loading images work so I don't expect any issues here. Api requests are currently handled separately.
I've probably missed something crucial here. If I understand correctly, we're talking about mobileview API testing in general:
In my opinion, property testing is best performed in the Content Service or MobileFrontend itself. It's easy to examine expected fields in a response.
How the HTML in a response will look is tricky. You can run the Android screenshot tests but these haven't worked as well as hoped as they're integration level (slow and flaky) and don't currently have support for diffing. I think we've focused more lately on moving transforms out of the WebView instead and trusted that it will behave similar to the stock browser. Our loading logic will need a lot more refactoring before we can make these tests effective.
The new Android screenshot tests won't be useful to this end as they are very unit-y, would never make a network request, and only consume completely formed models. These tests are useful for detecting regressions against all supported configurations (locales, strings, images, and other variable inputs) but the data is mocked for each.
The new data client tests use MockWebServer with a mock response but maybe we can make a script to update these periodically. However, most of these data clients only make a dumb network request and provide a fully hydrated model with little manual parsing. They're unlikely to catch bugs affecting presentation.
BTW, AFAIK the Android app still has some native and JS transforms for both Content Service and MediaWiki API responses.
@Niedzielski we will sync with you when we actually plan to make any change. Right now we'll still focused on deciding whether to do this or not so any discussion may be premature. If we do make the change we can easily avoid changes to the mobileview api. We may also want to prioritise work on T139003. Apologies for the confusion.
(@phuedx in case you got lost by the comments my original comment/question is here - https://phabricator.wikimedia.org/T143803#2619937 - please move to sign off if you are happy with the suggested idea for an approach)
It's our (Reading Web's) job to ensure that any changes that we make to the API are tested, which includes your data client tests.
I'm happy that we even explored a CSS-based solution as part of this task.
Generally speaking, any work that we can do server-side, and cache (and edge-cache) should be done server-side. PHP, despite it's failings, is far more flexible than CSS.
Our first task is to create a diverse corpus of pages to test our pages against.
@bearND, @Niedzielski, @Mholloway: Do you have one of these? Would you like one of these?
Per T143803#2622476, I'm happy with the proposed solution – the most flexible, easy to (unit) test one,
@dr0ptp4kt: There's coordination required around when and how to signal making changes to MobileFormatter and mobileview API. AFAICT there's also an opportunity to create a shared test suite for the API, the value of which will have to be demonstrated to our POs /cc @ovasileva
@Jdlrobson: Let's create a separate task about your concerns about the lack of a shared cache (?) between ExtMobileFrontend::DOMParse and the mobileview API.
Change 309202 abandoned by Bmansurov:
Beta: Show the lead section first paragraph before infoboxes
Reason:
Resurrect if needed. Done for now.
Change 309117 abandoned by Bmansurov:
Show the lead section first paragraph before infoboxes
Reason:
Resurrect if needed. Done for now.
@dr0ptp4kt: There's coordination required around when and how to signal making changes to MobileFormatter and mobileview API. AFAICT there's also an opportunity to create a shared test suite for the API, the value of which will have to be demonstrated to our POs /cc @ovasileva
Definitely! As a start, do you think there would be a quick and handy way to watch certain paths in https://www.mediawiki.org/wiki/Git/Reviewers#mediawiki.2Fextensions.2FMobileFrontend and friends? I guess there's PageImages, RelatedArticles, TextExtracts, as well - I don't know if the simplest solution for those is for people to auto add for any change in those repos, given that they're very much API.php oriented (contrast this with MobileFrontend, where all kinds of other changes have little impact on api.php).
The best most future proof solution to this is to [[ T139003 | write tests ]] and run them as part of MobileFrontend which we now do as of this week.
I've created T145216 to capture the outcome of this spike and T145219 to capture my MobileFormatter concerns and a more long term way of using it.
I consider this spike closed.