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

Commit #2652 breaks the Featured module's layout #2886

Closed
TiborBesze opened this issue Apr 8, 2015 · 7 comments
Closed

Commit #2652 breaks the Featured module's layout #2886

TiborBesze opened this issue Apr 8, 2015 · 7 comments

Comments

@TiborBesze
Copy link
Contributor

The "product-layout" class added to the 4th line of the catalog/view/theme/default/template/module/featured.tpl in commit #2652 incorrectly breaks 4 products into 2 lines, when either a left or right column present in the layout. Example:

opencart_featured_module_clearfix_break

The Latest and the Bestseller modules are not affected.

@aWeirdo
Copy link

aWeirdo commented May 7, 2015

It is mostlikely caused by your right content not having enough "available" width to show all four products on the same line.

Several easy ways to fix this in css, fastest:
Reduce width of your navigation/category sidebar via css.
This is found in Bootstrap's css.

@media (min-width: 768px)
.col-sm-3 {
  width: 25%;
}

Change to something like this ->

@media (min-width: 768px)
.col-sm-3 {
  width: 23%;
}

Keep reducing by 1% until it looks correct,
If that dosen't fix the issue then i would guess that it is because you have a fixed width for your right content which is not wide enough. Find your right content container and increase it's width.

@TiborBesze
Copy link
Contributor Author

Have you ever used Bootstrap? I can't see a reason why should I fix it, as it was the commit I've mentioned which broke it in the first place. Also, Bootstrap is using box-sizing, so 4 x 25% will always fit in one row, rather than two. The extra "product-layout" css class added to Line 4 of catalog/view/theme/default/template/module/featured.tpl is making it break into two lines. Also, I don't use any fixed width containers, as it would go completely against the concept of Bootstrap. This issue can be reproduced anytime, using the default template, without altering any of the CSS or TPL files, so it's a bug in the core files, rather than something I've just messed up.

FYI, the "product-layout" class is automatically adding a clearfix div after every 3rd product divs (incorrectly), as shown on the image below:
image

... so because of this, it doesn't matter what width you apply to the .col-sm-3 CSS class, it won't ever fix it. Clearfix clears the floating divs, and there's no way to fix that using only CSS.

Please, have a look at the actual problem before trying to fix it.

@veksen
Copy link
Contributor

veksen commented May 8, 2015

The problem is the clearfix being inserted by javascript in common.js (https://github.com/opencart/opencart/blob/master/upload/catalog/view/javascript/common.js#L26)

// Adding the clear Fix
cols1 = $('#column-right, #column-left').length;

if (cols1 == 2) {
    $('#content .product-layout:nth-child(2n+2)').after('<div class="clearfix visible-md visible-sm"></div>');
} else if (cols1 == 1) {
    $('#content .product-layout:nth-child(3n+3)').after('<div class="clearfix visible-lg"></div>');
} else {
    $('#content .product-layout:nth-child(4n+4)').after('<div class="clearfix"></div>');
}

It should instead be treated by CSS like so:

@media (min-width: 1200px) {
  #content .col-lg-2:nth-child(6n+1),
  #content .col-lg-2:nth-child(6n+1),
  #content .col-lg-3:nth-child(4n+1),
  #content .col-lg-4:nth-child(3n+1),
  #content .col-lg-6:nth-child(2n+1) {
    clear:left;
  }
}
@media (min-width: 992px) and (max-width: 1199px) {
  #content .col-md-2:nth-child(6n+1),
  #content .col-md-2:nth-child(6n+1),
  #content .col-md-3:nth-child(4n+1),
  #content .col-md-4:nth-child(3n+1),
  #content .col-md-6:nth-child(2n+1) {
    clear:left;
  }
}
@media (min-width: 768px) and (max-width: 991px) {
  #content .col-sm-2:nth-child(6n+1),
  #content .col-sm-2:nth-child(6n+1),
  #content .col-sm-3:nth-child(4n+1),
  #content .col-sm-4:nth-child(3n+1),
  #content .col-sm-6:nth-child(2n+1) {
    clear:left;
  }
}

Note that elsewhere, like in related products, the clearfix is inserted via PHP, this should also get taken care of. Just my 2 cents... would do a PR to fix the whole thing, but my last experiences on here were bad (no comment on why it gets closed or not, etc).

Additionally, if I recall during my hotfixing here, the left_column and right_column are not considered empty if some modules are enabled by empty (say, no specials), so here, you are considered having a left and right column, and the clearfix is being applied every 3 .product-layout.

@joolfe
Copy link

joolfe commented Jul 20, 2015

I think the problem wih feature module is that have the class "product-layout" so the common.js insert the clear fix based on the number of columns (like veksen sais) but in pages where no grid control exist the common.js not adjust the product-layout grid. This is the function:

    // Product Grid
$('#grid-view').click(function() {
    $('#content .product-layout > .clearfix').remove();

    // What a shame bootstrap does not take into account dynamically loaded columns
    cols = $('#column-right, #column-left').length;

    if (cols == 2) {
        $('#content .product-layout').attr('class', 'product-layout product-grid col-lg-6 col-md-6 col-sm-12 col-xs-12');
    } else if (cols == 1) {
        $('#content .product-layout').attr('class', 'product-layout product-grid col-lg-4 col-md-4 col-sm-6 col-xs-12');
    } else {
        $('#content .product-layout').attr('class', 'product-layout product-grid col-lg-3 col-md-3 col-sm-6 col-xs-12');
    }

     localStorage.setItem('display', 'grid');
});

If this function is executed the featured module looks ok but i think the responsive adjuts based on javascript execution is very bad solution...

@danielkerr
Copy link
Member

#3224

@danielkerr
Copy link
Member

just reopen a new issue if this is not fixed.

@stoyanov1984
Copy link

Hello,

I have the same problem using v 2.0.3.1
Is there any solution of that problem?
featured_122_159lo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants