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

Remove duplicate CSS caused by mixins #10731

Closed
wants to merge 1 commit into from
Closed

Remove duplicate CSS caused by mixins #10731

wants to merge 1 commit into from

Conversation

danieljuhl
Copy link
Contributor

Removed parentheses in declaration of mixins (in mixins.less) which is going to be accessible as a class.

Removed the classes (in utilities.less) which has already been declared as mixins without parentheses, as they will be added to the final CSS.

In LESS any class can be used as a mixin, but when we declare a mixin and afterwards add a class with the same name, LESS actually duplicates the mixin. When we apply e.g. the .clearfix to the .row-selector, the clearfix CSS is added twice to the final CSS.

…s going to be accessible as a class.

Removed the classes which has already been declared as mixins without parentheses, as they will be added to the final CSS.

In LESS any class can be used as a mixin, but when we declare a mixin and afterwards add a class with the same name, LESS actually duplicates the mixin. When we apply e.g. the .clearfix to the .row-selector, the clearfix CSS is added twice to the final CSS.
@cvrebert
Copy link
Collaborator

X-Ref: #8947, less/less.js#1437

@mdo
Copy link
Member

mdo commented Sep 20, 2013

The problem with this is that the mixins.less generates CSS, something a number of folks absolutely hate. Last I saw it wasn't solvable fully until Less 1.4? Unsure about that last part. Definitely not 100% happy about that.

More than that, however, are all the deletions in the compiled CSS. Note that many of the clearfix-ed elements no longer have any clearfix properties. Whatever we change for this cannot affect the generated CSS.

@mdo mdo closed this Sep 20, 2013
mdo added a commit that referenced this pull request Dec 9, 2013
Original discussion:
less/less.js#1437 (comment).

Since we’re switching to `grunt-contrib-less`, we can take advantage of
newer LESS features than what RECESS supported. Included in that is the
ability to `:extend`, and not only that, but `:extend(.mixin-name
all)`. By doing so, we remove duplicate CSS for all our elements that
were being clearfix-ed.

Fixes #8947, #8968, #8991, #9257, #9268, #9291, #9430, #9604, #9686,
#9929, #10731, #10793, #11305, #11498, #11533, #11570, #11604, #11652.

(dem issues, tho)
stempler pushed a commit to stempler/bootstrap that referenced this pull request Apr 11, 2014
Original discussion:
less/less.js#1437 (comment).

Since we’re switching to `grunt-contrib-less`, we can take advantage of
newer LESS features than what RECESS supported. Included in that is the
ability to `:extend`, and not only that, but `:extend(.mixin-name
all)`. By doing so, we remove duplicate CSS for all our elements that
were being clearfix-ed.

Fixes twbs#8947, twbs#8968, twbs#8991, twbs#9257, twbs#9268, twbs#9291, twbs#9430, twbs#9604, twbs#9686,
twbs#9929, twbs#10731, twbs#10793, twbs#11305, twbs#11498, twbs#11533, twbs#11570, twbs#11604, twbs#11652.

(dem issues, tho)
stempler pushed a commit to stempler/bootstrap that referenced this pull request Nov 4, 2014
Original discussion:
less/less.js#1437 (comment).

Since we’re switching to `grunt-contrib-less`, we can take advantage of
newer LESS features than what RECESS supported. Included in that is the
ability to `:extend`, and not only that, but `:extend(.mixin-name
all)`. By doing so, we remove duplicate CSS for all our elements that
were being clearfix-ed.

Fixes twbs#8947, twbs#8968, twbs#8991, twbs#9257, twbs#9268, twbs#9291, twbs#9430, twbs#9604, twbs#9686,
twbs#9929, twbs#10731, twbs#10793, twbs#11305, twbs#11498, twbs#11533, twbs#11570, twbs#11604, twbs#11652.

(dem issues, tho)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants