Skip to content
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

Add visibility option to heading entities #22064

Merged
merged 2 commits into from
Sep 24, 2024
Merged

Add visibility option to heading entities #22064

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0a2451f
Add visibility option to heading entity
piitaya Sep 23, 2024
d4e7c6c
Fix types
piitaya Sep 24, 2024
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
70 changes: 69 additions & 1 deletion src/panels/lovelace/cards/heading/hui-heading-entity.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,15 @@
import { CSSResultGroup, LitElement, css, html, nothing } from "lit";
import {
CSSResultGroup,
LitElement,
PropertyValues,
css,
html,
nothing,
} from "lit";
import { customElement, property } from "lit/decorators";
import { ifDefined } from "lit/directives/if-defined";
import memoizeOne from "memoize-one";
import { MediaQueriesListener } from "../../../../common/dom/media_query";
import "../../../../components/ha-card";
import "../../../../components/ha-icon";
import "../../../../components/ha-icon-next";
Expand All @@ -12,6 +20,10 @@ import { HomeAssistant } from "../../../../types";
import { actionHandler } from "../../common/directives/action-handler-directive";
import { handleAction } from "../../common/handle-action";
import { hasAction } from "../../common/has-action";
import {
attachConditionMediaQueriesListeners,
checkConditionsMet,
} from "../../common/validate-condition";
import type { HeadingEntityConfig } from "../types";

@customElement("hui-heading-entity")
Expand All @@ -20,6 +32,10 @@ export class HuiHeadingEntity extends LitElement {

@property({ attribute: false }) public config!: HeadingEntityConfig | string;

@property({ type: Boolean }) public preview = false;

private _listeners: MediaQueriesListener[] = [];

private _handleAction(ev: ActionHandlerEvent) {
const config: HeadingEntityConfig = {
tap_action: {
Expand All @@ -46,6 +62,58 @@ export class HuiHeadingEntity extends LitElement {
}
);

public disconnectedCallback() {
super.disconnectedCallback();
this._clearMediaQueries();
}

public connectedCallback() {
super.connectedCallback();
this._listenMediaQueries();
this._updateVisibility();
}

protected update(changedProps: PropertyValues<typeof this>): void {
super.update(changedProps);
if (changedProps.has("hass") || changedProps.has("preview")) {
this._updateVisibility();
}
}

Comment on lines +76 to +81
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Include 'config' changes in the visibility update condition

Currently, _updateVisibility() is called when hass or preview changes. If the config property changes, the visibility may also need to be updated to reflect the new configuration. Consider adding changedProps.has("config") to ensure visibility is updated when the configuration changes.

Apply this diff to implement the change:

 protected update(changedProps: PropertyValues<typeof this>): void {
   super.update(changedProps);
-  if (changedProps.has("hass") || changedProps.has("preview")) {
+  if (
+    changedProps.has("hass") ||
+    changedProps.has("preview") ||
+    changedProps.has("config")
+  ) {
     this._updateVisibility();
   }
 }
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
protected update(changedProps: PropertyValues<typeof this>): void {
super.update(changedProps);
if (changedProps.has("hass") || changedProps.has("preview")) {
this._updateVisibility();
}
}
protected update(changedProps: PropertyValues<typeof this>): void {
super.update(changedProps);
if (
changedProps.has("hass") ||
changedProps.has("preview") ||
changedProps.has("config")
) {
this._updateVisibility();
}
}

private _updateVisibility(forceVisible?: boolean) {
const config = this._config(this.config);
const visible =
forceVisible ||
this.preview ||
!config.visibility ||
checkConditionsMet(config.visibility, this.hass);
this.toggleAttribute("hidden", !visible);
}

private _clearMediaQueries() {
this._listeners.forEach((unsub) => unsub());
this._listeners = [];
}

private _listenMediaQueries() {
const config = this._config(this.config);
if (!config?.visibility) {
return;
}
const conditions = config.visibility;
const hasOnlyMediaQuery =
conditions.length === 1 &&
conditions[0].condition === "screen" &&
!!conditions[0].media_query;

this._listeners = attachConditionMediaQueriesListeners(
config.visibility,
(matches) => {
this._updateVisibility(hasOnlyMediaQuery && matches);
}
);
}

Comment on lines +98 to +115
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Prevent duplicate media query listeners by clearing existing listeners

If connectedCallback() is called multiple times without a corresponding disconnectedCallback(), multiple media query listeners could be attached, potentially leading to unexpected behavior or memory leaks. To avoid this, consider calling _clearMediaQueries() at the beginning of the _listenMediaQueries() method to ensure that existing listeners are removed before new ones are added.

Apply this diff to implement the change:

 private _listenMediaQueries() {
+   this._clearMediaQueries();
    const config = this._config(this.config);
    if (!config?.visibility) {
      return;
    }
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private _listenMediaQueries() {
const config = this._config(this.config);
if (!config?.visibility) {
return;
}
const conditions = config.visibility;
const hasOnlyMediaQuery =
conditions.length === 1 &&
conditions[0].condition === "screen" &&
!!conditions[0].media_query;
this._listeners = attachConditionMediaQueriesListeners(
config.visibility,
(matches) => {
this._updateVisibility(hasOnlyMediaQuery && matches);
}
);
}
private _listenMediaQueries() {
this._clearMediaQueries();
const config = this._config(this.config);
if (!config?.visibility) {
return;
}
const conditions = config.visibility;
const hasOnlyMediaQuery =
conditions.length === 1 &&
conditions[0].condition === "screen" &&
!!conditions[0].media_query;
this._listeners = attachConditionMediaQueriesListeners(
config.visibility,
(matches) => {
this._updateVisibility(hasOnlyMediaQuery && matches);
}
);
}

protected render() {
const config = this._config(this.config);

Expand Down
8 changes: 7 additions & 1 deletion src/panels/lovelace/cards/hui-heading-card.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ export class HuiHeadingCard extends LitElement implements LovelaceCard {

@property({ attribute: false }) public hass?: HomeAssistant;

@property({ type: Boolean }) public preview = false;

@state() private _config?: HeadingCardConfig;

public setConfig(config: HeadingCardConfig): void {
Expand Down Expand Up @@ -94,7 +96,11 @@ export class HuiHeadingCard extends LitElement implements LovelaceCard {
<div class="entities">
${this._config.entities.map(
(config) => html`
<hui-heading-entity .config=${config} .hass=${this.hass}>
<hui-heading-entity
.config=${config}
.hass=${this.hass}
.preview=${this.preview}
>
</hui-heading-entity>
`
)}
Expand Down
1 change: 1 addition & 0 deletions src/panels/lovelace/cards/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -508,6 +508,7 @@ export interface HeadingEntityConfig {
content?: string | string[];
icon?: string;
tap_action?: ActionConfig;
visibility?: Condition[];
}

export interface HeadingCardConfig extends LovelaceCardConfig {
Expand Down
68 changes: 64 additions & 4 deletions src/panels/lovelace/editor/heading-entity/hui-heading-entity-editor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,28 @@
import { mdiGestureTap } from "@mdi/js";
import { mdiEye, mdiGestureTap } from "@mdi/js";
import { css, html, LitElement, nothing } from "lit";
import { customElement, property, state } from "lit/decorators";
import memoizeOne from "memoize-one";
import { array, assert, object, optional, string, union } from "superstruct";
import {
any,
array,
assert,
object,
optional,
string,
union,
} from "superstruct";
import { fireEvent } from "../../../../common/dom/fire_event";
import "../../../../components/ha-expansion-panel";
import "../../../../components/ha-form/ha-form";
import type {
HaFormSchema,
SchemaUnion,
} from "../../../../components/ha-form/types";
import type { HomeAssistant } from "../../../../types";
import type { HeadingCardConfig, HeadingEntityConfig } from "../../cards/types";
import type { HeadingEntityConfig } from "../../cards/types";
import { Condition } from "../../common/validate-condition";
import type { LovelaceGenericElementEditor } from "../../types";
import "../conditions/ha-card-conditions-editor";
import { configElementStyle } from "../config-elements/config-elements-style";
import { actionConfigStruct } from "../structs/action-struct";

Expand All @@ -20,6 +31,7 @@ const entityConfigStruct = object({
content: optional(union([string(), array(string())])),
icon: optional(string()),
tap_action: optional(actionConfigStruct),
visibility: optional(array(any())),
});
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve type safety for the visibility property

Using any() in visibility: optional(array(any())) reduces type safety and can lead to runtime errors. To enhance maintainability and catch potential issues at compile time, consider defining a specific struct for the visibility conditions.

Define a struct for the conditions, for example:

const conditionStruct = object({
  entity: string(),
  state: optional(string()),
  // Add other required properties
});

const entityConfigStruct = object({
  // existing properties
  visibility: optional(array(conditionStruct)),
});


@customElement("hui-heading-entity-editor")
Expand All @@ -29,6 +41,8 @@ export class HuiHeadingEntityEditor
{
@property({ attribute: false }) public hass?: HomeAssistant;

@property({ type: Boolean }) public preview = false;

@state() private _config?: HeadingEntityConfig;

public setConfig(config: HeadingEntityConfig): void {
Expand Down Expand Up @@ -79,6 +93,7 @@ export class HuiHeadingEntityEditor

const schema = this._schema();

const conditions = this._config.visibility ?? [];
return html`
<ha-form
.hass=${this.hass}
Expand All @@ -87,6 +102,27 @@ export class HuiHeadingEntityEditor
.computeLabel=${this._computeLabelCallback}
@value-changed=${this._valueChanged}
></ha-form>
<ha-expansion-panel outlined>
<h3 slot="header">
<ha-svg-icon .path=${mdiEye}></ha-svg-icon>
${this.hass!.localize(
"ui.panel.lovelace.editor.card.heading.entity_config.visibility"
)}
</h3>
<div class="content">
<p class="intro">
${this.hass.localize(
"ui.panel.lovelace.editor.card.heading.entity_config.visibility_explanation"
)}
</p>
<ha-card-conditions-editor
.hass=${this.hass}
.conditions=${conditions}
@value-changed=${this._conditionChanged}
>
</ha-card-conditions-editor>
</div>
</ha-expansion-panel>
`;
}

Expand All @@ -96,11 +132,30 @@ export class HuiHeadingEntityEditor
return;
}

const config = ev.detail.value as HeadingCardConfig;
const config = ev.detail.value as HeadingEntityConfig;

fireEvent(this, "config-changed", { config });
}

private _conditionChanged(ev: CustomEvent): void {
ev.stopPropagation();
if (!this._config || !this.hass) {
return;
}

const conditions = ev.detail.value as Condition[];

const newConfig: HeadingEntityConfig = {
...this._config,
visibility: conditions,
};
if (newConfig.visibility?.length === 0) {
delete newConfig.visibility;
}

fireEvent(this, "config-changed", { config: newConfig });
}

Comment on lines +140 to +157
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Avoid using the delete operator for better performance

The delete operator can negatively impact performance because it changes the object's shape in V8 JavaScript engine. Instead of deleting the visibility property, set it to undefined or omit it when creating newConfig.

Apply this diff to address the performance issue:

       const newConfig: HeadingEntityConfig = {
         ...this._config,
-        visibility: conditions,
+        visibility: conditions.length > 0 ? conditions : undefined,
       };
-      if (newConfig.visibility?.length === 0) {
-        delete newConfig.visibility;
-      }
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private _conditionChanged(ev: CustomEvent): void {
ev.stopPropagation();
if (!this._config || !this.hass) {
return;
}
const conditions = ev.detail.value as Condition[];
const newConfig: HeadingEntityConfig = {
...this._config,
visibility: conditions,
};
if (newConfig.visibility?.length === 0) {
delete newConfig.visibility;
}
fireEvent(this, "config-changed", { config: newConfig });
}
private _conditionChanged(ev: CustomEvent): void {
ev.stopPropagation();
if (!this._config || !this.hass) {
return;
}
const conditions = ev.detail.value as Condition[];
const newConfig: HeadingEntityConfig = {
...this._config,
visibility: conditions.length > 0 ? conditions : undefined,
};
fireEvent(this, "config-changed", { config: newConfig });
}
Tools
Biome

[error] 153-153: Avoid the delete operator which can impact performance.

Unsafe fix: Use an undefined assignment instead.

(lint/performance/noDelete)

private _computeLabelCallback = (
schema: SchemaUnion<ReturnType<typeof this._schema>>
) => {
Expand Down Expand Up @@ -128,6 +183,11 @@ export class HuiHeadingEntityEditor
display: block;
margin-bottom: 24px;
}
.intro {
margin: 0;
color: var(--secondary-text-color);
margin-bottom: 8px;
}
`,
];
}
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 @@ -6007,7 +6007,9 @@
},
"entities": "Entities",
"entity_config": {
"content": "Content"
"content": "Content",
"visibility": "Visibility",
"visibility_explanation": "The entity will be shown when ALL conditions below are fulfilled. If no conditions are set, the entity will always be shown."
},
"default_heading": "Kitchen"
},
Expand Down
Loading