Page MenuHomePhabricator

On long infoboxes, section 1 is pushed down page despite space available
Closed, ResolvedPublic5 Story Points

Description

The body of the article doesn't appear until after bottom of the right-hand sidebar, which can be pretty far down the page

  • Desktop with Windows 10, ver 1709
  • Tested with 2 browsers, Opera 51.0.2830.55 and Edge 41.16299.248.0
  • Not sure when problem began. I first noticed it a few days ago, but am pretty spaced out at times.
  • Tested all of the skins in Preferences. Problem only occurs with MinervaNeue skin

To reproduce:

  1. Go to Preferences>Appearance
  2. Select MinervaNeue skin
    • Don't use Preview. Problem doesn't appear in Preview, which only shows the Home page in the test skin format. If a link in Preview pg is clicked, the page appears in the user's saved preference skin, not the test skin.
  3. Click Save
  4. Go to[ https://en.wikipedia.org/wiki/Black-capped_chickadee | Black-capped chickadee ]] or Henry VIII
  5. After intro section, body of article doesn't start until the bottom of right-hand infobox

Comparison: any other of the skins

bugs to fix

developer notes

Alex provided a mock to demonstrate how this should behave
https://codepen.io/alexhollender/pen/yKvrMx
top-section and panel correspond to mf-section-{number} elements

This shouldn't require any HTML changes.

Testing

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
Jdlrobson updated the task description. (Show Details)Mar 14 2018, 4:29 PM

I've split the TOC issue into T189699!

@Jdlrobson I vote for the content wrapping around the infobox like it does in Vector

@alexhollender the problem is the current hierarchy of the page due to section wrapping.

  • div.mf-section-0
    • p
    • .infobox
  • h2
  • div.mf-section-1

We'd also need to work out how the break point so that it snaps to the right without impacting the collapse/expand behaviour of desktop


I'd suggest creating a prototype would be the way forward to determine whether

  1. We can achieve this via CSS alone
  2. If we can achieve it via HTML what those HTML changes should be.
ovasileva triaged this task as Normal priority.Mar 28 2018, 3:56 PM
alexhollender added a comment.EditedMar 29 2018, 6:36 PM

@Jdlrobson alright so I got the behavior we're going for here, and I think the HTML structure is the same as on Wikipedia (i.e. this can be done with CSS alone). Check it out: https://codepen.io/alexhollender/pen/yKvrMx

Nirzar pointed out that it would also be good to default TOC to open for larger/desktop screen sizes.

Nirzar pointed out that it would also be good to default TOC to open for larger/desktop screen sizes.

The reason we collapse it is because we render this via JavaScript and want to avoid a flash of unstyled content. If we want to restore this we'll need to use the HTML for the Vector table of contents and start shipping it in the HTML.

Jdlrobson updated the task description. (Show Details)Apr 2 2018, 5:19 PM
Jdlrobson set the point value for this task to 5.Apr 3 2018, 4:45 PM

When working on this we'll be worried about edge cases and different browsers.

TheDJ added a subscriber: TheDJ.Apr 4 2018, 12:54 PM

Change 428605 had a related patch set uploaded (by Jdrewniak; owner: Jdrewniak):
[mediawiki/skins/MinervaNeue@master] Display content beside infobox instead of below.

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

Edge-cases abound, but I think these few lines of CSS get us pretty far.

In the Empire Of Japan article for example, a special infobox is still pushing down the content below it.


Could be resolved by changing the clear: right to a clear: left

Jdrewniak added a subscriber: ovasileva.

I've put this patch on staging per request from @alexhollender and putting in design review.

I've noticed double infoboxes no longer stacking like they use to on: http://reading-web-staging.wmflabs.org/wiki/Iowa%20and%20http://reading-web-staging.wmflabs.org/wiki/Nebraska - let's address that in a follow up patch though.

TheDJ added a comment.EditedApr 25 2018, 6:22 AM

Could be resolved by changing the clear: right to a clear: left

hmm. if you don't clear right, why would you introduce a clear left than though ? Why not simply clear:none ? I don't really understand.

It also doesn't really solve the problem. The problem here is floating content on the left and the right side interleaving. With the floating content on the left being the ToC. This is why the ToC in Vector uses display:table instead of float/clear. It sidesteps the problem. But even on desktop you can see this problem, if people start interleaving thumbnails on the left with infobox and images on the right. At some point, it will start clearing regardless, that's the nature of float. We can't really fix that unless we stop using floating thumbnails.

Oh, and when your headers are no longer 100% wide, you might have to set overflow:hidden on them to start a new layout context, to avoid the [[WP:BUNCH]]ing problem, see also: T28449. [edit] Actually, inspection shows that headers use table layout in Minerva, which also forces a new context and thus has the same effect (already works).

@Jdlrobson alright, just checked out ~60 pages. Here are a few things I'm seeing:

  1. not working on certain pages (e.g. New Delhi, Rotary International, Ulaanbaatar, Sapporo, Castro District, NBC News, Jackfruit)
  1. gray bg on .hatnote within section not respecting infobox (examples here: Kolkata #Etymology, West Bengal #Etymology)

  1. Taiwan - funky situation where section has 2 infoboxes
  1. Commonwealth of Nations #Structure example of image spanning multiple sections
  1. Ford Model T #Characteristics example of sub-section (Engine) not wrapping
  1. San Francisco Ferry Building #History example of random white space in section (scroll down a bit)

Additionally, because of long infoboxes, sometimes image are getting pushed down past the section that they correspond with. Compare Nepal (production) with Nepal (staging), and notice the position of the first image ("Lumbini"). The image, I believe, is meant to be part of the "Ancient" sub-section of "History", however because of the long infobox it gets pushed down.

I wonder if adding a max-height on infoboxes, with some kind of expand functionality, might alleviate a bunch of the issues above. I also don't know enough about how editors position images relative to text in order to understand whether or not images getting shifted down a bit within a section is a big deal or not.

San Francisco Ferry Building #History example of random white space in section (scroll down a bit)

Not sure what you mean here.

Additionally, because of long infoboxes, sometimes image are getting pushed down past the section that they correspond with. Compare Nepal (production) with Nepal (staging), and notice the position of the first image ("Lumbini"). The image, I believe, is meant to be part of the "Ancient" sub-section of "History", however because of the long infobox it gets pushed down.

This is also the case on Vector so this is actually more consistent with desktop which is a good thing - https://en.wikipedia.org/wiki/Nepal?useskin=vector

Jdlrobson updated the task description. (Show Details)Apr 25 2018, 8:33 PM

^ @Jdrewniak I think is a great first start. I've summarised the bugs in the description

@Jdlrobson this is what I was seeing re:

San Francisco Ferry Building #History example of random white space in section (scroll down a bit)

Watch gif. Let me know if this clarifies

Jdlrobson updated the task description. (Show Details)Apr 25 2018, 9:56 PM
TheDJ added a comment.Apr 26 2018, 5:56 AM

@alexhollender number 2, is the same as the last point regarding headers in my comment T189688#4156218.
It is not related to this ticket, but is something that should probably be fixed, I already made the same fix recently on en.wp

@TheDJ thank you for your insights and mentioning the concept of "layout contexts". This concept is key in managing floats, and I think some of the techniques used in Vector can be handy here (in the latest patch I switched the TOC to display: table like in Vector).

As mentioned, floated thumbnails can be a major issue, since they might be pushed below/next to the last floated element.


To prevent this, I've used the overflow:hidden technique on content sections to contain their floated images, and removed their width:100% so that they may take up the available space next to the infobox. This results in a (more than) slightly different layout than the desired one. The content section sits beside the infobox, but it doesn't take up space below the infobox. The proceeding sections however, do take up the available width. Hopefully this is still preferable to the current situation.

TheDJ added a comment.EditedApr 26 2018, 6:35 PM

I misremembered, the correct terminology is block formatting context

TheDJ added a comment.Apr 26 2018, 6:46 PM

@Jdrewniak I think this is an interesting approach. Worth of trying out in production.

I think you've mostly nailed this, but I'm still seeing side by side infoboxes on http://reading-web-staging.wmflabs.org/wiki/Taiwan when I close the table of contents. I definitely fix this is an improvement however on this existing styles. @alexhollender what do you think about merging this patch and iterating off of it for the remaining edge cases?

@alexhollender @TheDJ I've updated the staging environment http://reading-web-staging.wmflabs.org/ if you want to try these style changes out on different articles. I'd love to see this go out in the train next week.

@Jdrewniak @Jdlrobson damn this is awesome o_0 !!!! It feels so great to get rid of all of that white space. I'm cool with releasing this then iterating further. In terms of follow up items, I think we've got:

any others?

Change 428605 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Display content beside infobox instead of below.

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

The change is merged and now reflected on staging and beta cluster. Please have a test and think about anything you'd like Anthony to look at during QA. If passing to him, be sure to let him know specifically what to test!

@Jdlrobson not seeing any new issues. Will add some notes and pass to Anthony.

Noting a few things here so it's documented for creating follow-up tasks:

-doesn't seem to be working on: Hawaii
-curious if the left-aligned table on United_States#Demographics has anything to do with this change?

alexhollender updated the task description. (Show Details)

doesn't seem to be working on: Hawaii

This is behaving correctly. It uses the "clear" template. Check desktop out.

United_States#Demographics

I think I've got a solution for this: https://phabricator.wikimedia.org/T193418#4169629

@Jdlrobson just took a look around. Not seeing many issues. For some reason a few funky things with this page, which I couldn't find on any other pages:

  • go to Ulaanbaatar — check out the first section, Names and etymology, with the toc collapsed (notice the image that should be floated left appears at the bottom of the section), then open the toc and look at it again (displays correctly).
  • if you navigate to this URL (Ulaanbaatar#Names_and_etymology) the toc automatically expands. This doesn't seem to be the case with any other section. Also this doesn't seem to be happening on production
  • clicking the first item in the toc (Names and etymology) on Ulaanbaatar doesn't take you to that section

@Jdlrobson just took a look around. Not seeing many issues. For some reason a few funky things with this page, which I couldn't find on any other pages:

  • go to Ulaanbaatar — check out the first section, Names and etymology, with the toc collapsed (notice the image that should be floated left appears at the bottom of the section), then open the toc and look at it again (displays correctly).

I've submitted a new patch and added it to staging which removes "clear" commands. Do you think this helps things? It does mean in certain cases in the lead section there may be an image next to the infobox (e.g. this particular example) or 2 images next to each other (e.g. http://reading-web-staging.wmflabs.org/wiki/Salinity)
Extreme example: http://reading-web-staging.wmflabs.org/wiki/Black-figure_pottery

  • if you navigate to this URL (Ulaanbaatar#Names_and_etymology) the toc automatically expands. This doesn't seem to be the case with any other section. Also this doesn't seem to be happening on production

Yeh, unrelated. I opened T193515 but then declined it. Probably something weird about our setup but doesn't seem to be an actual problem.

  • clicking the first item in the toc (Names and etymology) on Ulaanbaatar doesn't take you to that section

Unrelated.. it seems the table of contents is re-rendering the span: T193520

It looks like what's happening here is editors are floating items based on the table of contents on desktop. I think we're gonna struggle to fight this, so we may have to live with the whitespace unless @Jdrewniak our resident CSS expert has some good ideas when he gets back!

Jdlrobson removed ABorbaWMF as the assignee of this task.May 1 2018, 9:09 PM
Jdlrobson added a subscriber: ABorbaWMF.

Does this need QA or sign off @alexhollender ? it's unclear.

@Jdlrobson you probably know better than I do. Considering the large scope of impact here I imagine it's worth having a professional QA pass. I wrote testing instructions in the ticket.

@ABorbaWMF can you take a look at various pages on a tablet device using reading web staging and see if you notice anything unusual. By unusual I mean text being scrunched up into small spaces or large chunks of whitespace in the article.
e.g.

Thanks!

Looks good to me on a variety of browsers.






Confirmed all the current edge cases, @Jdrewniak, @Jdlrobson - should we open a new task for these/are any of them actionable? @ABorbaWMF - did you get a chance to test through a number of different pages? I just want to make sure we're not missing further edge cases.

ABorbaWMF added a comment.EditedMay 3 2018, 5:17 PM

Testing the remaining articles. I will update this as I go.

Rotary International - Looks good, except for an issue on IE 11 where I am seeing a vertical title in the infobox

MAC/SafariMAC/ChromeMAC/Firefox
Windows/EdgeWindows/IE 11Windows/Opera

Ulaanbaatar - There is a gap (zoomed out for screenshots)

MAC/SafariMAC/ChromeWindows/Chrome
Windows/IE 11Windows/EdgeWindows/Opera

Sapporo - Looks ok

MAC/SafariWindows/ChromeMAC/Firefox
Windows/IE 11Windows/EdgeMAC/Opera

Castro District - Looks ok

MAC/SafariWindows/ChromeMAC/Firefox
Windows/IE 11Windows/EdgeMAC/Opera

NBC News - Looks ok

MAC/SafariMAC/ChromeMAC/Firefox
Windows/EdgeWindows/IE 11Windows/Opera

Jackfruit - Looks ok

MAC/SafariMAC/ChromeMAC/Firefox
Windows/EdgeWindows/IE 11Windows/Opera

Commonwealth of Nations - Looks ok

MAC/SafariMac/OperaMac/Chrome
Windows/FirefoxWindows/IE 11Windows/Edge

Ford Model T - Looks ok

MAC/SafariMAC/FirefoxMAC/Opera
Windows/ChromeWindows/IE 11Windows/Edge

San Francisco Ferry Building - Looks ok

MAC/SafariMAC/FirefoxMAC/Opera
Windows/ChromeWindows/IE 11Windows/Edge

@ABorbaWMF thanks for shedding light on that IE11 issue, I've opened a ticket for that over here T193881

ovasileva updated the task description. (Show Details)May 9 2018, 11:08 AM

@Jdrewniak - not sure if this is due to other issues with staging, but I'm seeing a few weird things:

This comment was removed by Jdlrobson.
This comment was removed by Jdlrobson.

Staging was having some issues. I've updated the code there and it seems to have fixed a few of the issues I was seeing. @ovasileva can you take another look?

The toc is back! But I am still seeing the issue with this article (subheading not filling white space):

http://reading-web-staging.wmflabs.org/wiki/Ford_Model_T#Engine

@ovasileva we can only do so much... that space is there because the editors are using a {{Clear}} template that intentionally pushes down the content. 🤷‍♂️

ovasileva closed this task as Resolved.May 10 2018, 2:36 PM

Sounds good to me