fix: make the style for the state match the value being displayed when hov…

[galo2099]

…ering over a point.

The color should match the color of the point being hovered, not the color of the latest point.

fixes #698

[ildar170975]

Please consider adding some descriptions & screenshots.

[akloeckner]

Nice catch! Would you mind porting the fix also to the adaptive icon and name colours? They were changed together last, I think. (Maybe there's even more places where this should be fixed.)

[galo2099]

I tested and the current PR works for the name and icon too.

[ildar170975]

With this PR still there is this issue:

Here we have 2 graphs, red & green.
Selected a point from a green graph, but it is colored as red.

type: custom:mini-graph-card
entities:
  - entity: sensor.xiaomi_cg_1_temperature
    name: Temp
    show_state: true
    color: red
    state_adaptive_color: true
  - entity: sensor.xiaomi_cg_2_temperature
    name: CO2
    show_state: true
    color: green
    state_adaptive_color: true
    y_axis: secondary
show:
  name: false
  icon: false

Also, I honestly do not understand a purpose of isPrimary var...

[galo2099]

I don't have the data to reproduce the issue above. Are we ok merging this fix and then having someone else fix this new case?

[akloeckner]

I tested and the current PR works for the name and icon too.

Great! 👍

Are we ok merging this fix and then having someone else fix this new case?

Yes, I would consider this a different issue.

I will want to double-check before merging - and have Ildar's OK, too. But I don't see any major risk.

[ildar170975]

Imho no risk at all, this is not a nuclear reactor.
Of course we can merge it for testing.
But as I said, I do not understand a logic behind that “isPrimary” thing, but may be this is probably because my C/C++ genome is incompatible with this JS things.

[akloeckner]

Ah, now I see why you're making the connection to the secondary state display, @ildar170975. Because the isPrimary is used in the condition... I'll have a look, maybe this weekend... Or maybe, @galo2099 can quickly point me in the right direction, why it's used?

[akloeckner]

Ok, got it and tested it:

• The code is used for all state elements, the primary on the left and the secondary on the right of the card.
• isPrimary is true, when rendering the primary state element, i.e. on the left-hand side of the card. It has nothing to do with y_axis: secondary, btw.
• This "primary" stateelement is used to display the first entity's state and it is also used for displaying the tooltip.
• So, it makes sense to only use the tooltip value for colouring the state, if it is the primary element and if the tooltip is actually defined.

Which is what the PR does.

[ildar170975]

@akloeckner , thanks for explaining.

So, what I see during tests:
Assume we have this nice card with 3 entities (same color_thresholds):

type: custom:mini-graph-card
entities:
  - entity: sensor.xiaomi_cg_1_co2
    <<: &ref_settings
      show_state: true
      show_fill: false
      show_points: true
      state_adaptive_color: true
  - entity: sensor.xiaomi_cg_2_co2
    <<: *ref_settings
  - entity: sensor.mijia_300_1_co2
    <<: *ref_settings
color_thresholds:
  - value: 450
    color: "#ff0000"
  - value: 500
    color: orange
  - value: 550
    color: red
  - value: 600
    color: "#00ff00"
  - value: 650
    color: cyan
  - value: 700
    color: blue
  - value: 750
    color: violet
  - value: 800
    color: yellow
color_thresholds_transition: hard
name: CO2
hours_to_show: 24
points_per_hour: 60
line_width: 3
height: 600
show:
  fill: fade
  points: true
  labels: true
  icon_adaptive_color: true
  name_adaptive_color: true

And whatever graph's whatever point I select - I see a proper color for a state, name, icon:

This is very good.

But shouldn't we see same behaviour for this config where a point's color is defined based on color option, not color_thresholds?

Sorry, if the question is silly.
Perhaps this PR is only about "take the CURRENT (not the latest) state, match it with color_thresholds -> then use a corr. color".

[akloeckner]

The question is far from silly. The behaviour you describe should be the correct one, I agree. So, I took some time to sort this sub-routine out.

I pushed the changes to your branch, @galo2099 . I didn't mean to be intrusive. It was just the easiest way to get this into this PR.

@ildar170975, is this what you were looking for?

[ildar170975]

@akloeckner
Unfortunately, I see another glitch with a new version:

Note that for a "higher graph" a smaller state is shown
But this bug was added earlier, tested with a prev version:

So, this seems to be a separate issue, will register it.

I will continue testing new changes now.

[ildar170975]

So, the latest change:

1. If color option is defined per-entity - then a selected point is colored properly (this issue seems to be solved).
2. But colors and STATES are wrongly shown if color_thresholds are defined:
   
   Here the selected point is cyan (same as name & icon), but the state is red (like for the CURRENT state) - and the value is for current state, not for the selected one.

[akloeckner]

Thanks. I didn't realize this. Should be fixed now?

[ildar170975]

With the latest version I see correct colors both for "color_threasholds case" & "per-entity defined colors" case.
But values of states are wrong: "lower" points may have bigger states.
Imho this could be related to points_per_hours / smoothing / etc.
Probably another issue...

Just noticed that the card has wrong names - "temperature" & "co2". In fact these are "temperature" entities, same Y-axis, same unit.

[akloeckner]

Are you sure about the "same axis"? You have a scale on the right side of the graph.

But: If the colors are fixed now, let's merge it. And investigate the lower and higher points separately.

[akloeckner]

Thanks a lot, @galo2099!

[github-actions]

🎉 This PR is included in version 0.13.0-dev.2 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

[ildar170975]

Are you sure about the "same axis"? You have a scale on the right side of the graph.

You are right! My mistake. Will retest.
Update: that was too stupid of me, I compared graphs from different Y-axises & thus having different min bounds.