Convert hui-call-service-row to TypeScript/LitElement #1894

iantrich · 2018-10-28T01:45:18Z

No description provided.

zsarnett · 2018-10-28T01:54:58Z

src/panels/lovelace/special-rows/hui-call-service-row.ts

+      !config.name ||
+      !config.action_name ||
+      !config.service ||
+      !config.service_data


This is more of a question: Does Service Data need to be required? Isn't service data by default the entity_Id? Idk if that what is passed to this though

The docs say it is required which is what I was going by 🤷‍♂️

There is no entity_id in this config, so it can't default to entity ID. However, it's totally fine to have an empty service data. We should make it not required.

zsarnett · 2018-10-28T01:56:23Z

src/panels/lovelace/special-rows/hui-call-service-row.ts

+  static get properties() {
+    return {
+      hass: {},
+      config: {},


Should this be _config?

iantrich · 2018-10-28T02:37:14Z

Not sure why Travis is complaining?

balloob · 2018-10-28T08:10:55Z

Travis is complaining about the polymer linter, which apparently decided to throw errors 🤔 which is weird as we use yarn install and have a lock file.

balloob · 2018-10-28T08:12:55Z

src/panels/lovelace/special-rows/hui-call-service-row.ts

        </div>
-        <paper-button on-click="_callService">[[_config.action_name]]</paper-button>
+        <paper-button
+          @click="${() => callService(this._config, this.hass)}"


Instead of doing this, can you make it a private function so we don't have to create a new arrow function on each render.

Is that just more overhead to create the arrow functions?

render is called many times, each time we define a function. That function, just being generated, won't be unique to the previous one, causing lit to update the click listener. (oldVal !== newVal). It also means that you create a function per instance of the element.

Now with a class method, the definition of the method just lives on the prototype chain. There is only 1 in memory even if you have 1000 instances. No updates are triggered when setting (oldVal === newVal). And as added bonus, if someone would want to inherit from this class, they could override the method. Overall a lot better 👍

home-assistant/frontend#1894

balloob · 2018-10-28T19:53:19Z

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

+
+  public setConfig(config: Config) {
+    const entities = processConfigEntities(config.entities);
+    for (const entity of entities) {


We should drop this check. The createRowElement function will render an error card when a type is invalid.

Not sure why git thinks I changed all of this? I plan to clean up entities-card config checks in a future PR though.

balloob · 2018-10-28T19:54:36Z

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

+                    .hass="${this._hass}"
+                    .entities="${this._configEntities!.map(
+                      (conf) => conf.entity
+                    )}"


add .filter(Boolean) to filter out any undefined which are returned when fetching entity from weblink or service call config.

balloob · 2018-10-28T19:55:50Z

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

+    }
+
+    this._config = config;
+    this._configEntities = entities;


We should call it _configRows, as not all are entities.

I think prettier ran against this file and made it look like I touched everything. Fail on my part.

My only intended change was to remove the !entity.service_data check when entity.type === "call-service" as I moved that to being optional. The full check should be removed in a future PR

balloob · 2018-10-28T19:57:31Z

src/panels/lovelace/special-rows/hui-call-service-row.ts

+  public setConfig(config: CallServiceConfig): void {
+    if (
+      !config ||
+      !config.icon ||


We should do a default icon.

Do you have a suggestion? I was thinking play-circle-outline

what icon do we use for services in the dev tools?

balloob · 2018-10-28T19:57:50Z

src/panels/lovelace/special-rows/hui-call-service-row.ts

+      !config ||
+      !config.icon ||
+      !config.name ||
+      !config.action_name ||


Let's do a default action_name, what about "Run" ?

* Made `service_data` optional and verified that `callService` does use `entity` if it is available in the passed config. * Removed check in `entities-card` for `service-data` and will remove the full config check once other PRs have been applied to avoid merge conflicts * Will create a docs PR to update the docs

* Removed entity config check in entities-card * Made `icon` optional. Default now `remote` * Made `action_name` optional. Default now 'Run'

balloob · 2018-10-29T18:49:19Z

src/panels/lovelace/special-rows/hui-call-service-row.ts

+
+    return html`
+      ${this.renderStyle()}
+      <ha-icon icon="${this._config.icon}"></ha-icon>


balloob

Ok to merge when comment addressed.

balloob · 2018-10-30T08:31:56Z

Not sure why Polymer linter "woke up" and started to spit all over your PR

balloob · 2018-10-30T10:13:25Z

Figured it out, TypeScript was checking all type declarations of our node modules, making a mess. Fixed it in #1885 so will go ahead and merge this one.

* service-data now optional home-assistant/frontend#1894 * additional fields made optional `icon` and `action_name` will be optional as well * 🚑 Fixes build

homeassistant added the cla-signed label Oct 28, 2018

ghost assigned iantrich Oct 28, 2018

ghost added the in progress label Oct 28, 2018

zsarnett reviewed Oct 28, 2018

View reviewed changes

balloob reviewed Oct 28, 2018

View reviewed changes

iantrich added a commit to home-assistant/home-assistant.io that referenced this pull request Oct 28, 2018

service-data now optional

b1aac2f

home-assistant/frontend#1894

iantrich mentioned this pull request Oct 28, 2018

Some service-row fields are now optional home-assistant/home-assistant.io#7201

Merged

2 tasks

balloob reviewed Oct 28, 2018

View reviewed changes

cgarwood added the hacktoberfest label Oct 28, 2018

iantrich added 3 commits October 29, 2018 13:19

Convert hui-call-service-row to TypeScript/LitElement

f6e7757

Update on _config change

fb9fe14

iantrich force-pushed the ts-service-row branch from 522451d to 4a31640 Compare October 29, 2018 18:20

Addressed review comments

7d467e7

* Removed entity config check in entities-card * Made `icon` optional. Default now `remote` * Made `action_name` optional. Default now 'Run'

balloob reviewed Oct 29, 2018

View reviewed changes

balloob approved these changes Oct 29, 2018

View reviewed changes

.icon

b4cdabf

balloob merged commit 38bfe8c into home-assistant:dev Oct 30, 2018

ghost removed the in progress label Oct 30, 2018

balloob mentioned this pull request Nov 3, 2018

Beta release: 20181030.0 + 20181103.0 #1974

Merged

iantrich deleted the ts-service-row branch November 16, 2018 19:19

github-actions bot locked and limited conversation to collaborators Jul 7, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Convert hui-call-service-row to TypeScript/LitElement #1894

Convert hui-call-service-row to TypeScript/LitElement #1894

iantrich commented Oct 28, 2018

zsarnett Oct 28, 2018

iantrich Oct 28, 2018 •

edited

Loading

balloob Oct 28, 2018 •

edited

Loading

zsarnett Oct 28, 2018

iantrich Oct 28, 2018

iantrich commented Oct 28, 2018

balloob commented Oct 28, 2018

balloob Oct 28, 2018

iantrich Oct 28, 2018

balloob Oct 28, 2018

balloob Oct 28, 2018

iantrich Oct 29, 2018 •

edited

Loading

balloob Oct 28, 2018

balloob Oct 28, 2018

iantrich Oct 29, 2018

balloob Oct 28, 2018

iantrich Oct 29, 2018

balloob Oct 29, 2018

balloob Oct 28, 2018

balloob Oct 29, 2018

balloob left a comment

balloob commented Oct 30, 2018

balloob commented Oct 30, 2018

Convert hui-call-service-row to TypeScript/LitElement #1894

Convert hui-call-service-row to TypeScript/LitElement #1894

Conversation

iantrich commented Oct 28, 2018

Choose a reason for hiding this comment

iantrich Oct 28, 2018 • edited Loading

Choose a reason for hiding this comment

balloob Oct 28, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iantrich commented Oct 28, 2018

balloob commented Oct 28, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iantrich Oct 29, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

balloob left a comment

Choose a reason for hiding this comment

balloob commented Oct 30, 2018

balloob commented Oct 30, 2018

iantrich Oct 28, 2018 •

edited

Loading

balloob Oct 28, 2018 •

edited

Loading

iantrich Oct 29, 2018 •

edited

Loading