Skip to content

include entities not in entity registry in config entities #4867

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.

Sign up for GitHub

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

Merged
bramkragten merged 5 commits into from
Feb 17, 2020
Merged

include entities not in entity registry in config entities #4867

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
cecb870
include entities not in entity registry in config entities
bramkragten Feb 13, 2020
955824c
Update ha-data-table.ts
bramkragten Feb 13, 2020
0028bc4
Comments
bramkragten Feb 14, 2020
e660cce
Update ha-device-entities-card.ts
bramkragten Feb 14, 2020
456982f
Comments
bramkragten Feb 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions src/components/data-table/ha-data-table.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ export interface DataTableColumnData extends DataTableSortColumnData {

export interface DataTableRowData {
[key: string]: any;
selectable?: boolean;
}

@customElement("ha-data-table")
Expand Down Expand Up @@ -249,6 +250,7 @@ export class HaDataTable extends BaseElement {
data-row-id="${row[this.id]}"
@click=${this._handleRowClick}
class="mdc-data-table__row"
.selectable=${row.selectable !== false}
>
${this.selectable
? html`
Expand All @@ -258,6 +260,7 @@ export class HaDataTable extends BaseElement {
<ha-checkbox
class="mdc-data-table__row-checkbox"
@change=${this._handleRowCheckboxChange}
.disabled=${row.selectable === false}
.checked=${this._checkedRows.includes(
String(row[this.id])
)}
Expand Down Expand Up @@ -298,9 +301,12 @@ export class HaDataTable extends BaseElement {
protected createAdapter(): MDCDataTableAdapter {
return {
addClassAtRowIndex: (rowIndex: number, cssClasses: string) => {
if (!(this.rowElements[rowIndex] as any).selectable) {
return;
}
this.rowElements[rowIndex].classList.add(cssClasses);
},
getRowCount: () => this.data.length,
getRowCount: () => this.rowElements.length,
getRowElements: () => this.rowElements,
getRowIdAtIndex: (rowIndex: number) => this._getRowIdAtIndex(rowIndex),
getRowIndexByChildElement: (el: Element) =>
Expand All @@ -309,7 +315,7 @@ export class HaDataTable extends BaseElement {
isCheckboxAtRowIndexChecked: (rowIndex: number) =>
this._checkedRows.includes(this._getRowIdAtIndex(rowIndex)),
isHeaderRowCheckboxChecked: () => this._headerChecked,
isRowsSelectable: () => true,
isRowsSelectable: () => this.selectable,
notifyRowSelectionChanged: () => undefined,
notifySelectedAll: () => undefined,
notifyUnselectedAll: () => undefined,
Expand All @@ -332,6 +338,9 @@ export class HaDataTable extends BaseElement {
this._headerIndeterminate = indeterminate;
},
setRowCheckboxCheckedAtIndex: (rowIndex: number, checked: boolean) => {
if (!(this.rowElements[rowIndex] as any).selectable) {
return;
}
this._setRowChecked(this._getRowIdAtIndex(rowIndex), checked);
},
};
Expand Down Expand Up @@ -516,6 +525,7 @@ export class HaDataTable extends BaseElement {
padding-left: 16px;
/* @noflip */
padding-right: 0;
width: 40px;
}
[dir="rtl"] .mdc-data-table__header-cell--checkbox,
.mdc-data-table__header-cell--checkbox[dir="rtl"],
Expand Down Expand Up @@ -558,6 +568,7 @@ export class HaDataTable extends BaseElement {
.mdc-data-table__cell--icon {
color: var(--secondary-text-color);
text-align: center;
width: 24px;
}

.mdc-data-table__header-cell {
Expand Down
2 changes: 1 addition & 1 deletion src/dialogs/config-flow/step-flow-pick-handler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ class StepFlowPickHandler extends LitElement {
css`
div {
overflow: auto;
max-height: 600px;
max-height: calc(100vh - 300px);

@balloob balloob Feb 13, 2020

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.

Isn't this very limiting on mobile ?

@bramkragten bramkragten Feb 14, 2020

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.

This shouldn't have been in this PR 🙈 but, no It just makes sure the scroll container fits on the screen, will fine-tune it a bit.

image

}
paper-item {
cursor: pointer;
Expand Down
69 changes: 58 additions & 11 deletions src/panels/config/entities/ha-config-entities.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import "@polymer/paper-dropdown-menu/paper-dropdown-menu";
import "@polymer/paper-item/paper-icon-item";
import "@polymer/paper-listbox/paper-listbox";
import "@polymer/paper-tooltip/paper-tooltip";
import { UnsubscribeFunc } from "home-assistant-js-websocket";
import { UnsubscribeFunc, HassEntities } from "home-assistant-js-websocket";
import {
css,
CSSResult,
Expand All @@ -22,7 +22,6 @@ import { stateIcon } from "../../../common/entity/state_icon";
import {
DataTableColumnContainer,
DataTableColumnData,
HaDataTable,
RowClickedEvent,
SelectionChangedEvent,
} from "../../../components/data-table/ha-data-table";
Expand All @@ -47,6 +46,10 @@ import {
} from "./show-dialog-entity-registry-detail";
import { configSections } from "../ha-panel-config";
import { classMap } from "lit-html/directives/class-map";
import { computeStateName } from "../../../common/entity/compute_state_name";
import { fireEvent } from "../../../common/dom/fire_event";
// tslint:disable-next-line: no-duplicate-imports
import { HaTabsSubpageDataTable } from "../../../layouts/hass-tabs-subpage-data-table";

@customElement("ha-config-entities")
export class HaConfigEntities extends SubscribeMixin(LitElement) {
Expand All @@ -57,9 +60,11 @@ export class HaConfigEntities extends SubscribeMixin(LitElement) {
@property() private _entities?: EntityRegistryEntry[];
@property() private _showDisabled = false;
@property() private _showUnavailable = true;
@property() private _showReadOnly = true;
@property() private _filter = "";
@property() private _selectedEntities: string[] = [];
@query("ha-data-table") private _dataTable!: HaDataTable;
@query("hass-tabs-subpage-data-table")
private _dataTable!: HaTabsSubpageDataTable;
private getDialog?: () => DialogEntityRegistryDetail | undefined;

private _columns = memoize(
Expand Down Expand Up @@ -90,7 +95,7 @@ export class HaConfigEntities extends SubscribeMixin(LitElement) {
sortable: true,
filterable: true,
template: (_status, entity: any) =>
entity.unavailable || entity.disabled_by
entity.unavailable || entity.disabled_by || entity.readonly
? html`
<div
tabindex="0"
Expand All @@ -102,15 +107,21 @@ export class HaConfigEntities extends SubscribeMixin(LitElement) {
})}
.icon=${entity.unavailable
? "hass:alert-circle"
: "hass:cancel"}
: entity.disabled_by
? "hass:cancel"
: "hass:pencil-off"}

@balloob balloob Feb 13, 2020 •
edited
Loading

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 like the pencil-off icon, good choice.

@tdejneka tdejneka Feb 14, 2020

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Is the icon truly needed? The entity's check box is grayed-out, signalling read-only status. A quick glance on the left already indicates which entities can/cannot be edited. The no-edit icon repeats on the right what is already communicated on the left.

Whereas the exclamation icon draws one's attention to an anomaly (entity is unavailable), lacking a unique_id isn't an anomalous condition but shares the same column.

@balloob balloob Feb 14, 2020

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.

A user needs to know why the checkbox is disabled, there is no explanation now.

@tdejneka tdejneka Feb 14, 2020

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I agree. However, I don't believe the pencil-off icon provides that missing explanation. It simply reiterates the meaning of the dimmed checkbox: this table-row cannot be edited.

Suggestion:
Perhaps by hovering over the checkbox or table-row (and/or if you click the table-row) it reveals an explanation like:

Cannot edit this entity because it lacks a unique_id. Refer to configuration.yaml file.

Anyway, I don't mean to belabor the point but, in its current form, it tells me twice that I can't edit the table-row (and without a hint as to why).

@bramkragten bramkragten Feb 14, 2020

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.

It does show that it is not editable on click in the settings dialog.

@bramkragten bramkragten Feb 17, 2020

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.

Check the screenshot, it does tell you why.

@tdejneka tdejneka Feb 17, 2020

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Your original post's edit-history shows the screenshot was added 3 days ago, same time when I made my previous post. My mistake; I didn't see you had amended your original post.

I suggest the message also mention configuration.yaml to answer the reader's question "If not from the UI then where?" However, even without mentioning it, the message achieves the goal of clarifying why the entity cannot be edited in the UI.

BTW, I believe the message's text should use the conjunctive adverb therefore as opposed to the adverb therefor.

@bramkragten bramkragten Feb 17, 2020

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.

It doesn't necessarily have to be in configuration.yaml for example the group entities from hue don't have unique ID's but are set up with an integration flow.

@tdejneka tdejneka Feb 17, 2020

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

True, but all entities defined in configuration.yaml are not obligated to have a unique_id. Traditionally, it's a prime source of entities than do not appear in the Entities list and why I suggested it be mentioned. Nevertheless, even without referring to the YAML file, the message explains why you can't edit the entity and that's very helpful. It's all good.

FWIW, the few flow-based integrations I've used (Homekit_controller, LIFX, and the currently in-beta UPB) all generate entities with unique_id's. This gave me the impression that it's the norm for flow-based integrations.

Off-topic: As for the Hue integration, without knowing why it fails to generate unique id's (a design constraint?), I'd say it's anomalous (and probably should create them).

@bramkragten bramkragten Feb 17, 2020

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.

Well every integration should try to do it, also a lot of integrations that are set up with configuration.yaml have them. In the case of hue, it is a technical limitation just for groups, lights and sensors do have unique id's.

></ha-icon>
<paper-tooltip position="left">
${entity.unavailable
? this.hass.localize(
"ui.panel.config.entities.picker.status.unavailable"
)
: this.hass.localize(
: entity.disabled_by
? this.hass.localize(
"ui.panel.config.entities.picker.status.disabled"
)
: this.hass.localize(
"ui.panel.config.entities.picker.status.readonly"
)}
</paper-tooltip>
</div>
Expand Down Expand Up @@ -156,18 +167,38 @@ export class HaConfigEntities extends SubscribeMixin(LitElement) {
private _filteredEntities = memoize(
(
entities: EntityRegistryEntry[],
states: HassEntities,
showDisabled: boolean,
showUnavailable: boolean
showUnavailable: boolean,
showReadOnly: boolean
) => {
let stateEntities: EntityRegistryEntry[] = [];
if (showReadOnly) {
const regEntityIds = entities.map((entity) => entity.entity_id);
Comment thread
bramkragten marked this conversation as resolved.
Outdated
const stateEntityIds = Object.keys(states).filter(
(entityId) => !regEntityIds.includes(entityId)
);
stateEntities = stateEntityIds.map((entityId) => {
Comment thread
bramkragten marked this conversation as resolved.
Outdated
return {
name: computeStateName(states[entityId]),
entity_id: entityId,
platform: computeDomain(entityId),
disabled_by: null,
readonly: true,
selectable: false,
Comment thread
bramkragten marked this conversation as resolved.
};
});
}

if (!showDisabled) {
entities = entities.filter((entity) => !Boolean(entity.disabled_by));
}

return entities.reduce((result, entry) => {
const state = this.hass!.states[entry.entity_id];
return entities.concat(stateEntities).reduce((result, entry) => {
Comment thread
bramkragten marked this conversation as resolved.
Outdated
const state = states[entry.entity_id];

const unavailable =
state && (state.state === "unavailable" || state.attributes.restored); // if there is not state it is disabled
state && (state.state === "unavailable" || state.attributes.restored);
Comment thread
bramkragten marked this conversation as resolved.
Outdated

if (!showUnavailable && unavailable) {
return result;
Expand Down Expand Up @@ -322,6 +353,15 @@ export class HaConfigEntities extends SubscribeMixin(LitElement) {
"ui.panel.config.entities.picker.filter.show_unavailable"
)}
</paper-icon-item>
<paper-icon-item @tap="${this._showReadOnlyChanged}">

@balloob balloob Feb 13, 2020

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 know if this should be an option. They are active, they have states etc otherwise they wouldn't exist. I think that we should always show them.

<paper-checkbox
.checked=${this._showReadOnly}
slot="item-icon"
></paper-checkbox>
${this.hass!.localize(
"ui.panel.config.entities.picker.filter.show_readonly"
)}
</paper-icon-item>
</paper-listbox>
</paper-menu-button>
`;
Expand All @@ -336,8 +376,10 @@ export class HaConfigEntities extends SubscribeMixin(LitElement) {
.columns=${this._columns(this.narrow, this.hass.language)}
.data=${this._filteredEntities(
this._entities,
this.hass.states,
this._showDisabled,
this._showUnavailable
this._showUnavailable,
this._showReadOnly
)}
.filter=${this._filter}
selectable
Expand Down Expand Up @@ -369,6 +411,10 @@ export class HaConfigEntities extends SubscribeMixin(LitElement) {
this._showUnavailable = !this._showUnavailable;
}

private _showReadOnlyChanged() {
this._showReadOnly = !this._showReadOnly;
}

private _handleSearchChange(ev: CustomEvent) {
this._filter = ev.detail.value;
}
Expand Down Expand Up @@ -466,6 +512,7 @@ export class HaConfigEntities extends SubscribeMixin(LitElement) {
(entity) => entity.entity_id === entryId
);
if (!entry) {
fireEvent(this, "hass-more-info", { entityId: entryId });
Comment thread
bramkragten marked this conversation as resolved.
Outdated
return;
}
this.getDialog = showEntityRegistryDetailDialog(this, {
Expand Down
4 changes: 3 additions & 1 deletion src/translations/en.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1345,11 +1345,13 @@
"filter": {
"filter": "Filter",
"show_disabled": "Show disabled entities",
"show_unavailable": "Show unavailable entities"
"show_unavailable": "Show unavailable entities",
"show_readonly": "Show read-only entities"
},
"status": {
"unavailable": "Unavailable",
"disabled": "Disabled",
"readonly": "Read-only",
"ok": "Ok"
},
"headers": {
Expand Down