?
avatar ciar4n
ciar4n
26 Jan 2017

The Bootstrap responsive grid has some issues between 768px and 979px browser width. Spans/columns wrap when they should remain inline.

The issue appears to be due to an incorrect margin-left calculation (@fluidGridColumnWidth768)... https://github.com/joomla/joomla-cms/blob/staging/media/jui/less/variables.less#L273-L300

In Isis, commenting out each of the following does resolve the issue however before I go creating a PR, I would appreciate some further thoughts from anyone with a better understand of each of these less files. I realise they are changing the gutter width however they appear to be doing so incorrectly for this instance. If they can't be fixed maybe it would be best to remove them..

https://github.com/joomla/joomla-cms/blob/staging/administrator/templates/isis/less/template.less#L62
https://github.com/joomla/joomla-cms/blob/staging/administrator/templates/isis/less/template.less#L64

EDIT: On further inspection, comparing versions compiled by lessc and an alternative compiler, it is the span widths that are incorrect rather than the margin-left.. https://github.com/joomla/joomla-cms/blob/staging/administrator/templates/isis/css/template.css#L352-L399

Steps to reproduce the issue

Navigate to article edit and reduce the browser window. Columns wrap prematurely..

Expected result

grid2

Actual result

grid1

System information (as much as possible)

Additional comments

avatar ciar4n ciar4n - open - 26 Jan 2017
avatar joomla-cms-bot joomla-cms-bot - change - 26 Jan 2017
Labels Added: ?
avatar joomla-cms-bot joomla-cms-bot - labeled - 26 Jan 2017
avatar ciar4n ciar4n - edited - 26 Jan 2017
avatar ciar4n ciar4n - change - 26 Jan 2017
The description was changed
avatar ciar4n ciar4n - edited - 26 Jan 2017
avatar mbabker
mbabker - comment - 26 Jan 2017

I don't think it's the LESS files. I think it's the compiler. I have fought with similar grid related issues on the joomla.org site template and ultimately said "screw it" and changed to using Bootstrap's old recess compiler versus the lessc library. Fixed all of my issues.

avatar ciar4n
ciar4n - comment - 27 Jan 2017

You have mentioned this before but only now checking it out for myself.. using an alternative compiler does indeed resolve the issue. Considering this and presuming changing the compile method won't fly maybe getting Isis to load it's own version of responsive-768px-979px.less might be the best option. We can replace the calculated variables with the correct static values, making it less likely for lessc to mess up?

avatar Bakual
Bakual - comment - 27 Jan 2017

Can't we fix the lessc compiler upstream then? I have no clue where the issue comes from but sounds like the right approach to fix it ?

avatar C-Lodder
C-Lodder - comment - 27 Jan 2017

It's over 2 years old. Perhaps consider using a more modern solution?

avatar dgt41
dgt41 - comment - 27 Jan 2017

I could write a small grunt script for compiling less to CSS and .min.css if that's ok for 3.x

avatar C-Lodder
C-Lodder - comment - 27 Jan 2017

That would mean everyone having to download Node. I wouldn't suggest that for a minor update. It also doesn't really help the end user who may want to make LESS changes via the Template Manager

avatar dgt41
dgt41 - comment - 27 Jan 2017

The other option is to switch to https://github.com/oyejorge/less.php which apparently also seems outdated...

avatar C-Lodder
C-Lodder - comment - 27 Jan 2017

or a JS library

avatar mbabker
mbabker - comment - 28 Jan 2017

That less.php library doesn't have a compatible license for us to ship it. We got away with it for Bootstrap but I doubt it would fly for a PHP library.

avatar ciar4n
ciar4n - comment - 31 Jan 2017

I think the issue with lessc is what is suggested here.. leafo/lessphp#352

Personally I think compiling with Node (for Isis at least) is the way to go. If a user is familiar with LESS, the chances are they are also familiar with using Node.

avatar C-Lodder
C-Lodder - comment - 31 Jan 2017

I wouldn't agree with making people have to use Node to compile in a minor version update. Should stick to 4.x for that

avatar ciar4n
ciar4n - comment - 31 Jan 2017

After that then the only likely alternative is to override the lessc output with a corrected version in the template.less?

avatar mbabker
mbabker - comment - 31 Jan 2017

I don't know what's being cached by lessc, but I know something is. Whether it's the mixin results, variable values, or something else, I have no idea. But I know it has resulted in me labeling that library as unusable.

avatar ciar4n ciar4n - edited - 31 Jan 2017
avatar ciar4n ciar4n - change - 31 Jan 2017
The description was changed
avatar ciar4n ciar4n - edited - 31 Jan 2017
avatar ciar4n
ciar4n - comment - 1 Feb 2017

PR replacing the values calculated by lessc with the correct values.. #13845

avatar ciar4n ciar4n - change - 3 Feb 2017
The description was changed
Status New Closed
Closed_Date 0000-00-00 00:00:00 2017-02-03 19:36:39
Closed_By ciar4n
avatar ciar4n ciar4n - close - 3 Feb 2017

Add a Comment

Login with GitHub to post a comment