Add Heading card #22008
Add Heading card #22008
Conversation
WalkthroughWalkthroughThe changes introduce the Changes
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (1)
Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
f40f076 to
827da74
Compare
| } | ||
|
|
||
| protected render() { | ||
| if (!this.entities || !this.hass) { |
There was a problem hiding this comment.
This means nothing is rendered (not even an add) if there are not entities in the config?
| if (!this.entities || !this.hass) { | |
| if (!this.hass) { |
| } | ||
| } | ||
|
|
||
| @customElement("hui-sub-form-editor") |
There was a problem hiding this comment.
Are planning on migrating the rest of the sub forms to this one too?
There was a problem hiding this comment.
Or should we make a base version that can be used by hui-sub-element-editor and hui-sub-form-editor, seems like there is too much duplicate code now.
src/panels/lovelace/editor/config-elements/hui-entities-editor.ts
Outdated
Show resolved
Hide resolved
src/panels/lovelace/editor/config-elements/hui-heading-card-editor.ts
Outdated
Show resolved
Hide resolved
6c0b9bc to
71947ae
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- src/panels/lovelace/cards/hui-heading-card.ts (1 hunks)
- src/panels/lovelace/editor/config-elements/hui-entities-editor.ts (1 hunks)
- src/panels/lovelace/editor/config-elements/hui-heading-card-editor.ts (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- src/panels/lovelace/cards/hui-heading-card.ts
- src/panels/lovelace/editor/config-elements/hui-entities-editor.ts
Additional context used
GitHub Check: Lint and check format
src/panels/lovelace/editor/config-elements/hui-heading-card-editor.ts
[failure] 156-156:
Object is possibly 'undefined'.
[failure] 160-160:
Object is possibly 'undefined'.
[failure] 180-180:
Object is possibly 'undefined'.
[failure] 192-192:
Object is possibly 'undefined'.
Additional comments not posted (6)
src/panels/lovelace/editor/config-elements/hui-heading-card-editor.ts (6)
1-36: LGTM!The import statements look good and are importing the necessary dependencies for the component.
47-47: The following past review comment is still applicable:bramkragten: here entities is set to
any, but below is a struct for entities? Why not use it?Do we support both a string and object for entities? Because the editor seems to not work when using a string.
Please address this comment if it hasn't been resolved yet.
51-55: LGTM!The
entityConfigStructdefinition looks good and properly defines the schema for individual entities usingsuperstruct.
57-320: LGTM!The
HuiHeadingCardEditorclass is well-implemented and follows best practices for a LitElement component. The rendering logic, event handling, and styling are all properly structured and enhance the user experience for configuring heading cards.Some key highlights:
- The use of
memoizeOnefor efficient schema generation.- Clean and understandable rendering logic.
- Proper event handling for configuration changes and entity editing.
- Localized label computation for form fields.
- Encapsulated styling using CSS literals.
Overall, this component improves the functionality and flexibility of the Lovelace UI.
Tools
GitHub Check: Lint and check format
[failure] 156-156:
Object is possibly 'undefined'.
[failure] 160-160:
Object is possibly 'undefined'.
[failure] 180-180:
Object is possibly 'undefined'.
[failure] 192-192:
Object is possibly 'undefined'.
156-156: Address the static analysis hints related to possible 'undefined' objects.The static analysis tool has flagged the following lines as potentially accessing 'undefined' objects:
- Line 156:
this.hass- Line 160:
this._entityFormEditorData.data- Line 180:
this.hass.localize- Line 192:
this._config.entitiesPlease double-check these lines and ensure that the objects are properly defined before accessing them. If necessary, add appropriate null checks or default values to prevent potential runtime errors.
Also applies to: 160-160, 180-180, 192-192
Tools
GitHub Check: Lint and check format
[failure] 156-156:
Object is possibly 'undefined'.
192-192: The following past review comment is still applicable:bramkragten: use
processEditorEntities(this._config.entities)Please address this comment if it hasn't been resolved yet.
Tools
GitHub Check: Lint and check format
[failure] 192-192:
Object is possibly 'undefined'.
69a24f8 to
b09a66f
Compare
src/panels/lovelace/editor/config-elements/hui-heading-card-editor.ts
Outdated
Show resolved
Hide resolved
b09a66f to
01a47c0
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range and nitpick comments (1)
src/panels/lovelace/editor/hui-sub-form-editor.ts (1)
164-167: Refine typing in_valueChangedmethodIn the
_valueChangedmethod,ev.targetis cast toanyto access thevalueproperty. This usage ofanycan be avoided by specifying the event type more precisely, enhancing type safety and readability.Consider defining a custom event type or using a generic type to avoid using
any:- const value = (ev.detail.value ?? (ev.target as any).value ?? {}) as T; + const value = (ev.detail.value ?? (ev.target as HTMLInputElement).value ?? {}) as T;
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (13)
- src/panels/lovelace/cards/hui-heading-card.ts (1 hunks)
- src/panels/lovelace/cards/types.ts (1 hunks)
- src/panels/lovelace/components/hui-card-edit-mode.ts (1 hunks)
- src/panels/lovelace/create-element/create-card-element.ts (2 hunks)
- src/panels/lovelace/editor/config-elements/hui-entities-editor.ts (1 hunks)
- src/panels/lovelace/editor/config-elements/hui-heading-card-editor.ts (1 hunks)
- src/panels/lovelace/editor/config-elements/hui-map-card-editor.ts (1 hunks)
- src/panels/lovelace/editor/hui-sub-form-editor.ts (1 hunks)
- src/panels/lovelace/editor/lovelace-cards.ts (1 hunks)
- src/panels/lovelace/editor/process-editor-entities.ts (1 hunks)
- src/panels/lovelace/editor/types.ts (1 hunks)
- src/panels/lovelace/sections/hui-grid-section.ts (2 hunks)
- src/translations/en.json (4 hunks)
Files skipped from review as they are similar to previous changes (11)
- src/panels/lovelace/cards/hui-heading-card.ts
- src/panels/lovelace/cards/types.ts
- src/panels/lovelace/components/hui-card-edit-mode.ts
- src/panels/lovelace/create-element/create-card-element.ts
- src/panels/lovelace/editor/config-elements/hui-entities-editor.ts
- src/panels/lovelace/editor/config-elements/hui-heading-card-editor.ts
- src/panels/lovelace/editor/lovelace-cards.ts
- src/panels/lovelace/editor/process-editor-entities.ts
- src/panels/lovelace/editor/types.ts
- src/panels/lovelace/sections/hui-grid-section.ts
- src/translations/en.json
Additional comments not posted (2)
src/panels/lovelace/editor/config-elements/hui-map-card-editor.ts (1)
184-184: LGTM!The change adds a safeguard to prevent potential runtime errors when
this._config.entitiesis undefined. It reflects a defensive programming approach to handle missing data gracefully.src/panels/lovelace/editor/hui-sub-form-editor.ts (1)
1-190: Overall, the component is well-implementedThe
HuiSubFormEditorcomponent is thoughtfully designed, providing flexibility between YAML and visual editing modes. It adheres to best practices in the codebase, and the logic is clear and maintainable.
| protected willUpdate(changedProperties: PropertyValues<this>) { | ||
| if (changedProperties.has("data")) { | ||
| if (this.assertConfig) { | ||
| try { | ||
| this.assertConfig(this.data); | ||
| this._warnings = undefined; | ||
| this._errors = undefined; | ||
| } catch (err: any) { | ||
| const msgs = handleStructError(this.hass, err); | ||
| this._warnings = msgs.warnings ?? [err.message]; | ||
| this._errors = msgs.errors || undefined; | ||
| this._yamlMode = true; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Add unit tests for configuration validation logic
The willUpdate lifecycle method contains critical logic for validating the configuration and handling errors and warnings. Implementing unit tests for this method will help ensure that the validation behaves correctly and prevent future regressions.
| public schema!: LovelaceConfigForm["schema"]; | ||
|
|
||
| public assertConfig?: (config: T) => void; | ||
|
|
||
| public computeLabel?: LovelaceConfigForm["computeLabel"]; | ||
|
|
||
| public computeHelper?: LovelaceConfigForm["computeHelper"]; |
There was a problem hiding this comment.
Annotate reactive properties with @property decorator
The properties schema, assertConfig, computeLabel, and computeHelper are not annotated with the @property decorator. Without this decorator, changes to these properties won't trigger re-rendering, which may lead to the component not updating as expected.
Apply this diff to add the necessary decorators:
+ @property({ attribute: false }) public schema!: LovelaceConfigForm["schema"];
+ @property({ attribute: false }) public assertConfig?: (config: T) => void;
+ @property({ attribute: false }) public computeLabel?: LovelaceConfigForm["computeLabel"];
+ @property({ attribute: false }) public computeHelper?: LovelaceConfigForm["computeHelper"];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.
| public schema!: LovelaceConfigForm["schema"]; | |
| public assertConfig?: (config: T) => void; | |
| public computeLabel?: LovelaceConfigForm["computeLabel"]; | |
| public computeHelper?: LovelaceConfigForm["computeHelper"]; | |
| @property({ attribute: false }) public schema!: LovelaceConfigForm["schema"]; | |
| @property({ attribute: false }) public assertConfig?: (config: T) => void; | |
| @property({ attribute: false }) public computeLabel?: LovelaceConfigForm["computeLabel"]; | |
| @property({ attribute: false }) public computeHelper?: LovelaceConfigForm["computeHelper"]; |
Proposed change
The heading card can have 2 styles : title or subtitle.
The icon, the navigation are also configurable.
Optional entities can be added on the right to display some information with the following configuration :
entity_id,iconandstate content.From a technical POV, this PR also contain an attempt to bring a generic sub form for entity edition.
Type of change
Example configuration
Additional information
Checklist
If user exposed functionality or configuration variables are added/changed:
Summary by CodeRabbit
Release Notes
New Features
hui-entities-editorfor adding, editing, and reordering entities in a drag-and-drop interface.hui-sub-form-editor.Improvements
Translations