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

Minify skip-link-focus and show how to flexibly include minified js. #892

Closed
wants to merge 1 commit into from

Conversation

jrfnl
Copy link
Contributor

@jrfnl jrfnl commented Dec 15, 2015

I've seen #181 and agree that minifying all css and js would not make sense for a starter theme.

However the skip-link-focus-fix.js file is one which typically won't be changed and makes for a nice example on how to flexibly enqueue minified files to satisfy the secondary teaching purpose of the theme.

@philiparthurmoore
Copy link
Collaborator

This won't fly. WordPress.com refuses to accept scripts that are minified, and _s is used by Automattic developers to kick off .com themes. Minified scripts are impossible to review so unfortunately this is probably a no-go. In general I like your approach but don't see this getting support for inclusion into _s.

@philiparthurmoore
Copy link
Collaborator

There's also the issue of most minification and concatenation happening server-side on WordPress.com (and also on the .org side of things via PageSpeed if it's configured on the server), so take that for what it's worth.

@jrfnl
Copy link
Contributor Author

jrfnl commented Dec 15, 2015

WordPress.com refuses to accept scripts that are minified ... Minified scripts are impossible to review

Isn't that only when no non-minified version of the same is available ? The non-minified versions will still be there too.

There's also the issue of most minification and concatenation happening server-side on WordPress.com (and also on the .org side of things via PageSpeed if it's configured on the server), so take that for what it's worth.

Makes sense for WP.com, but what about the rest of the world ?

@philiparthurmoore
Copy link
Collaborator

Isn't that only when no non-minified version of the same is available ? The non-minified versions will still be there too.

Basically it doesn't make sense to package code if it's not going to be used. So if minification and concatenation happen server-side on WordPress.com and theme reviewers need the non-minified versions, then the min versions are cruft and add an extra step into launching themes.

Makes sense for WP.com, but what about the rest of the world ?

Developers who don't handle min and concat server-side will just have to deal with this. Every attempt to ever ship minified versions of scripts to .com, with the exception of html5.js (which isn't accepted anyway outside of the default themes) isn't accepted, and _s is used every single day by Automattic. Adding more work to their workflow won't really fly.

And for the rest of us, there's PageSpeed and Grunt to help us minify and concatenate this stuff quickly. (So this PR is in effect a pretty good argument for Gruntifying _s at some point in another branch or on underscores.me, but this specific PR I highly, highly doubt would ever get accepted.)

Disclaimer: former Automattic employee who has reviewed more lines of theme code than Santa has eaten cookies.

@jrfnl
Copy link
Contributor Author

jrfnl commented Dec 15, 2015

I keep struggling with _s being positioned and promoted as a starter theme and a teaching tool for the wider WP community and at the same time all decisions about it seemingly being made based on the WP.com policies.

Disclaimer: former Automattic employee who has reviewed more lines of theme code than Santa has eaten cookies.

I feel for you ((hug))

@philiparthurmoore
Copy link
Collaborator

I keep struggling with _s being positioned and promoted as a starter theme and a teaching tool for the wider WP community and at the same time all decisions about it seemingly being made based on the WP.com policies.

Yes, this came up a while back and still is a major issue IMO:
#659

There is a disparity between WordPress.com, WordPress.org, and the default themes when it comes to standards, and unfortunately _s as the starting tool for .org developers reaches a certain point where a very good argument (at this point) can be made to not use it and use a home-grown version of _s for your own projects.

I cannot disagree with anyone who is frustrated by _s being positioned as a starter theme for all when it's really not there yet; it fits some needs well and for other needs, it's just not suitable. Wish I had better news.

@karmatosed
Copy link
Contributor

We aren't going to include minified files - that's up to the author. Please note even on .org as has been said you include minified and non. As a result it's not a .com decision - note .com authors are .org authors in even Automattic. I'm not going to get into the .org/.com debate as that's beyond this ticket. I will note that those of us making themes at Automattic are passionate equally about .org and .com and the cases aren't that different.

@karmatosed karmatosed closed this Feb 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants