Add more config option to heading entity element #22063
Conversation
b261f3b to
49cf7ca
Compare
49cf7ca to
7e5df8e
Compare
WalkthroughWalkthroughThe changes involve updates to the Changes
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 (
|
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range and nitpick comments (4)
src/panels/lovelace/cards/heading/hui-heading-entity.ts (3)
58-58: LGTM: Merging DEFAULT_CONFIGThe inclusion of
DEFAULT_CONFIGusing the spread operator is a good approach. It ensures default values are applied while allowing user configurations to override them.For improved clarity, consider destructuring the config object:
- ...DEFAULT_CONFIG, - tap_action: { - action: "none", - }, - ...config, + ...DEFAULT_CONFIG, + tap_action: { + action: "none", + }, + ...config, + show_icon: config.show_icon ?? DEFAULT_CONFIG.show_icon, + show_state: config.show_state ?? DEFAULT_CONFIG.show_state,This explicitly shows which new properties are being added and allows for easier debugging if needed.
138-146: LGTM: Conditional rendering for show_iconThe implementation of the
show_iconoption is correct and aligns with the PR objectives.For consistency with other parts of the code, consider using the
nothingliteral instead of an empty string for the false case:- ${config.show_icon - ? html` - <ha-state-icon - .hass=${this.hass} - .icon=${config.icon} - .stateObj=${stateObj} - ></ha-state-icon> - ` - : nothing} + ${config.show_icon + ? html` + <ha-state-icon + .hass=${this.hass} + .icon=${config.icon} + .stateObj=${stateObj} + ></ha-state-icon> + ` + : nothing}This change makes the code more consistent with the
show_statecondition below.
Line range hint
1-200: Overall implementation looks good, some additional steps neededThe changes successfully implement the new
show_iconandshow_stateconfiguration options for the heading entity element. The code is well-integrated with the existing structure and follows consistent patterns.To complete this PR, please consider the following steps:
- Add unit tests to verify the new functionality.
- Update the documentation to reflect these new configuration options.
- Provide an example configuration snippet in the PR description to help with testing.
- Confirm that you've tested these changes locally and update the PR checklist accordingly.
To ensure maintainability and ease of testing, consider extracting the icon and state rendering logic into separate methods. This would make the
rendermethod cleaner and allow for easier unit testing of each part independently.src/panels/lovelace/cards/types.ts (1)
508-511: Update documentation and provide migration guideWith the introduction of new properties and the renaming of
contenttostate_content, it's crucial to update the documentation and provide a migration guide for users.Please ensure the following:
- Update the Home Assistant documentation to reflect these new configuration options.
- Provide a migration guide for users to update their existing configurations.
- Update any relevant code examples or templates that use the
HeadingEntityConfig.Consider opening a separate PR for documentation updates or linking to a documentation PR in this PR's description.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- src/panels/lovelace/cards/heading/hui-heading-entity.ts (3 hunks)
- src/panels/lovelace/cards/types.ts (1 hunks)
- src/panels/lovelace/editor/heading-entity/hui-heading-entity-editor.ts (6 hunks)
- src/translations/en.json (1 hunks)
Additional context used
Biome
src/panels/lovelace/editor/heading-entity/hui-heading-entity-editor.ts
[error] 199-199: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
Additional comments not posted (9)
src/panels/lovelace/cards/heading/hui-heading-entity.ts (2)
27-27: LGTM: Import of DEFAULT_CONFIGThe addition of
DEFAULT_CONFIGimport is consistent with the PR objectives and follows good practices by providing default configurations.
147-155: LGTM: Conditional rendering for show_stateThe implementation of the
show_stateoption is correct and aligns with the PR objectives. The use ofconfig.state_contentis consistent with the existing code, and the conditional rendering is properly implemented.src/panels/lovelace/cards/types.ts (1)
510-511: LGTM: New configuration options addedThe addition of
show_stateandshow_iconproperties aligns with the PR objectives and provides users with more flexibility in configuring heading entities.src/panels/lovelace/editor/heading-entity/hui-heading-entity-editor.ts (6)
31-34: Definition ofDEFAULT_CONFIGenhances default handlingThe addition of
DEFAULT_CONFIGensures that default values forshow_stateandshow_iconare consistently applied, enhancing code maintainability and readability.
63-66: MergingDEFAULT_CONFIGwith user config applies defaults correctlyThe updated
setConfigmethod correctly mergesDEFAULT_CONFIGwith the provided configuration, ensuring that default values are applied when they are not specified by the user.
77-115: New appearance schema improves user customization optionsThe addition of the "appearance" section in the schema allows users to customize the display of the heading entity, including the icon, displayed elements, and state content. This enhances the flexibility and user experience of the configuration editor.
136-141: Efficient computation ofdisplayed_elementsThe
_displayedElementsfunction correctly constructs thedisplayed_elementsarray based on theshow_stateandshow_iconproperties. UsingmemoizeOneimproves performance by preventing unnecessary recalculations.
150-153: Data preparation for form binding is accurateIncluding
displayed_elementsin thedataobject ensures that the form is properly bound to the current configuration, facilitating user interaction with the new options.
228-230: Update label computation for new schema fieldsAdding cases for
"state_content","displayed_elements", and"appearance"in_computeLabelCallbackensures that labels for these new fields are correctly localized, improving the user interface.
| @@ -505,8 +505,10 @@ export interface TileCardConfig extends LovelaceCardConfig { | |||
|
|
|||
| export interface HeadingEntityConfig { | |||
| entity: string; | |||
| content?: string | string[]; | |||
| state_content?: string | string[]; | |||
There was a problem hiding this comment.
Breaking change: Rename content to state_content
The property content has been renamed to state_content. While this change improves clarity, it's a breaking change that will affect existing configurations using the content property.
To address this:
- Consider keeping both
contentandstate_contentfor backward compatibility, deprecatingcontent. - Update the implementation to support both properties, prioritizing
state_contentif both are present. - Add a deprecation warning when
contentis used.
Example:
export interface HeadingEntityConfig {
// ...
/** @deprecated Use state_content instead */
content?: string | string[];
state_content?: string | string[];
// ...
}
Proposed change
Add show_state and show_icon option to heading entities
Type of change
Example configuration
Additional information
Checklist
If user exposed functionality or configuration variables are added/changed:
Summary by CodeRabbit
New Features
HuiHeadingEntitycomponent, enhancing flexibility in display options.Documentation
These changes improve user customization and control over how heading entities are displayed in the interface.