Skip to content

Convert hui-toggle-entity-row to TypeScript/LitElement#1939

Merged
balloob merged 4 commits intohome-assistant:devfrom
iantrich:ts-toggle-row
Nov 7, 2018
Merged

Convert hui-toggle-entity-row to TypeScript/LitElement#1939
balloob merged 4 commits intohome-assistant:devfrom
iantrich:ts-toggle-row

Conversation

@iantrich
Copy link
Copy Markdown
Member

No description provided.

return html``;
}

const stateObj = this.hass.states[this._config.entity];
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.

Should we be checking if this exists? Or is this a case of to even call this element there has to be a state obj?

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 should check if stateObj is defined. Might want to consider updating the type of hass.states to indicate that we don't have a value for every key.

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.

Ooh, hui-generic-entity-row will render a yello block if the entity doesn't exist, we just need to make sure we don't crash on line 46.

`;
}

private _computeState(stateObj: HassEntity) {
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.

Return Type?

import "../entity-rows/hui-text-entity-row.js";
import "../entity-rows/hui-timer-entity-row.js";
import "../entity-rows/hui-toggle-entity-row.js";
import "../entity-rows/hui-toggle-entity-row";
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 should just have a PR that rips out all .js from all imports throughout the whole repo.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Just remember that you asked for this: #1941
I started small with only all of src 😄

}

private _computeState(stateObj: HassEntity) {
return stateObj && computeStateDisplay(this.localize, stateObj);
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.

No need to check for stateObj here, if it didn't exist, you would already have crashed on line 46 😉

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.

I think that you can just put computeStateDisplay directly in to the template above.

}

protected render(): TemplateResult {
if (!this._config || !this.hass || !this.hass.states[this._config.entity]) {
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 not good. Right now we won't render anything if the entity doesn't exist.

The old behavior was that if the entity doesn't exist, we still render <hui-generic-entity-row> which would then go ahead and render a yellow warning that the entity didn't exist.

@balloob balloob merged commit 9ce74e2 into home-assistant:dev Nov 7, 2018
@ghost ghost removed the in progress label Nov 7, 2018
@iantrich iantrich deleted the ts-toggle-row branch November 16, 2018 19:18
@github-actions github-actions bot locked and limited conversation to collaborators Jul 7, 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.

4 participants