Colors in light theme too bright

[cribskip]

When using valuecolor=[ON="green"], basic UI shows a darker green than the app. Even when chaning the basic-ui theme, an intensiv light green (seems `#00FF00') is shown.

This is too bright for a white background.

We should change the colors at least in the basic-ui theme to match the basic ui and provide better accessibility through higher contrast.

Here's a quick comparison between basic UI (first) and app (second)

[mueller-ma]

Can you do some research which colors are used in basic ui?

[FlorianSW]

This green is the default green when you parse a color string (green) through Color.parseColor. I'm not quite sure, if we should change this behaviour :/

@mueller-ma This are the color codes, I think: https://docs.openhab.org/configuration/sitemaps.html#label-and-value-colors

However, in my opinion, this documentation is quite inaccurate. The color code is just implemented in core products of openHAB (eclipse smarthome), however, there's no guarantee, that these color codes are used by other clients, too. If the color codes are guaranteed, then the server should send us the color code and not the name of the color (where the assigned color code for this color name van vary from client to client). IMHO: When using the color name, instead of the code, the user should currently accept, that the exact color used can be different on different clients.

TL;DR: I wouldn't change that in the app, I would suggest changing the behaviour of the server (to send the exact color code, which should be used, instead of the name).

What do you think?

[mueller-ma]

When we have names, we can change it easier when having a dark color on a dark theme.

I opened an issue in esh since we should discuss it there: https://github.com/eclipse/smarthome/issues/4916

[maniac103]

@FlorianSW From my understanding, the color names are guaranteed, not the color codes:

Generally, you can expected that valid HTML colors will be accepted (e.g. “green”, “lightgrey”, “#334455”), but note that a UI may only accept internally defined colors, or work with a special theme. The color names above are agreed on between all openHAB UIs and are therefor your safest choice

(Emphasis mine)

So AFAICT we could have a lookup table of known colors to (per-theme) color resources and fall back to Color.parseColor if that lookup fails.

[FlorianSW]

It wouldn't be hard to hardcode the color codes for the color names on the list. However, what I question myself is: SHould we really do such things in our client side code (in the android app, in the iOS app, in the basic and classic UI, in HABPanel and so on) or wouldn't it be smarter to have this conversion from color names to color codes in the openHAB server itself? This would avoid to duplicate the logic from each client to each client (without any guarantee that another third-party client implements it in the same way).

[maniac103]

wouldn't it be smarter to have this conversion from color names to color codes in the openHAB server itself?

IMHO it would be smarter to do this in the client, because the client defines the environment the color needs to blend well into (first and foremost: the background color).

[mueller-ma]

I'm not quite sure, if we should change this behaviour :/

We already do it for orange: https://github.com/openhab/openhab-android/blob/master/mobile/src/main/java/org/openhab/habdroid/model/OpenHABWidget.java#L279 And we can save the color codes here so it is theme dependend.

[FlorianSW]

We do it for orange only, because Color.parseColor does not know orange. But, if you'll agree, that this should be handled in the app, then let's do it!

[mueller-ma]

I'm a bit unsure about the implementation. Is there a better way than to create the colors in colors.xml and a mapping for every color in every theme here?

[maniac103]

You don't need to add them to colors.xml at least; just put the HTML color codes into themes.xml. I wonder whether there's a way to put colors into resource arrays; will check this later.

[mueller-ma]

IMO it's easier to read when you have references to colors.xml

[maniac103]

But even if that's the case (I don't agree personally), I don't think it's worth creating 80 additional resources (5 themes * 16 colors ... or something, didn't check exact numbers) just for that purpose.

[maniac103]

One pointer for resource arrays: http://www.davidebasso.me/android/android-color-list-arrays-in-xml/ ... just needs the attribute-to-resource-ID added on top.

In particular, we'd need one such array per them defined in arrays.xml and then need a theme attribute that references the respective array.

[mueller-ma]

I don't think it's worth creating 80 additional resources (5 themes * 16 colors ... or something, didn't check exact numbers)

We have 5 themes, but only the background color really matters, which is bright, dark and black. Maybe we can even use the same colors for dark and black, so we only need two arrays in the end.

[maniac103]

That's right, variants for light and dark background are probably sufficient. I still find it kinda pointless to create color resources with no intention of reusing them (or, IOW, with the intention of using them only once), but anyway, that's not the most important part here.

[mueller-ma]

I started working on it here.

need a theme attribute that references the respective array.

@maniac103 Can you look at my branch and give me a hint how to do that?

[maniac103]

@mueller-ma Working diff to your current state: https://pastebin.com/8atxWmMZ ... however, I would strongly suggest to

• make OpenHABWidget store the string representation only, not the parsed color and
• move parsing to the view adapter (optimization: resolve the arrays only once, maybe by adding an additional setter to ViewHolder called from onCreateViewHolder)

Benefit of that is that OpenHABWidget and friends stay dumb data holders without any theme dependency; after all they're not reloaded on theme changes.