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

Add Less CSS preprocessing engine, some refactoring #4

Merged
merged 1 commit into from
Jun 16, 2016

Conversation

MusikAnimal
Copy link
Member

@MusikAnimal MusikAnimal commented Jun 16, 2016

Why use Less?

  • Our Twig templates have conditionals to add the proper CSS classes on individual elements based on the triage state, the same is true with the JavaScript in saveState
  • To accomplish what we had in mind for the undo review task, we'd end up having to copy/paste Bootstrap styles, or use several lines of JavaScript with unnecessary event listeners to accomplish what should be a simple thing
  • With Less, we can add a single CSS class name to .status-div that represents the triage state. From there we can use Bootstrap.less (not included in this PR, but easy to add) to extend the necessary classes and get the styles we want. So we have the initial state, but on :hover we just extend a different Bootstrap class. A few lines of code instead of a bunch of lines of code that need refactoring, but we can't refactor because native CSS prevents any attempt at being DRY
  • There is little overhead considering the index.css file is cached, and is only regenerated when the Less files change (so once per deploy)
  • You can use normal ole CSS in Less. No need to hesitate and feel like you need to read the docs. Just write and we can refactor as needed.
  • MediaWiki does it, and much in the same way. This isn't really a compelling reason to use it on Tool Labs, but it'd at least be familiar to those comfortable with the MediaWiki frontend
  • CSS preprocessors are just awesome :) They are incredibly easier to work with (nesting alone will win you over), and they can bring a huge CSS file down to a fraction of the size, making it much more maintainable

$options = array( 'cache_dir' => APP_ROOT . '/src/Less/cache' );
$cssFileName = Less_Cache::Get( $lessFiles, $options );
$slim->response->headers->set( 'Content-Type', 'text/css' );
$slim->response->setBody( file_get_contents( APP_ROOT . '/src/Less/cache/' . $cssFileName ) );
Copy link
Member Author

@MusikAnimal MusikAnimal Jun 16, 2016

Choose a reason for hiding this comment

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

This is where the magic happens. Requests to index.css will have to go through the stack, but the file is cached until the .less file is changed, so this should have no noticeable affect on page load time. This is how it is done in ResourceLoader. Normally we could just add these 5 lines of code within a <?php block in the <head> tag of our HTML, but here we can't because we're using Twig, which does not interpret PHP. We could create a Twig function that we could reference in the template, but it's essentially the same effect, except here we're taking out that small amount of Twig processing.

We only need to compile index.less. Within that file we can import Bootstrap or any other Less files, but saving that for another PR :)

I could not get a /css/index.css route to work, for whatever reason. I figure index.css works just as well, especially since once we start using bootstrap.less we'd only have one .css file anyway.


// make this easier when working locally
$numRecords = $_SERVER['HTTP_HOST'] === 'localhost' ? 5 : 50;
return $this->dao->getPlagiarismRecords( $numRecords, $filter, $filterUser );
Copy link
Member Author

Choose a reason for hiding this comment

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

We had talked about how we prefer to change to a lower number of records when working locally, so I figure this would be a handy dandy way to automate it. I've forgotten to change the 5 back to 50 a number of times now.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Good idea! :D

Some refactoring in CopyPatrol.php
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants