Make WordPress Core

Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#25389 closed defect (bug) (fixed)

Twenty Fourteen: Browser height is not filled for short content

Reported by: wycks's profile wycks Owned by: lancewillett's profile 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 11 years ago.
25389.2.diff (1.3 KB) - added by taupecat 11 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 11 years ago.
Supercedes 25389.2.diff; flips that patch in the way Lance suggested.

Download all attachments as: .zip

Change History (16)

#1 @lancewillett
11 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
11 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
11 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 11 years ago by wycks (previous) (diff)

#4 follow-up: @taupecat
11 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
11 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
11 years ago

#6 @wycks
11 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 11 years ago by wycks (previous) (diff)

#7 follow-up: @taupecat
11 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
11 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
11 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 11 years ago by wycks (previous) (diff)

#10 in reply to: ↑ 8 @taupecat
11 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 11 years ago by taupecat (previous) (diff)

@taupecat
11 years ago

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

#11 @lancewillett
11 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
11 years ago

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

#12 @lancewillett
11 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
11 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.