Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fluid Thumbnails Broken After First Row #3494

Closed
ehoch opened this issue May 15, 2012 · 72 comments
Closed

Fluid Thumbnails Broken After First Row #3494

ehoch opened this issue May 15, 2012 · 72 comments
Labels

Comments

@ehoch
Copy link

ehoch commented May 15, 2012

Hey guys,

Love that you are working on fluid thumbnails, but they're still broken for me after the first row. I set up a fiddle based on your example in the css tests:

http://jsfiddle.net/D2RLR/79/

By broken, which I realize is the least descriptive word possible, I mean they do not form a grid anymore. Starting with row two, only two span4's can fit ina row. It's missing the 0'ing out of the margin to make it so all 3 span4's can fit.

@ehoch
Copy link
Author

ehoch commented May 17, 2012

How did this get assigned to 2.0.5? Can we move this to 2.0.4?

@mdo
Copy link
Member

mdo commented May 17, 2012

I moved it to 2.0.5 because in the interest of getting 2.0.4 out the door, I'm limiting the number of bugs it addresses.

I spend nearly two hours fiddling with how best to do this and nothing seemed right or easy. It requires additional time, more than I want to spend on getting 2.0.4 out. Sorry for the delay!

@ehoch
Copy link
Author

ehoch commented May 17, 2012

Oh, okay, just making sure I didn't do that by mistake! Thank you for looking into. I will patiently await until 2.0.5 then :)

I have the following workaround / hack if anyone else needs it:

ul.thumbnails li.span4:nth-child(3n + 4) {
  margin-left : 0px;
}

ul.thumbnails li.span3:nth-child(4n + 5) {
  margin-left : 0px;
}

ul.thumbnails li.span12 + li {
  margin-left : 0px;
}

@ghotiphud
Copy link

I wrote some code to fix a very similar issue I was having with the fluid grid. #2984

@kwent
Copy link

kwent commented May 28, 2012

Thank you for this hack @ehoch.Very helpful !

@leeaston
Copy link

leeaston commented Jun 5, 2012

Want to say thanks to @ehoch as well - Thanks man!

@spfaffly
Copy link

I also wanted to thank @ehoch . Thanks so much. I ended up using something like:

.row-fluid ul.thumbnails li.span12 + li { margin-left : 0px; }
.row-fluid ul.thumbnails li.span6:nth-child(2n + 3) { margin-left : 0px; }
.row-fluid ul.thumbnails li.span4:nth-child(3n + 4) { margin-left : 0px; }
.row-fluid ul.thumbnails li.span3:nth-child(4n + 5) { margin-left : 0px; }

@djgeekdout
Copy link

good lookin sonnnnnnnnn!!!!!!!! @ehoch @spfaffly

I was about to straight-up write a custom PHP count iterator just to fix this for the wordpress site I'm working on… so glad I looked here first…

@ehoch
Copy link
Author

ehoch commented Jun 14, 2012

I appreciate everyone enjoying / using this hack, but realize it won't work with anything that doesn't support nth-child. Like IE < 9.

Maybe someone wants to do a jquery work-around and post that here?

@saibayadon
Copy link

(function($){
    $('.row-fluid ul.thumbnails li.span6:nth-child(2n + 3)').css('margin-left','0px');
    $('.row-fluid ul.thumbnails li.span4:nth-child(2n + 3)').css('margin-left','0px');
    $('.row-fluid ul.thumbnails li.span3:nth-child(2n + 3)').css('margin-left','0px'); 
})(jQuery);`

That should do it for now.

@Toze
Copy link

Toze commented Jun 22, 2012

little mistake :)

here is the correct one :

(function($){
    $('.row-fluid ul.thumbnails li.span6:nth-child(2n + 3)').css('margin-left','0px');
    $('.row-fluid ul.thumbnails li.span4:nth-child(3n + 4)').css('margin-left','0px');
    $('.row-fluid ul.thumbnails li.span3:nth-child(4n + 5)').css('margin-left','0px'); 
})(jQuery);

@saibayadon
Copy link

Yeah, didn't notice it! Thanks for the correction. Is this ever gonna be fixed?

@bokkagroup
Copy link

+1 on this problem. Really delaying our project.

@leeaston
Copy link

I ended up not using .thumbnails at all. Instead I made my CMS (ExpressionEngine) create a new row when needed - including empty spans (to complete row) if necessary.

@ghost
Copy link

ghost commented Jun 22, 2012

I wrote this function that will also work with mixed thumbnail spans and different thumbnail heights.
Demo: http://jsfiddle.net/Sanjo/xevK7/24/

.first-in-row { margin-left: 0 !important; }​
/**
 * Adds 0 left margin to the first thumbnail on each row that don't get it via CSS rules.
 * Recall the function when the floating of the elements changed.
 */
function fixThumbnailMargins() {
    $('.row-fluid .thumbnails').each(function () {
        var $thumbnails = $(this).children(),
            previousOffsetLeft = $thumbnails.first().offset().left;
        $thumbnails.removeClass('first-in-row');
        $thumbnails.first().addClass('first-in-row');
        $thumbnails.each(function () {
            var $thumbnail = $(this),
                offsetLeft = $thumbnail.offset().left;
            if (offsetLeft < previousOffsetLeft) {
                $thumbnail.addClass('first-in-row');
            }
            previousOffsetLeft = offsetLeft;
        });
    });
}

@mainerror
Copy link

Using the nt-child() CSS3 selector would not work if IE 7 and IE 8 support is to be kept.

@ehoch
Copy link
Author

ehoch commented Jul 3, 2012

@mainerror That was mentioned in this thread. That's why I recommended jquery as a backup.

@mainerror
Copy link

Oh, my bad. Didn't notice it.

@mdo
Copy link
Member

mdo commented Jul 30, 2012

Punting this for now. In the future, I want to get back to a single grid system—fixed or fluid, it'll be just one for simplicity and flexibility. We'll solve this problem when we get that done.

@neersighted
Copy link

Still broken...

@neersighted
Copy link

@Tozee's fix works for now.

@alepee
Copy link
Contributor

alepee commented Oct 1, 2012

Thank you for this fix, I added some more lines to feet every case:

.row-fluid {
  ul.thumbnails {
    li.span12 + li { margin-left : 0px; }
    li.span6:nth-child(2n + 3) { margin-left : 0px; }
    li.span4:nth-child(3n + 4) { margin-left : 0px; }
    li.span3:nth-child(4n + 5) { margin-left : 0px; }
    li.span2:nth-child(6n + 7) { margin-left : 0px; }
    li.span1:nth-child(12n + 13) { margin-left : 0px; }
  }
}

CSS should be best than JS for this kind of fix :)

@erlingur
Copy link

erlingur commented Oct 3, 2012

@alepee Thank you very much! Using this right now in a project :)

@ehoch
Copy link
Author

ehoch commented Oct 3, 2012

@alepee The reason for JS is on TOP of CSS. If you've read, that solution does not work on IE7-8.

@IwanUswak
Copy link

thank you guys, I was just about to write a fix but luckily I googled first :-)

@jahvi
Copy link

jahvi commented Nov 30, 2012

This is still broken in 2.2.1 😢

@alepee 's fix works great though since it takes care of all variations, that's if your using the same width for all thumbnails, if not you're out of luck

@omarinov
Copy link

Unfortunately this solution don't work with drag & drop. If you reorder the thumbnails and call the JS again this won't work as expected every time.

@ghost
Copy link

ghost commented Dec 24, 2012

@omarinov I made a little update that resets the previously set margins when you call the function again. I hope that fixes your problem. http://jsfiddle.net/Sanjo/xevK7/24/

@ghost
Copy link

ghost commented Apr 11, 2013

Just in case anyone searches this problem and finds this post, doing something like this, below, stops that annoying habit of thumbnails bumping into each other when they start a new row, the clear:left stops that.

.row-fluid ul.thumbnails li.span12 + li { margin-left : 0px;clear:left }
.row-fluid ul.thumbnails li.span6:nth-child(2n + 3) { margin-left : 0px;clear:left }
.row-fluid ul.thumbnails li.span4:nth-child(3n + 4) { margin-left : 0px;clear:left }
.row-fluid ul.thumbnails li.span3:nth-child(4n + 5) { margin-left : 0px; clear:left}
.row-fluid ul.thumbnails li.span2:nth-child(6n + 7) { margin-left : 0px;clear:left }
.row-fluid ul.thumbnails li.span1:nth-child(12n + 13) { margin-left : 0px;clear:left }

@muayyad-alsadi
Copy link

I fix it with this css

https://gist.github.com/muayyad-alsadi/5720281

@mufaddalmw
Copy link

@sanjo taking your code created one more example http://codepen.io/mufaddalmw/pen/dGzaw

@rpbaguio
Copy link

rpbaguio commented May 9, 2013

@mufaddalmw your example works like a charm. Thanks

@webdevsenior
Copy link

I've found another solution, this is my example: http://jsfiddle.net/ruRs7/2/
Work well with IE8

@tobias74
Copy link

@muayyad-alsadi Thanks for your solution!

I only had to put a margin-left:0 into your fix to make it work for me:

.row-fluid .thumbnails [class*="span"] {
margin-right: 2.5641%;
margin-left:0;
}

@tobias74
Copy link

... the fix breaks the layout on mobile devices, though.

@muayyad-alsadi
Copy link

@tobias74 welcome, you might need to replace margin-right with margin-left because I use -RTL version of bootstrap http://muayyad-alsadi.github.com/bootstrap-rtl/
I'll edit my solution

@webdevsenior are referring to my fix

@emage
Copy link

emage commented May 31, 2013

Still a bug in 2.3.2 :( be nice to not use any JS

@pongstr
Copy link

pongstr commented Jun 1, 2013

@emage, hey man there's actually a fix for this but might need some JS to force it on IE6-IE8.

try this out:

.thumbnails > li.span2:nth-child(6n+1),
.thumbnails > li.span3:nth-child(4n+1),
.thumbnails > li.span4:nth-child(3n+1),
.thumbnails > li.span6:nth-child(2n+3){ margin-left: 0 !important; }

and then add this (if you'd like to support IE6/7/8)

$(window).load(function(){
   if ( $('.lt-ie9 .thumbnails', '.lt-ie8 .thumbnails').length > 0 ) {      
    // Don't break function
    function dontBreak( column ){
      var ie7 = '.lt-ie8' + ' .thumbnails > ' + column,
          ie8 = '.lt-ie9' + ' .thumbnails > ' + column;        
      $( ie7, ie8 ).css({ 'margin-left' : '0' });
    }

    // Don't break .span2 
    dontBreak('.span2:nth-child(6n+1)');
    // Don't break .span3 
    dontBreak('.span3:nth-child(4n+1)');
    // Don't break .span4 
    dontBreak('.span4:nth-child(3n+1)');
    // Don't break .span26
    dontBreak('.span6:nth-child(2n+3)');
  }    
});

or just view the updated results here

@luiscarlosjayk
Copy link

I tested solutions posted here but since my thumbnails might change their column sizes upon some calls and/or have multiple sizes of columns per row, css solutions posted here do not fit on my needs. (likely using css transitions on width of each thumbnail to make a nice effect to resize thumbnail boxes)

Instead I ended writing a js script to fix it.
http://jsfiddle.net/Ciul/amxL9/ This fiddle is based on ehoch jsfiddle.

It is a function named fixThumbs, and you can pass the number of columns for each row. By default it is 12.

Regards,
Ciul

@appastair
Copy link

Although I don't see the reluctance to include support for something that's so frequently-requested, @mdo has stated that the current implementation is as stated in the documentation for thumbnails:

...the thumbnails component uses existing grid system classes - like .span2 or .span3 - for control of thumbnail dimensions.

A patch to fix this would be easy to pull! 😆

https://gist.github.com/appastair/5723225#file-thumbnails-less-diff

@igal-getrailo
Copy link

@appastair -- I used your fix, thanks. I modified it a little to LESS style so it now looks like so:

.row-fluid {
    .thumbnails {
          .span2:nth-child( 6n + 1 )
        , .span3:nth-child( 4n + 1 )
        , .span4:nth-child( 3n + 1 )
        , .span6:nth-child( 2n + 1 )   {    // Responsive Thumbnail "Rows" (https://github.com/twitter/bootstrap/issues/3494)

            margin-left: 0;
        }
    }
}

@lvanderbijl
Copy link

There's an additional problem in that it seems the nth child selectors only get applied on first "view". I have a thumbnail grid where I dynamically show and hide thumbnails. When doing this, even in an nthChild supported browser, the styles do not get properly applied when

  • elements are hidden and then re-displayed (using display:none);

    I have there reverted to solely using @Paul424's fixThumbnails method to re-calculate margins everytime the display changes.

    This is far from perfect, but at least it works.

  • @appastair
    Copy link

    @lvanderbijl if you apply .hidden instead of adding styles directly to each element, you can modify the nth-child fix to be:

    .row-fluid .thumbnails .span2:nth-child( 6n + 1 ):not(.hidden){...}
    

    Although jQuery is aware of their visibility, CSS isn't. 😉

    @barrymcm
    Copy link

    barrymcm commented Jul 1, 2013

    Thanks the jQuery solution worked like a dream.

    @emersonthis
    Copy link

    Anyone know why this bug has persisted for over a year? Doesn't seem like a super tough fix.

    @cvrebert
    Copy link
    Collaborator

    cvrebert commented Jul 2, 2013

    This is almost certainly fixed in 3.0.0-wip.

    @igal-getrailo
    Copy link

    @cvrebert -- that may be the case, but not everyone is ready to switch to 3.0 and go with a mobile-first approach

    @musha68k
    Copy link

    @appastair @igal-getrailo thanks folks, thanks a lot! <3

    @SlimDeluxe
    Copy link

    Since my client has a gallery that is loaded with AJAX and it is a result of filtering (meaning diff. no. or results), I just used a ".first" class on the first item of each row and added
    .row-fluid .thumbnails li.first {
    margin-left: 0;
    }
    into the Less file.
    The class is added in PHP by checking if the index is dividable by 6 ($idx%6 == 0).
    Another solution is to chunk the results into groups of 6 (array_chunk() in PHP) and echo each line within it's own ul class="thumbnails" parent element.
    These are hacks but they work.

    @tiesont
    Copy link
    Contributor

    tiesont commented Jul 28, 2013

    @pongstr Seems worth mentioning that $.browser was removed in jQuery 1.9, so your fix won't work as-is if someone is using anything newer than 1.8.x unless the Migrate plugin is added. Other than that small problem, very nice.

    @pongstr
    Copy link

    pongstr commented Jul 28, 2013

    @tiesont thanks, here's an update to the solution I've suggested if anyone needs it.

    conditional ie tags

    <!doctype html>
    <!--[if IE 7]><html class="no-js lt-ie9 lt-ie8"><![endif]-->
    <!--[if IE 8]><html class="no-js lt-ie9"><![endif]-->
    <!--[if gt IE 8]><!--><html class="no-js"><!--<![endif]--><head>

    the javascript

    $(window).load(function(){
       if ( $('.lt-ie9 .thumbnails', '.lt-ie8 .thumbnails').length > 0 ) {      
        // Don't break function
        function dontBreak( column ){
          var ie7 = '.lt-ie8' + ' .thumbnails > ' + column,
              ie8 = '.lt-ie9' + ' .thumbnails > ' + column;        
          $( ie7, ie8 ).css({ 'margin-left' : '0' });
        }
    
        // Don't break .span2 
        dontBreak('.span2:nth-child(6n+1)');
        // Don't break .span3 
        dontBreak('.span3:nth-child(4n+1)');
        // Don't break .span4 
        dontBreak('.span4:nth-child(3n+1)');
        // Don't break .span6
        dontBreak('.span6:nth-child(2n+3)');
      }    
    });

    @donmb1
    Copy link

    donmb1 commented Aug 1, 2013

    Thank you guys for this. @pongstr - yours worked for me!

    @pongstr
    Copy link

    pongstr commented Aug 2, 2013

    👍 @MARTINB83 you're welcome man

    @chenhunghan
    Copy link

    @pongstr your fix will destroy the layout on mobile device when there are two span6 thumbnail row with captions...

    @pongstr
    Copy link

    pongstr commented Aug 20, 2013

    can you make a fiddle for that?

    @twbs twbs locked and limited conversation to collaborators Jun 14, 2014
    Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
    Labels
    Projects
    None yet
    Development

    No branches or pull requests