Update Hold/Tap Actions to Objects

[zsarnett]

    tap_action:
      action: call-service
      service: light.turn_off
      service_data:
        entity_id: light.office
    hold_action:
      action: call-service
      service: light.toggle
      service_data:
        entity_id: light.office

[zsarnett]

UI Editor will need updated after this is merged

[iantrich]

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]

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

[thomasloven]

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]

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

[balloob]

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

[iantrich]

Travis threw an error here:
https://github.com/home-assistant/home-assistant-polymer/pull/2182/files#diff-2e86734b42da11910dbfae0763c5d602R77
Need to look inside the ActionConfigs now

Will also need to look at .action in the comparison

[iantrich]

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]

Thats why it has the check for action field

[iantrich]

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

[iantrich]

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

[zsarnett]

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

[balloob]

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

[iantrich]

duplicate

[thomasloven]

This doesn't allow for the "simple" configuration scheme tap_action: toggle, or did I miss something?

[zsarnett]

Correct. I though we decided against that. So that it was better configurable via the editor

[thomasloven]

Rereading the discussion it seems that way... but it was not what I thought I agreed to.

It looks much nicer in a text config, and is similar to the entity list, but as you say it'd have to be converted by the editor on first edit...

[balloob]

I did think we would support the short-form too, when possible. However, given that editors are going to rewrite it and that by not defaulting service-data, action: service becomes semi-useless (but at least not unexpected behavior), we should probably just drop it.

I expect that within reasonable time, 95% of our users won't need to write YAML, especially since custom cards can also add config UI.

[balloob]

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

[balloob]

In a future PR we need to get rid of this catch all element config. We need to define a type per element, then have 1 type that joins them

[iantrich]

fixes #2180