bpmn-io / bpmn-js

A BPMN 2.0 rendering toolkit and web modeler.
https://bpmn.io/toolkit/bpmn-js/
Other
8.64k stars 1.33k forks source link

Inconsistent Color Management #1967

Closed lukagerlach closed 1 year ago

lukagerlach commented 1 year ago

Describe the Bug

i tried getting the color for a specific element by using the getFillColor(element), getStrokeColor(element) functions. The output format of this methods seem to be inconsistent.

E.g for a newly created element the fill color method returns "white" while the stroke color method returns "hsl(225, 10%, 15%)". As soon as i assign a custom color to the element using the modeling.setColor(someHexCodeValues) method or the color-picker extension, the color is returned as a hex-colorcode (this is how i would expect it to work).

I think this behavior comes from the implementation of the getFillColor/getStrokeColor methods in the BpmnRenderUtil.js file.

export function getFillColor(element, defaultColor) {
  var di = getDi(element);

  return di.get('color:background-color') || di.get('bioc:fill') || defaultColor || 'white';
}

export function getStrokeColor(element, defaultColor) {
  var di = getDi(element);
  var black = 'hsl(225, 10%, 15%)'
  return di.get('color:border-color') || di.get('bioc:stroke') || defaultColor || black;
}

Expected Behavior

While this inconsistency can be bypassed by handing a default color in hex format to the getFillColor/getStrokeColor, it can still be confusing as the bpmn-js colorpicker extension uses different formats for different color picker entries (The black&white entry (default color) sets the colors to fill: white, stroke: hsl(225, 10%, 15%)while the other colors use hex codes). I think the easiest fix for this would be to adjust the default values 'white' and 'hsl(225, 10%, 15%)' to their corresponding hex-codes. As hex codes are the most commonly used color input format i think this would be a good approach.

If i am overseeing something and this behavior is on purpose i would be happy to hear why. In case you agree with my view i would be happy to contribute!

Environment

nikku commented 1 year ago

As soon as i assign a custom color to the element using the modeling.setColor(someHexCodeValues) method or the color-picker extension, the color is returned as a hex-colorcode (this is how i would expect it to work).

@lukagerlach if you unset the color using the color picker extension, then you'd get the default value to hsl (white) again. Where is your pain coming from? Why do you need the colors to be in the same format?

lukagerlach commented 1 year ago

If i see it correctly, when unsetting the color using the color picker extension i would get the default value for the fill color as 'white' and the stroke color as hsl(black), as it just sets fill and stroke to undefined and therefore falls back to the default values in the getFillColor/getStrokeColor methods i mentioned above.

Here's where i'm coming from:

I am building a custom color picker that shows the user which color is currently selected. If multiple elements are selected, the user should either see the color of the elements as a hex code (given they have the same color) or a little flag that informs the user that the selected elements are of different color. Now i also include a html color input that just passes down it's value to the modeling.setColor() function (naturally as a hex code). Let's say the user chooses the default colors black and white in the html color input. This then leads to the case that the 'custom default' color is passed down by the html color input and saved as a hex code. But when creating a new element, it's colors are saved as 'white' and hsl(black). So checking if two colors are the same would require some extra translation step from each format to a hex code.

While this is a highly specific use case, i do not see a reason to not stick to the standard of hex color codes as it is the most commonly used color input format and, as far as i see, does not seem to have any negative side effects.

nikku commented 1 year ago

I think what you're really looking for is a way to convert color codes, or to retrieve the actual hex color value.

For us our default colors are not public API. Even if we did adjust it (which I agree, we could do), then we'd

nikku commented 1 year ago

Closing this as won't fix as we do not plan to give any guarantees on color codes.