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

More entity improvement #675

Merged
merged 3 commits into from
Nov 25, 2017
Merged

Conversation

balloob
Copy link
Member

@balloob balloob commented Nov 24, 2017

This PR depends on #674 so this PR merges into that branch.

  • Add entity picker to the script and automation editors
  • Allow filtering by entity and domain inside entity picker
  • Add new service picker for scripts editor
  • Add service picker and entity picker to dev services tool

In the automation editor: when configuring a zone we only autocomplete entities that have a location.

screen shot 2017-11-23 at 23 39 41

@andrey-git
Copy link
Contributor

Do we really want to go in the preact direction?

I didn't actually try it, so I'm not aware of the advantages, but the disadvantages I see are:

  • Polymer is easier to understand: "Here is a file which says how the element looks and what it does".
  • Requires rollup. This became less of an issue with ES6 rollup, but still the code you debug is not the code you wrote.

@balloob
Copy link
Member Author

balloob commented Nov 24, 2017

For this PR: All the logic here is added in Polymer components. The preact components are just pointing at the polymer components.

the code you debug is not the code you wrote.

It is very very similar. The only thing that is transpiled is that <paper-menu-button bla={5} /> is rewritten to h('paper-menu-button', {bla: 5}) (try it out here). Rollup (which will power Polymer 3) is just bundling all the files.

The reason I went for Preact for the automation/script editor is because of speed and as an interoperability experiment. Dynamically figuring out which component to render is just a lot more straightforward in Preact. I don't have to do something like this for each type:

<template is='dom-if' if='[[isTriggerState(row)]]'>
  <trigger-state-editor
    hass='[[hass]]'
    trigger='{{trigger}}'
  ></trigger-state-editor>
</template>

but instead can do this to match all types at once (since it's JS):

const Comp = TYPES[row.type];
return <Comp hass={hass} trigger={trigger} />

We could probably rewrite this using window.hassUtil.dynamicContentUpdater and Polymer and make it similar smooth.

I'm not against removing Preact. I just don't feel like doing it 😉

@home-assistant home-assistant deleted a comment from houndci-bot Nov 24, 2017
@home-assistant home-assistant deleted a comment from houndci-bot Nov 24, 2017
@home-assistant home-assistant deleted a comment from houndci-bot Nov 24, 2017
@home-assistant home-assistant deleted a comment from houndci-bot Nov 24, 2017
@home-assistant home-assistant deleted a comment from houndci-bot Nov 24, 2017
@home-assistant home-assistant deleted a comment from houndci-bot Nov 24, 2017
@home-assistant home-assistant deleted a comment from houndci-bot Nov 24, 2017
@home-assistant home-assistant deleted a comment from houndci-bot Nov 24, 2017
@home-assistant home-assistant deleted a comment from houndci-bot Nov 24, 2017
@home-assistant home-assistant deleted a comment from houndci-bot Nov 24, 2017
@home-assistant home-assistant deleted a comment from houndci-bot Nov 24, 2017
@home-assistant home-assistant deleted a comment from houndci-bot Nov 24, 2017
@home-assistant home-assistant deleted a comment from houndci-bot Nov 24, 2017
@home-assistant home-assistant deleted a comment from houndci-bot Nov 24, 2017
@home-assistant home-assistant deleted a comment from houndci-bot Nov 24, 2017
@home-assistant home-assistant deleted a comment from houndci-bot Nov 24, 2017
@home-assistant home-assistant deleted a comment from houndci-bot Nov 24, 2017
@home-assistant home-assistant deleted a comment from houndci-bot Nov 24, 2017
@home-assistant home-assistant deleted a comment from houndci-bot Nov 24, 2017
@home-assistant home-assistant deleted a comment from houndci-bot Nov 24, 2017
@home-assistant home-assistant deleted a comment from houndci-bot Nov 24, 2017
@home-assistant home-assistant deleted a comment from houndci-bot Nov 24, 2017
@home-assistant home-assistant deleted a comment from houndci-bot Nov 24, 2017
@home-assistant home-assistant deleted a comment from houndci-bot Nov 24, 2017
@andrey-git
Copy link
Contributor

The polymer part looks fine. I don't quite understand the preact part :)

@balloob
Copy link
Member Author

balloob commented Nov 25, 2017

The preact part is rendering the new component and dealing with the data that comes out of it. If you want we can jump on a call and I can give you a preact 101.

@balloob
Copy link
Member Author

balloob commented Nov 25, 2017

Is this ok to merge?

@andrey-git
Copy link
Contributor

ok to merge.

I understand the theory, it is just much harder (relative to Polymer) to follow that everything is correct.

@balloob balloob merged commit c155da0 into enttiy-dropdown-improvement Nov 25, 2017
balloob added a commit that referenced this pull request Nov 25, 2017
* Update script and automation editor to use entity picker

* Add entity and service picker to service dev panel

* Lint
@balloob balloob deleted the more-entity-improvement branch November 25, 2017 23:44
balloob added a commit that referenced this pull request Nov 26, 2017
* Ignore hass changes while dropdown is open

* Upgrade vaadin-combo-box

* Fix styling on dev-service panel

* Fix styling for ha-entity-dropdown

* Fix height vaadin-combo-box dropdown

* Rename ha-entity-dropdown to ha-entity-picker

* More entity improvement (#675)

* Update script and automation editor to use entity picker

* Add entity and service picker to service dev panel

* Lint
@github-actions github-actions bot locked and limited conversation to collaborators Jul 8, 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

3 participants