From dfbe4af42857750cc54ba56294f342802657e085 Mon Sep 17 00:00:00 2001 From: Erik Date: Fri, 23 Aug 2019 16:57:54 +0200 Subject: [PATCH] Fix review comments, improve usability. --- src/components/device/ha-device-picker.ts | 39 +++--- .../device/ha-device-trigger-picker.ts | 123 ++++++++---------- src/data/device_automation.ts | 32 +++++ src/panels/config/js/trigger/device.js | 28 +--- 4 files changed, 117 insertions(+), 105 deletions(-) create mode 100644 src/data/device_automation.ts diff --git a/src/components/device/ha-device-picker.ts b/src/components/device/ha-device-picker.ts index e07347e1c1a2..83e94bee199d 100644 --- a/src/components/device/ha-device-picker.ts +++ b/src/components/device/ha-device-picker.ts @@ -1,6 +1,5 @@ -import "@polymer/paper-icon-button/paper-icon-button"; import "@polymer/paper-input/paper-input"; -import "@polymer/paper-item/paper-icon-item"; +import "@polymer/paper-item/paper-item"; import "@polymer/paper-item/paper-item-body"; import "@polymer/paper-dropdown-menu/paper-dropdown-menu-light"; import "@polymer/paper-listbox/paper-listbox"; @@ -17,18 +16,18 @@ import { HomeAssistant } from "../../types"; import { fireEvent } from "../../common/dom/fire_event"; import { DeviceRegistryEntry, - fetchDeviceRegistry, + subscribeDeviceRegistry, } from "../../data/device_registry"; import { compare } from "../../common/string/compare"; -class HaEntityPicker extends LitElement { +class HaDevicePicker extends LitElement { public hass?: HomeAssistant; @property() public label?: string; @property() public value?: string; @property() public devices?: DeviceRegistryEntry[]; + private _unsubDevices?: UnsubscribeFunc; private _sortedDevices = memoizeOne((devices?: DeviceRegistryEntry[]) => { - console.log(devices); if (!devices || devices.length === 1) { return devices || []; } @@ -46,14 +45,14 @@ class HaEntityPicker extends LitElement { attr-for-selected="data-device-id" @iron-select=${this._deviceChanged} > - + No device - + ${this._sortedDevices(this.devices).map( (device) => html` - + ${device.name_by_user || device.name} - + ` )} @@ -65,13 +64,23 @@ class HaEntityPicker extends LitElement { return this.value || ""; } + public disconnectedCallback() { + super.disconnectedCallback(); + if (this._unsubDevices) { + this._unsubDevices(); + this._unsubDevices = undefined; + } + } + protected firstUpdated(changedProps) { super.firstUpdated(changedProps); if (this.devices === undefined) { - // TODO: Subscribe to device registry updates? - fetchDeviceRegistry(this.hass.connection!).then((devices) => { - this.devices = devices; - }); + this._unsubDevices = subscribeDeviceRegistry( + this.hass.connection!, + (devices) => { + this.devices = devices; + } + ); } } @@ -98,11 +107,11 @@ class HaEntityPicker extends LitElement { paper-listbox { min-width: 200px; } - paper-icon-item { + paper-item { cursor: pointer; } `; } } -customElements.define("ha-device-picker", HaEntityPicker); +customElements.define("ha-device-picker", HaDevicePicker); diff --git a/src/components/device/ha-device-trigger-picker.ts b/src/components/device/ha-device-trigger-picker.ts index 175699b9ea9b..aee8af34dfec 100644 --- a/src/components/device/ha-device-trigger-picker.ts +++ b/src/components/device/ha-device-trigger-picker.ts @@ -1,6 +1,5 @@ -import "@polymer/paper-icon-button/paper-icon-button"; import "@polymer/paper-input/paper-input"; -import "@polymer/paper-item/paper-icon-item"; +import "@polymer/paper-item/paper-item"; import "@polymer/paper-item/paper-item-body"; import "@polymer/paper-dropdown-menu/paper-dropdown-menu-light"; import "@polymer/paper-listbox/paper-listbox"; @@ -16,76 +15,48 @@ import { import { HomeAssistant } from "../../types"; import { fireEvent } from "../../common/dom/fire_event"; import { compare } from "../../common/string/compare"; -import { LocalizeFunc } from "../../common/translations/localize"; import computeStateName from "../../common/entity/compute_state_name"; +import { + DeviceTrigger, + fetchDeviceTriggers, + triggersEqual, +} from "../../data/device_automation"; -export interface DeviceTrigger { - platform: string; - domain: string; - device_id: string; - entity_id?: string; - type: string; -} - -const fetchDeviceTriggers = (hass, deviceId) => - hass.callWS({ - type: "device_automation/list_triggers", - device_id: deviceId, - }); - -function isObject(v) { - return "[object Object]" === Object.prototype.toString.call(v); -} - -JSON.sort = function(o) { - if (Array.isArray(o)) { - return o.sort().map(JSON.sort); - } else if (isObject(o)) { - return Object.keys(o) - .sort() - .reduce(function(a, k) { - a[k] = JSON.sort(o[k]); - - return a; - }, {}); - } - - return o; -}; - -class HaEntityPicker extends LitElement { +class HaDeviceTriggerPicker extends LitElement { public hass?: HomeAssistant; - public localize?: LocalizeFunc; @property() public label?: string; - @property() public value?: string; @property() public deviceId?: string; - @property() public triggers?: DeviceTrigger[]; + @property() public triggers?: DeviceTrigger[] = []; + @property() public trigger?: DeviceTrigger; + @property() public presetTrigger?: DeviceTrigger; + private noTrigger = { device_id: this.deviceId, platform: "device" }; private _sortedTriggers = memoizeOne((triggers?: DeviceTrigger[]) => { return triggers || []; }); protected render(): TemplateResult | void { + const noTriggers = this._sortedTriggers(this.triggers).length == 0; return html` - + - - No trigger - + ${noTriggers + ? html` + + No triggers + + ` + : ""} ${this._sortedTriggers(this.triggers).map( (trigger) => html` - - ${this.localize( + + ${this.hass.localize( `ui.panel.config.automation.editor.triggers.type.device.trigger_type.${ trigger.type }`, @@ -104,31 +75,49 @@ class HaEntityPicker extends LitElement { `; } - private get _value() { - return this.value || ""; + private get _trigger() { + return this.trigger; } - protected updated(oldProps) { - console.log("_triggerChanged"); - if (oldProps.has("deviceId") && oldProps.get("deviceId") != this.deviceId) { + protected updated(changedProps) { + super.updated(changedProps); + + // Reset trigger if device-id has changed + if (changedProps.has("deviceId")) { + this.noTrigger = { device_id: this.deviceId, platform: "device" }; if (this.deviceId) { fetchDeviceTriggers(this.hass!, this.deviceId).then((trigger) => { this.triggers = trigger.triggers; + if (this.triggers.length > 0) { + // Set first trigger as default + this.trigger = this.triggers[0]; + } else if (triggersEqual(this.noTrigger, this.presetTrigger)) { + this.trigger = this.noTrigger; + } + for (var trigger of this.triggers) { + // Try to find a trigger matching existing trigger loaded from stored automation + if (triggersEqual(trigger, this.presetTrigger)) + this.trigger = trigger; + } }); } else { + // No device, clear the list of triggers this.triggers = []; + this.trigger = this.noTrigger; } } + + // The triggers property has changed, force the listbox to update + if (changedProps.has("triggers")) { + this.shadowRoot.getElementById("listbox")._selectSelected(); + } } private _triggerChanged(ev) { - console.log("_triggerChanged"); - const tmp = JSON.parse(ev.detail.item.dataset.trigger); - const newValue = ev.detail.item.dataset.trigger; - if (newValue !== this._value) { - this.value = newValue; + const newValue = ev.detail.item.trigger; + if (newValue !== this._trigger) { + this.trigger = newValue; setTimeout(() => { - fireEvent(this, "value-changed", { value: newValue }); fireEvent(this, "change"); }, 0); } @@ -145,11 +134,11 @@ class HaEntityPicker extends LitElement { paper-listbox { min-width: 200px; } - paper-icon-item { + paper-item { cursor: pointer; } `; } } -customElements.define("ha-device-trigger-picker", HaEntityPicker); +customElements.define("ha-device-trigger-picker", HaDeviceTriggerPicker); diff --git a/src/data/device_automation.ts b/src/data/device_automation.ts new file mode 100644 index 000000000000..4a12afc305de --- /dev/null +++ b/src/data/device_automation.ts @@ -0,0 +1,32 @@ +import { HomeAssistant } from "../../types"; +import { fireEvent } from "../../common/dom/fire_event"; +import { compare } from "../../common/string/compare"; +import { LocalizeFunc } from "../../common/translations/localize"; +import computeStateName from "../../common/entity/compute_state_name"; + +export interface DeviceTrigger { + platform: string; + domain: string; + device_id: string; + entity_id?: string; + type: string; +} + +export const fetchDeviceTriggers = (hass: HomeAssistant, deviceId: string) => + hass.callWS({ + type: "device_automation/list_triggers", + device_id: deviceId, + }); + +export const triggersEqual = (a: any, b: any) => { + if (typeof a != typeof b) return false; + + for (var property in a) { + if (!Object.is(a[property], b[property])) return false; + } + for (var property in b) { + if (!Object.is(a[property], b[property])) return false; + } + + return true; +}; diff --git a/src/panels/config/js/trigger/device.js b/src/panels/config/js/trigger/device.js index 30e2766df10e..e1921bc21dc8 100644 --- a/src/panels/config/js/trigger/device.js +++ b/src/panels/config/js/trigger/device.js @@ -5,25 +5,7 @@ import "../../../../components/device/ha-device-trigger-picker"; import { onChangeEvent } from "../../../../common/preact/event"; -function isObject(v) { - return "[object Object]" === Object.prototype.toString.call(v); -} - -JSON.sort = function(o) { - if (Array.isArray(o)) { - return o.sort().map(JSON.sort); - } else if (isObject(o)) { - return Object.keys(o) - .sort() - .reduce(function(a, k) { - a[k] = JSON.sort(o[k]); - - return a; - }, {}); - } - - return o; -}; +import { triggersEqual } from "../../../../data/device_automation"; export default class DeviceTrigger extends Component { constructor() { @@ -36,6 +18,7 @@ export default class DeviceTrigger extends Component { devicePicked(ev) { this.deviceId = ev.target.value; let deviceTrigger = {}; + // Reset the trigger if device is changed deviceTrigger.platform = "device"; deviceTrigger.device_id = this.deviceId; @@ -43,14 +26,14 @@ export default class DeviceTrigger extends Component { } deviceTriggerPicked(ev) { - let deviceTrigger = JSON.parse(ev.target.value); + let deviceTrigger = ev.target.trigger; this.props.onChange(this.props.index, (this.props.trigger = deviceTrigger)); } /* eslint-disable camelcase */ render({ trigger, hass, localize }) { const { device_id } = trigger; - const jsontrigger = JSON.stringify(JSON.sort(trigger)); + const jsontrigger = trigger; return (
@@ -61,11 +44,10 @@ export default class DeviceTrigger extends Component { label="Device" />