Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
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
7 changes: 7 additions & 0 deletions src/components/ha-related-items.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,13 @@ export class HaRelatedItems extends SubscribeMixin(LitElement) {
if (!this._related) {
return html``;
}
if (Object.keys(this._related).length === 0) {
return html`
<p>
${this.hass.localize("ui.components.related-items.no_related_found")}
</p>
`;
}
return html`
${this._related.config_entry && this._entries
? this._related.config_entry.map((relatedConfigEntryId) => {
Expand Down
14 changes: 13 additions & 1 deletion src/dialogs/config-flow/step-flow-pick-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import "../../common/search/search-input";
import { styleMap } from "lit-html/directives/style-map";
import { FlowConfig } from "./show-dialog-data-entry-flow";
import { configFlowContentStyles } from "./styles";
import { classMap } from "lit-html/directives/class-map";

interface HandlerObj {
name: string;
Expand Down Expand Up @@ -69,7 +70,10 @@ class StepFlowPickHandler extends LitElement {
.filter=${this.filter}
@value-changed=${this._filterChanged}
></search-input>
<div style=${styleMap({ width: `${this._width}px` })}>
<div
style=${styleMap({ width: `${this._width}px` })}
class=${classMap({ advanced: Boolean(this.showAdvanced) })}
>
${handlers.map(
(handler: HandlerObj) =>
html`
Expand Down Expand Up @@ -143,6 +147,14 @@ class StepFlowPickHandler extends LitElement {
overflow: auto;
max-height: 600px;
}
@media all and (max-height: 1px) {
div {
max-height: calc(100vh - 205px);
}
div.advanced {
max-height: calc(100vh - 300px);
}
}
paper-item {
cursor: pointer;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ export class HaDeviceEntitiesCard extends LitElement {
const entry = (ev.currentTarget! as any).entry;
showEntityRegistryDetailDialog(this, {
entry,
entity_id: entry.entity_id,
});
}

Expand Down
35 changes: 21 additions & 14 deletions src/panels/config/entities/dialog-entity-registry-detail.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ export class DialogEntityRegistryDetail extends LitElement {
return html``;
}
const entry = this._params.entry;
const stateObj: HassEntity | undefined = this.hass.states[entry.entity_id];
const entityId = this._params.entity_id;
const stateObj: HassEntity | undefined = this.hass.states[entityId];

return html`
<ha-paper-dialog
Expand All @@ -68,9 +69,7 @@ export class DialogEntityRegistryDetail extends LitElement {
dialog-dismiss
></paper-icon-button>
<div class="main-title" main-title>
${stateObj
? computeStateName(stateObj)
: entry.name || entry.entity_id}
${stateObj ? computeStateName(stateObj) : entry?.name || entityId}
</div>
${stateObj
? html`
Expand Down Expand Up @@ -99,20 +98,28 @@ export class DialogEntityRegistryDetail extends LitElement {
</paper-tabs>
${cache(
this._curTab === "tab-settings"
? html`
<entity-registry-settings
.hass=${this.hass}
.entry=${entry}
.dialogElement=${this._dialog}
@close-dialog=${this._closeDialog}
></entity-registry-settings>
`
? entry
? html`
<entity-registry-settings
.hass=${this.hass}
.entry=${entry}
.dialogElement=${this._dialog}
@close-dialog=${this._closeDialog}
></entity-registry-settings>
`
: html`
<paper-dialog-scrollable>
${this.hass.localize(
"ui.dialogs.entity_registry.no_unique_id"
)}
</paper-dialog-scrollable>
`
: this._curTab === "tab-related"
? html`
<paper-dialog-scrollable>
<ha-related-items
.hass=${this.hass}
.itemId=${entry.entity_id}
.itemId=${entityId}
itemType="entity"
@close-dialog=${this._closeDialog}
></ha-related-items>
Expand All @@ -139,7 +146,7 @@ export class DialogEntityRegistryDetail extends LitElement {

private _openMoreInfo(): void {
fireEvent(this, "hass-more-info", {
entityId: this._params!.entry.entity_id,
entityId: this._params!.entity_id,
});
this._params = undefined;
}
Expand Down
76 changes: 60 additions & 16 deletions src/panels/config/entities/ha-config-entities.ts
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

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.

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.

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.

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).

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.

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.

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.

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.

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).

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}">

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 @@ -461,15 +507,13 @@ export class HaConfigEntities extends SubscribeMixin(LitElement) {
}

private _openEditEntry(ev: CustomEvent): void {
const entryId = (ev.detail as RowClickedEvent).id;
const entityId = (ev.detail as RowClickedEvent).id;
const entry = this._entities!.find(
(entity) => entity.entity_id === entryId
(entity) => entity.entity_id === entityId
);
if (!entry) {
return;
}
this.getDialog = showEntityRegistryDetailDialog(this, {
entry,
entity_id: entityId,
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ import { EntityRegistryEntry } from "../../../data/entity_registry";
import { DialogEntityRegistryDetail } from "./dialog-entity-registry-detail";

export interface EntityRegistryDetailDialogParams {
entry: EntityRegistryEntry;
entry?: EntityRegistryEntry;
entity_id: string;
}

export const loadEntityRegistryDetailDialog = () =>
Expand All @@ -21,12 +22,12 @@ const getDialog = () => {

export const showEntityRegistryDetailDialog = (
element: HTMLElement,
systemLogDetailParams: EntityRegistryDetailDialogParams
entityDetailParams: EntityRegistryDetailDialogParams
): (() => DialogEntityRegistryDetail | undefined) => {
fireEvent(element, "show-dialog", {
dialogTag: "dialog-entity-registry-detail",
dialogImport: loadEntityRegistryDetailDialog,
dialogParams: systemLogDetailParams,
dialogParams: entityDetailParams,
});
return getDialog;
};
Loading