Skip to content

Update Hold/Tap Actions to Objects #2182

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.

Sign up for GitHub

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

Merged
balloob merged 11 commits into from
Dec 5, 2018
Merged

Update Hold/Tap Actions to Objects #2182

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
d7ce047
Update Action to Object for entity-button
zsarnett Dec 5, 2018
9151f9b
Update Actions to Object for Glance Card
zsarnett Dec 5, 2018
bb2672b
Update to use HandleClick function
zsarnett Dec 5, 2018
ab19765
Add navigation
zsarnett Dec 5, 2018
5382335
Updating to use navigation path from Action Config
zsarnett Dec 5, 2018
efbda47
Type handle events in glance
zsarnett Dec 5, 2018
a4e3957
Update Service checks
zsarnett Dec 5, 2018
f62ec0b
Update Picture elements
zsarnett Dec 5, 2018
e4d525c
oops
zsarnett Dec 5, 2018
d9b0179
Adding nav back until we convert picture
zsarnett Dec 5, 2018
3a9c5b5
Extend typing
balloob Dec 5, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/data/lovelace.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,12 @@ export interface LovelaceCardConfig {
[key: string]: any;
}

export interface ActionConfig {

@balloob balloob Dec 5, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is the wrong approach. We should have a separate interface for each action and then have ActionConfig be a union interface: ActionConfig = CallServiceActionConfig & MoreInfoActionConfig

action?: "toggle" | "call-service" | "more-info" | "none";
Comment thread
zsarnett marked this conversation as resolved.
Outdated
service?: string;
service_data?: object;
Comment thread
zsarnett marked this conversation as resolved.
Outdated
}

export const fetchConfig = (hass: HomeAssistant): Promise<LovelaceConfig> =>
hass.callWS({
type: "lovelace/config",
Expand Down
49 changes: 12 additions & 37 deletions src/panels/lovelace/cards/hui-entity-button-card.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,23 +15,20 @@ import stateIcon from "../../../common/entity/state_icon";
import computeStateDomain from "../../../common/entity/compute_state_domain";
import computeStateName from "../../../common/entity/compute_state_name";
import applyThemesOnElement from "../../../common/dom/apply_themes_on_element";
import { toggleEntity } from "../common/entity/toggle-entity";
import { HomeAssistant, LightEntity } from "../../../types";
import { hassLocalizeLitMixin } from "../../../mixins/lit-localize-mixin";
import { LovelaceCard } from "../types";
import { LovelaceCardConfig } from "../../../data/lovelace";
import { LovelaceCardConfig, ActionConfig } from "../../../data/lovelace";
import { longPress } from "../common/directives/long-press-directive";
import { fireEvent } from "../../../common/dom/fire_event";
import { handleClick } from "../common/handle-click";

interface Config extends LovelaceCardConfig {
entity: string;
name?: string;
icon?: string;
theme?: string;
tap_action?: "toggle" | "call-service" | "more-info";
hold_action?: "toggle" | "call-service" | "more-info";
service?: string;
service_data?: object;
tap_action?: ActionConfig;
hold_action?: ActionConfig;
}

class HuiEntityButtonCard extends hassLocalizeLitMixin(LitElement)
Expand Down Expand Up @@ -82,8 +79,8 @@ class HuiEntityButtonCard extends hassLocalizeLitMixin(LitElement)
return html`
${this.renderStyle()}
<ha-card
@ha-click="${() => this.handleClick(false)}"
@ha-hold="${() => this.handleClick(true)}"
@ha-click="${this._handleTap}"
@ha-hold="${this._handleHold}"
.longPress="${longPress()}"
>
${
Expand Down Expand Up @@ -187,34 +184,12 @@ class HuiEntityButtonCard extends hassLocalizeLitMixin(LitElement)
return `hsl(${hue}, 100%, ${100 - sat / 2}%)`;
}

private handleClick(hold: boolean): void {
const config = this._config;
if (!config) {
return;
}
const stateObj = this.hass!.states[config.entity];
if (!stateObj) {
return;
}
const entityId = stateObj.entity_id;
const action = hold ? config.hold_action : config.tap_action || "more-info";
switch (action) {
case "toggle":
toggleEntity(this.hass!, entityId);
break;
case "call-service":
if (!config.service) {
return;
}
const [domain, service] = config.service.split(".", 2);
const serviceData = { entity_id: entityId, ...config.service_data };
this.hass!.callService(domain, service, serviceData);
break;
case "more-info":
fireEvent(this, "hass-more-info", { entityId });
break;
default:
}
private _handleTap() {
handleClick(this, this.hass!, this._config!, false);
}

private _handleHold() {
handleClick(this, this.hass!, this._config!, true);
}
}

Expand Down
39 changes: 13 additions & 26 deletions src/panels/lovelace/cards/hui-glance-card.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,12 @@ import {
import { TemplateResult } from "lit-html";
import { classMap } from "lit-html/directives/classMap";

import { fireEvent } from "../../../common/dom/fire_event";
import { hassLocalizeLitMixin } from "../../../mixins/lit-localize-mixin";
import { HomeAssistant } from "../../../types";
import { LovelaceCard, LovelaceCardEditor } from "../types";
import { LovelaceCardConfig } from "../../../data/lovelace";
import { LovelaceCardConfig, ActionConfig } from "../../../data/lovelace";
import { longPress } from "../common/directives/long-press-directive";
import { EntityConfig } from "../entity-rows/types";
import { toggleEntity } from "../common/entity/toggle-entity";
import { processConfigEntities } from "../common/process-config-entities";

import computeStateDisplay from "../../../common/entity/compute_state_display";
Expand All @@ -24,12 +22,11 @@ import applyThemesOnElement from "../../../common/dom/apply_themes_on_element";
import "../../../components/entity/state-badge";
import "../../../components/ha-card";
import "../../../components/ha-icon";
import { handleClick } from "../common/handle-click";

export interface ConfigEntity extends EntityConfig {
tap_action?: "toggle" | "call-service" | "more-info";
hold_action?: "toggle" | "call-service" | "more-info";
service?: string;
service_data?: object;
tap_action?: ActionConfig;
hold_action?: ActionConfig;
}

export interface Config extends LovelaceCardConfig {
Expand Down Expand Up @@ -203,8 +200,8 @@ export class HuiGlanceCard extends hassLocalizeLitMixin(LitElement)
<div
class="entity"
.entityConf="${entityConf}"
@ha-click="${(ev) => this.handleClick(ev, false)}"
@ha-hold="${(ev) => this.handleClick(ev, true)}"
@ha-click="${this._handleTap}"
@ha-hold="${this._handleHold}"
.longPress="${longPress()}"
>
${
Expand Down Expand Up @@ -243,24 +240,14 @@ export class HuiGlanceCard extends hassLocalizeLitMixin(LitElement)
`;
}

private handleClick(ev: MouseEvent, hold: boolean): void {
private _handleTap(ev) {
Comment thread
zsarnett marked this conversation as resolved.
Outdated
const config = (ev.currentTarget as any).entityConf as ConfigEntity;
const entityId = config.entity;
const action = hold ? config.hold_action : config.tap_action || "more-info";
switch (action) {
case "toggle":
toggleEntity(this.hass!, entityId);
break;
case "call-service":
const [domain, service] = config.service!.split(".", 2);
const serviceData = { entity_id: entityId, ...config.service_data };
this.hass!.callService(domain, service, serviceData);
break;
case "more-info":
fireEvent(this, "hass-more-info", { entityId });
break;
default:
}
handleClick(this, this.hass!, config, false);
}

private _handleHold(ev) {
Comment thread
zsarnett marked this conversation as resolved.
Outdated
const config = (ev.currentTarget as any).entityConf as ConfigEntity;
handleClick(this, this.hass!, config, true);
}
}

Expand Down
12 changes: 5 additions & 7 deletions src/panels/lovelace/cards/hui-picture-entity-card.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import computeStateName from "../../../common/entity/compute_state_name";
import { longPress } from "../common/directives/long-press-directive";
import { hassLocalizeLitMixin } from "../../../mixins/lit-localize-mixin";
import { HomeAssistant } from "../../../types";
import { LovelaceCardConfig } from "../../../data/lovelace";
import { LovelaceCardConfig, ActionConfig } from "../../../data/lovelace";
import { LovelaceCard } from "../types";
import { handleClick } from "../common/handle-click";
import { UNAVAILABLE } from "../../../data/entity";
Expand All @@ -25,10 +25,8 @@ interface Config extends LovelaceCardConfig {
camera_image?: string;
state_image?: {};
aspect_ratio?: string;
tap_action?: "toggle" | "call-service" | "more-info" | "navigate";
hold_action?: "toggle" | "call-service" | "more-info" | "navigate";
service?: string;
service_data?: object;
tap_action?: ActionConfig;
hold_action?: ActionConfig;
show_name?: boolean;
show_state?: boolean;
}
Expand Down Expand Up @@ -109,7 +107,7 @@ class HuiPictureEntityCard extends hassLocalizeLitMixin(LitElement)
}"
.entity="${this._config.entity}"
.aspectRatio="${this._config.aspect_ratio}"
@ha-click="${this._handleClick}"
@ha-click="${this._handleTap}"
@ha-hold="${this._handleHold}"
.longPress="${longPress()}"
class="${
Expand Down Expand Up @@ -157,7 +155,7 @@ class HuiPictureEntityCard extends hassLocalizeLitMixin(LitElement)
`;
}

private _handleClick() {
private _handleTap() {
handleClick(this, this.hass!, this._config!, false);
}

Expand Down
36 changes: 24 additions & 12 deletions src/panels/lovelace/common/handle-click.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,31 @@ import { LovelaceElementConfig } from "../elements/types";
import { fireEvent } from "../../../common/dom/fire_event";
import { navigate } from "../../../common/navigate";
import { toggleEntity } from "../../../../src/panels/lovelace/common/entity/toggle-entity";
import { LovelaceCardConfig } from "../../../data/lovelace";
import { LovelaceCardConfig, ActionConfig } from "../../../data/lovelace";
import { EntityConfig } from "../entity-rows/types";

export interface ConfigEntity extends EntityConfig {
tap_action?: ActionConfig;
hold_action?: ActionConfig;
navigation_path?: string;
}

export const handleClick = (
node: HTMLElement,
hass: HomeAssistant,
config: LovelaceElementConfig | LovelaceCardConfig,
config: LovelaceElementConfig | LovelaceCardConfig | ConfigEntity,
hold: boolean
): void => {
let action = config.tap_action || "more-info";

let actionConfig;
if (hold && config.hold_action) {
action = config.hold_action;
actionConfig = config.hold_action;
} else if (!hold && config.tap_action) {
actionConfig = config.tap_action;
}

const action =
actionConfig && actionConfig.action ? actionConfig.action : "more-info";

@iantrich iantrich Dec 5, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not all the types handled by config have an action field. I don't think any elements are likely working right now with this

@zsarnett zsarnett Dec 5, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thats why it has the check for action field

@iantrich iantrich Dec 5, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

But you can still define an action type for the elements beyond "more-info". That is lost here.

@iantrich iantrich Dec 5, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Perhaps we should just align the elements now as well? icon, image, state-icon and state-label

@zsarnett zsarnett Dec 5, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oh you are saying the elements of picture elements
Yea I havent gotten there yet. Next commit. Looking at that now

@balloob balloob Dec 5, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It's fine to have multiple handleClick methods to handle different supported actions for different places in the code


if (action === "none") {
return;
}
Expand All @@ -34,14 +45,15 @@ export const handleClick = (
toggleEntity(hass, config.entity!);
break;
case "call-service": {
if (config.service) {
const [domain, service] = config.service.split(".", 2);
const serviceData = {
entity_id: config.entity,
...config.service_data,
};
hass.callService(domain, service, serviceData);
if (!actionConfig.service) {
return;
}
const [domain, service] = actionConfig.service.split(".", 2);
const serviceData = {
entity_id: config.entity,

@iantrich iantrich Dec 5, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This has caused issues for some services that don't want the entity_id. I can't recall them off the top of my head, but should require that the service_data be explicitly stated instead?

@zsarnett zsarnett Dec 5, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If it doesnt want it, the service should ignore it... I think that should be a service fix

@thomasloven thomasloven Dec 5, 2018 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Most services don't work that way. Probably by design.

Invalid service data for light.turn_on: extra keys not allowed @ data['unknown_key']. Got 'someting'

Invalid service data for system_log.write: extra keys not allowed @ data['entity_id']. Got 'light.my_lamp'

@balloob balloob Dec 5, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yeah we should stop trying to be smart and adding the entity automatically.

@balloob balloob Dec 5, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We can still automatically populate service data for toggle, but not for normal services.

...actionConfig.service_data,
};
hass.callService(domain, service, serviceData);
}
}
};