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

Non-standard use of rgba(.., .., .., ..) #18

Open
rupertlssmith opened this issue May 17, 2018 · 10 comments
Open

Non-standard use of rgba(.., .., .., ..) #18

rupertlssmith opened this issue May 17, 2018 · 10 comments
Assignees
Labels
Milestone

Comments

@rupertlssmith
Copy link
Contributor

Setting the style on an element like this:

[ fillOpacity <| Opacity 1.0
]

Results in SVG like this:

fill="rgba(255, 255, 255, 1)"

Which renders correctly in browsers as "rgba(..)" is valid in CSS. However, the SVG 1.0 standard does not allow rgba, and this should be specified as

fill="#FFFFFF"

The alpha part should be set independently using the 'fill-opacity' parameter.

This problem is only seen if the rendered SVG is exported and loaded into an SVG tool that does not support CSS, such as Inkscape.

rupertlssmith added a commit that referenced this issue May 17, 2018
@rupertlssmith
Copy link
Contributor Author

This also afffects the 'stroke' attribute which has a separate 'stroke-opacity' attribute for the alpha channel. Corrected in the commit above.

@rupertlssmith
Copy link
Contributor Author

It is a little hard to know what to do alpha values from colors in general. If one is set, should adding a 'stroke', 'fill' automatically set the opacity?

Also the 'color' attribute does not have a corresponding 'opacity' attribute. In browsers setting a color with alpha will work just fine, but in SVG 1.1 it should really only set the color. It would seem correct if adhering to 1.1 to discard the alpha value from an Elm Color in that case.

It would seem more friendly to make a better API that lets Elm Color values be used with alpha, and to automatically set the correct opacity somehow.

I decided to leave 'color' alone for now, until a better design can be thought of.

@rupertlssmith rupertlssmith added this to the v2 milestone May 17, 2018
@rupertlssmith rupertlssmith self-assigned this May 17, 2018
@rupertlssmith
Copy link
Contributor Author

Fixed in 2.0.1

@madasebrof
Copy link

Ahhh. Okay, now I see.

Anyway, as I mentioned, this is a breaking change. Our entire app blew up, as suddenly everything no longer has proper transparency.

To be backward compatible, you could write some logic to check if you are being passed an RGBA from Elm. If so, add the opacity as an additional field fill-opacity and set it to the opacity.

Not sure what to do now... maybe I'll make a pull request to fix this...

@rupertlssmith rupertlssmith reopened this May 21, 2018
@rupertlssmith
Copy link
Contributor Author

As discussed in issue #19, the patch has been removed as it was a breaking change.

@rupertlssmith
Copy link
Contributor Author

Options for fixing this are:

  1. "Write some logic to check if you are being passed an RGBA from Elm, if so, add the opacity as an additional field fill-opacity"

Each SVG attribute in typed-svg maps onto one VirtualDom.Property, so creating 2 attributes from 1 probably doesn't work. Unless there is some kind of List Property -> Property that lets you wrap a list as just one property, like Cmd.batch does for Cmds.

This would still be possible but it means not mapping Attribute 1:1 onto VirtualDom.Property but introducing an intermediate layer. This would cost a little in terms of performance.

  1. Could add a separate attribute that does maintain SVG 1.1 compatability for fill and stroke, like this:

default:

fill : Fill -> Attribute

when you want to ensure compatability:

fillCompat : Fill -> Attribute

  1. Could do it dynamically depending on whether there is an alpha value < 1:

This will output 'rgba(123, 232, 45, 0.9)':

fill Color.rgba (123, 232, 45, 0.9)

These would output '#7be82d':

fill Color.rgba (123, 232, 45, 1.0)
fill Color.rgb (123, 232, 45)

This would be documented in the comments warning users that non-compliant SVG may be output when setting the alpha channel and directing them to the fill-opacity attribute as an alternative.

===

Other suggestions welcome or say which of the above you think works best.

@rupertlssmith
Copy link
Contributor Author

rupertlssmith commented May 21, 2018

  1. Add more options to the Fill type (and introduce a similar Stroke type):
type Fill
    = Fill Color
    | FillNone

Changes to:

type Fill
    = Fill Color
    | FillNoAlpha Color
    | FillNone

other suitable names might be FillCompat, or FillColorOnly?

@rupertlssmith
Copy link
Contributor Author

Should also look at what is going to happen to the Color module in Elm 0.19, it changes in some way for that I believe.

@madasebrof
Copy link

Personally, I'd probably go with option #3, as it seems like most use-cases for this library would be geared for SVG browser compatibility, e.g. SVG fill rgba works on IE 11, Chrome, FF, Safari, Edge, which is pretty much everything.

But maybe I'm wrong!

@rupertlssmith rupertlssmith modified the milestones: v2, v3 May 22, 2018
@rupertlssmith
Copy link
Contributor Author

I'm good with option 3 too.

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