-
Notifications
You must be signed in to change notification settings - Fork 237
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
Legend indicator: color may not follow color_thresholds #1116
Comments
TL;DR We should try to substitute Line 659 in 49ca1cd
I did some digging and I think: We're sorting the thresholds in descending order here: mini-graph-card/src/buildConfig.js Line 91 in 49ca1cd
We then look for the first threshold Lines 655 to 657 in 49ca1cd
If all thresholds are below the state, there is no Lines 662 to 664 in 49ca1cd
However, the "normal" branch seems to be broken. This is the case, where we actually have Lines 659 to 660 in 49ca1cd
I also guess that this is no issue for the graph itself, because it seems to be coloured by an SVG gradient (which I am not familiar with). Line 446 in 49ca1cd
|
Nope, that didn't help... |
Ok... Next guess: the Lines 16 to 30 in 49ca1cd
Try converting your colours to that notation. This works well for me. Example: color_thresholds:
- value: 450
color: '#ff0000'
- value: 550
color: '#00ff00'
- value: 650
color: '#0000ff' I am not sure how to fix this. We'd need to internally convert named colours to hexadecimal notation... Or go the lazy way used for the state: do not interpolate and use Line 297 in 49ca1cd
Line 608 in 49ca1cd
|
I wonder is there any standard function to do it? |
I also wondered... Since 2023 we can even directly use CSS's Then, I found |
d3 seems to work well. See #1118 . I'll migrate |
@akloeckner |
🎉 This issue has been resolved in version 0.12.2-dev.2 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Fixed in 0.12.2-dev.2 |
Hi @akloeckner, first of all thanks for your continued help in maintaining mini-graph-card! I sadly enough only got freed up enough now to notice #1118 getting merged. The original implementation is actually very efficient, despite being limited to hex. We can certainly extend the feature set; and use a (minimal) lib to parse named colors to hex on the configuration-side (as that's definitely a value-add) if required.
is sadly enough the problem here :) /cc @ildar170975 p.s.: thanks that you didn't :)
|
Have a look at https://stackoverflow.com/a/1573141/620141 The canvas-method might be more future-proof as it's per spec, but is quite slow (https://jsperf.app/sobipi). I'd personally consider the risk of many new css colors being added on the low side & just hardcode it. |
😄 I also found the canvas method and decided to remember this as a fun creative way of doing things. But never to do this myself. 😝 Sorry for calling the efficient implementation "weird"... Are you experiencing problems on slow devices? We can always re-introduce the original interpolation and just convert everything to hex internally. Or we first wait if someone complains and then possibly re-visit this. |
No worries, I fully understand the perception if you're not used to binary. The slow device support is mainly an extra advantage (I have noticed issues on my slowest device with other graphs, so it's definitely something to stay aware of) The main reason for reopening is to keep the core code of the card (graphs) dependency-free as intended by kalkih. I'm sadly still quite short on extra time to code but if you're willing & able to contribute I'd be glad to continue assisting architecturally meanwhile (just @ me)! In this case, we preferably revert the introduction of d3.interpolate in graph.js & use one of the SO methods when importing the config. Thanks again! |
Initial report: #1115 (comment)
With #1115 merged:
a legend indicator's color may follow
color_thresholds
:i.e. here a color = PINK for both entities.
With changed
color_thresholds
:there is different picture - an indicator is BLACK for the 2nd entity:
Same - when graphs are built for attributes.
The text was updated successfully, but these errors were encountered: