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

grid with more than 2 columns and multiple rows has incorrect css #4835

Closed
diamondq opened this Issue Aug 12, 2012 · 3 comments

Comments

Projects
None yet
2 participants
@diamondq

diamondq commented Aug 12, 2012

The grid code injects ui-block-XXX for each column within a given row. For grids with 1 or 2 columns, having multiple rows works correctly, since it injects a ui-block-a every 2 widgets (for 2 columns) and a ui-block-b every 2 widgets + 1 (for 2 columns).

$kids.filter( ":nth-child(" + iterator + "n+1)" ).addClass( "ui-block-a" );
if ( iterator > 1 ) {
    $kids.filter( ":nth-child(" + iterator + "n+2)" ).addClass( "ui-block-b" );
}

However, for more than 2 columns, the code doesn't take into account the actual column count. Thus, this code

if ( iterator > 2 ) {
   $kids.filter( ":nth-child(3n+3)" ).addClass( "ui-block-c" );
}

injects a ui-block-c every 3n+3, so if you're using a 5 column wide grid, you'd get a ui-block-c at the 6th space, but it should be the 8th space.

I believe that the correct code would be:

if ( iterator > 2 ) {
   $kids.filter( ":nth-child(" + iterator + "n+3)" ).addClass( "ui-block-c" );
}

And the same for 4n and 5n.

For an example of it failing with the latest code (1.2 branch), see http://jsfiddle.net/diamondq/xZQnH/2/

@jaspermdegroot

This comment has been minimized.

Show comment
Hide comment
@jaspermdegroot

jaspermdegroot Aug 12, 2012

Member

@diamondq

Thanks for reporting the issue. You are right. Do you want to create a PR for it or want us to change it?

BTW - Not branch "1.2" but "master" is latest code. See http://jsbin.com/atahip/87/edit for links to latest code on jQuery CDN.

Member

jaspermdegroot commented Aug 12, 2012

@diamondq

Thanks for reporting the issue. You are right. Do you want to create a PR for it or want us to change it?

BTW - Not branch "1.2" but "master" is latest code. See http://jsbin.com/atahip/87/edit for links to latest code on jQuery CDN.

@ghost ghost assigned jaspermdegroot Aug 12, 2012

@diamondq

This comment has been minimized.

Show comment
Hide comment
@diamondq

diamondq Aug 12, 2012

Perhaps you can. I'm less familiar with Git at this point.

diamondq commented Aug 12, 2012

Perhaps you can. I'm less familiar with Git at this point.

@jaspermdegroot

This comment has been minimized.

Show comment
Hide comment
@jaspermdegroot

jaspermdegroot Aug 23, 2012

Member

@diamondq - Ok, done. Thanks again for your help!

Member

jaspermdegroot commented Aug 23, 2012

@diamondq - Ok, done. Thanks again for your help!

arschmitz added a commit to arschmitz/jquery-mobile that referenced this issue Oct 16, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment