Make Homebox only equalize heights of columns within the same row.

I confirm, it's a hudge problem for me. It looks like horrible when moving blocks.

Also confirming this!

Can you try my fix from here? http://drupal.org/node/950120#comment-3785844

I know it's a different problem but perhaps it will begin us on a course of fixing the auto-height function in homebox.js.

Your fix refers to the 2.1 version rather than the dev version, so I will not be trying it (as I am running the dev version due to compatibility with my version of jquery ui).

Gotcha, but the bug is still in the "autoheight" function in homebox.js. The jQuery code is pretty easy to read... . I think what you need to do is divide by how many columns can fit in your layout and then do an autoheight on each row instead of as if they're all in the same row.

If I get a chance I'll install dev and see if I can drum up a patch that fixes both problems since they are closely related.

I have a big 100% sized column followed by 3 columns (33%, 34%, 33%). I run into this problem with the IE (in Firefox I didn't notice it) where I got a big white space between the first column and the rest. A workaround for me and all who do not need the auto height feature will be to get out of the corresponding function with return $columns; right after the function get called. Like this:

/*
 * Set all column heights equal
 */
Drupal.homebox.equalizeColumnsHeights = function(columns) {

  return $columns;
  // the following stuff of this function will be ignored

Fix in the last dev version. Thanks!

Aye, it certainly appears to be fixed in the latest dev. Nice!

Good news. I was unable to reproduce, so I agree, this one looks fixed.

Oh dear... This has been reintroduced in 6.x-3.x-dev.

Yes, there is a bit of a problem here where we have to choose between the lesser of too evils. This was reintroduced to fix another problem:

This is done so that all the area below the 'shorter' columns is a targettable drop area. This makes dropping blocks to the bottom of the shorter columns much easier, especially if you have a long enough source column that the bottom of the target column isn't in the browser window.

However, when you have multiple non-100% width rows, then you have the bug described in this issue.

Essentially, the solution needed is to have the JS be row-aware, and only equalize column heights within the current row (note that whatever is done would need be be very performant, as this function is called *alot*). However, I am not sure how to make that happen. Patches are welcome.

In the meantime, I would suggest only using layouts that have only a single row of non-100% width columns.

I am going to revert part of #1063928: Remove unnecessary width check - the width check it removed prevents the 100% width columns from having their height equalized. Beyond that, I don't have a solution for this one.

Ah. I don't suppose you could provide a patch for those of us that didn't have the issue you described?
My homebox setup didn't have any problems with the target drop areas and was working rather well previously, so for me this is the greater evil.

#1063928: Remove unnecessary width check was reverted to prevent 100% width columns from taking part in or being resized by the equalizeColumnHeights() function. This won't appear in the -dev release until it is rerolled, but should be available in CVS.

My suggested solution for the main issue implement the following:

We probably need to add a 'row' input beside the custom column widths, so we can assign columns to rows, with some validation to ensure that all 100%-width columns get their own row, and the sum of the widths of the custom columns within a row do not exceed 100%.

Then we add a class to each column ('homebox-row-1' etc.), and make the equalizeColumnHeights() function only calculate the maximum height for columns within the same row.

@Shadlington: A new copy from CVS should work for you, and should return the functionality to where it was previously.

Hmmm. I'm not sure if I misunderstood but the new copy I installed doesn't revert the functionality I want.
I meant could you provide a patch that would return the functionality to what it was a couple of weeks ago - i.e. only resizing columns to their own size, not to that of the largest. This was ideal for my setup (I have a 3 - 2 - 1 column layout).

I would do it myself but I don't have a copy of the previous version I was using and so don't know what the different javascript you were using was.

Shadlington,

That was not my intent. The version you want was a broken version, which happened when I accidentally flipped some logic, and resized no columns. We cannot go back to that version.

The patch I reverted affects this only in that 100%-width columns will not be taken into the maximum height calculations and have that height applied (being 100% width, this is not a problem!).

For now, until someone sends a patch to implement #14, or I have a significant amount of suddenly free time on my hands, you may need to stick with a layout that has only one row with multiple columns (in addition to however many 100% width columns you need).

-Deleted-

Hi to everybody,
possible patch for #14 without the need to add a new field "row" in the database:
the trick is to use the width of the entire homebox area to find where and when the next column starts a new row.
Hope this helps, please let me know what do you think about it.

Cheers,
Daniele

That's an interesting approach. I don't have time to test it thoroughly, but perhaps Shadlington wouldn't mind giving it a try.

I'm on it :)

Seems good!

Am I right in thinking the reason the '==null' bits I liked were dropped (or rather, were put back in) was so that the drop area was larger on the shorter columns? If that was the issue then it is resolved with this patch.

The behaviour I'm seeing is each row is only as tall as the tallest column in that row whilst the shorter columns in each row retain a drop area the height of the row. Nice! Win-win? :)

You are right.
I have a homebox with 6 columns: 100% - 50% - 50% - 100% - 50% - 50% that renders in 4 rows.
Before the patch the rows with columns not set at 100% were all high as the highest column in the box, even when i collapsed some of them.
Now each row get the right height even when collapsing.
I think the '==null' bits are used to not automatically evaluate the height for columns with height set at 100%. This behaviour is not changed by the patch.

Thanks for trying it and for your reply! :-)

I just realised I got a bit muddled with what I was trying to say.
Didn't really make myself clear, but I was directing that question at brianV really, as we had previously had a conversation about this behaviour (it had sort of been fixed at one point by removing the '==null's but this introduced a separate problem where each column was only as big as it needed to be, leaving the drop areas small so the '==null's were put back).
I guessed that you knew what your patch was doing :P

Anyway the point is I think this works. It solves the issue I had and it keeps the nice big drop areas.
I've now tested it on a few layouts, (especially on my preferred layout of 50/50 - 33/34/33 - 100) and everything is great.
Oh and I didn't notice any performance hit either, in case anyone wondered.

Nice work jellicle_it!

Ops...sorry for misunderstanding, it's all clear now, thanks! :-))

Shadlington:

See #1037968: Homebox layout broken in IE7/8 - in that bug, we moved from checking if the homebox class had a string in it's style attribute matching 'width: 100' to using a case-insensitive regex as IE7 and IE8 would sometime return a style attribute where everything was capitalized for no apparent reason.

The intent of the line is to exclude full-width blocks from the equalizing calculation, as their optimal height isn't dictated by other blocks.

Unfortunately, I didn't read what match() does closely enough, and accidentally inverted the logic. While we used to check if the width *was not* 100%, this new check with the regex checked if the width *was* 100% and based the calculation on only 100%-width blocks.

I had to add the '== null' comparison to restore the logic back to what it should have been.

Ah, I see. So my non-100% blocks were not getting resized at all... Right. So that pretty much didn't work at all then - I get it now! Took me long enough ;)

Still, the issue right now is this patch and I think its a winner. You should try it out when you get the chance!

I intend to. :)

Here's how we solved it:

1) Added a div resize command to the "homebox.js_.patch"

Drupal.homebox.equalizeColumnsHeights = function () {

  $('#homebox-add').width(950);     // resize the homebox-add div section to desired width...
  ....
  .... <rest of code in patch>    
  ....
};

2)Added an empty div row to the homebox.tpl.php file after:

...
  <?php if (!empty($add_links)): ?>
    <div id="homebox-add" class="homebox-column-wrapper"><?php print $add_links; ?>  <div class="submitted">click on above to add to home page</div> </div>
  <?php endif; ?>
  
  <!--- add empty div row here -->
  <div id="empty_row4ie" style="width: 80%; height: 1px;" >&nbsp; </div>
...
...

3) Also, for other styling issues replace the outline statements with border statements in the homebox CSS files. IE7/8 does not support outline statements.

The above patch doesn't seem to work with homebox.js dated 2/14/2011 in the 6.x-3.x-beta2 set.

The current file doesn't have the "if ($(this).parent('.homebox-column-wrapper').attr('style').match(/width: 100%/i) == null) {" line.

Could you upload a patch against the 2/14 file or put the Drupal.homebox.equalizeColumnsHeights = function () function in text?

Thank you.

I changed equalizeColumnsHeights based on the patch in #18. This works against 6.x-3.x-beta2

I didn't include the test to exclude 100% columns. I don't think it would save much time against a single pass through the loop. It also seems to cause a problem when you don't set custom widths for the regions. http://drupal.org/node/1062360

Working with Homebox 6.x-3.0-beta2, I ran into this same problem, my multi-column layout was bloated with even tiny rows being assigned the maxHeight of all the rows on that homebox page. Here was my workaround:

In /homebox.js, at line 78, I changed this

Drupal.homebox.equalizeColumnsHeights = function () {
  var maxHeight = 0;
  Drupal.homebox.$columns.each(function () {
    $(this).height('auto');
    maxHeight = Math.max($(this).height(), maxHeight);
  }).height(maxHeight);
};

by just commenting out the part where the height is assigned:

Drupal.homebox.equalizeColumnsHeights = function () {
  var maxHeight = 0;
  Drupal.homebox.$columns.each(function () {
    $(this).height('auto');
    maxHeight = Math.max($(this).height(), maxHeight);
  });//.height(maxHeight); <-- this is where it's assigned
};

...the effect is that every "column" is just the size of itself, not the height of it's tallest sibling. For the time being, that's an acceptable trade-off!

Thanks baisong. #31 fixed my problem. The only thing was that I found that code on line 86. Works great now, thanks.

As you seem to have started up development on Homebox again, I wondered if this might get some attention...

Which is the latest patch up for consideration? There are several new patches since the one I last looked at.

Honestly I haven't tried any of them in some time...
However, I believe #18 was working.
But #30 claims to be another patch based on #18 :/

Ok, I don't have time to test either patch, but if someone wouldn't mind putting the time into it and marking one or the other RTBC, I will commit it.

I encountered this same issue and thanks to the guys who came up with patches for this. Quite frankly, I'm surprised why it hasn't been committed after over six months. I have tested the patch in #18 and #30 and they are working well.

A further optimization can be done by moving the logic that separates columns into different rows from the .js and placing it in the .module code - remember, this needs to be done only once per page load, not each time equalizeColumnsHeights() is called.

So, slight modification in homebox.module to generate row numbers (a la #14)

  // Sort each region/column based on key value
  // Also separate the regions into rows based on the region widths
  $sum_width = 0;
  $row = 0;
  for ($i = 1; $i <= count($regions); $i++) {
    ksort($regions[$i]);
    $sum_width += $page->settings['widths'][$i];
    if ($sum_width > 100) {
      $row++;
      $sum_width = 0;
    }
    $page->settings['rows'][$i] = $row;
  }

Then, slight modification in homebox.tpl.php to add the class homebox-row-x (a la #14 again). The benefit of using css class names for each row is that different rows can be styled differently

    <div class="homebox-column-wrapper homebox-column-wrapper-<?php print $i; ?> homebox-row-<?php print $page->settings['rows'][$i]+0; ?>"<?php print count($page->settings['widths']) ? ' style="width: ' . $page->settings['widths'][$i] . '%;"' : ''; ?>>

Finally, the code in homebox.js to implement row sizing in equalizeColumnsHeights().

/**
 * Set all column heights equal
 */
Drupal.homebox.equalizeColumnsHeights = function () {
  var maxHeight = [];
  Drupal.homebox.$columns.each(function () {
    row = $(this).parent('.homebox-column-wrapper').attr('class').match(/homebox-row-\d+/i)[0].replace('homebox-row-', '') * 1;
    $(this).height('auto');
    maxHeight[row] = maxHeight[row] ? Math.max($(this).height(), maxHeight[row]) : $(this).height();
  }).each(function () {
    row = $(this).parent('.homebox-column-wrapper').attr('class').match(/homebox-row-\d+/i)[0].replace('homebox-row-', '') * 1;
    $(this).height(maxHeight[row]);
  });
};

I would like to avoid the regular expression and string operation to speed up the function more. The alternative being using custom tag attribute e.g.
<div class="homebox-column-wrapper ..." row=0>...</div>

I've tested this code on a 20-20-30-30-60-40 layout, a 100-25-25-25-25-50-50 and also on a 50-50-40-30-30 layout. They all work well.

Sorry I could not attach the patches. I got an error message. Will try again.

Patches finally attached.

I did some further cleanup and committed this to 6.x-3.x. Still needs to happen in 7.x-2.x.

• Changed maxHeight to an object so casting to int is not necessary.
• Added row to the var declaration.
• Shortened the row finding with a regular expression grouping.
• Removed unnecessary casting in the template.

Here is the same commit applied to 7.x with whitespace cleanup. It looks like it will work, but needs testing.

Committed to 7.x-2.x.

I'm having an issue with this calculation. The row numbers render are not correct in my application.
100- 70 - 30 - 35 - 35 - 30 - 100 results in row number
0 - 1 - 1 - 1 - 1 - 2 - 1 which is not correct.

Should the new row width sum not start with the width of the current row? This is information is lost when the loop continues?

  // Sort each region/column based on key value
  // Also separate the regions into rows based on the region widths
  $sum_width = 0;
  $row = 0;
  for ($i = 1; $i <= count($regions); $i++) {
    ksort($regions[$i]);
    $sum_width += $page->settings['widths'][$i];
    if ($sum_width > 100) {
      $row++;
      $sum_width = $page->settings['widths'][$i]; // Current items width instead of 0.
     }
    $page->settings['rows'][$i] = $row;
  }

That sounds like it could work. Have you had good luck with it? Can you provide a patch?

Yes, it works correct now with multiple rows heights with my project.
Patch is attached.

Looks okay, committed.