Skip to content
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

Show battery charging state in the config panels #6356

Merged
merged 6 commits into from
Jul 12, 2020

Conversation

bdraco
Copy link
Member

@bdraco bdraco commented Jul 10, 2020

Proposed change

The battery charging state is now checked when generating the battery icon.

Screen_Shot_2020-07-09_at_3_35_46_PM

Screen_Shot_2020-07-09_at_3_36_11_PM

Screen_Shot_2020-07-09_at_3_36_20_PM

Screen_Shot_2020-07-09_at_3_36_25_PM

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:

import { batteryIcon } from "../../common/entity/battery_icon";
import "../ha-icon";

class HaBatteryIcon extends PolymerElement {
Copy link
Member

Choose a reason for hiding this comment

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

I know you just copied this, but we should not add new PolymerElements, we are trying to get rid of them 😄

Copy link
Member Author

Choose a reason for hiding this comment

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

What should I do instead here?

Copy link
Member

Choose a reason for hiding this comment

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

LitElement and typescript

Copy link
Member

Choose a reason for hiding this comment

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

@customElement("ha-battery-icon")
class HaBatteryIcon extends LitElement {
    
  @property() public batteryStateObj;
  @property() public batteryChargingStateObj;

  protected render() {
    return html`
      <ha-icon
        .icon=${batteryIcon(this.batteryStateObj, this.batteryChargingStateObj)}
      ></ha-icon>
    `;
  }
}

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you. So - much - cleaner.

Copy link
Member

Choose a reason for hiding this comment

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

Instead of using an icon, we should probably import the icon paths directly and use ha-svg-icon ?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, but that would only work for this element so would need separate logic.

Copy link
Member Author

Choose a reason for hiding this comment

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

import { mdiBatteryCharging40, mdiBatteryCharging50, .... } from '@mdi/js';

Seems like I'd need to import about 25 items and make an object to look them up in order to implement ha-svg-icon, unless there is a better way??

Copy link
Member

Choose a reason for hiding this comment

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

No, that would be the way.

The benefit of that is that we don't have to load an entire (or multiple) icon chunk of MDI icons and requests to the database where they are cached.

@bramkragten bramkragten merged commit 3bc54aa into home-assistant:dev Jul 12, 2020
@bramkragten bramkragten mentioned this pull request Jul 14, 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.

None yet

4 participants