custom-cards / circle-sensor-card

A custom component for displaying sensor values as cards or elements
Apache License 2.0
174 stars 23 forks source link

Additional font configurations, handling commas, units and using attribute for display #7

Closed iantrich closed 6 years ago

iantrich commented 6 years ago

PR for the following issues: Units of meausrement Handling commas Using attribute instead of state Support font_family Support text_shadow

I am not a web dev (C++) so hopefully I didn't botch these too bad :)

jeradM commented 6 years ago

Thanks for the PR, just a couple comments. I can try to update this when I have time, but feel free to update yourself if you want to. My biggest concern is with having a separate config option for each style. This way we would have to add a new option for every style someone might want to use. I prefer to follow the way picture-elements does it and just allow people to add any style they want and apply them all. Then we can remove the font_color option as well

iantrich commented 6 years ago

I think that is a solid plan. I am on vacation for the next week so won't be able to touch it till at least next weekend.

iantrich commented 6 years ago

I believe I have addressed all your concerns. Let me know if there is anything more I can do! Thanks.

jeradM commented 6 years ago

Thanks for updating this. I am good with most of this other than globally replacing commas as they mean different things depending on where you are in the world. Do you have an example of a platform that returns a numerical value as a string with separators?

iantrich commented 6 years ago

Side note; this card is working great for me:

https://imgur.com/a/8jB0CPx

jeradM commented 6 years ago

Wow that looks awesome. Good work. I'm glad someone is getting some use out of this 🙂

iantrich commented 6 years ago

I believe this change should satisfy the "issue" with numeric strings

jeradM commented 6 years ago

Sensor values are not always integers 🙁

jeradM commented 6 years ago

I still think this needs to be fixed in the platform and we shouldn't try to massage arbitrary strings into numerical values here

iantrich commented 6 years ago

Ugh, you're right.

iantrich commented 6 years ago

I don't think this sensor is alone though. Could check if float or int

function isInt(n){
    return Number(n) === n && n % 1 === 0;
}

function isFloat(n){
    return Number(n) === n && n % 1 !== 0;
}

https://stackoverflow.com/questions/3885817/how-do-i-check-that-a-number-is-float-or-integer

I imagine as the history graph has values for these values, something similar is done there as well.

Regardless, I'm going to back this off again and you can merge it and I'll just use template sensors for now.

jeradM commented 6 years ago

Okay sounds good. I'm sure we could come up with a way to coerce the string values, but I think that ignores the bigger problem of sensors returning strings where there should be numbers

iantrich commented 6 years ago

Absolutely. I'm off onto bigger and better things now...like the reason I need to track things with my Fitbit: https://imgur.com/a/T1bb6RH