search

AnthonMS / my-cards

Bundle of my custom Lovelace cards for Home Assistant. Includes: my-slider, my-slider-v2, my-button
Other
104 stars 27 forks source link

Bug with initial value when minSet and maxSet are set #2

Closed jncanches closed 3 years ago

jncanches commented 3 years ago

Hi, great card ! There is a bug when using minSet and maxSet. In this case (for example using a light with Warmth function, the slider is not initialized with the right value, event when the value is set between min and max sets. I found a correction updating the template used. Instead of this `

this._setWarmth(entity, e.target, minSet, maxSet)}> 
                </ha-card>`

use `

this._setWarmth(entity, e.target, minSet, maxSet)}> 
                </ha-card>`

Just write .value attribute after min and max ones.

Hope this helps.

AnthonMS commented 3 years ago

So what you're saying, I just need to add this line of code in the html ha-card element? .value="${entity.state === "off" ? 0 : entity.attributes.color_temp}"

Edit: sorry about formatting, I'm on mobile.

AnthonMS commented 3 years ago

And I'm guessing this piece is only for the warmth. Is the bugs also present when using just brightness? Because if it does, then this needs to be added in all the different sliders.

jncanches commented 3 years ago

It is not about adding some code but only reorganize the order of attributes in the input element. I don't know why it is working ( probably a problem with events triggered on the input before all attributes are set. Weird but I can't see a better explanation) Sorry I haven't test this behavior with other slider but we can reasonnably expect the same behavior so I would do the change for all of them.

Hope this helps

AnthonMS commented 3 years ago

Yeah okay. But thank you anyways, I will change the order of the props as soon as I can. Hopefully it resolves it.

If you know how, then you can make a pull request with changes. Just remember to run the build script before pushing, otherwise the changes won't be in the imported resource.

jncanches commented 3 years ago

I can do a merge request. But can you send me the command line to build the project before push ?

[EDIT] found it. I was thinking of a custom script or else. It as just an npm run build.

I cannot push a new branch on your project to create a pull request :(

AnthonMS commented 3 years ago

Yeah just the npm command, sorry for not specifying.

Ohh really? To be honest, I actually don't know much git, and I have never personally made a PR for another project, so I don't know much about it. On the last repo, a guy made a lot of PR's with some great updates to the card, but I have no idea how he made the PR's, I just know how to accept, delay and deny them when they are incoming.

But I can very quickly update it directly on github I suppose with their new VSCode webview.

jncanches commented 3 years ago

Generally, to made a PR, you first create a branch, commit on it and push it to Git. On github you use the "pull request" tab and "create pull request" button with your "master" branch (called "main" on my-cards projects) as base branch and the new branch (the one with the fix) as the compare branch.

I have make the branch, commit it but I cannot push it to your github. I think you have to authorize my login on your project as a developper (don't the exact term on Github, I am more confortable and used to Gitlab, but the basics are the same)

Yeah of course you can update the file using Gitpod

AnthonMS commented 3 years ago

Yeah okay. But I usually don't have to authorize people to make PRs. You only authorize them if they need to be able to directly pull/push changes. Normally I just have to accept the PRs.

But I have made the change really quick, I hope it fixes the issue. Otherwise let me know.