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

Update of less compiler (leafo) #7687

Merged
merged 8 commits into from
Jan 26, 2016
Merged

Update of less compiler (leafo) #7687

merged 8 commits into from
Jan 26, 2016

Conversation

dgrammatiko
Copy link
Contributor

Joomla is using an outdated version of lessc

B/C

This should be 100% compatible with Bootstrap > 2.3 or any other less files that are using the same notation

Testing

Try to compile any less files (other than the ones Joomla is already shipping) and compare it with the previous copy. Should be identical.
It would be nice if you can apply #6979 and try to run generatecss.php, should work fine! Also since to already have applied that patch do some template switching and give to that one as well a response!

PS. There seems to be some border color clash for a sub template of Hathor, that needs some attention.
Protostar seems to get one more class for IE10, https://github.com/joomla/joomla-cms/blob/staging/media/jui/less/responsive-utilities.less#L9-L11

@mbabker
Copy link
Contributor

mbabker commented Aug 12, 2015

You have to run "composer update leafo/lessphp" and update the Composer
lock file too. Otherwise things start to get very out of whack.

Run "composer update leafo/lessphp" then "composer install --no-dev" after
changing the version constraint in composer.json. The second command
uninstalls the dev dependencies that will get pulled in (by design so they
are locked to specific versions for the CI systems).

On Wednesday, August 12, 2015, Dimitris Grammatiko [email protected]
wrote:

Joomla is using an outdated version of lessc B/C

This should be 100% compatible with Bootstrap > 2.3 or any other less
files that are using the same notation
Testing

Try to compile any less files (other than the ones Joomla is already
shipping) and compare it with the previous copy. Should be identical.

PS. There seems to be some border color clash for a sub template of
Hathor, that needs some attention.
Protostar seems to get one more class for IE7 (Useless Joomla supports

IE>8)

You can view, comment on, or merge this pull request online at:

#7687
Commit Summary

  • Sync with leafo's repo ver 0.5

File Changes

Patch Links:


Reply to this email directly or view it on GitHub
#7687.

@wilsonge
Copy link
Contributor

Joomla 3.x is IE8 and higher

@dgrammatiko
Copy link
Contributor Author

@wilsonge that is quite a paradox bootstrap 2.x is >IE7 but Joomla is >IE8.
Anyway the extra class comes from bootstrap 2.x mixins which is IE7 compatible, so we either leave it as is, or we touch the bootstrap less

@dgrammatiko
Copy link
Contributor Author

@mbabker done

@wilsonge
Copy link
Contributor

@dgrammatiko
Copy link
Contributor Author

@Bakual
Copy link
Contributor

Bakual commented Aug 13, 2015

The reason why we never updated beyong 0.3.9 was the B/C warning for 0.4.0:
http://leafo.net/lessphp/changelog.html

Warning: this release has some breaking changes. If you the old selector expression syntax (eg. Bootstrap 2.3) then it’s recommended to use 0.3.9.

Interesting that on leafo.net, the latest version still shows as 0.4.0, while the 0.5.0 release is tagged in GitHub since Nov 2014.

@vess
Copy link
Contributor

vess commented Aug 13, 2015

@Bakual Bootstrap moved away from that old selector expression before 0.4.0 even got out, so it should be ok to update our version.

@Bakual
Copy link
Contributor

Bakual commented Aug 13, 2015

@Bakual Bootstrap moved away from that old selector expression before 0.4.0 even got out, so it should be ok to update our version.

I have no clue what it refers to. If Bootstrap 2.3.2 doesn't use that selector expression (whatever that is), then I guess we can safely update it.
However according to @nonumber in #6451 (comment), there are issues with 0.5.0.

@dgrammatiko
Copy link
Contributor Author

@Bakual The errors @nonumber found are already been fixed in this PR (/cms/less/less.php)

v0.5.0 was not fine. Found issues with it (not inheriting overwritten variables)

Also @vess comment in another workgroup, which pointed to the fix:

multiple files in a row with the same instance fails... issue leafo/lessphp#594 fix that works is leafo/lessphp#607

@vess
Copy link
Contributor

vess commented Aug 13, 2015

I have no clue what it refers to. If Bootstrap 2.3.2 doesn't use that selector expression (whatever that is), then I guess we can safely update it.

@Bakual
old: (~".span@{index}") {.span(@Index);}
new: .span@{index} {.span(@Index);}
lukeapage/bootstrap@6b9bb54

@Bakual
Copy link
Contributor

Bakual commented Aug 13, 2015

Thanks!

@roland-d
Copy link
Contributor

roland-d commented Nov 4, 2015

So are we OK to get this thing in or are there B/C concerns?

@dgrammatiko
Copy link
Contributor Author

No B/C break with this

@roland-d
Copy link
Contributor

roland-d commented Nov 4, 2015

In that case, this needs to be tested.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/7687.

@Bakual Bakual mentioned this pull request Nov 12, 2015
@RickR2H
Copy link
Member

RickR2H commented Dec 12, 2015

I have tested this item ✅ successfully on 657c027

Applied the patch and changed the .less files in the isis and the protostar template to monitor the output. After that executed generatecss.php to generate the stylesheets. Everything seems to work and there where no errors.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/7687.

@joomla-cms-bot
Copy link

This PR has received new commits.

CC: @RickR2H


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/7687.

@810
Copy link
Contributor

810 commented Jan 17, 2016

I have tested this item ✅ successfully on 675a878

>=IE10 support has been ended by Microsoft. So we can also end the support for >= J3.5


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/7687.

@brianteeman
Copy link
Contributor

(MS only ended support - they did not stop anyone from using it.)


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/7687.

@dgrammatiko
Copy link
Contributor Author

@RickR2H can you retest this one? Thanks!

@RickR2H
Copy link
Member

RickR2H commented Jan 25, 2016

What is the best way to test this? I'll test asap ;)

@dgrammatiko
Copy link
Contributor Author

Just run php build/generate.php and see if the css's are all ok

@RickR2H
Copy link
Member

RickR2H commented Jan 25, 2016

@dgt41 I'll do that.

Just for my information, why is the LESS compiler there when developers mostly use grunt etc. to compile less?

@dgrammatiko
Copy link
Contributor Author

It has to be php, for the low end servers that don't have grunt etc

@810
Copy link
Contributor

810 commented Jan 25, 2016

its php build/generatecss.php right not php build/generate.php

@dgrammatiko
Copy link
Contributor Author

@810 yes you are right!

@RickR2H
Copy link
Member

RickR2H commented Jan 26, 2016

I have tested this item ✅ successfully on 675a878

Test was successful! To test it I added extra styling to the end of the template.less file. Sources are getting compiled nicely!


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/7687.

@dgrammatiko
Copy link
Contributor Author

RTC then

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jan 26, 2016
@wilsonge wilsonge added this to the Joomla! 3.5.0 milestone Jan 26, 2016
wilsonge added a commit that referenced this pull request Jan 26, 2016
Update of less compiler (leafo)
@wilsonge wilsonge merged commit 175fc16 into joomla:staging Jan 26, 2016
@dgrammatiko dgrammatiko deleted the ____less_0.5 branch January 26, 2016 23:24
@brianteeman brianteeman removed the RTC This Pull Request is Ready To Commit label May 11, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants