modernizr-loader
modernizr-loader copied to clipboard
Improve documentation
Hi, This is really useful project thanks for that. Anyway I would like to suggest some README improvements:
- add info about
.modernizrrc
available options by linking https://github.com/Modernizr/Modernizr/blob/master/lib/config-all.json (like in https://www.npmjs.com/package/modernizr-webpack-plugin) - rearrange instructions. As it is a webpack loader I think that
require
andimport
code blocks should be placed after webpack.config example file with an information where you should writerequire
code (it should be written in your code not in the webpack.config which may not be so obvious)
If you like this suggestions I may send a PR.
Hey! Thanks for your suggestions. Feel free to send a PR 👍
I'm happy to help too as I stumbled a bit on not setting "setClasses: true". Without that I was not getting all css classes added for my tests.
As I finished my minimal example for #13 I think I may share it as a reference for other WP Sage theme users. Here is my repo and specific example commit which adds modernizr-loader https://github.com/jmarceli/modernizr-loader-sage/commit/f895441cef232e957e0f5feaa2eafc01a64ca9fd
What do you think? Should I modify README or place it in the Wiki or maybe leave it alone :)
@flootr I'd like to add to the README to include the option of adding modernizr to an entry point in webpack.config.js if only used in CSS and therefore importing into modules isn't necessary. For those just using it for CSS this is far simpler and avoids an unused import or finding an arbitrary place to require Modernizr.
// ...
entry: {
main: ['modernizr', './your/javascript/modules'],
},
// ...
This sounds like an useful info for me but it should be somehow separated from base configuration in order not to mislead people. Maybe new section name e.g. Alternative configuration, and current would be named Base configuration (or the other way round).
Ok why don't I do a pull request for what I've done and we can iterate in that?
Sent from my iPhone
On 29 Jun 2016, at 17:45, jmarceli [email protected] wrote:
This sounds like an useful info for me but it should be somehow separated from base configuration in order not to mislead people.
— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.
Ok why don't I do a pull request for what I've done and we can iterate in that?
Go for it 👍 @stevegibbings
@stevegibbings investigating further it is even easier to import 'modernizr'
than modify webpack config file. I think that the result is similar but this way if we need it in JS it is enough to change import 'modernizr'
to import Modernizr from 'modernizr'
.
I see no drawbacks of that approach and I would favour it over modifying a config file.
Yes I can see how that would work.
Sent from my iPhone
On 8 Nov 2016, at 11:51, jmarceli [email protected] wrote:
@stevegibbings investigating further it is even easier to import 'modernizr' than modify webpack config file. I think that the result is similar but this way if we need it in JS it is enough to change import 'modernizr' to import Modernizr from 'modernizr'. I see no drawbacks of that approach and I would favour it over modifying a config file.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.