Page MenuHomePhabricator

Investigate solutions for updating presentation of page issues
Closed, ResolvedPublicSpike

Description

Design proposal is in the parent ticket. Investigate and report back possible solutions with an emphasis on work complexity and impact to the existing functionality.


~ Report ~

Intro and Overview

On most wikis page issues in the main namespace are wrapped in an element, usually a table, with the ambox (Article message box) class. Other wikis might use different classes (itwiki uses the class avviso instead). Other wikis use no page issues-specific class (e.g. dewiki) AFAICS.
A couple of years ago the web team had a project to improve the display of page issues from a small link to something pretty close to what we want: https://www.mediawiki.org/wiki/Reading/Web/Projects/Mobile_Page_Issues. Here's an overview of how these are styled in several wikis: https://people.wikimedia.org/~jdrewniak/page_issues_inventory/.
There is code in the MobileFrontend extension and the MinervaNeue skin for dealing with page issues, some of which could be borrowed or at least give a heads-up to potential traps.

Task overview
  1. Unhide ambox
  2. Move lead paragraph up should also move page issues up
  3. Hide collapsible and small text on page
  4. Modify styling of ambox
  5. Add "Learn more" link
  6. Emit event when "Learn more" is tapped
  7. Display page issues when event is received [multiple options]
  8. Optional: remove original severity icon images from DOM
Task details
1) Unhide ambox

In PCS code: Sections.less: remove or modify following rule, or trigger pcs-v2 to be emitted by clients using version: 2 in the Page.setup() call. (Currently version 1 is the default. AFAICS none of the two native apps sets version 2 yet.) When using a browser you could also manually change the class on the root html element from pcs-v1 to pcs-v2 as a quick but temporary solution.

/* Hide page issues */
html:not(.pcs-v2) .ambox {
    display: none;
}

E.g. on enwiki Pinsetter article the table element containing the multiple page issues has following classes:

class="box-Multiple_issues plainlinks metadata ambox ambox-content ambox-multiple_issues compact-ambox"
2) Move lead paragraph up should also move page issues up

In the lead section, we want to page issue to appear before the lead paragraph.
Change the code in lib/mobile/MobileHTML.js to something similar to this:

moveLeadParagraphUp(sectionId, section) {
  let afterElement;
  let child = section.firstElementChild;
  // skip empty elements, consider using isParagraphEligible instead
  while (child && child.textContent.length === 0) {
    child = child.nextElementSibling;
  }
  while (child && child.classList && (child.classList.contains('hatnote') || child.classList.contains('ambox'))) {
    afterElement = child;
    child = child.nextElementSibling;
  }
  LeadIntroduction.moveLeadIntroductionUp(this.doc, section, afterElement);
}
3) Hide collapsible and small text on page

Make sure class="mw-collapsible-content" is still hidden to only show the compact version of the message text on mobile platforms.

mw-collapsible-content,
.mw-collapsible small {
  display: none;
}

On the Pinsetter article this would result in "This article has multiple issues. Please help improve it or discuss these issues on the talk page." This is more text than the design prescribes but it's pretty close. If we really need to hide the 2nd sentence it would get more complicated. Maybe this is good enough for an initial iteration? The web team also leaves this sentence visible.

4) Modify styling of ambox
.ambox td > div, .ambox td > span {
    font-size: 0.8125em; // TODO: find the correct constant
}

/* ensure text is not bolded nor italicized anymore */
.ambox td > div b, .ambox td > span b {
  font-weight: normal;
}

/* remove border */
/* need higher specificity here */
table.ambox-content {
  border: none;
}

TODO:

  • change icon to the correct SVG per design, see the note on "type" and "issue" parameters.
  • probably also need to adjust padding
  • remove links (the only link remaining should be the new "Learn more" link).
5) Add "Learn more" link

As inspiration and for consistency, MFE uses <span class="ambox-learn-more">Learn more</span>. Not sure why they prepend it instead of appending.
To avoid DOM manipulation on the client side consider adding this new DOM element on the server in mobileHTML.js. Remember to I18N the "Learn more" string.
One thing to note is that for multiple page issues the icon is based on the highest severity of all contained issues. See Multiple issues.

6) Emit event when "Learn more" is tapped

Add onclick handler for .ambox-learn-more.

7) Display page issues when event is received
Option A: Very hacky way: Just go to the mobile web page for it

For that we need to know the section the page issue is in, which shouldn't be too hard.
Example:
https://en.wikipedia.org/api/rest_v1/page/mobile-html/Pinsetter?theme=dark
Then in console:

window.location = "https://en.m.wikipedia.org/wiki/Pinsetter#/issues/0"

Pro: Easy
Cons: No themeing, slightly different styles, flash of page content (mobile web) since we are loading that page again. (Probably not going to work for us based on the themeing issue alone.)

Option B: Use web overlay to display page issues

Do similar things that the web team did in skins/MinervaNeue/resources/skins.minerva.scripts/page-issues (https://github.com/wikimedia/mediawiki-skins-MinervaNeue/tree/d01dcb4d6a2f48181a022254974f7ba1d2753726/resources/skins.minerva.scripts/page-issues) but with our layout and our svgs, plus the different colors for the different themes. I think we should at least use that code to learn from it. There is special handling for different namespaces, different selectors beyond .ambox.

Some of the functionality can likely be reused, especially the selectors and cases handled for other languages as well.
We can probably learn a lot from the parsing portion of the code.

The UI looks similar to what we want in principle. A few adjustments are needed:

  • close button should be on the right instead of the left
  • "Page issues" title should be centered horizontally.
  • The <hr>s have lighter color.
  • The date should be moved up, parentheses removed from text. This part is a bit more work but should be doable.

Components:

  • index.js -> initPageIssues(OverlayManager, PageHTMLParser) -> various ways to call insertBannersOrNotice() # adds a "learn more" message (variant B of the A/B test)
  • pageHTMLParser
  • pageIssuesParser with extract()
  • Overlay

We probably don't need to add Overlay and OverlayManager (extensions/MobileFrontend/src/mobile.startup/OverlayManager.js) just to add a single overlay. The OverlayManager in MobileFrontend has routing capability. I believe this changes the window location to add a fragment, like #/issues/1 for issues in section 1, or #/issues/0 for the whole page.
The overlay styles are in extensions/MobileFrontend/skinStyles/mobile.startup/vector.less.
.overlay-enabled .mw-overlays-container rule to show an overlay for the entire page to avoid page interactions, to get the effect of a modal dialogue.

Pros:

  • Only need to implement the UI once
  • Can skip reliance on version 2 and do it for older client versions, too (we could then just remove the CSS rule in step 1)
Option C: Native display of page issues
  • Add a page_issues interaction event, as mentioned in pcs.md.
  • Include event data: an array of page issues, each with a severity for the icon and HTML for the actual full, uncollapsed text.

Note: PCS server already has a simple page issues extract function in lib/transformations/pageextracts/extractPageIssues.js.
Example output: see "issues" property in https://en.wikipedia.org/api/rest_v1/page/mobile-sections/Pinsetter

  • Apps to implement the display of the array of page issues.
8) Optional: remove original severity icon images from DOM

Minor: for performance reasons consider not loading original severity icon images we don't show.

QA ideas

Consider using the Template:Ambox/testcases page as a test case for styling and modifications of icons:

Event Timeline

Restricted Application changed the subtype of this task from "Task" to "Spike". · View Herald TranscriptApr 9 2020, 5:55 PM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
LGoto triaged this task as Medium priority.Apr 14 2020, 3:40 PM
JoeWalsh renamed this task from adopting mobile web presentation of page issues to Investigate solutions for updating presentation of page issues.Apr 14 2020, 4:00 PM
JoeWalsh updated the task description. (Show Details)

Updated the description with the report. Shout-out to the web team for their work in this area. //CC: @Jdrewniak @Jdlrobson.

I left the implementation of the detailed display of the page issues a little high-level for now since it's not clear if we should go with Option B (web UI) or C (native UI).

It would probably be good to find some good test pages for page issues in later sections.

@JoeWalsh, I've updated the task description with the report. Let me know if you have any questions about it.

@bearND thanks for the thorough writeup! No questions at the moment.

Thanks @bearND. I'll follow up with the other PMs and we'll get back to you with any questions/next steps.

bearND subscribed.

Hey @JoeWalsh and @JMinor, do you need any further investigation on this? I'm inclined to mark this task as resolved if you have enough information from Bernds comments.

MSantos assigned this task to bearND.