Twenty Thirteen: fix sidebar overflow: long sidebars overlap footer

[mercime]

Per image attached, sidebar - footer areas on category pages, single post pages with comments disabled, some pages, etc.

[lancewillett]

This is intentional, as part of the design. Leaving ticket open for now, for further discussion and brainstorming ideas to make this look better.

[bpetty]

In the ​Twenty Thirteen announcement it mentions:

"but it supports a sidebar, if you really insist"

I don't see how the theme can "support a sidebar", but you're saying it shouldn't and that this is intended to not work.

The sidebar has still been enabled by default for years, and even if that is changed in core just for this theme in 3.6, there's still millions of blogs with it enabled when they switch to this theme. So while this theme may have been intended to be used without a sidebar, in reality, it's going to be used with a sidebar far more frequently than without (including in a default WP installation if core isn't changed to disabled the sidebar by default).

[lancewillett]

Replying to bpetty:

I don't see how the theme can "support a sidebar"

Themes must explicitly support a sidebar. See register_sidebar() -- ​http://codex.wordpress.org/Function_Reference/register_sidebar. It's an optional theme feature that each theme author can choose to enable if they wish.

[srajbr]

Replying to lancewillett:

This is intentional, as part of the design. Leaving ticket open for now, for further discussion and brainstorming ideas to make this look better.

Hi, I am new to wordpress trac.
From the files, it is purely intentional. But I believe most of the users are going to use sidebar. So, may be it can be made to look like the max-width:999px view (bottom) by default. Its just an option. Or if its really worth to keep the sidebar to right, I guess lots of css changes need to be done. I can help if you want.

[lancewillett]

We're very much open to ideas for improving the footer / sidebar clearing.

[sharonaustin]

Hi, that Twenty Thirteen theme is jaw-dropping gorgeous; one of the most beautiful things I've seen. But if you would, please, reconsider the sidebar issue.

Please don't get rid of the sidebar, or make it difficult for the average user to implement one. As someone who works with institutions who are attempting to use Wordpress as a content management system, not a blog, I personally think much harm would be done for the average Joe to use Wordpress if a determination is made that special steps must be taken to include a sidebar. Please don't do this. Twenty Thirteen is gorgeous, I want it, I love it, but I could not recommend it to others as a CMS if the sidebar is not readily available for the multiple navigation links needed by many agencies.

Again, the theme is absolutely gorgeous. Breathtakingly beautiful. I am always amazed at what the Wordpress team does, always grateful for the determination to make it a platform of the people.

Thanks for listening.

[aaroncampbell]

Replying to sharonaustin:

Please don't get rid of the sidebar, or make it difficult for the average user to implement one. As someone who works with institutions who are attempting to use Wordpress as a content management system, not a blog, I personally think much harm would be done for the average Joe to use Wordpress if a determination is made that special steps must be taken to include a sidebar.

The Twenty Thirteen theme is specifically designed for blogs. Twenty Twelve will still be shipping with WordPress and is probably a better starting place (at least in my opinion) for a WordPress-as-a-CMS theme, especially if you're going to be relying heavily on a sidebar.

[mindctrl]

Related, but different. If this needs to be on a separate ticket, let me know.

Footer widgets that contain a lot of content overflow at the bottom, too. See attached 2013-footer-overflow.png.

[lancewillett]

@mindctrl -- Yes, that's a new ticket.

[lancewillett]

Technique discussed today in IRC during office hours, use JavaScript to calculate the height of the sidebar. If it's greater than the main container + footer, change the main container height to match.

[bpetty]

Is that planned to go into the theme itself then, or are you just saying that's the technique that anyone who wants this behavior to change on their installation has to use in a child theme to fix it?

Also, if anyone else missed it like I almost did, this ticket was actually really addressed in #23644.

[lancewillett]

Just an idea for a technique to go into the theme, nothing planned yet.

[lancewillett]

I didn't intend to close this earlier.

[lancewillett]

@mindctrl Could you re-test the issue you had with the overflow in footer? If it's still occurring, please open a new ticket for us.

[mercime]

use JavaScript to calculate the height of the sidebar. If it's greater than the main container + footer, change the main container height to match.

I was thinking of a simpler solution such as triggering a body class e.g. "add-secondary-widget", if widget is added to main-widget-area-now-secondary-widget-area via functions.php so that there's a hook to manipulate layout when that secondary widget area is used and no more absolute positioning :-)

[srajbr]

Replying to mercime:

use JavaScript to calculate the height of the sidebar. If it's greater than the main container + footer, change the main container height to match.

I was thinking of a simpler solution such as triggering a body class e.g. "add-secondary-widget", if widget is added to main-widget-area-now-secondary-widget-area via functions.php so that there's a hook to manipulate layout when that secondary widget area is used and no more absolute positioning :-)

But this will create another problem- for screen width <= 999px, extra space will be added between the post and the sidebar.

[lancewillett]

Replying to mercime:

I was thinking of a simpler solution such as triggering a body class e.g. "add-secondary-widget", if widget is added to main-widget-area-now-secondary-widget-area via functions.php so that there's a hook to manipulate layout when that secondary widget area is used and no more absolute positioning :-)

That already exists, "sidebar" is added to body class.

[markoheijnen]

Can't we just drop masonry for the footer. We really don't need it there. It can easily be arranged with PHP. It seems we only need to add an additional class every 4th item.

[mercime]

Replying to lancewillett:

That already exists, "sidebar" is added to body class.

You're right, of course. Been mulling about this, CSS won't cut it as it'll ruin the intended design. It can be implement via JS. I saw it done before for absolute positioned sidebar but can't recall where. Shades of: you see it when you don't need it, then when you need it, you can't find it :-)

[aaroncampbell]

Replying to markoheijnen:

Can't we just drop masonry for the footer. We really don't need it there. It can easily be arranged with PHP. It seems we only need to add an additional class every 4th item.

With masonry:

Without masonry (but with .site-footer .widget:nth-child(4n){margin-right:0;}):

I think Masonry goes a long way toward improving the display of the footer widgets when the sizes differ.

[markoheijnen]

Ah didn't noticed that it would also drop in. I thought only lines so without masonry it would drop from 4 to 7.

[mindctrl]

@lancewillet, the footer overflow is no longer happening.

[lancewillett]

Replying to mindctrl:

@lancewillet, the footer overflow is no longer happening.

?? Can you please elaborate? Link to a URL, screenshots?

Scratch that. I misunderstood. Thanks. :)

[sbrajesh]

Here is another take on the patch.
we can simply set the min height on the #main div with the max(height of the contained element, i.e the maximum height from the #tertiary or #primary).
The patch takes care of the margin/padding/positioning of the elements.

[obenland]

Cleaned up version of sbrajesh's approach.

[obenland]

sbrajesh, can you point me to resources about jQuery's height() problem with absolut positioned elements?
I tested it both with .height() and .outterHeight( true ) and came to the same results.

[sbrajesh]

obenland, since we both have used the height/outerHeight on the child of absolutely positioned element, there is no difference.
The height/outerHeight are not recommended to use for the absolutely positioned element(in our case #tertiary). That will give unpredictable result.

​http://api.jquery.com/height/

The only difference in my approach was I used outerHeight to account for margin/padding.

[mercime]

23557.diff

[mercime]

Thanks obenland and sbrajesh! Tested the patches in Chrome, FF, Safari, IE and Opera.

Attached screenshots for tests made on patches:

23557.diff - #tertiary sidebar overlaps the #secondary sidebar below content but clears #site-info per first screenshot above

23557.1.diff and 23557.2.diff - #tertiary sidebar clears <footer> which contains #secondary sidebar and #site-info per second screenshot. Looking good.

Cheers.

[obenland]

Thanks for the side-by-side comparison of the two patches! Let's discuss tomorrow during office hours in which direction we should go.

[mercime]

23557.1.diff and 23557.2.diff

[lancewillett]

@obenland Minor in the JS file, but a few parentheses need spacing, and the 0 and 999 checks can be on the right side of comparisons (not using Yoda here since they are not equal signs).

[obenland]

Can I leave the 0 and 999 checks as they are? I don't see why we should intentionally switch them to non-yoda?

[lancewillett]

Replying to obenland:

Can I leave the 0 and 999 checks as they are? I don't see why we should intentionally switch them to non-yoda?

We discussed this at length in Tucson. :)

Yoda is to avoid assignment using = instead of == or ===. In cases of numerical comparison, it doesn't help and is harder to read.

[lancewillett]

.4 patch tests out well.

In committing this I'm adding one set of parentheses in the tertiary notation on line 16:

secondary = ( 0 == sidebar.length ) ? -40 : sidebar.height(),

for better readability around the comparison statement.

Also updating the adjustFooter comment a tiny bit.

[lancewillett]

In 23676:

Twenty Thirteen: enable a JavaScript-based fix for long sidebar areas that overflow the footer. Props obenland, fixes #23557.