CSS-Tricks / MovingBoxes

Simple horizontal slider which grows up the current box when it's in focus (image, title & text) and back down when it's not in focus.
http://css-tricks.github.io/MovingBoxes/
GNU Lesser General Public License v3.0
280 stars 147 forks source link

Appending and then sliding to the new panel gets the wrong size #54

Closed udi closed 12 years ago

udi commented 12 years ago

When you add a panel in code, and then move to the new panel, the size of the box (height) is wrong. It is as the size of the box after the one you just added.

To see it, use the demo. Remove all the panels (first one will remain). Then add a new one, and switch to it. If the added panel is larger than the current one, the height of the box will be smaller than it should.

Great plugin, btw!

Mottie commented 12 years ago

Hi udi!

I wonder if it's just the demo... if I click really fast it does seem to mess up, but not consistently. Like sometimes I remove all of the panels and the arrows will still be there, or the last image will be off-center. Add a few panels and it corrects itself. I did get the behavior you mention, but again, inconsistently. It only seemed to happen when adding the panel with the cat image (it is taller).

So I mention this because in a practical sense, you shouldn't be adding a panel, updating, adding another panel, updating like you do with the demo. I don't think this would be a problem when using the plugin as intended... the demo doesn't count because of crazy button spamming.

What are your thoughts on this?

udi commented 12 years ago

It is quite consistent, and not only in the demo. I just used the demo to explain the behavior I see in my page. It happens whenever the panel that you add is larger than the current one. When you slide to the new (taller) panel then the size of the box remains the same instead of adjusting.

I'm using it as follows - whenever the user does several things in a panel, I add another panel so that the use can keep on providing feedback. I never remove panels, just add new ones. No need to do anything fast.

Mottie commented 12 years ago

Are you using the update() method? Or did you try setting the fixedHeight option to true. It sets the height of the slider to the height of the tallest panel.

Maybe providing me with a quick demo would help? I have a demo "playground" set up on jsFiddle.net that you can modify.

udi commented 12 years ago

Do you mean update() after append()? My code was: .append(data) .movingBoxes();

Do I need to update after this? I thought that calling .movingBoxes() should take care of that, no?

Btw, playing with this again, it seems that when I'm in the last panel, and appending a new one, moving to the new one just collapses the slider to a very small size. My initialization is empty (only defaults): $("#boxes").movingBoxes();

I don't want to have a fixedHeight cause I like the auto adjust, and also, I will probably need to call update() again, no? so that it can adjust to the new tallest panel (in case the new panel is taller than all the rest)?

I will try setting up some demo, but it might take me a while.

Mottie commented 12 years ago

Hmm, ok, I think the main issue is in webkit browsers right? The problem seems to be related to images, so I bet if you set an image width and height the problems will go away. I need to do some more testing and maybe work out a better method.

udi commented 12 years ago

I'll try setting the width and height and see. Indeed on Firefox it seems to be working fine (however, sadly, the star ratings plugin fails). Chrome on Linux and Mac fails.

And btw, when running on chrome using the debug console, you get the following warning: event.layerX and event.layerY are broken and deprecated in WebKit. They will be removed from the engine in the near future.

Mottie commented 12 years ago

So, I wrote this jQuery function/plugin that runs a callback when all images have loaded and paired it with MovingBoxes. It seems to be working but now only fails when adding a panel to an empty slider. I think I have it solved in the version I'm working on, but I wanted you to check out the demo and see if there are any other times you notice it mess up.

Here is the "imagesLoaded" plugin I wrote:

jQuery.fn.extend({
    imagesLoaded: function(callback) {
        var i, c = true, t = this, l = t.length;
        for (i = 0; i < l; i++) {
            if (this[i].tagName === "IMG") {
                c = (c && this[i].complete && this[i].height !== 0);
            }
        }
        if (c) {
            if (typeof callback === "function") { callback(); }
        } else {
            setTimeout(function() {
                jQuery(t).imagesLoaded(callback);
            }, 200);
        }
    }
});
udi commented 12 years ago

Sorry it took me a while to setup a small demo, but try looking at: http://74.95.195.227:8888

(tested on Chrome)

When you rate a movie it adds a new item. Then try to move to the item that was added, and see what happens. If you rate a bunch of movies, then it adds several new movies (as the number you rated), and the last one will have wrong height. Try playing with it a bit and see how jumpy it is. It is completely stateless, so just refresh if you want to start over.

Send me any corrections you want me to include and I'll add them.

On Sun, Dec 18, 2011 at 5:38 AM, Rob G < reply@reply.github.com

wrote:

So, I wrote this jQuery function/plugin that runs a callback when all images have loaded and paired it with MovingBoxes. It seems to be working but now only fails when adding a panel to an empty slider. I think I have it solved in the version I'm working on, but I wanted you to check out the demo and see if there are any other times you notice it mess up.

Here is the "imagesLoaded" plugin I wrote:

jQuery.fn.extend({
   imagesLoaded: function(callback) {
       var i, c = true, t = this, l = t.length;
       for (i = 0; i < l; i++) {
           if (this[i].tagName === "IMG") {
               c = (c && this[i].complete && this[i].height !== 0);
           }
       }
       if (c) {
           if (typeof callback === "function") { callback(); }
       } else {
           setTimeout(function() {
               jQuery(t).imagesLoaded(callback);
           }, 200);
       }
   }
});

Reply to this email directly or view it on GitHub: https://github.com/chriscoyier/MovingBoxes/issues/54#issuecomment-3194731

Mottie commented 12 years ago

Hi Udi!

I cleaned up the code for you (download this page and remove the <base> tag at the top if you replace the original file)...

Basically you only need one copy of the code to control all of the star ratings:

$(function(){
    $('.auto-submit-star').rating({
        callback: function(value, link){
            var id = $(this).closest('form')[0].id.replace('ratingForm_','');
                mb = $('#boxes').getMovingBoxes();
                mb.goForward();
            $.post('ajax/ratings.html', {item_id: id, rating: value}, function(data) {
                $('#boxes')
                // divs appended here, assuming you are using the DIV layout
                .append(data)
                .movingBoxes()
                // .update(); // EDIT: oops this is used wrong... better to just remove it!
            });
            return false; // this makes the mb.goForward() work
        }
    });
    $("#boxes").movingBoxes();
});

And I added this bit of extra css to set a starting panel height - so it doesn't start out short then grow after all of the images have loaded - this is completely optional. The second line adds a height to the form which contains the stars - oddly, setting it to 1px works fine, if you set it larger, it adds more padding.

.mb-panel { min-height: 650px; } /* this makes the Moving Boxes start at full height */
form { height: 1px; } /* this includes the height of the rating stars */

Oh, and you'll need to include a <!DOCTYPE html> at the top or IE will go into quirks mode

udi commented 12 years ago

thanks!

However, now when I add new panels to the box, the script (and init of the stars) do not work properly. I guess that when adding the new panel when a rating was done, I should somehow call the $('.auto-submit-star').rating(..) function, so that it initializes the stars properly.

See the updated demo http://74.95.195.227:8888/ page.

On Wed, Dec 21, 2011 at 3:59 PM, Rob G < reply@reply.github.com

wrote:

Hi Udi!

I cleaned up the code for you (download this page and remove the <base> tag at the top if you replace the original file)...

Basically you only need one copy of the code to control all of the star ratings:

$(function(){
       $('.auto-submit-star').rating({
               callback: function(value, link){
                       var id =
$(this).closest('form')[0].id.replace('ratingForm_','');
                               mb = $('#boxes').getMovingBoxes();
                               mb.goForward();
                       $.post('ajax/ratings.html', {item_id: id, rating:
value}, function(data) {
                               $('#boxes')
                               // divs appended here, assuming you are
using the DIV layout
                               .append(data)
                               .movingBoxes()
                               .update();
                       });
                       return false; // this makes the mb.goForward() work
               }
       });
       $("#boxes").movingBoxes();
});

And I added this bit of extra css to set a starting panel height - so it doesn't start out short then grow after all of the images have loaded - this is completely optional. The second line adds a height to the form which contains the stars - oddly, setting it to 1px works fine, if you set it larger, it adds more padding.

.mb-panel { min-height: 650px; } /* this makes the Moving Boxes start at
full height */
form { height: 1px; } /* this includes the height of the rating stars */

Reply to this email directly or view it on GitHub: https://github.com/chriscoyier/MovingBoxes/issues/54#issuecomment-3241599

Mottie commented 12 years ago

Ok, try this code:

$(function(){
    var rating = function(){
        $('.auto-submit-star:not(.processed)').rating({
            callback: function(value, link){
                var id = $(this).closest('form')[0].id.replace('ratingForm_','');
                mb = $('#boxes').getMovingBoxes();
                mb.goForward();
                $.post('ajax/ratings.html', {item_id: id, rating: value}, function(data) {
                    $('#boxes').append(data); // divs appended here, assuming you are using the DIV layout
                    mb.update();
                    rating();
                });
            return false; // this makes the mb.goForward() work
            }
        }).addClass('processed');
    }
    rating();
$("#boxes").movingBoxes();
});
udi commented 12 years ago

it seems to work, but it is not very smooth...when the pictures have different sizes (especially when they are very small) then everything is a bit jumpy. There is an option to have all panels with the same fixed height, right? this will make everything much smoother, even when adding and removing panels on the fly.

On Thu, Dec 22, 2011 at 6:17 AM, Rob G < reply@reply.github.com

wrote:

Ok, try this code:

$(function(){
       var rating = function(){
               $('.auto-submit-star:not(.processed)').rating({
                        callback: function(value, link){
                               var id =
$(this).closest('form')[0].id.replace('ratingForm_','');
                               mb = $('#boxes').getMovingBoxes();
                               mb.goForward();
                               $.post('ajax/ratings.html', {item_id: id,
rating: value}, function(data) {
                                        $('#boxes').append(data); // divs
appended here, assuming you are using the DIV layout
                                       mb.update();
                                       rating();
                                });
                       return false; // this makes the mb.goForward() work
                       }
                }).addClass('processed');
       }
       rating();
$("#boxes").movingBoxes();
});

Reply to this email directly or view it on GitHub: https://github.com/chriscoyier/MovingBoxes/issues/54#issuecomment-3249397

Mottie commented 12 years ago

Yes, it's named fixedHeight. I was going to point you to the wiki page describing it, but umm, I guess I still need to write it LOL. Anyway, here is all of the options listed out with a brief description.

beshur commented 12 years ago

Sorry for not getting too deep in the issue... Well, I had the problem when asynchnroniously appended more images to the slider, when reaching the new images, the div.mb-scroll suddenly changed its height to 34px (less than needed). I added min-height style propery and it is ok now.

Mottie commented 12 years ago

Cool! Thanks for sharing :)

beshur commented 12 years ago

Whoops, I have read the whole topic now and noticed that you mentioned the min-height ))

Actually, I now added an .update(); after .movingBoxes() just as you showed. I works fine for the slider itself, the height issue is resolved. But no javascript code is executed after this update(); - I have more things to do like remove classes and other stuff and this does not happen. e.g.

jQuery("#slider") .append(response_html) .movingBoxes() .update(); countpage = countpage+1; // this doesn't work

Do you anythings about this?

Mottie commented 12 years ago

Hmmm, yeah, update should be used as follows, I guess I screwed that up LOL. All of the methods below are equivalent:

// each line does the exact same thing
jQuery('#slider').data('movingBoxes').update();
jQuery('#slider').getMovingBoxes().update();
jQuery('#slider').movingBoxes();
Mottie commented 12 years ago

Oh, and I forgot to mention that the "imagesLoaded" plugin I shared above works well except in IE (suprised?) when there is an image error. So I've updated it and made it into an real plugin: https://github.com/Mottie/imagesLoaded.

Also, I didn't know this at the time, but David Desandro (creator of Masonry and Isotope) already has a similar plugin. It uses the images loaded event instead of the completed flag & image height like I use: https://github.com/desandro/imagesloaded

beshur commented 12 years ago

Well, thank for the code, Mottie! The movingBoxesd(); work fine. // Yet, nothing after movingBoxesd(); does not work.

UPD Just my fault - used $ instead of jQuery (for one of the actions). As the code was in an external file, no error messages shown). Sorry for bothering! Have a good Sunday night :)

Mottie commented 12 years ago

I did it again... I have no idea how I added the "d" on the end of "movingBoxesd();"... man I think I need more sleep or something LOL.

beshur commented 12 years ago

I noticed and thought that it will be not very easy to distinguish it in the code and used .update() instead.

It's all because of dumb users like me ))