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

Change SVG colors to not use RGBA (technically not allowed). #586

Merged
merged 2 commits into from
Mar 22, 2013

Conversation

emackey
Copy link
Contributor

@emackey emackey commented Mar 22, 2013

All our target browsers allowed us to use RGBA for a linearGradient: stop-color, but technically it's not in the SVG spec. SVG defines stop-opacity and stop-color separately, and the stop-opacity is the only place where alpha values work.

When I tried to export a new copy of the Animation widget into Inkscape (which hasn't happened in a long time), the colors came out black because we're not following the SVG spec.

So, this pull request brings us back into compliance, but there should be no visible changes in the browser as a result. In particular, the translucent parts of the Animation widget should be completely un-changed by this pull.

* @return {String} The CSS equivalent of this color, minus its Alpha.
* @see <a href="http://www.w3.org/TR/css3-color/#rgb-color">CSS RGB color values</a>
*/
Color.prototype.toCssColorStringNoAlpha = function() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would toCssRgbString and toCssRgbaString make better names?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be. It would be a breaking API change, but I don't know if this API really has that many users. I'm fine renaming these to whatever people think is best. Any other opinions on this before I rename?

@mramato
Copy link
Contributor

mramato commented Mar 22, 2013

Also, how about some tests for the new version (by just copying the rgba ones and removing the alpha).

@emackey
Copy link
Contributor Author

emackey commented Mar 22, 2013

I'll add some tests.

@shunter
Copy link
Contributor

shunter commented Mar 22, 2013

We could change toCssColor to use either rgb or rgba syntax depending on whether alpha is 1 or not. If you are using it with SVG then you should already be using alpha of 1, right?

@emackey
Copy link
Contributor Author

emackey commented Mar 22, 2013

Ready for another review.

mramato added a commit that referenced this pull request Mar 22, 2013
Change SVG colors to not use RGBA (technically not allowed).
@mramato mramato merged commit eaa46e3 into master Mar 22, 2013
@mramato mramato deleted the no-rgba-in-svg branch March 22, 2013 18:01
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