Remove side effects of render and Add types - Button/Glance (#1919)

* Remove side effects of render and Add types

* Addressing changes

* Updating when to apply theme

* Review Updates

* Fixing last reviews

* Updates from travis

src/panels/lovelace/cards/hui-entity-button-card.ts

@@ -1,5 +1,12 @@
-import { html, LitElement, PropertyDeclarations } from "@polymer/lit-element";
+import {
-import { fireEvent } from "../../../common/dom/fire_event.js";
+  html,
+  LitElement,
+  PropertyDeclarations,
+  PropertyValues,
+} from "@polymer/lit-element";
+import { HassEntity } from "home-assistant-js-websocket";
+import { TemplateResult } from "lit-html";
+import { styleMap } from "lit-html/directives/styleMap.js";
 
 import "../../../components/ha-card.js";
 
@@ -9,11 +16,11 @@ import stateIcon from "../../../common/entity/state_icon.js";
 import computeStateDomain from "../../../common/entity/compute_state_domain.js";
 import computeStateName from "../../../common/entity/compute_state_name.js";
 import applyThemesOnElement from "../../../common/dom/apply_themes_on_element.js";
-import { styleMap } from "lit-html/directives/styleMap.js";
+import { HomeAssistant, LightEntity } from "../../../types.js";
-import { HomeAssistant } from "../../../types.js";
 import { hassLocalizeLitMixin } from "../../../mixins/lit-localize-mixin";
 import { LovelaceCard, LovelaceConfig } from "../types.js";
 import { longPress } from "../common/directives/long-press-directive";
+import { fireEvent } from "../../../common/dom/fire_event.js";
 
 interface Config extends LovelaceConfig {
   entity: string;
@@ -29,38 +36,47 @@ interface Config extends LovelaceConfig {
 class HuiEntityButtonCard extends hassLocalizeLitMixin(LitElement)
   implements LovelaceCard {
   public hass?: HomeAssistant;
-  protected config?: Config;
+  private _config?: Config;
 
   static get properties(): PropertyDeclarations {
     return {
       hass: {},
-      config: {},
+      _config: {},
     };
   }
 
-  public getCardSize() {
+  public getCardSize(): number {
     return 2;
   }
 
-  public setConfig(config: Config) {
+  public setConfig(config: Config): void {
     if (!isValidEntityId(config.entity)) {
       throw new Error("Invalid Entity");
     }
 
-    this.config = { theme: "default", ...config };
+    this._config = { theme: "default", ...config };
-
-    if (this.hass) {
-      this.requestUpdate();
-    }
   }
 
-  protected render() {
+  protected shouldUpdate(changedProps: PropertyValues): boolean {
-    if (!this.config) {
+    if (changedProps.has("_config")) {
+      return true;
+    }
+
+    const oldHass = changedProps.get("hass") as HomeAssistant | undefined;
+    if (oldHass) {
+      return (
+        oldHass.states[this._config!.entity] !==
+        this.hass!.states[this._config!.entity]
+      );
+    }
+    return true;
+  }
+
+  protected render(): TemplateResult {
+    if (!this._config || !this.hass) {
       return html``;
     }
-    const stateObj = this.hass!.states[this.config.entity];
+    const stateObj = this.hass.states[this._config.entity];
-
-    applyThemesOnElement(this, this.hass!.themes, this.config.theme);
 
     return html`
       ${this.renderStyle()}
@@ -72,7 +88,7 @@ class HuiEntityButtonCard extends hassLocalizeLitMixin(LitElement)
         ${
           !stateObj
             ? html`<div class="not-found">Entity not available: ${
-                this.config.entity
+                this._config.entity
               }</div>`
             : html`
               <paper-button>
@@ -80,20 +96,14 @@ class HuiEntityButtonCard extends hassLocalizeLitMixin(LitElement)
                   <ha-icon
                     data-domain="${computeStateDomain(stateObj)}"
                     data-state="${stateObj.state}"
-                    .icon="${
+                    .icon="${this._config.icon || stateIcon(stateObj)}"
-                      this.config.icon ? this.config.icon : stateIcon(stateObj)
-                    }"
                     style="${styleMap({
                       filter: this._computeBrightness(stateObj),
                       color: this._computeColor(stateObj),
                     })}"
                   ></ha-icon>
                   <span>
-                    ${
+                    ${this._config.name || computeStateName(stateObj)}
-                      this.config.name
-                        ? this.config.name
-                        : computeStateName(stateObj)
-                    }
                   </span>
                 </div>
               </paper-button>
@@ -103,7 +113,17 @@ class HuiEntityButtonCard extends hassLocalizeLitMixin(LitElement)
     `;
   }
 
-  private renderStyle() {
+  protected updated(changedProps: PropertyValues): void {
+    if (!this._config || !this.hass) {
+      return;
+    }
+    const oldHass = changedProps.get("hass") as HomeAssistant | undefined;
+    if (!oldHass || oldHass.themes !== this.hass.themes) {
+      applyThemesOnElement(this, this.hass.themes, this._config.theme);
+    }
+  }
+
+  private renderStyle(): TemplateResult {
     return html`
     <style>
       ha-icon {
@@ -143,7 +163,7 @@ class HuiEntityButtonCard extends hassLocalizeLitMixin(LitElement)
     `;
   }
 
-  private _computeBrightness(stateObj) {
+  private _computeBrightness(stateObj: HassEntity | LightEntity): string {
     if (!stateObj.attributes.brightness) {
       return "";
     }
@@ -151,20 +171,19 @@ class HuiEntityButtonCard extends hassLocalizeLitMixin(LitElement)
     return `brightness(${(brightness + 245) / 5}%)`;
   }
 
-  private _computeColor(stateObj) {
+  private _computeColor(stateObj: HassEntity | LightEntity): string {
     if (!stateObj.attributes.hs_color) {
       return "";
     }
-    const hue = stateObj.attributes.hs_color[0];
+    const { hue, sat } = stateObj.attributes.hs_color;
-    const sat = stateObj.attributes.hs_color[1];
     if (sat <= 10) {
       return "";
     }
     return `hsl(${hue}, 100%, ${100 - sat / 2}%)`;
   }
 
-  private handleClick(hold) {
+  private handleClick(hold: boolean): void {
-    const config = this.config;
+    const config = this._config;
     if (!config) {
       return;
     }

src/panels/lovelace/cards/hui-glance-card.ts

@@ -1,4 +1,9 @@
-import { html, LitElement } from "@polymer/lit-element";
+import {
+  html,
+  LitElement,
+  PropertyValues,
+  PropertyDeclarations,
+} from "@polymer/lit-element";
 import { classMap } from "lit-html/directives/classMap.js";
 
 import computeStateDisplay from "../../../common/entity/compute_state_display.js";
@@ -17,6 +22,7 @@ import { hassLocalizeLitMixin } from "../../../mixins/lit-localize-mixin";
 import { HomeAssistant } from "../../../types.js";
 import { LovelaceCard, LovelaceConfig } from "../types.js";
 import { longPress } from "../common/directives/long-press-directive";
+import { TemplateResult } from "lit-html";
 
 interface EntityConfig {
   name: string;
@@ -40,27 +46,25 @@ interface Config extends LovelaceConfig {
 export class HuiGlanceCard extends hassLocalizeLitMixin(LitElement)
   implements LovelaceCard {
   public hass?: HomeAssistant;
-  protected config?: Config;
+  private _config?: Config;
-  protected configEntities?: EntityConfig[];
+  private _configEntities?: EntityConfig[];
 
-  static get properties() {
+  static get properties(): PropertyDeclarations {
     return {
       hass: {},
-      config: {},
+      _config: {},
     };
   }
 
-  public getCardSize() {
+  public getCardSize(): number {
-    const columns =
-      this.config!.columns || Math.min(this.config!.entities.length, 5);
     return (
-      (this.config!.title ? 1 : 0) +
+      (this._config!.title ? 1 : 0) +
-      2 * Math.ceil(this.configEntities!.length / columns)
+      Math.ceil(this._configEntities!.length / 5)
     );
   }
 
-  public setConfig(config: Config) {
+  public setConfig(config: Config): void {
-    this.config = { theme: "default", ...config };
+    this._config = { theme: "default", ...config };
     const entities = processConfigEntities(config.entities);
 
     for (const entity of entities) {
@@ -78,26 +82,43 @@ export class HuiGlanceCard extends hassLocalizeLitMixin(LitElement)
     const columns = config.columns || Math.min(config.entities.length, 5);
     this.style.setProperty("--glance-column-width", `${100 / columns}%`);
 
-    this.configEntities = entities;
+    this._configEntities = entities;
 
     if (this.hass) {
       this.requestUpdate();
     }
   }
 
-  protected render() {
+  protected shouldUpdate(changedProps: PropertyValues): boolean {
-    if (!this.config || !this.hass) {
+    if (changedProps.has("_config")) {
+      return true;
+    }
+
+    const oldHass = changedProps.get("hass") as HomeAssistant | undefined;
+    if (oldHass && this._configEntities) {
+      for (const entity of this._configEntities) {
+        if (
+          oldHass.states[entity.entity] !== this.hass!.states[entity.entity]
+        ) {
+          return true;
+        }
+      }
+      return false;
+    }
+    return true;
+  }
+
+  protected render(): TemplateResult {
+    if (!this._config || !this.hass) {
       return html``;
     }
-    const { title } = this.config;
+    const { title } = this._config;
-
-    applyThemesOnElement(this, this.hass!.themes, this.config.theme);
 
     return html`
       ${this.renderStyle()}
       <ha-card .header="${title}">
         <div class="entities ${classMap({ "no-header": !title })}">
-          ${this.configEntities!.map((entityConf) =>
+          ${this._configEntities!.map((entityConf) =>
             this.renderEntity(entityConf)
           )}
         </div>
@@ -105,7 +126,18 @@ export class HuiGlanceCard extends hassLocalizeLitMixin(LitElement)
     `;
   }
 
-  private renderStyle() {
+  protected updated(changedProperties: PropertyValues): void {
+    if (!this._config || !this.hass) {
+      return;
+    }
+
+    const oldHass = changedProperties.get("hass") as HomeAssistant | undefined;
+    if (!oldHass || oldHass.themes !== this.hass.themes) {
+      applyThemesOnElement(this, this.hass.themes, this._config.theme);
+    }
+  }
+
+  private renderStyle(): TemplateResult {
     return html`
       <style>
         .entities {
@@ -147,7 +179,7 @@ export class HuiGlanceCard extends hassLocalizeLitMixin(LitElement)
     `;
   }
 
-  private renderEntity(entityConf) {
+  private renderEntity(entityConf): TemplateResult {
     const stateObj = this.hass!.states[entityConf.entity];
 
     if (!stateObj) {
@@ -165,7 +197,7 @@ export class HuiGlanceCard extends hassLocalizeLitMixin(LitElement)
         .longPress="${longPress()}"
       >
         ${
-          this.config!.show_name !== false
+          this._config!.show_name !== false
             ? html`<div class="name">${
                 "name" in entityConf
                   ? entityConf.name
@@ -178,7 +210,7 @@ export class HuiGlanceCard extends hassLocalizeLitMixin(LitElement)
           .overrideIcon="${entityConf.icon}"
         ></state-badge>
         ${
-          this.config!.show_state !== false
+          this._config!.show_state !== false
             ? html`<div>${computeStateDisplay(this.localize, stateObj)}</div>`
             : ""
         }
@@ -186,7 +218,7 @@ export class HuiGlanceCard extends hassLocalizeLitMixin(LitElement)
     `;
   }
 
-  private handleClick(ev: MouseEvent, hold) {
+  private handleClick(ev: MouseEvent, hold: boolean): void {
     const config = (ev.currentTarget as any).entityConf as EntityConfig;
     const entityId = config.entity;
     const action = hold ? config.hold_action : config.tap_action || "more-info";