Remove scrollbar and max-height from help tabs

[ocean90]

In Firefox there are two scrollbars: ​http://cl.ly/BZQg
In Chrome is only one for all columns: ​http://cl.ly/BYJE

[ocean90]

Okay, in Chrome the window was wide enough, so the text fit into the box. But the scrollbar for the links is still there.

[olleicua]

My scrollbars in FF7 are in a completely different place see ​help.png. Wouldn't it make the most sense to just not define a maximum height for the help panels so that it is as tall as it needs to be, no scrolling necessary. This seems like it would be a bit friendlier to me.

[jane]

Replying to olleicua:

My scrollbars in FF7 are in a completely different place see ​help.png. Wouldn't it make the most sense to just not define a maximum height for the help panels so that it is as tall as it needs to be, no scrolling necessary. This seems like it would be a bit friendlier to me.

Yes, the intention was for no scrolling... that was the point of breaking up help content into tabs in the first place. Removing max height good with me.

[ocean90]

olleicua, the problem is, that the height isn't calculated new. max-height is 300px, and on your screen aren't 300px. Could reproduce it here too.

[chexee]

So, per what Jane said, there shouldn't be any scrollbars here. I poked around and just removing the max-height doesn't do it (looks like the heights are calculated and explicitly set in the JS).

Could we remove max height and make the help height animate and expand to longer lengths of text and you roll through the side tabs?

[sorich87]

The issue was that the height was not properly recalculated when navigating through the panels . The initial height that was previously set when you clicked on the help was just kept.

19155.diff resets the height before recalculating it again. In addition, it also removes max-height.

[koopersmith]

This works, but setting the heights via JS is no longer necessary. It can all be done via CSS.

[koopersmith]

In [19361]:

Help tabs: remove scrollbar and max-height. see #19155.

[koopersmith]

I've left this ticket open because I'm not sure whether or not we should animate between the changing panel heights.

That said, the bug has been fixed. Hooray!

[olleicua]

Considering it animates initially it would be more consistent to animate it always. It doesn't look bad and it's definitely way better but I think it would be best to animate it..

I was trying to look into this just now but it seems the fix has been committed since It works correctly (although without animation). However when I look at the wp-admin/js/common.dev.js the fix isn't there. I tried grep -r "columns.height" * (since columns.height is what was added in the patch) but nothing turns up. Did someone refactor this without posting here..? If so where is the code that currently changes the size so we can animate it? I'm probably missing something obvious here..

[ocean90]

olleicua, see ​http://core.trac.wordpress.org/changeset/19361#file7

[jane]

Replying to koopersmith:

I've left this ticket open because I'm not sure whether or not we should animate between the changing panel heights.

I would just as soon not.

[olleicua]

Replying to jane:

I would just as soon not.

It seems inconsistent not to to me. I'm curious why?

[jane]

Because gratuitous animation makes things feel slower. Animation should only be done if there's a purpose to it. The tab sliding down is reinforcing position and where the tabs live to the user's brain. If we're just changing height based on content, there's no real purpose that trumps speed. If I'm overlooking something, am open to being convinced otherwise.

[olleicua]

Either way seems has advantages. The slow down for the animation is a valid reason not to. It just seems awkward to me if it slides down and then jumps erratically once it's down there. Perhaps we should get a third opinion, I'll put UI-feedback back on for now.

[jane]

FYI, the ui-feedback tag is generally shorthand for, "Jane, please sign off on this." Anyone is welcome to leave an opinion on how to make the ui, but that tag won't necessarily make that happen.

[olleicua]

That makes sense, sorry I'm new.

[koopersmith]

I agree with Jane here — in this case, there's no benefit to the animation, only delay. As for consistency, most of our other tabs and features that shift box heights change instantly. The animation when opening screen options/help is the exception, not the rule.

I'm going to close this ticket as fixed.

Replying to olleicua:

I was trying to look into this just now but it seems the fix has been committed since It works correctly (although without animation). Did someone refactor this without posting here..?

Yes, instead of the JS patch, I reworked it to use CSS as mentioned in ​comment 8 and implemented in [19361] (shown here in ​comment 9).

The help columns are arranged in a way that will increase the height of the help container to the height of the tallest column (regardless of that column's height), so code to calculate the column height is unnecessary.

The tricky part here was getting the three columns to each appear to stretch to the bottom of the help container. That is now accomplished by an absolutely positioned div that sits behind the three columns and paints the middle column/borders.

Replying to olleicua:

That makes sense, sorry I'm new.

Trac keywords definitely take some getting used to. We use them to manage our workflow — you'll get the hang of them soon. :)