Opened 12 years ago

Closed 12 years ago

Last modified 8 years ago

#3652 closed defect (fixed)

dojo.Color.setColor("rgba(nR, nG, nB, nA") ignores alpha value

Reported by: guest Owned by: Eugene Lazutkin
Priority: high Milestone: 0.9
Component: General Version: 0.9
Keywords: Cc: Eugene Lazutkin, Adam Peller
Blocked By: Blocking:

Description

Passing a string argument to dojo.Color.setColor containing an alpha value results in a returned dojo.Color object with an alpha of 1.

Change History (20)

comment:1 Changed 12 years ago by Adam Peller

Owner: changed from anonymous to Eugene Lazutkin

while we're at it, we should probably consolidate constructor arg types -- an Array instead of arg lists and hashes would be more consistent with the rest of dojo.Color

comment:2 Changed 12 years ago by Eugene Lazutkin

Milestone: 0.9
Status: newassigned

comment:3 Changed 12 years ago by Eugene Lazutkin

Resolution: wontfix
Status: assignedclosed

There is no rgba(r, g, b, a) notation. Where did you get it from? Is it documented anywhere? Marking wontfix for now.

comment:4 Changed 12 years ago by Eugene Lazutkin

Resolution: wontfix
Status: closedreopened

I stand corrected: it is part of CSS3. Reopening.

comment:5 Changed 12 years ago by Eugene Lazutkin

Another part of CSS3 color module is the ability to specify color values like this:

rgb(100%, 0%, 0%)
rgba(100%, 0%, 0%, 1.0)
hsl(0, 100%, 50%)

All of them represent "red". Question to all interested parties: do we want to support it as part of the base? We can move it to dojo.colors like we did with CSS3 named colors. In the latter case it will augment existing dojo.Color functions.

Thoughts?

comment:6 Changed 12 years ago by bill

My thought is, why support three ways to do the same thing? I think one way is enough (and makes our code more consistent).

comment:7 Changed 12 years ago by Eugene Lazutkin

It depends on what we want to achieve. If we want to parse valid CSS strings to rgba values, it is one thing. If all we want is to support programmatic creation of colors, we don't need to parse strings at all. In the latter case we need to support following things in the base:

  1. Conversion from a string is done only for named colors --- it is convenient.
  2. Programmatic creation:
    • new dojo.Color(255, 0, 0, 0.5)
    • new dojo.Color("red")
    • new dojo.Color([255, 0, 0])
  3. Conversion to strings:
    • color.toRgb()
    • color.toHex()

The rest goes to the core (dojo.color), which can include enhanced functionality. This code can augment a string conversion method with all possible color formats. New functionality will be used transparently by all other methods/constructors.

In addition Adam made a valid point about changing the constructor signature from 3-4 arguments to just 1 argument --- use array, if you want to specify colors as a set of numbers. Personally I support him but it is a judgment call --- need more input on that.

comment:8 Changed 12 years ago by Eugene Lazutkin

Cc: elazutkin@… added

comment:9 Changed 12 years ago by Adam Peller

Cc: Eugene Lazutkin Adam Peller added; elazutkin@… removed

Taking this to an extreme, do we need the dojo.Color object abstraction at all? Can we just deal with tuples as an Array for rgb or rgba, with helper methods to convert to/from strings? I'm for moving as much as possible out of base into 'core'. How much actually has to be in base? Probably just enough to support color animation and blends?

Trying to mimic the CSS spec sounds like the kind of thing we did prior to 0.9. OTOH, we probably need this support for gfx, in which case dojo core would seem appropriate.

comment:10 Changed 12 years ago by Eugene Lazutkin

Like I said before on several occasions I don't know what was the original idea behind dojo.Color, so I don't know what level of support is expected.

Typically we use objects to accomplish two things:

  1. Keep all values of the state together.
  2. Enforce rules (e.g., value clamping for color components), so we have a guarantee of well-formed-ness. It allows to skip checks in any functional components (functions, methods, constructors, and so on) speeding the whole thing up.

We can accomplish #1 with array. And we don't use #2 in our current codebase. So in my opinion we can go with array. The only drawback is a necessity to remember that color[2] is a green component. Or is it blue? :-)

Or we can go in the opposite direction and support only strings (named color and hex in the base) and dictionary objects getting rid of _cache completely (data duplication raises questions of consistency). We can still use all array-based methods with minimal overhead:

var _rgb = ["r", "g", "b"];
...
var s = new Array(3);
dojo.forEach(_rgb, function(n, i){ s[i] = this[n]; }, color);
var cssRgbColor = "rgb(" + s.join(", ") + ")";

comment:11 Changed 12 years ago by skinner

Eugene asked for input from interested parties, so I'll chime in as a user of dojo.Color...

On the question of what args the constructor should be able to take, for my needs the one thing I care about is having a constructor that can take whatever string you get from calling toString. Being able to "round-trip" the string values is useful if you want to be able to easily serialize and de-serialize Color objects without treating color as a special case. So, you should be able to do this:

var colorTwo = new dojo.Color(colorOne.toString());
doh.assertTrue(colorOne.toString() == colorTwo.toString());

I don't care whether Color is in base vs. core vs. dojox, or what part is where, but it would be convenient to have a Color object available somewhere. If dojo doesn't provide one then we'll end up having to make our own for OpenRecord.

Using an array would work fine if we always knew that the array held info that represented a color, but in a world without strong typing the Color object is more useful than the array because you know that the Color object represents a color.

comment:12 Changed 12 years ago by Eugene Lazutkin

OK. It gives us the following plan:

  1. We support two types of string representations in the base: hex, and named colors. It includes a conversion to/from.
  2. The constructor takes following arguments: strings (see #1), array (3 or 4 elements), another dojo.Color object.

The rest goes to dojo.colors and augments the existing functionality. It allows us to reduce the base footprint, yet we have everything we possibly need in the core.

Yay? Nay?

comment:13 Changed 12 years ago by Adam Peller

iiuc, you also need to be able to generate "rgba(r,g,b,a)" sort of like our current toCss(), as I think that's the only external representation which can convey alpha information. I just converted animateProperty() over to use this in anticipation of solving this problem.

So, assuming we need a dojo.Color object, I'm ok with the plan. I'm just skeptical 'cause it's the kind of thing we avoid elsewhere in Dojo.

-Adam

comment:14 Changed 12 years ago by Eugene Lazutkin

Some input from Alex:

  • dojo.Color is there to support blinding for animations (fx.js). It means:
    • It has to be able to read and write CSS colors normalizing the difference between browsers.
    • This functionality should be in the base.
    • Note: caching of calors was implemented to speed up fx.js.
  • General observation: dojo.Color should be an object.

comment:15 Changed 12 years ago by Adam Peller

note that in order to express an alpha channel in CSS, rgba() notation must be used (is this supported on all of our browsers?) Using #rrggbb I don't think we can do blends with alpha.

comment:16 Changed 12 years ago by Eugene Lazutkin

(In [9657]) New implementation of dojo.Color as an object, support for common CSS formats, updates to utility functions, optimizations for fx.js, expanded test cases, minor updates to dojox.gfx. Refs #3652.

comment:17 Changed 12 years ago by Eugene Lazutkin

Now dojo.Color supports following string formats: named colors, rgb, rgba, hex. They are supported for input as well as output. toString() produces "rgba(r, g, b, a)" representation, which can be used for serialization/deserialization preserving all four components.

All factory functions (dojo.colorFromString(), dojo.colorFromArray(), and so on) support an optional in-place creation without allocating a new dojo.Color object.

The rest of CSS3 support will be added to dojo.colors.

comment:18 Changed 12 years ago by Eugene Lazutkin

(In [9658]) Added one more test for dojo.Color. Fixed dojo.data test, which used dojo.Color. Refs #3652.

comment:19 Changed 12 years ago by Eugene Lazutkin

Resolution: fixed
Status: reopenedclosed

(In [9659]) Added full CSS3 component recognition (rgb, rgba, hsl, hsla, and rgb/rgba with percentage values). Added tests for enhanced functionality (copied from CSS3 examples). Added "transparent" color as descrobed by CSS3. Fixes #3652.

comment:20 Changed 8 years ago by bill

(In [25750]) Remove duplicate and conflicting definition of "transparent". Add "transparent" to NLS message file. Using [0,0,0,0] for transparent as per the CSS3 Color Module spec, and dojo/tests/colors.js unit test.

Refs #3652, #6399, #13370.

Note: See TracTickets for help on using tickets.