Roots 7.0.0 #982

Merged
merged 61 commits into from Jul 3, 2014

Projects

None yet
Owner
retlehs commented Feb 5, 2014
  • Add Bower (#922, #978)
  • Better LESS organization (#961)
  • Different Grunt workflow (#933)
  • Leaner Roots (#992) — clean up, relative URLs, and nice search moved to Soil
  • Update screenshot to 880x660 (maybe just use a blank image)
  • Merge in @Foxaii's nav rejig
  • Move language files outside repo
  • Add grunt-autoprefixer
  • Setup dist/ folder?
  • Generate new roots.pot
  • Update CHANGELOG
  • Update README
  • Update roots.io docs

What's new

Leaner Roots

Soil will now be a plugin that contains:

  • Markup changes/clean up
    • wp_head() clean up
    • Remove WP version from RSS feeds
    • Clean up <html> attributes
    • Clean up <link> tags
    • Clean up body_class()
    • Wrap embedded media as suggested by Readability
    • Use <figure> and <figcaption> for WP captions
    • Remove unnecessary dashboard widgets
    • Remove unnecessary self-closing tags
  • Nice search
  • Relative URLs

The above features will be enabled in Roots (or any other theme) if Soil is activated by using add_theme_support:

  • add_theme_support('soil-clean-up');
  • add_theme_support('soil-nice-search');
  • add_theme_support('soil-root-relative-urls');

Removed from Roots:

  • lib/cleanup.php - Move roots_get_search_form to lib/utils.php
  • lib/widgets.php - Remove vCard widget, move register sidebars to lib/init.php
  • lib/relative-urls.php

Bower for front-end package management

Bower is now being used to pull in Modernizr, jQuery, Bootstrap and Respond to assets/vendor/. Whenever you're adding front-end assets to your project, you can use bower install <package> to add them to the vendor directory.

We're also now using grunt-modernizr to make a lean build based on what tests you use in your styles and JS.

If you check out the 7.0.0 branch, run npm install from the theme dir and bower install will run after the node dependencies have been installed.

LESS organization

LESS files are organized differently, see #961 for details. tl;dr:

├── components
│   ├── _buttons.less
│   ├── _forms.less
│   ├── _media.less
│   └── _wp-classes.less
├── layouts
│   │── pages
│   │   └── _home.less
│   │── _footer.less
│   │── _general.less
│   │── _header.less
│   │── _pages.less
│   │── _posts.less
│   └── _sidebar.less
├── _bootstrap.less
├── _global.less
├── _variables.less
└── main.less

Grunt workflow

Previously, every time you updated your CSS or JS, a minified files and changes to lib/scripts.php had to be committed to the repo. The repo no longer includes any generated front-end assets. Grunt tasks are now split into two: dev and build.

The dev task runs JSHint, compiles your LESS (non-minified) and concatenates your JS

The build task runs JSHint, compiles and minifies your LESS, runs UglifyJS for concatenation and minification, generates a lean Modernizr build, and revisions CSS and JS based on the hashes.

During build, assets-manifest.json is generated with the names of the revisioned files. lib/scripts.php reads that file and tells WordPress the correct path to the assets.

Testing

Check out the 7.0.0 branch and run npm install from the theme directory. If you don't use Bedrock, you'll need to add the following to your wp-config.php:

define('WP_ENV', 'development');

Grunt tasks:

  • grunt dev
  • grunt watch (same as dev except watches for file changes)
  • grunt build — for staging/production

Also grab https://github.com/roots/soil's new branch and enable the plugin

Note: things are subject to change!

These changes are not yet final. Keep an eye on this PR for updates.

retlehs added some commits Feb 3, 2014
@retlehs retlehs added this to the 7.0.0 milestone Feb 5, 2014
Member

Looking good. I'd be very happy to use Bower for Bootstrap in 7.0, I've been using it plenty for other projects, and even for grabbing extra Javascript/jQuery plugins for Wordpress themes/plugins

Not to go all crazy, but has anyone had experience with Gulp? http://gulpjs.com/

It's newer than GruntJS, but so far has been getting rave reviews, and looks easier to set up than Grunt, by a long shot. Even though I've gotten my head wrapped around the Gruntfile, it's still not very intuitive IMO.

@retlehs retlehs referenced this pull request Feb 5, 2014
Closed

Better LESS organization #961

talves commented Feb 5, 2014

This branch is definitely a +1. I include the file assets for bootstrap in my version (tags) commits on my private repos, so I know what version of bootstrap I included in a release of a particular theme. The alternative was targeting a specific release in the bower file. Still not sure what way I like best.

@retlehs Thanks for considering the bower solution
@kalenjohnson Thanks for recommending it

raulghm commented Feb 5, 2014

@kalenjohnson #gulpjs is pretty, all gruntjs plugins are compatible with gulpjs, but not there is one as "grunt-wp-version" for cache styles and script files

talves commented Feb 5, 2014

@retlehs I propose we copy (grunt-contrib-copy) the dist files the theme requires back into the appropriate asset folders for release.

I feel this will make it easier for us to ignore the whole vendor directory when deploying and deploy only the needed asset folders.

Here is a compare of my proposed changes

hariadi and others added some commits Feb 20, 2014
@hariadi hariadi Use grunt-wp-assets for version task
* default use filename version revving instead of querystring
* versioning file will be ignore
77e2fdb
@retlehs retlehs Merge pull request #990 from hariadi/7.0.0-grunt-wp-asset
Use grunt-wp-assets for version task
55994a8
@retlehs retlehs Run bower install after npm install c3263f0
@retlehs retlehs Fix parseFiles in grunt-modernizr task 53c2829
@retlehs retlehs Update clean task for versioned assets 4e70c60
@retlehs retlehs Use latest Bootstrap f5e03e9
@retlehs retlehs Ref #933 - dev and build tasks for Grunt
Dev: don't minify
Build: minify, lean Modernizr build

Dev assets will load if you have defined your WP_ENV as development
If you don't use Bedrock, you'll need to add this to your wp-config.php:

define('WP_ENV', 'development');

TODO: lib/scripts.php shouldn't need to be committed for changes after
running the version task
7b25599
@retlehs retlehs Use load-grunt-tasks 94cc294
@retlehs retlehs Replace grunt-wp-assets with grunt-filerev and grunt-filerev-assets
By writing to assets-manifest.json with grunt-filerev-assets, it's
no longer necessary to commit changes to lib/scripts.php when developing
5b23b7f

How about creating dist folder that will also be in .gitignore and where grunt will place built files.

The reason for this, amongst other things, is that you won't have to worry about any git conflicts in files built with grunt, which can be pita sometimes... =D

Owner
retlehs commented Feb 22, 2014

Files built with grunt are now gitignored, but would still like to add
dist to have better separation

Sent from my iPhone

On Feb 22, 2014, at 6:29 AM, "Miloš Gavrilović" notifications@github.com
wrote:

How about creating dist folder that will also be in .gitignore and where
grunt will place built files.

The reason for this, amongst other things, is that you won't have to worry
about any git conflicts in files built with grunt, which can be pita
sometimes... =D


Reply to this email directly or view it on
GitHubhttps://github.com/roots/roots/pull/982#issuecomment-35801456
.

@retlehs retlehs referenced this pull request Feb 22, 2014
Closed

Leaner Roots #992

Owner
retlehs commented Feb 22, 2014

note: updated grunt workflow is not final. renaming the assets brings concerns about possible 404s related to caching. might go back to the querystring method and get rid of the new grunt-filerev and grunt-filerev-assets additions

@Foxaii Foxaii referenced this pull request in roots/grunt-wp-assets Feb 22, 2014
Closed

Add JSON Manifest #20

@QWp6t QWp6t commented on an outdated diff Feb 26, 2014
- version: {
- options: {
- file: 'lib/scripts.php',
- css: 'assets/css/main.min.css',
- cssHandle: 'roots_main',
- js: 'assets/js/scripts.min.js',
- jsHandle: 'roots_scripts'
+ filerev: {
+ assets: {
+ src: ['assets/css/main.min.css', 'assets/js/scripts.min.js']
+ }
+ },
+ filerev_assets: {
+ assets: {
+ options: {
+ dest: 'assets-manifest.json'
QWp6t
QWp6t Feb 26, 2014 Member

Because this writes paths with \\ on Windows (i.e., assets\\css\\main.min.css), I'd recommend adding grunt-text-replace with the following task:

    replace: {
      dist: {
        src: ['<%= filerev_assets.assets.options.dest %>'],
        overwrite: true,
        replacements: [{
          from: '\\\\',
          to: '/'
        }]
      }
    }

That completely resolved the issue for me.

Member
QWp6t commented Feb 26, 2014

I have also been using a .appconfig file that adds JSON config that I pull into Grunt. It adds a lot of flexibility for me. It requires a slightly modified directory structure, however.

├── assets
│   ├── css
│   ├── fonts
│   ├── img
│   └── js
│       └── vendor
├── dev
│   ├── coffee
│   ├── css
│   ├── fonts
│   ├── img
│   ├── js
│   ├── less
│   └── vendor
├── .appconfig
└── Gruntfile.js

My Grunt file is also slightly different. I run less and output to dev/css, and then I use autoprefixer and cssmin. I do the same pattern with coffee (output to dev/js and then uglify or concat to assets/js).

Anyway, here is an example config

{
  "devPath": "dev",
  "distPath": "assets",
  "autoPrefix": ["last 2 versions", "ie 8", "Opera 12.1"],
  "liveReload": true
}

It is trivial to write a function to retrieve those values or fallback to a default of the file or key don't exist.

Member
Foxaii commented Feb 26, 2014

Thanks for the heads up, but filerev and filerev_assets are likely to be dropped when grunt-wp-assets has added the JSON manifest.

Your Grunt workflow is very similar to what we were hoping to incorporate, so thanks for sharing.

Member
QWp6t commented Feb 26, 2014

When grunt-wp-assets uses a JSON manifest, I'd recommend being cognizant of Windows directory separators so that it doesn't have the same problem that Windows users have with filerev and filerev_assets.

I really love that you move functionality to plugins!! 👍

Just to put it up for consideration: Follow WP code formatting (SEE: https://core.trac.wordpress.org/browser/trunk/.editorconfig?rev=27198&order=name)

Owner
retlehs commented Feb 28, 2014

that editorconfig is almost the same as ours except it uses tabs instead of spaces, which we will never do. roots will also not follow other wordpress core coding standards including spaces around braces/parens & yoda conditions

Member
QWp6t commented Feb 28, 2014

I think roots should use 4 spaces instead of 2 to be psr-2 compliant.

There is one WordPress standard that roots uses that I don't like: prefixes. Instead of using proper namespacing, roots (and WordPress) uses prefixes as a form of namespacing.

I think Roots should loosen the coupling between its library and templates, i.e., the templates ought not call library functions directly but instead apply WP filters, and library functions should add WP filters. For example, instead of roots_main_class(), you'd use apply_filters('roots/main_class', 'col-sm-8') in your template and your library might do something like add_filter('roots/main_class', [...] ); or something. This allows libraries to be more modular and paves the way for eventually converting roots into a dependency or plugin that can be updated with Composer. This might also make it easier to use Roots with other template engines like Twig.

Owner

@QWp6t

2 vs 4 spaces: this is the one coding standard I'm most likely to accept since its PSR (which is actually important) and not WP (which isn't).

Namespaces: we've had many internal discussions about it and I'm personally in favour of requiring PHP 5.3 and going with namespaces. We've already done that for some plugins we've made that are meant to be used with Bedrock since it obviously requires at least 5.3.2.

Filters: interesting idea that I'm not sure we've discussed. Not that this would be a problem, but I'm curious if you've ever heard anything about performance issues with a lot of hooks? I assume they'd start to have an effect at some point (obviously caching mitigates this).

Member
QWp6t commented Mar 1, 2014

I think even a 5.4 requirement would be okay since 5.4 is two years old now. #movethewebforward (lol)

I've never gotten crazy with filters before, but I don't think Roots meddles enough in the theme for filters to start getting out of hand. Come to think of it, I guess filters per se aren't really necessary. I was mostly just trying to think of a way to modularize the different parts of Roots.

Right now I overhaul the Grunt file (see above) to add a bit more flexibility and configurability. I intend to do the same sort of thing with other parts of Roots, like abstracting away the Bootstrap dependency so that Foundation can be used as an alternative, or SASS instead of LESS, etc. I guess I viewed using WP filters as way of abstracting away the Roots library, but there are probably better ways to do it.

Owner
retlehs commented Mar 1, 2014

since we're making certain things required such as grunt and bower with 7.0.0, i think it'd be okay if we require a newer php version.. even though this is depressing. would love to start moving in the direction of decoupling bootsrap from roots, along with supporting other css frameworks like foundation

Member

Hey Ben,

How can I help move v7 along? Ok if I just fork your branch and have at it?

I might have some time coming up in the next few weeks to take care of a few of those checkboxes you've got up there so we can launch 7.

Owner
retlehs commented Mar 16, 2014

@JulienMelissas sure! i've been extremely busy with other work that's got priority over getting this done 😓

Member

@retlehs, understood! I've got some things this week I need to take care of but next week I'm going to put this on my list to get on so I can use it on some upcoming stuff... I think I can take care of quite a few things for you.

Owner
retlehs commented Mar 31, 2014
Member
QWp6t commented Mar 31, 2014

Awesome! 👍

Nice to see autoprefixer being added as well.

Contributor
hariadi commented Apr 1, 2014

How about support querystring and filerev method for assets versioning? Example build task:

grunt build
--> main.min.css?060865602e1c6ad3e02ee2ebf60799a0

grunt build:filerev
--> main.06086560.min.css

More details: hariadi@roots:7.0.0...7.0.0-grunt-wp-asset

If it's ok I'll send a new PR.

JulienMelissas and others added some commits Apr 5, 2014
@JulienMelissas JulienMelissas - fixing weird double negative thing on line 22
- using unpackages/unoptimized modernizr when in the WordPress development environment
ea35b85
@chriscarr chriscarr Move main/sidebar class filters into config.php (fix missing arg erro…
…rs).
31d4e8a
Owner
retlehs commented Apr 16, 2014

@hariadi sorry for the late reply! we think it's best to support just the querystring method in roots, and maybe just add an article on the site about using the filerev method

fwiw, i prefer the filerev method but we settled on keeping it as the querystring to avoid issues that people could run into with possible 404s after making updates

@retlehs retlehs Merge pull request #1019 from JulienMelissas/7.0.0
Using un-optimized version of modernizr in dev environment.
cc17c14
Member

Just thinking about decoupling Roots from Bootstrap, with how mixins work for both LESS and SASS, we could potentially remove all Bootstrap classes from the markup and set up something like:

.main { .col-sm-8 }

I think the issue could be from the necessity of the .row class, that would still need to be wrapped somehow.

Member

I was thinking about this the other day too and ran into the same issue.
The yeoman generator someone made was able to decouple some things - I
think if we remove Bootstrap we're going to open another can of worms.

I guess the question is is "do we open that can"? I don't think 7.0.0 is
going to deal with it. 8.0 however, hmmmm

  • Julien

On Fri, May 30, 2014 at 12:56 PM, Kalen Johnson notifications@github.com
wrote:

Just thinking about decoupling Roots from Bootstrap, with how mixins work
for both LESS and SASS, we could potentially remove all Bootstrap classes
from the markup and set up something like:

.main { .col-sm-8 }

I think the issue could be from the necessity of the .row class, that
would still need to be wrapped somehow.


Reply to this email directly or view it on GitHub
#982 (comment).

Owner
retlehs commented May 30, 2014

yeah, i need to just get 7.0.0 out the door at this point. i'm going to finish up some docs for the site and then release it as is (maybe this weekend?)

sorry it's taken so long, lots going on outside of roots right now 😓

Member

Understood. You are the man!

  • Julien

On Fri, May 30, 2014 at 1:37 PM, Ben Word notifications@github.com wrote:

yeah, i need to just get 7.0.0 out the door at this point. i'm going to
finish up some docs for the site and then release it as is (maybe this
weekend?)

sorry it's taken so long, lots going on outside of roots right now [image:
😓]


Reply to this email directly or view it on GitHub
#982 (comment).

Contributor

What do people think about adding imagemin (https://github.com/gruntjs/grunt-contrib-imagemin) to the gruntfile to compress any images included in the assets dir? I've found it helpful in projects using yeoman (spec. angular.) to slim down any images that can't be fetched dynamically. Not absollutely necessary, per se, but could help and easy to configure.

@retlehs thanks again for your work on this!

schurig commented May 31, 2014

@markthethomas great idea! I would also suggest to add spritesmith (https://github.com/Ensighten/grunt-spritesmith)

Member
QWp6t commented May 31, 2014

I think Imagemin and Spritesmith are too unstable/unreliable across various platforms and architectures, and they have nothing to do with Roots.

I personally use them in my projects, but I would be against including them. They sometimes even fail during npm install which will lead to unnecessary headaches for Roots devs as they have to deal with the support requests here and on the forums.

Member

I 100% agree with @QWp6t!

On Saturday, May 31, 2014, QWp6t notifications@github.com wrote:

I think Imagemin and Spritesmith are too unstable/unreliable across
various platforms and architectures, and they have nothing to do with Roots.

I personally use them in my projects, but I would be against including
them. They sometimes even fail during npm install which will lead to
unnecessary headaches for Roots devs as they have to deal with the support
requests here and on the forums.


Reply to this email directly or view it on GitHub
#982 (comment).

  • Julien
Contributor

Huh; I've never had any problems with it but that could just be my interaction with it so far. I'd suggest the relation to roots though would be compressing a filetype that's usually included (images of some kind) in a project.

But since there are so many image compression tools (CLI and otherwise) and since people can easily include it on their own if they like, I'll agree with @QWp6t and @JulienMelissas and I don't think inclusion should happen here.

And, I suppose, a good WP theme shouldn't have many non-dynamic images anyhow :)

here's a gist for a gruntfile with imagemin and autoprefixer if anyone wants to include them on their own: https://gist.github.com/markthethomas/6b114f5fd7d55d29eca3

retlehs and others added some commits Jun 6, 2014
@retlehs retlehs Merge pull request #1049 from QWp6t/patch-1
Retrieve manifest.json from local file system. Fixes  #1048
4653b9c
@al-the-x al-the-x Make `grunt watch:js` include `assets/js/plugins/**/*.js`
If I add JavaScript packages to my `assets/js/plugins/` directory, they must be concatenated, uglified, and added to `scripts.js` to be used in my theme. The `watch:js` task should also note changes to those files and directories.
0804074
@retlehs retlehs Merge pull request #1055 from al-the-x/watch-js-plugins
Make `grunt watch:js` include `assets/js/plugins/**/*.js`
dffa1b4
@retlehs retlehs Ref #1071 - Update to Bootstrap 3.2.0 34bef3e
@retlehs retlehs README updates 9441ec5
@retlehs retlehs README updates b1645b5
@retlehs retlehs referenced this pull request Jun 30, 2014
Closed

Use build directory #1070

A little bit matter of taste - but I would much prefer nested rules. It avoids repetition and makes it easier to read (esp. if the elements follow the order on the page).

Also, if would ask you to think about if you now have gone to far with the splitting of the LESS files into components. The old version (app.less) was too monolithic - but this makes it hard for a developer to get a overview.

@retlehs retlehs merged commit 17716e5 into master Jul 3, 2014
@retlehs retlehs deleted the 7.0.0 branch Jul 3, 2014
Contributor
asuh commented on 70054b7 Jul 7, 2014

I'm confused about enqueuing script.js. After running npm install and it added everything, script.js is nowhere to be found. The only js file in /assets/js is main.js. Is this an error?

Owner

you need to run grunt to generate the assets first, see the theme development section in the readme

Contributor
asuh replied Jul 7, 2014

Ah, interesting. I see now what's going on. To get up and running with everything takes several steps, didn't realize how involved this process has become! Thanks for correcting me.

Owner

maybe we could include another npm postinstall script that does the initial grunt build..

Contributor
asuh replied Jul 7, 2014

That sounds like a good idea. What grunt dev is doing might be better automated in the process like you said. Is this command only used the one time initially?

Owner

to generate dev assets yeah, but you'll want to be using grunt dev or grunt watch as you're developing the theme as well. then, once you move to staging/production you need to run grunt build

Contributor
asuh replied Jul 7, 2014

Sure, I get that. I can't see any reason to run grunt dev beyond the first time if grunt watch practically does the same thing. Better UX to incorporate that extra step into initial install as you said.

Contributor
asuh commented on eb5ff6f Aug 5, 2014

Should 'roots' always be the name of the theme? I have a child theme and I assume I wouldn't be changing 'roots' with a custom version of this file for other library files?

Member
QWp6t replied Aug 5, 2014

Technically locate_template() accepts an array as its first parameter (and loads the first value in the array that it finds). So 'lib/scripts.php' could potentially be ['lib/scripts.php','roots/lib/scripts.php'] and it would be an acceptable value. But then if it fails, I believe you would see "Error locating Array for inclusion" thus making the error message meaningless.

Do we care supporting that syntax?

Owner

@asuh are you just referring to 'roots' in here:

trigger_error(sprintf(__('Error locating %s for inclusion', 'roots'), $file), E_USER_ERROR);

That's just the translation domain (see http://codex.wordpress.org/Function_Reference/_2) which should be roots since the translations come from our files. That wouldn't change with Roots as a parent theme.

Contributor
asuh replied Aug 5, 2014

@swalkinshaw yes that's what I was referring to, thanks. Makes sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment