search

sirkitree / angular-directive.g-signin

:triangular_ruler: AngularJS Directive for Google Plus Sign-in Button
http://jeradbitner.com/angular-directive.g-signin/
148 stars 84 forks source link

Expanding tag name to avoid HTML parsing issues, switching to bower.json, updating Angular dependency #7

Closed doingweb closed 10 years ago

doingweb commented 10 years ago

The grunt-contrib-htmlmin task throws a parse error when run on an HTML file that includes the <g+signin> tag. This pull request avoids this and other syntax-related issues that come from using a + in the tag name, by expanding the name to google-plus-signin.

I also renamed component.json to bower.json to silence the deprecation complaint from bower, and updated the dependency version on Angular. If these changes are not desired, I can branch on an earlier commit and update the PR.

sirkitree commented 10 years ago

Sweet! I'll give this a good review tomorrow. Thank you!

djds4rce commented 10 years ago

This will break already present code(apps) if we dont make it a new release?

sirkitree commented 10 years ago

This will also need re-rolled since #8 is merged.

sirkitree commented 10 years ago

Another question, does the grunt task throw an error if you use <ANY data-g+signin=""></ANY> instead?

doingweb commented 10 years ago

I haven't tested a case like that specifically, but I believe it would.

The grunt-contrib-htmlmin task uses html-minifier, which matches start tags (including attributes) with the following regex:

/^<([\w:-]+)((?:\s*[\w:-]+(?:\s*=\s*(?:(?:"[^"]*")|(?:'[^']*')|[^>\s]+))?)*)\s*(\/?)>/

I tossed this in the nearest regex tester and it matched neither <g+signin> nor <any data-g+signin="">. It looks like it's the [\w:-]+ bits that match the tag name and attributes.

doingweb commented 10 years ago

Merged in today's upstream changes and bumped version to 0.1.0.

sirkitree commented 10 years ago

Looks good. Thanks!