Skip to content
This repository has been archived by the owner on Feb 12, 2022. It is now read-only.

Conversation

chriscorwin
Copy link
Contributor

Purpose

The proposed change would allow us to begin to use the & parent selector that LESS provides more effectively.

Overview

The less build should be able to be slightly restructured so that the .fuelux wrapper class name is not in the parent-chain (&) during the main LESS compilation, and instead gets tacked onto the front of the rules after the fact.

The CSS file ultimately output ought not be any different after making this change, since the & selector currently goes under-utilized (due, of course, to the quirks of our current process).

Simple example

From the LESS documentation on Changing Selector Order.

Given the following code:

.header {
  .menu {
    border-radius: 5px;
    .no-borderradius & {
      background-image: url('images/button-background.png');
    }
  }
}

...the way we are currently building the LESS would render the .css as:

.fuelux .header .menu {
  border-radius: 5px;
}
.no-borderradius .fuelux .header .menu {
  background-image: url('images/button-background.png');
}

Note the stray .fuelux that is inserted after the .no-borderradius selector, which is not what the developer intended.

The restructure would output as:

.fuelux .header .menu {
  border-radius: 5px;
}
.fuelux .no-borderradius .header .menu {
  background-image: url('images/button-background.png');
}

The Actual Restructure Proposal

Change the fuelux.less file so that it does the its initial compilation not inside the .fuelux wrapper.

It would output a temporary "non-namespaced" file, perhaps located at /less/temp.less.

At that point we would process a new LESS file which imports the temporary file we created -- this time inside the wrapper -- tacking the .fuelux "namespace" onto the rules.

@charset "UTF-8";

.fuelux {
    @import (less) "temp.less";
}

The grunt task doing this would output the resulting .css into it's final home at /dist/css/fuelux.css, and then delete the temporary file.

The temporary file should also be added to .gitignore to ensure that a failed LESS build does not result in the temp file not being deleted and it being inadvertently checked into the repository.

Chris Corwin added 6 commits February 6, 2015 14:24
…x-in-parents) Restrucure LESS so that initial compiliation does not have fuelux wrapper, which gets tacked on after the fact. Gruntfile deletes temporrary less file created in process which is also added to gitignore
…x-in-parents) All seems well, works as expected
…x-in-parents) Pulled in dist directory from upstream master
node_modules.tar.gz

# Temp file created int he LESS build

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo "int he"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, thanks.

@cmcculloh-kr cmcculloh-kr self-assigned this Feb 6, 2015
@cmcculloh-kr cmcculloh-kr added this to the 3.6.0 milestone Feb 6, 2015
/* -------------
SERVEFAST
------------- */
grunt.registerTask('servefast', 'Serve the files with no "dist" build or tests. Optional --no-less to also disabled compiling less into css.', function() {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

disabled -> disable

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

several trivial jshint errors here too actually, I'm fixing so I can merge the PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I couldn't figure out on a Friday afternoon what the heck was going on with travis errors.

Nor am I sure why or how they were introduced.

I am gonna dig in and look at what happened, but... you didn't remove the enhancements to "servefast" did you?

Those kinda sneaked in this, but were the result of an afternoon playing with it with @interactivellama .

@cmcculloh-kr cmcculloh-kr merged commit b39d366 into ExactTarget:master Feb 6, 2015
@chriscorwin chriscorwin deleted the GH1044---restructure-less-build-to-allow-ampersand-to-not-have-fuelux-in-parents branch April 2, 2015 16:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants