-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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.
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 min/max row/columns to resize card editor #21244
Conversation
Note Reviews pausedUse the following commands to manage reviews:
WalkthroughThe updates enhance Lovelace panels and components by incorporating dynamic grid layout calculations. Key additions include the introduction of Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as Lovelace UI
participant Card as LovelaceCard
participant Editor as HaGridSizeEditor
User->>UI: Load Lovelace Panel
UI->>Card: Instantiates Card
Card->>Card: Call getLayoutOptions()
Card->>UI: Return layout options
UI->>Editor: Open Layout Editor
Editor->>Editor: Calculate disabledColumns and disabledRows
Editor->>UI: Render with updated grid layout and sliders
sequenceDiagram
participant User
participant UI as Lovelace UI
participant Slider as HaGridLayoutSlider
User->>UI: Load Grid Layout Editor
UI->>Slider: Instantiate Slider
Slider->>Slider: Adjust track structure, background and active elements
Slider->>UI: Render with new styles and behaviors
These diagrams illustrate the interaction flow for loading Lovelace cards with dynamic layout options and updating the grid layout editor with redesigned sliders. 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Outside diff range comments (4)
src/panels/lovelace/types.ts (1)
Line range hint
118-118
: Avoid usingany
type.Replace
any
with a more specific type to avoid disabling type checking rules.- export interface LovelaceGenericElementEditor<C = any> extends HTMLElement { + export interface LovelaceGenericElementEditor<C = unknown> extends HTMLElement { - setConfig(config: any): void; + setConfig(config: Record<string, unknown>): void;Also applies to: 122-122
src/panels/lovelace/editor/card-editor/hui-card-layout-editor.ts (1)
Line range hint
187-187
: Avoid usingdelete
operator and non-null assertion.Replace
delete
withundefined
assignment and non-null assertion with optional chaining.- delete newConfig.layout_options; + newConfig.layout_options = undefined; - if (newConfig.layout_options!.grid_columns === undefined) { + if (newConfig.layout_options?.grid_columns === undefined) { - if (newConfig.layout_options!.grid_rows === undefined) { + if (newConfig.layout_options?.grid_rows === undefined) { - if (Object.keys(newConfig.layout_options!).length === 0) { + if (Object.keys(newConfig.layout_options ?? {}).length === 0) { - delete newConfig.layout_options; + newConfig.layout_options = undefined;Also applies to: 205-205, 206-206, 208-208, 209-209, 211-211, 212-212
src/panels/lovelace/sections/hui-grid-section.ts (1)
Line range hint
91-91
: Avoid using non-null assertion.Replace non-null assertion with optional chaining.
- return this._cardConfigKeys.get(cardConfig)!; + return this._cardConfigKeys.get(cardConfig) ?? ''; - const { rows, columns } = computeSizeOnGrid(layoutOptions!); + const { rows, columns } = computeSizeOnGrid(layoutOptions ?? {}); - this.lovelace!.saveConfig(newConfig); + this.lovelace?.saveConfig(newConfig); - this.lovelace!.config, + this.lovelace?.config,Also applies to: 133-133, 189-189, 193-193
src/panels/lovelace/editor/card-editor/ha-grid-layout-slider.ts (1)
Line range hint
141-141
: Add explicit type annotation forsavedValue
.The variable
savedValue
implicitly has theany
type. Declare a type or initialize the variable with some value.- let savedValue; + let savedValue: number | undefined;
const grid_columns = 2; | ||
let grid_rows = 1; | ||
if (this._config?.features?.length) { | ||
options.grid_rows += Math.ceil((this._config.features.length * 2) / 3); | ||
grid_rows += Math.ceil((this._config.features.length * 2) / 3); | ||
} | ||
if (this._config?.vertical) { | ||
options.grid_rows++; | ||
grid_rows!++; | ||
} | ||
return options; | ||
return { | ||
grid_columns, | ||
grid_rows, | ||
grid_min_rows: grid_rows, | ||
grid_max_rows: grid_rows, | ||
grid_min_columns: grid_columns, | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replace non-null assertions with optional chaining.
Non-null assertions can lead to runtime errors. Use optional chaining instead.
const grid_columns = 2;
let grid_rows = 1;
if (this._config?.features?.length) {
grid_rows += Math.ceil((this._config.features.length * 2) / 3);
}
if (this._config?.vertical) {
grid_rows++;
}
return {
grid_columns,
grid_rows,
grid_min_rows: grid_rows,
grid_max_rows: grid_rows,
grid_min_columns: grid_columns,
};
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.
const grid_columns = 2; | |
let grid_rows = 1; | |
if (this._config?.features?.length) { | |
options.grid_rows += Math.ceil((this._config.features.length * 2) / 3); | |
grid_rows += Math.ceil((this._config.features.length * 2) / 3); | |
} | |
if (this._config?.vertical) { | |
options.grid_rows++; | |
grid_rows!++; | |
} | |
return options; | |
return { | |
grid_columns, | |
grid_rows, | |
grid_min_rows: grid_rows, | |
grid_max_rows: grid_rows, | |
grid_min_columns: grid_columns, | |
}; | |
const grid_columns = 2; | |
let grid_rows = 1; | |
if (this._config?.features?.length) { | |
grid_rows += Math.ceil((this._config.features.length * 2) / 3); | |
} | |
if (this._config?.vertical) { | |
grid_rows++; | |
} | |
return { | |
grid_columns, | |
grid_rows, | |
grid_min_rows: grid_rows, | |
grid_max_rows: grid_rows, | |
grid_min_columns: grid_columns, | |
}; |
Tools
Biome
[error] 131-131: Forbidden non-null assertion.
(lint/style/noNonNullAssertion)
const disabledColumns = | ||
this.columnMin !== undefined && | ||
this.columnMin !== undefined && | ||
this.columnMin === this.columnMax; | ||
const disabledRows = | ||
this.rowMin !== undefined && | ||
this.rowMin !== undefined && | ||
this.rowMin === this.rowMax; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct the redundant condition in disabledColumns
and disabledRows
.
The conditions this.columnMin !== undefined
and this.rowMin !== undefined
are repeated.
- const disabledColumns =
- this.columnMin !== undefined &&
- this.columnMin !== undefined &&
- this.columnMin === this.columnMax;
+ const disabledColumns =
+ this.columnMin !== undefined &&
+ this.columnMin === this.columnMax;
- const disabledRows =
- this.rowMin !== undefined &&
- this.rowMin !== undefined &&
- this.rowMin === this.rowMax;
+ const disabledRows =
+ this.rowMin !== undefined &&
+ this.rowMin === this.rowMax;
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.
const disabledColumns = | |
this.columnMin !== undefined && | |
this.columnMin !== undefined && | |
this.columnMin === this.columnMax; | |
const disabledRows = | |
this.rowMin !== undefined && | |
this.rowMin !== undefined && | |
this.rowMin === this.rowMax; | |
const disabledColumns = | |
this.columnMin !== undefined && | |
this.columnMin === this.columnMax; | |
const disabledRows = | |
this.rowMin !== undefined && | |
this.rowMin === this.rowMax; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
Outside diff range comments (2)
src/panels/lovelace/cards/hui-thermostat-card.ts (1)
Line range hint
84-84
: Replace non-null assertions with optional chaining.Non-null assertions can lead to runtime errors. Use optional chaining instead.
- entityId: this._config!.entity, + entityId: this._config?.entity, - .label=${this.hass!.localize( + .label=${this.hass?.localize( - .features=${this._config.features} + .features=${this._config?.features}Also applies to: 128-128, 153-153
src/panels/lovelace/cards/hui-humidifier-card.ts (1)
Line range hint
92-92
: Replace non-null assertions with optional chaining.Non-null assertions can lead to runtime errors. Use optional chaining instead.
- entityId: this._config!.entity, + entityId: this._config?.entity, - .label=${this.hass!.localize( + .label=${this.hass?.localize( - .features=${this._config.features} + .features=${this._config?.features}Also applies to: 136-136, 161-161
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
public getLayoutOptions(): LovelaceLayoutOptions { | ||
if (this._config?.footer) { | ||
return { | ||
grid_columns: 2, | ||
}; | ||
} | ||
return { | ||
grid_columns: 2, | ||
grid_rows: 2, | ||
grid_min_rows: 2, | ||
}; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refactor for readability and maintainability.
The getLayoutOptions
method can be refactored to improve readability and maintainability. Consider extracting the logic into smaller helper functions.
public getLayoutOptions(): LovelaceLayoutOptions {
const grid_columns = 2;
const grid_min_columns = 2;
const { grid_rows, grid_min_rows, grid_max_rows } = this.calculateGridRows();
return {
grid_columns,
grid_rows,
grid_min_rows,
grid_max_rows,
grid_min_columns,
};
}
private calculateGridRows() {
let grid_rows = 2;
let grid_min_rows = 2;
let grid_max_rows = 2;
if (this._config?.footer) {
grid_rows = 1;
grid_min_rows = 1;
grid_max_rows = 1;
}
return { grid_rows, grid_min_rows, grid_max_rows };
}
const grid_columns = 2; | ||
let grid_rows = 1; | ||
if (this._config?.features?.length) { | ||
options.grid_rows += Math.ceil((this._config.features.length * 2) / 3); | ||
const featureHeight = Math.ceil((this._config.features.length * 2) / 3); | ||
grid_rows += featureHeight; | ||
} | ||
if (this._config?.vertical) { | ||
options.grid_rows++; | ||
grid_rows!++; | ||
} | ||
return options; | ||
return { | ||
grid_columns, | ||
grid_rows, | ||
grid_min_rows: grid_rows, | ||
grid_min_columns: grid_columns, | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refactor for readability and maintainability.
The getLayoutOptions
method can be refactored to improve readability and maintainability. Consider extracting the logic into smaller helper functions.
public getLayoutOptions(): LovelaceLayoutOptions {
const grid_columns = 2;
const grid_min_columns = 2;
const { grid_rows, grid_min_rows, grid_max_rows } = this.calculateGridRows();
return {
grid_columns,
grid_rows,
grid_min_rows,
grid_max_rows,
grid_min_columns,
};
}
private calculateGridRows() {
let grid_rows = 1;
let grid_min_rows = 1;
let grid_max_rows = 1;
if (this._config?.features?.length) {
const featureHeight = Math.ceil((this._config.features.length * 2) / 3);
grid_rows += featureHeight;
grid_min_rows += featureHeight;
grid_max_rows += featureHeight;
}
if (this._config?.vertical) {
grid_rows++;
grid_min_rows++;
grid_max_rows++;
}
return { grid_rows, grid_min_rows, grid_max_rows };
}
Tools
Biome
[error] 132-132: Forbidden non-null assertion.
(lint/style/noNonNullAssertion)
Replace non-null assertions with optional chaining.
Non-null assertions can lead to runtime errors. Use optional chaining instead.
const grid_columns = 2;
let grid_rows = 1;
if (this._config?.features?.length) {
const featureHeight = Math.ceil((this._config.features.length * 2) / 3);
grid_rows += featureHeight;
}
if (this._config?.vertical) {
grid_rows++;
}
return {
grid_columns,
grid_rows,
grid_min_rows: grid_rows,
grid_min_columns: grid_columns,
};
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.
const grid_columns = 2; | |
let grid_rows = 1; | |
if (this._config?.features?.length) { | |
options.grid_rows += Math.ceil((this._config.features.length * 2) / 3); | |
const featureHeight = Math.ceil((this._config.features.length * 2) / 3); | |
grid_rows += featureHeight; | |
} | |
if (this._config?.vertical) { | |
options.grid_rows++; | |
grid_rows!++; | |
} | |
return options; | |
return { | |
grid_columns, | |
grid_rows, | |
grid_min_rows: grid_rows, | |
grid_min_columns: grid_columns, | |
}; | |
const grid_columns = 2; | |
let grid_rows = 1; | |
if (this._config?.features?.length) { | |
const featureHeight = Math.ceil((this._config.features.length * 2) / 3); | |
grid_rows += featureHeight; | |
} | |
if (this._config?.vertical) { | |
grid_rows++; | |
} | |
return { | |
grid_columns, | |
grid_rows, | |
grid_min_rows: grid_rows, | |
grid_min_columns: grid_columns, | |
}; |
Tools
Biome
[error] 132-132: Forbidden non-null assertion.
(lint/style/noNonNullAssertion)
@coderabbitai pause |
dc45e58
to
efb0ef7
Compare
Proposed change
Allows card to provide a min and max rows and columns for resize card editor in a grid.
I implemented it on 11 cards (future PRs will be open for others cards):
Type of change
Example configuration
Additional information
Checklist
If user exposed functionality or configuration variables are added/changed:
Summary by CodeRabbit
New Features
Improvements
HuiEntityCard
,HuiHumidifierCard
,HuiThermostatCard
, andHuiTileCard
with new methods to determine layout options based on card configurations.HaGridLayoutSlider
with refined styles and behaviors for better user interaction.HuiCardLayoutEditor
to better handle default and computed grid size options.Refactor
computeSizeOnGrid
function to ensure values are within specified bounds.