Skip to content

Fix sensor card from not working with detail#7060

Merged
zsarnett merged 2 commits intodevfrom
fix-sensor-card
Sep 22, 2020
Merged

Fix sensor card from not working with detail#7060
zsarnett merged 2 commits intodevfrom
fix-sensor-card

Conversation

@zsarnett
Copy link
Contributor

Breaking change

Proposed change

I do not know how we left this for so long. The detail field was looking at this._config.number instead of this._config.detail. Like its been that way since creation.

image

My change makes this a switch. There are only two values this can be and I have 0 idea why it was made a number field in the first place

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New feature (thank you!)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Example configuration

Additional information

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue:
  • Link to documentation pull request:

Checklist

  • The code change is tested and works locally.
  • There is no commented out code in this PR.
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

.configValue=${"unit"}
@value-changed=${this._valueChanged}
></paper-input>
<ha-formfield label="Show more detail">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lokalise

Comment on lines +197 to +206
const target = ev.target! as EditorTarget;
const value = target.checked ? 2 : 1;

if (this[`_${target.configValue}`] === value) {
return;
}

this._config = {
...this._config,
[target.configValue!]: value,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is really specific for the detail option, so let's not make the rest generic

Comment on lines +162 to +166
<ha-switch
.checked=${this._detail === 2}
.configValue=${"detail"}
@change=${this._change}
></ha-switch>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want this to be a switch or a slider?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Slider? Its only 2 values

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Detail or no detail

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh... why it is a number then? 😝

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no idea. Mentioned that above. 0 idea why this was made a number. I converted it to Typescript. Can't remember if I created this or not.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, should I read the description of the PR? 🙈

@zsarnett zsarnett merged commit 0304c0e into dev Sep 22, 2020
@zsarnett zsarnett deleted the fix-sensor-card branch September 22, 2020 17:06
@bramkragten bramkragten mentioned this pull request Sep 30, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Jul 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants