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

Having some problems with using this with WSK #24

Closed
gauntface opened this issue Jan 13, 2015 · 9 comments
Closed

Having some problems with using this with WSK #24

gauntface opened this issue Jan 13, 2015 · 9 comments
Labels

Comments

@gauntface
Copy link

I'm just trying to add in the new version of this task into WSK and I'm having a few issues:

At the moment the task is:

gulp.task('favicon', function () {
  favicons({
    files: {
        src: 'app/images/favicons/favicon-1024x1024.png',   // Path for file to produce the favicons. `string` or `object`
        dest: 'dist/',                                      // Path for writing the favicons to. `string`
        html: 'dist/index.html',                            // Path for HTML file to write metadata. `string`
        iconsPath: null,                                    // Path for overriding default icons path. `string`
        androidManifest: 'dist/manifest.json',              // Path for an existing android_chrome_manifest.json. `string`
        browserConfig: null,                                // Path for an existing browserconfig.xml. `string`
        firefoxManifest: 'dist/manifest.webapp',            // Path for an existing manifest.webapp. `string`
        yandexManifest: null                                // Path for an existing yandex-browser-manifest.json. `string`
    },
    icons: {
        android: true,            // Create Android homescreen icon. `boolean`
        appleIcon: true,          // Create Apple touch icons. `boolean`
        appleStartup: true,       // Create Apple startup images. `boolean`
        coast: true,              // Create Opera Coast icon. `boolean`
        favicons: true,           // Create regular favicons. `boolean`
        firefox: true,            // Create Firefox OS icons. `boolean`
        opengraph: true,          // Create Facebook OpenGraph. `boolean`
        windows: true,            // Create Windows 8 tiles. `boolean`
        yandex: true              // Create Yandex browser icon. `boolean`
    },
    settings: {
        appName: 'Web Starter Kit',             // Your application's name. `string`
        appDescription: 'Web Starter Kit',      // Your application's description. `string`
        developer: null,                        // Your (or your developer's) name. `string`
        developerURL: null,                     // Your (or your developer's) URL. `string`
        background: '#3372DF',                  // Background colour for flattened icons. `string`
        index: null,                            // Path for the initial page on the site. `string`
        url: null,                              // URL for your website. `string`
        logging: true                           // Print logs to console?
    }
  });
});

The log for this is minimal:

$ gulp favicon
[14:33:39] Using gulpfile ~/Programming/Code/web-starter-kit/gulpfile.js
[14:33:39] Starting 'favicon'...
[14:33:39] Finished 'favicon' after 538 μs
Created apple-touch-startup-image-320x460.png
Created apple-touch-startup-image-640x920.png
Created apple-touch-startup-image-640x1096.png
Created apple-touch-startup-image-748x1024.png
Created apple-touch-startup-image-768x1004.png
Created apple-touch-startup-image-1496x2048.png
Created apple-touch-startup-image-1536x2008.png

No images seem to get created anywhere (assuming /dist/app-touch-startup-image-*.png would be the location it would be created.

  • Any ideas of how to debug this easier?
  • Any obvious issues where I'm going wrong?
  • Is it possible to give this more than one html file?

Thanks,
Matt

@haydenbleasel
Copy link
Contributor

Heya @gauntface, I'll download the WSK and have a look into the issue. It's a bit hard to debug this new version as most of the processing is done with a RealFaviconGenerator API call, but I think the error is due to the lack of a callback. Also there's a Gulp plugin now so I recommend using that: https://www.npmjs.com/package/gulp-favicons. Can you send me your favicon image file you're using?

@gauntface
Copy link
Author

Any issues you find, it would be good to reveal them in the logs, always a black box at the moment. I'll try out the gulp-favicons.

Just to let your know I'm working on the material-sprint branch AND I have an image not in the repo - happy to share, but any image to test would be fine.

@haydenbleasel
Copy link
Contributor

@gauntface I tried out gulp-favicons by adding a new task to WSK and it appears to work okay:

gulp.task('favicons', function() {
  gulp.src('app/index.html')
    .pipe($.favicons({
      files: {
        dest: '../dist/images/',
        html: '../dist/index.html',
        androidManifest: '../dist/manifest.json',
        browserConfig: null,
        firefoxManifest: '../dist/manifest.webapp',
        yandexManifest: null
      },
      icons: {
        android: true,
        appleIcon: true,
        appleStartup: true,
        coast: true,
        favicons: true,
        firefox: true,
        opengraph: true,
        windows: true,
        yandex: true
      },
      settings: {
        appName: 'Web Starter Kit',
        appDescription: 'Web Starter Kit',
        developer: null,
        developerURL: null,
        background: '#3372DF',
        index: null,
        url: null,
        logging: true
      }
    }))
    .pipe(gulp.dest('dist'));
})

Long story short, the Gulp plugin works a little differently by parsing an HTML tag:

<link rel="favicons" href="images/logo.png" />

It removes it upon processing so there's no chance of a W3C validation error or anything. I'd like to make it accept an HTML file or an image when I get around to it.

@gauntface
Copy link
Author

Thanks for your help @haydenbleasel.

I'm getting different results to you:

$ gulp favicon
[15:15:48] Using gulpfile ~/Programming/Code/web-starter-kit/gulpfile.js
[15:15:48] Starting 'favicon'...
[15:15:48] Finished 'favicon' after 154 ms
Created apple-touch-startup-image-320x460.png

/Users/mattgaunt/Programming/Code/web-starter-kit/node_modules/gulp-favicons/node_modules/underscore/underscore.js:474
    for (var i = 0, length = input.length; i < length; i++) {
                                  ^
TypeError: Cannot read property 'length' of undefined
    at flatten (/Users/mattgaunt/Programming/Code/web-starter-kit/node_modules/gulp-favicons/node_modules/underscore/underscore.js:474:35)
    at Function._.flatten (/Users/mattgaunt/Programming/Code/web-starter-kit/node_modules/gulp-favicons/node_modules/underscore/underscore.js:489:12)
    at /Users/mattgaunt/Programming/Code/web-starter-kit/node_modules/gulp-favicons/index.js:87:50
    at /Users/mattgaunt/Programming/Code/web-starter-kit/node_modules/favicons/index.js:256:24
    at /Users/mattgaunt/Programming/Code/web-starter-kit/node_modules/favicons/node_modules/async/lib/async.js:544:30
    at /Users/mattgaunt/Programming/Code/web-starter-kit/node_modules/favicons/index.js:241:21
    at /Users/mattgaunt/Programming/Code/web-starter-kit/node_modules/favicons/index.js:130:24
    at /Users/mattgaunt/Programming/Code/web-starter-kit/node_modules/favicons/node_modules/async/lib/async.js:254:17
    at done (/Users/mattgaunt/Programming/Code/web-starter-kit/node_modules/favicons/node_modules/async/lib/async.js:129:15)
    at /Users/mattgaunt/Programming/Code/web-starter-kit/node_modules/favicons/node_modules/async/lib/async.js:32:16

I'm also slightly lost as to what this is now doing.

The gulp.src is app/index.html, we also have the html set to dist/index.html (If we can drop the dist/index.html one then thats awesome).

I'm also not following what dest: '../dest/images/', is doing in this if the gulp.dest is defining dist AND is gulp.dest for the images and the index.html or just one of them?

Pulling info out of the HTML file is actually really helpful for our usecase as it means we can push this to a build only step.

@haydenbleasel
Copy link
Contributor

@gauntface I definitely agree it's confusing, I'd like to spend some time on making it simpler. Basically we're reading an HTML file instead of an image for streaming reasons - you can pipe it onto your html task in WSK, making it (as you said) a build-only step e.g.

return gulp.src('app/**/*.html')
    .pipe(assets)
    .pipe(favicons({ ... })) // Accepts HTML file(s), returns HTML file(s).
    .pipe($.if('*.js', $.uglify({preserveComments: 'some'})))
    ...

It accepts an HTML file with a custom metatag and returns an HTML file with new metatags. The produced images are basically an "add-on" which is why you need the files.dest property. I think I'll take your advice on removing files.html, it was there in case you don't want metadata printed to your HTML file but whatever, it's bad practice not to.

As for the error you're getting, I usually get that if my metatag is missing or the image reference is wrong. For you, this should be something like:

<link rel="favicons" href="images/favicons/favicon-1024x1024.png" />

I really need to either write down a list of these errors in the readme or make better error messages. Hope this helps and sorry for the hassle :S

@gauntface
Copy link
Author

Right, further digging (I'll post my code to get to this point), but the issue with a spawn task:

$ gulp favicons
[17:39:53] Using gulpfile ~/Programming/Code/web-starter-kit/gulpfile.js
[17:39:53] Starting 'favicons'...
[17:39:53] Finished 'favicons' after 123 ms
file.cwd = /Users/mattgaunt/Programming/Code/web-starter-kit
Created apple-touch-startup-image-320x460.png

/Users/mattgaunt/Programming/Code/web-starter-kit/node_modules/gulp-favicons/index.js:105
throw error;
^
Error: spawn ENOENT
at errnoException (child_process.js:988:11)
at Process.ChildProcess._handle.onexit (child_process.js:779:34)

I'm assuming this is an issue with the imagemagick but after installing it - I still can't get the damn thing to work :(

@gauntface
Copy link
Author

Ah - requires graphicsmagick too: https://www.npmjs.com/package/gm

@gauntface
Copy link
Author

So I've finally got it looking like it's working, but mashed things up into this PR: #25

@haydenbleasel
Copy link
Contributor

@gauntface Awesome, thanks so much. Was it really necessary as part of the commit to change all my indentation and coding style though? I'm a huge fan of JSLint so that's from where most of my style is based.

Also, Addy proposed splitting gulp-favicons into a new repository so the new PR has to go here: https://github.com/haydenbleasel/gulp-favicons. Sorry about that :/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants