WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#25389 closed defect (bug) (fixed)

Twenty Fourteen: Browser height is not filled for short content

Reported by: wycks Owned by: lancewillett
Milestone: 3.8 Priority: normal
Severity: normal Version: 3.8
Component: Bundled Theme Keywords: 2nd-opinion ui-feedback
Focuses: Cc:

Description

When posting content that doesn't fill up the browser's height, for example a short page with comments turned off, the sidebar and footer does not expand to fill 100%.

Current problem:
http://i.imgur.com/E6orA74.jpg

What it should look like:
http://i.imgur.com/DqXWciB.jpg

Possible solutions: javascript, cross browser hacks (CSS), changing the box model to adhere to a 100% height.

Attachments (3)

25389.diff (383 bytes) - added by wycks 7 years ago.
25389.2.diff (1.3 KB) - added by taupecat 7 years ago.
Supercedes 25389.diff; tests for presence of footer widgets and only applies min-height if no footer widgets exist.
25389.3.diff (1.3 KB) - added by taupecat 7 years ago.
Supercedes 25389.2.diff; flips that patch in the way Lance suggested.

Download all attachments as: .zip

Change History (16)

#1 @lancewillett
7 years ago

  • Cc iamtakashi added
  • Keywords ui-feedback added
  • Milestone changed from Awaiting Review to 3.8

#2 in reply to: ↑ description ; follow-up: @iamtakashi
7 years ago

Replying to wycks:

I personally don't think that's a huge deal but if we can achieve that with only CSS without JS and not to compromise anything else, that could be a "nice to have" improvement. @wycks, are you able to cook a patch?

#3 in reply to: ↑ 2 @wycks
7 years ago

Replying to iamtakashi:

I also think it's minor, I tried a patch but ran into several problems, it's actually quite tricky.
The CSS hacks are not very browser compatible, especially for >IE8 and the mixture of media queries. For example there is an very easy way to do this just using something like:

#secondary{height:100vh;}

But it's not 100% compatible: http://caniuse.com/#feat=viewport-units

I don't like the idea of JS adjusting the DOM on the fly.

That leaves adjusting the structure which is a larger undertaking, for example making sure the html, body and sidebar descendants are all set to 100% height, this doesn't work outside the box so I will play around with it some more to see if a simple changes works.

Last edited 7 years ago by wycks (previous) (diff)

#4 follow-up: @taupecat
7 years ago

I like the 100vh solution: it's simple, it's performant, and it's IE9+ compatible. Couldn't we implement that and just not worry about older versions of IE? After all, http://dowebsitesneedtolookexactlythesameineverybrowser.com/

#5 in reply to: ↑ 4 @iamtakashi
7 years ago

Replying to taupecat:

Couldn't we implement that and just not worry about older versions of IE?

Agreed. I don't think we need to make things complicated for older versions of IE for this matter as it's not really broken or anything.

@wycks
7 years ago

#6 @wycks
7 years ago

Tested in Chrome 29, FF 22+ and ie9. It's not perfect but I think it's better than having it float mid page.

To test, create a post without comments and no content.

Last edited 7 years ago by wycks (previous) (diff)

#7 follow-up: @taupecat
7 years ago

The net result of this patch is the footer is now always "below the fold", no matter how tall the viewport is. If we're okay with this, then the patch works as advertised.

Food for thought for Twenty Fifteen: flexbox (a very promising CSS technique that's not QUITE ready for prime time, mostly b/c of IE support), has a better solution for the sticky footer problem. See http://philipwalton.github.io/solved-by-flexbox/demos/sticky-footer/

#8 in reply to: ↑ 7 ; follow-up: @lancewillett
7 years ago

Replying to taupecat:

The net result of this patch is the footer is now always "below the fold", no matter how tall the viewport is. If we're okay with this, then the patch works as advertised.

We could only apply the rule when no footer widgets exist, based on body_class value.

#9 @wycks
7 years ago

The flexbox technique will show the same result, the problem is not the viewport units, it's the dom structure which would require re-doing.

When widgets are present it's still below the fold, but it does look better.

Last edited 7 years ago by wycks (previous) (diff)

#10 in reply to: ↑ 8 @taupecat
7 years ago

Replying to lancewillett:

We could only apply the rule when no footer widgets exist, based on body_class value.

So right now, there is no change in the body_class value when there are footer widgets, based on my observations at least. So a patch for this would need to:

  • Set the body_class with a footer widget indicator
  • Change the CSS to require that body_class NOT be present in order to apply the 100vh

Additionally, we should probably change the "height: 100vh" to be "min-height: 100vh", as that's more precisely what we're trying to accomplish.

Last edited 7 years ago by taupecat (previous) (diff)

@taupecat
7 years ago

Supercedes 25389.diff; tests for presence of footer widgets and only applies min-height if no footer widgets exist.

#11 @lancewillett
7 years ago

I love the logic -- but it seems backward to always add the body class for the absence of footer widgets.

Can you turn it around so that the body class is only when footer widgets exist? Then the style can be applied to #secondary rule just above, and turned off by the body class: min-height: 0.

@taupecat
7 years ago

Supercedes 25389.2.diff; flips that patch in the way Lance suggested.

#12 @lancewillett
7 years ago

  • Owner set to lancewillett
  • Resolution set to fixed
  • Status changed from new to closed

In 25736:

Twenty Fourteen: fill browser height to 100% for views with short content. Accounts for footer widgets, in which case the height isn't set to 100% so that the widgets remain visible. Props wycks and taupecat, fixes #25389.

#13 @DrWeidinger
7 years ago

Hi Team,

How would one go about integrating this fix? I'm running WordPress 3.8.1 and Twenty Fourteen 1.0, but still experiencing the bug as described at the beginning of this thread. I've tried a few things, namely taupe cat's link: http://philipwalton.github.io/solved-by-flexbox/demos/sticky-footer/ but to no avail.

Suggestions would be appreciated. Thanks!

Note: See TracTickets for help on using tickets.