Page MenuHomePhabricator

Remove spacing between list items
Closed, ResolvedPublic1 Story Points

Description

Ordered lists and definition lists will not include spacing between the lead and the first item of the list, or between individual list items. (spacing will be removed as in T168332#3369896)

Examples:
Go to https://en.wikipedia.beta.wmflabs.org/wiki/Earth, and hover over "planet". For the "planet" preview, the spacing between "...stellar remnant that" and "is massive enough..." will be removed

Go to https://en.wikipedia.beta.wmflabs.org/wiki/Category:Page_previews_debug and hover over "definition list"
For the definition list below, remove spacing (padding/margins) between first paragraph and between each list item

Acceptance criteria

  • No margins/paddings on lists in previews
  • margins/paddings of lists are preserved outside previews.

Testing criteria

Go to https://en.wikipedia.beta.wmflabs.org/wiki/Category:Page_previews_debug and hover over the following items:

  • Definition list
  • Intro of planet articles
  • Ordered list
  • List 1

Ensure preview displays list and does not cut off the bottom items of the list

Event Timeline

I set up a category to play around with ordered and unordered lists and noticed the following:

preview for List1:

preview for planet:

I don't see anything in the wikitext that would make these different

Not seeing in description list:

ovasileva updated the task description. (Show Details)Jun 27 2017, 12:31 PM
ovasileva raised the priority of this task from Normal to High.Jun 27 2017, 2:27 PM
ovasileva moved this task from Incoming to Upcoming on the Readers-Web-Backlog board.

As I mentioned in an earlier task comment this seems tricky with how we currently cut off previews.

Does anybody have an idea of how we'd actually do this without introducing a monospace font? :)

Jdlrobson updated the task description. (Show Details)Jun 27 2017, 4:19 PM
Jdlrobson updated the task description. (Show Details)
phuedx updated the task description. (Show Details)Jun 27 2017, 4:21 PM
phuedx set the point value for this task to 1.
ovasileva updated the task description. (Show Details)Jun 27 2017, 4:23 PM
phuedx added a comment.EditedJun 27 2017, 5:25 PM

As I mentioned in an earlier task comment this seems tricky with how we currently cut off previews.
Does anybody have an idea of how we'd actually do this without introducing a monospace font? :)

We answered this question during grooming/estimation: @Nirzar has approved removing the vertical margins from list items in previews. They should occupy a line, meaning that our truncation mechanism will still work.

pmiazga claimed this task.Jun 30 2017, 6:07 PM

Change 362454 had a related patch set uploaded (by Pmiazga; owner: Pmiazga):
[mediawiki/extensions/Popups@master] Enforce no top&bottom margins on lists on page previews

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

Change 362454 merged by jenkins-bot:
[mediawiki/extensions/Popups@master] Enforce no top&bottom margins on lists on page previews

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

phuedx added a comment.Jul 3 2017, 1:54 PM

👍

With my local PP client pointed at the Beta Cluster's RESTBase instance, I see the following:

You'll note that the text truncation mechanism is working correctly and that there's no over-/underflow of the last list element.

phuedx reassigned this task from pmiazga to ABorbaWMF.Jul 3 2017, 1:55 PM
phuedx added a subscriber: pmiazga.

This needs a test plan /cc @ovasileva @pmiazga

ovasileva updated the task description. (Show Details)Jul 3 2017, 6:39 PM

@phuedx, @pmiazga added test plan. A question though, in https://en.wikipedia.beta.wmflabs.org/wiki/Category:Page_previews_debug, the definition list is the only one that displays the correct preview.

Also, the preview looks great for https://en.wikipedia.beta.wmflabs.org/wiki/Planet, but not for https://en.wikipedia.beta.wmflabs.org/wiki/Intro_of_planet_articles, not sure what's going on.

... A question though, in https://en.wikipedia.beta.wmflabs.org/wiki/Category:Page_previews_debug, the definition list is the only one that displays the correct preview.

It looks like the changes aren't propagating to RESTBase on the Beta Cluster /cc @mobrovac @Pchelolo @GWicke

@phuedx I have just deployed RESTBase in BetaCluster, so it matches the latest code deploy.

Here is what I am seeing:






phuedx added a comment.Jul 6 2017, 9:06 AM

^ @ABorbaWMF: Is that a thumbs up or a thumbs down?

@phuedx I have just deployed RESTBase in BetaCluster, so it matches the latest code deploy.

It seems this still isn't functioning correctly? Note the middle four screenshots. That said, if the issues is just restbase acting up on the beta cluster, with all the other examples being okay, I suppose we're done?

phuedx added a comment.EditedJul 6 2017, 9:55 AM

I updated the "Intro of planet artices" page at 9:33 AM, 6th July 2017 (UTC) and the RESTBase response hasn't updated after 16 minutes [0].

[0]
{
  "title": "Intro of planet articles",
  "displaytitle": "Intro of planet articles",
  "pageid": 186832,
  "extract": "A planet is an astronomical body orbiting a star or stellar remnant that",
  "extract_html": "<p>A <b>planet</b> is an astronomical body orbiting a star or stellar remnant that</p>",
  "lang": "en",
  "dir": "ltr",
  "timestamp": "2017-07-03T18:37:12Z"
}

/cc @mobrovac

Nirzar added a comment.Jul 6 2017, 4:40 PM

@ABorbaWMF unrelated question: what browser is that? the outline for popover is missing. we use double shadow to create the outline.. if that's a very old browser then it's fine but if it's latest safari then I'm worried about the border :(

ovasileva moved this task from In Development to Done on the Page-Previews board.Jul 6 2017, 5:13 PM

@Nirzar I was using crossbrowsertesting and Safari 10

phuedx added a comment.Jul 6 2017, 5:25 PM

@ABorbaWMF unrelated question: what browser is that? the outline for popover is missing. we use double shadow to create the outline.. if that's a very old browser then it's fine but if it's latest safari then I'm worried about the border :(

Confirmed. That's Safari 10+ – I see it in Safari 10.1.1 on macOS Sierra (10.12.5).

@Jdlrobson created T169912 to track not working beta cluster as it seems unrelated to this. Will have a look today.

Sorry for the hold-up folks, RB in Beta works as expected now.

❤️

Null-editing the pages in the Page Previews debug category seems to have resolve the issue with the middle two articles:

and

@ovasileva: Over to you!

ovasileva closed this task as Resolved.Jul 7 2017, 10:24 AM

Whoo! All done! Lists look wonderful.