Resize card editor #21115
Resize card editor #21115
Conversation
frenck
added
the
Noteworthy
WalkthroughWalkthroughThe changes introduce a customizable grid size picker for the Lovelace card editor in Home Assistant, alongside revisions to existing UI components and properties. Key updates include a new prevent default function, the addition of the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant HaGridSizeEditor as ha-grid-size-picker
participant HuiCardElementEditor as hui-card-element-editor
participant HuiDialogEditCard as hui-dialog-edit-card
participant LovelaceCard
User->>+HuiDialogEditCard: Opens card editor
HuiDialogEditCard->>+HuiCardElementEditor: Initializes editor tabs
HuiCardElementEditor-->>-User: Renders config, visibility, layout tabs
User->>+HaGridSizeEditor: Adjusts grid size
HaGridSizeEditor-->>-User: Updates grid layout
User->>+LovelaceCard: Modifies layout
LovelaceCard-->>-User: Applies changes and emits event
User->>+HuiCardElementEditor: Switches to layout tab
HuiCardElementEditor-->>-User: Renders layout options
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (14)
Additional context usedBiome
Gitleaks
Additional comments not posted (8)
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
Outside diff range comments (8)
src/panels/lovelace/cards/hui-error-card.ts (1)
Line range hint
35-35: Consider replacinganywith a more specific error type to enhance type safety.- catch (err: any) { + catch (err: Error) {src/panels/lovelace/editor/card-editor/hui-card-visibility-editor.ts (1)
Line range hint
41-41: Consider avoiding thedeleteoperator to enhance performance. Set the property toundefinedinstead.- delete newConfig.visibility; + newConfig.visibility = undefined;src/panels/lovelace/editor/card-editor/hui-dialog-delete-card.ts (1)
Line range hint
62-62: Consider replacing the non-null assertion operators with optional chaining to ensure safer code.- ${this.hass!.localize("ui.common.cancel")} + ${this.hass?.localize("ui.common.cancel")} - ${this.hass!.localize("ui.common.delete")} + ${this.hass?.localize("ui.common.delete")}Also applies to: 65-65
src/panels/lovelace/editor/card-editor/hui-dialog-suggest-card.ts (1)
Line range hint
66-66: Several non-null assertion operators are used throughout the file. Consider replacing these with optional chaining to prevent potential runtime errors.- this.hass! + this.hass? - this._cardConfig! + this._cardConfig? - this._sectionConfig! + this._sectionConfig? - this._yamlEditor! + this._yamlEditor? - this.lovelace! + this.lovelace?Also applies to: 110-110, 133-134, 141-141, 160-160, 221-222, 223-224
src/panels/lovelace/sections/hui-grid-section.ts (1)
Line range hint
58-58: Several non-null assertion operators are used. It's recommended to replace these with optional chaining for safer code.- this.lovelace!.config + this.lovelace?.config - this.cards![idx] + this.cards?[idx] - [...oldPath, oldIndex] as [number, number, number] + [...(oldPath ?? []), oldIndex] as [number, number, number] - this._layoutElement!.cards = this._cards + this._layoutElement?.cards = this._cardsAlso applies to: 100-100, 158-158, 162-162
src/panels/lovelace/sections/hui-section.ts (1)
Line range hint
171-171: Several non-null assertion operators are used. Consider replacing these with optional chaining to enhance code safety and maintainability.- this.hass! + this.hass? - this.lovelace! + this.lovelace? - this._layoutElement! + this._layoutElement?Also applies to: 192-192, 193-193, 194-194, 195-195, 196-196, 197-197, 260-260, 262-262
src/panels/lovelace/views/hui-masonry-view.ts (1)
Line range hint
71-71: Avoid using non-null assertion operators to prevent runtime errors.Consider replacing non-null assertions with optional chaining or proper checks to ensure the object properties are defined before accessing them.
Also applies to: 91-91, 119-119, 134-134, 153-154, 165-165, 239-239, 260-260, 276-276
Tools
Biome
[error] 260-260: Forbidden non-null assertion. (lint/style/noNonNullAssertion)
src/panels/lovelace/editor/conditions/ha-card-condition-editor.ts (1)
Line range hint
56-56: Non-null assertion operators are used, which could lead to potential runtime errors if the assumptions fail.Consider replacing non-null assertions with optional chaining or proper existence checks to ensure robustness.
Also applies to: 169-169, 187-188, 191-191
src/panels/lovelace/editor/card-editor/hui-card-layout-editor.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range comments (1)
src/panels/lovelace/editor/card-editor/hui-dialog-edit-card.ts (1)
Line range hint
239-267: Enhance the conditional rendering logic for layout and visibility tabs.- .showLayoutTab=${this._shouldShowLayoutTab()} + .showLayoutTab=${this._params?.path.length === 2 && (!this._containerConfig.type || this._containerConfig.type === "grid")} - .showVisibilityTab=${this._cardConfig?.type !== "conditional"} + .showVisibilityTab=${this._cardConfig && this._cardConfig.type !== "conditional"}This change simplifies the conditional checks directly in the template rendering, making the code more concise and potentially easier to understand at a glance. It removes the need for separate methods that only contribute minimal logic, thus streamlining the component.
piitaya
force-pushed
the
resize_card_editor
branch
from
c6c0ca9
to
28d5609
Compare
There was a problem hiding this comment.
Actionable comments posted: 9
Outside diff range comments (8)
src/panels/lovelace/editor/card-editor/hui-card-visibility-editor.ts (1)
Line range hint
1-52: Consider optimizing property deletion for performance.The method
_valueChangeduses thedeleteoperator, which can impact performance. Consider setting the property toundefinedinstead to potentially enhance performance.- delete newConfig.visibility; + newConfig.visibility = undefined;src/panels/lovelace/sections/hui-grid-section.ts (2)
Line range hint
26-229: Refactor the render method to improve CSS handling and readability.The
rendermethod contains complex CSS and HTML structures that are difficult to read and maintain. Consider extracting CSS to external stylesheets or using CSS-in-JS solutions to improve maintainability and separation of concerns.- static get styles(): CSSResultGroup { + static get styles(): CSSResultGroup { + // Move CSS to external stylesheet or CSS-in-JS solution + }Tools
Biome
[error] 100-100: Forbidden non-null assertion. (lint/style/noNonNullAssertion)
Unsafe fix: Replace with optional chain operator ?. This operator includes runtime checks, so it is safer than the compile-only non-null assertion operator
Line range hint
26-229: Improve accessibility and user interaction in the_addCardmethod.The
_addCardmethod could be improved by adding keyboard accessibility features and providing feedback to the user when an action is taken. Consider adding keyboard shortcuts and visual indicators for successful card addition.Tools
Biome
[error] 100-100: Forbidden non-null assertion. (lint/style/noNonNullAssertion)
Unsafe fix: Replace with optional chain operator ?. This operator includes runtime checks, so it is safer than the compile-only non-null assertion operator
src/panels/lovelace/views/hui-view.ts (2)
Line range hint
92-92: Specify explicit types instead of usingany.Using
anydisables TypeScript's type checking, which can lead to bugs that are hard to detect. Specify explicit types to enhance type safety and maintainability.- catch (e: any) { + catch (e: Error) {This change should be applied wherever
anyis used for error handling in this file.Also applies to: 117-117, 173-173, 185-185, 214-214, 339-339, 344-344
Tools
Biome
[error] 214-214: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
Line range hint
261-261: Avoid non-null assertions; use optional chaining and default values where applicable.Non-null assertions can lead to runtime errors if the object is null. Replace these with optional chaining and provide default values to ensure robustness.
- this.hass!; + this.hass?.;Apply this change across all instances where non-null assertions are used.
Also applies to: 283-283, 284-284, 285-285, 286-286, 287-287, 288-288, 289-289, 290-290, 299-299, 325-325, 327-327
Tools
Biome
[error] 214-214: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
src/panels/lovelace/editor/card-editor/hui-dialog-edit-card.ts (3)
Line range hint
146-146: Consider replacing non-null assertions with safer alternatives.The use of non-null assertions (
!) is detected in multiple lines. While these may be intended to suppress nullability checks, they can lead to runtime errors if assumptions about non-nullability prove incorrect. Consider using optional chaining (?.) or additional runtime checks to ensure safety.- this._containerConfig! + this._containerConfig?. // Apply this pattern to similar instancesAlso applies to: 149-149, 154-154, 170-170, 182-182, 186-186, 192-192, 196-196, 198-198, 226-226, 286-286, 296-296, 312-312, 353-353, 387-387, 399-399, 402-402, 405-405, 406-406
Line range hint
164-164: Reduce complexity in the_savemethod.The
_savemethod has been flagged for excessive cognitive complexity. Consider refactoring this method by breaking it down into smaller, more manageable functions, each handling a specific part of the save process. This will enhance readability and maintainability.
Line range hint
154-154: Specify a more precise type instead ofany.The use of
anyfor the event parameter in the method_enableEscapeKeyClosecan lead to potential issues as it bypasses TypeScript's static type checking. Consider specifying a more precise type to enhance type safety and code clarity.- private _enableEscapeKeyClose = (ev: any) => { + private _enableEscapeKeyClose = (ev: KeyboardEvent) => {
|
I think we should tweak the responsive rules a bit, on some screen sizes it looks like this:
|
| protected firstUpdated(changedProps: PropertyValues<this>): void { | ||
| super.firstUpdated(changedProps); | ||
| try { | ||
| this._cardElement = document.createElement("hui-card"); |
There was a problem hiding this comment.
Instead of creating another card element, can we hook into the preview element we already display?
There was a problem hiding this comment.
Not really because the card is wrapped into a section component.
| } | ||
|
|
||
| private _boundedValue(value: number) { | ||
| return Math.min(Math.max(value, this.min), this.max); |
There was a problem hiding this comment.
frontend/src/common/number/clamp.ts
Line 1 in 28d5609
| data-row=${row} | ||
| data-column=${column} | ||
| ?disabled=${disabled} | ||
| @click=${this._cellClick} |
There was a problem hiding this comment.
Move the click listener to the div above, so we only have 1 listener for all cells.
|
@bramkragten Oh right for the responsive part. I thought I added a max width but no 😅 |
Proposed change
Add layout editor on card to resize cards on grid sections.
NEEDS #21065
Some points to improve in future PR :
Type of change
Example configuration
Additional information
Checklist
If user exposed functionality or configuration variables are added/changed:
Summary by CodeRabbit
New Features
ha-grid-size-pickerfor customizing grid sizes with sliders.ha-grid-layout-sliderfor numerical value control with enhanced accessibility.Enhancements
Bug Fixes
preventDefaultfunction.Style
Documentation