Skip to content

Separate row entity elements for Lovelace#1461

Merged
balloob merged 12 commits intomasterfrom
separate-row-entity-elements
Jul 18, 2018
Merged

Separate row entity elements for Lovelace#1461
balloob merged 12 commits intomasterfrom
separate-row-entity-elements

Conversation

@c727
Copy link
Copy Markdown
Contributor

@c727 c727 commented Jul 17, 2018

image

    cards:
      - type: entities
        entities:
          - scene.romantic_lights
          - entity: device_tracker.demo_paulus
            secondary_info: entity-id
          - cover.kitchen_window
          - group.kitchen
          - lock.kitchen_door
          - entity: light.bed_light
            secondary_info: last-changed

fix: home-assistant/ui-schema#97
fix: home-assistant/ui-schema#69

@ghost ghost assigned c727 Jul 17, 2018
@ghost ghost added the in progress label Jul 17, 2018
import fireEvent from '../../../common/dom/fire_event.js';

import stateCardType from '../../../common/entity/state_card_type.js';
import '../entity-row-elements/hui-generic-entity-row-element.js';
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 the folder name can just be entity-row.

@balloob
Copy link
Copy Markdown
Member

balloob commented Jul 17, 2018

I think this is great. Our current entity rows are too coupled between display on left side and controls on right side. We should be able to extract all the controls on the right side and make the left configurable. It's just things like input_select that might need an override. Which is fine.

@balloob
Copy link
Copy Markdown
Member

balloob commented Jul 17, 2018

It's also great as it allows to maybe just override the controls instead of the full entity row with a custom web component.

@balloob
Copy link
Copy Markdown
Member

balloob commented Jul 17, 2018

Since you're making a generic row, consider use slot to fill in control

const element = createEntityRowElement(entity, this.hass);
if (entityId && !DOMAINS_HIDE_MORE_INFO.includes(computeDomain(entityId))) {
element.classList.add('state-card-dialog');
element.addEventListener('click', () => this.fire('hass-more-info', { entityId }));
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 keep this here. We don't know where EntityRow will be used.

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.

Although, I do really like if we only attach click listener to the left part of the generic row. Interacting with controls should never open more info.

}

element.stateObj = stateObj;
element.hass = hass;
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.

Do we still need this or can we start removing hass from this file ? (as it should be)

color: var(--secondary-text-color);
}
</style>
<template is="dom-if" if="[[_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.

If we hide everything when there is no stateObj, can we instead show a light yellow div saying "State X not found"?

}

_computeStateObj(states, entityId) {
return states && entityId && entityId in states ? states[entityId] : null;
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 entityId, config is invalid and this component should not be instantiated with it?

static get template() {
return html`
<style>
paper-button {
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 don't see paper-button in this component

hass="[[hass]]"
config="[[_config]]"
>
<paper-input no-label-float=
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.

remove " at end

class HuiInputTextEntityRow extends PolymerElement {
static get template() {
return html`
<template is="dom-if" if="[[_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.

Really not a fan of these guards for stateObj :(

margin: 0;
}
</style>
<template is="dom-if" if="[[_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.

Not needed, already is in hui-generic-entity-row.

margin: 0;
}
</style>
<template is="dom-if" if="[[_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.

Already in hui-generic-entity-row

hass="[[hass]]"
config="[[_config]]"
>
<div>
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.

Why wrap it in div?

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.

I need an element for my flexbox

class HuiTextEntityRow extends PolymerElement {
static get template() {
return html`
<template is="dom-if" if="[[_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.

Done inside hui-generic-entity-row

this._config = config;
}

getCardSize() {
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.

?

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.

are we trying to keep the same API between cards and entity rows?

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.

hups I thought we use it also here, removed it

class HuiToggleEntityRow extends PolymerElement {
static get template() {
return html`
<template is="dom-if" if="[[_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.

Done inside hui-generic-entity-row

<state-info
hass="[[hass]]"
state-obj="[[stateObj]]"
in-dialog="[[inDialog]]"
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.

In retrospect, in-dialog is such a misnomer as attribute for state-info. It defines context instead of behavior (show-last-changed) while state info should not know anything about dialogs. Oh well, so we learn

@balloob
Copy link
Copy Markdown
Member

balloob commented Jul 18, 2018

This is already looking very good !

@ghost ghost assigned balloob Jul 18, 2018
@balloob balloob changed the title WIP: Separate row entity elements for Lovelace Separate row entity elements for Lovelace Jul 18, 2018
@balloob balloob merged commit 8c44e24 into master Jul 18, 2018
@ghost ghost removed the in progress label Jul 18, 2018
@balloob balloob deleted the separate-row-entity-elements branch July 18, 2018 15:25
@randellhodges
Copy link
Copy Markdown
Contributor

This merge removed the overrideName changes that was added on July 1. Was that on purpose, or an oversight? I'm attempting to create a state card in lovelace and want to pass along the name from the entity card configuration but I have no way to pass it into state-info since this property has been removed.

See the bottom of the diff:
8c44e24#diff-cc39ce33639eea793f68fa0b9315237f

@home-assistant home-assistant locked as resolved and limited conversation to collaborators Sep 13, 2018
@balloob
Copy link
Copy Markdown
Member

balloob commented Sep 13, 2018

No discussion in merged PRs.

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