Allow to hide and sort columns in data tables #21168
Conversation
WalkthroughWalkthroughThe recent updates enhance the DataTable components by adding features such as column manipulation, visibility control, and customization options. These changes empower users to reorder columns, hide/show columns dynamically, and utilize additional templates for enhanced rendering. The updates also introduce storage for column configurations, ensuring persistent customizations. Furthermore, similar enhancements have been applied across various panels, improving the user interface and interaction consistency. Changes
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (19)
Additional context usedBiome
Gitleaks
Additional comments not posted (48)
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: 5
Outside diff range comments (15)
src/panels/config/labels/ha-config-labels.ts (1)
[!TIP]
Codebase VerificationNon-null assertions are still present in the codebase and have not been universally replaced with optional chaining.
Occurrences of non-null assertions (
!) were found in the following files:
cast/src/receiver/layout/hc-main.tscast/src/receiver/layout/hc-lovelace.tssrc/util/toast-saved-success.tssrc/util/toast-deleted-success.tssrc/util/launch-screen.tssrc/state/translations-mixin.tssrc/state/themes-mixin.tssrc/state/haptic-mixin.tssrc/state/disconnect-toast-mixin.tssrc/state/context-mixin.tssrc/state/connection-mixin.tssrc/state/auth-mixin.tssrc/state-summary/state-card-script.tssrc/state-summary/state-card-number.tssrc/state-summary/state-card-input_number.tssrc/state-control/water_heater/ha-state-control-water_heater-temperature.tssrc/state-control/valve/ha-state-control-valve-position.tssrc/state-control/valve/ha-state-control-valve-buttons.tssrc/state-control/light/ha-state-control-light-brightness.tssrc/state-control/humidifier/ha-state-control-humidifier-humidity.tssrc/state-control/fan/ha-state-control-fan-speed.tssrc/state-control/cover/ha-state-control-cover-tilt-position.tssrc/state-control/cover/ha-state-control-cover-position.tssrc/state-control/cover/ha-state-control-cover-buttons.tssrc/state-control/climate/ha-state-control-climate-temperature.tssrc/state-control/climate/ha-state-control-climate-humidity.tssrc/state-control/alarm_control_panel/ha-state-control-alarm_control_panel-modes.tssrc/panels/todo/ha-panel-todo.tssrc/panels/todo/dialog-todo-item-editor.tssrc/panels/profile/ha-profile-section-security.tssrc/panels/profile/ha-profile-section-general.tssrc/panels/profile/ha-pick-dashboard-row.tssrc/panels/profile/ha-long-lived-access-tokens-card.tssrc/panels/profile/ha-long-lived-access-token-dialog.tssrc/panels/my/ha-panel-my.tssrc/panels/profile/dialog-ha-mfa-module-setup-flow.tssrc/panels/media-browser/ha-bar-media-player.tssrc/panels/logbook/ha-panel-logbook.tssrc/panels/map/ha-panel-map.tssrc/panels/mailbox/ha-dialog-show-audio-message.tssrc/panels/lovelace/views/hui-sidebar-view.tssrc/panels/lovelace/views/hui-view.tssrc/panels/lovelace/views/hui-sections-view.tssrc/panels/lovelace/hui-root.tssrc/panels/lovelace/special-rows/hui-cast-row.tssrc/panels/lovelace/hui-editor.tssrc/panels/lovelace/editor/view-editor/hui-dialog-edit-view.tssrc/panels/lovelace/editor/config-elements/hui-entities-card-row-editor.tssrc/panels/lovelace/editor/config-elements/hui-statistics-graph-card-editor.tssrc/panels/lovelace/editor/config-elements/hui-gauge-card-editor.tssrc/panels/lovelace/editor/config-elements/hui-select-options-card-feature-editor.tssrc/panels/lovelace/editor/config-elements/hui-tile-card-editor.tssrc/panels/lovelace/editor/config-elements/hui-thermostat-card-editor.tssrc/panels/lovelace/editor/config-elements/hui-sensor-card-editor.tssrc/panels/lovelace/editor/config-elements/hui-light-card-editor.tssrc/panels/lovelace/editor/config-elements/hui-iframe-card-editor.tssrc/panels/lovelace/editor/config-elements/hui-climate-preset-modes-card-feature-editor.tssrc/panels/lovelace/editor/config-elements/hui-conditional-card-editor.tssrc/panels/lovelace/editor/config-elements/hui-entities-card-editor.tssrc/panels/lovelace/editor/config-elements/hui-area-card-editor.tssrc/panels/lovelace/editor/config-elements/hui-logbook-card-editor.tssrc/panels/lovelace/editor/config-elements/hui-alarm-panel-card-editor.tssrc/panels/lovelace/entity-rows/hui-weather-entity-row.tssrc/panels/lovelace/entity-rows/hui-scene-entity-row.tssrc/panels/lovelace/entity-rows/hui-media-player-entity-row.tssrc/panels/lovelace/editor/card-editor/hui-dialog-edit-card.tssrc/panels/lovelace/editor/card-editor/hui-card-picker.tssrc/panels/lovelace/editor/card-editor/hui-entity-picker-table.tssrc/panels/lovelace/editor/card-editor/hui-dialog-create-card.tssrc/panels/lovelace/editor/card-editor/hui-dialog-suggest-card.tssrc/panels/lovelace/entity-rows/hui-lock-entity-row.tssrc/panels/lovelace/entity-rows/hui-input-select-entity-row.tssrc/panels/lovelace/entity-rows/hui-input-datetime-entity-row.tssrc/panels/lovelace/entity-rows/hui-button-entity-row.tssrc/panels/lovelace/elements/hui-state-icon-element.tssrc/panels/lovelace/elements/hui-state-label-element.tssrc/panels/lovelace/cards/hui-calendar-card.tssrc/panels/lovelace/cards/hui-light-card.tssrc/panels/lovelace/cards/hui-thermostat-card.tssrc/panels/lovelace/cards/hui-plant-status-card.tssrc/panels/lovelace/cards/hui-media-control-card.tssrc/panels/lovelace/cards/hui-statistics-graph-card.tssrc/panels/lovelace/cards/hui-tile-card.tssrc/panels/lovelace/cards/hui-glance-card.tssrc/panels/lovelace/cards/hui-starting-card.tssrc/panels/lovelace/cards/hui-logbook-card.tssrc/panels/lovelace/cards/hui-map-card.tssrc/panels/lovelace/cards/hui-button-card.tssrc/panels/lovelace/cards/hui-weather-forecast-card.tssrc/panels/lovelace/cards/hui-picture-glance-card.tssrc/panels/lovelace/cards/hui-picture-entity-card.tssrc/panels/lovelace/cards/hui-picture-elements-card.tssrc/panels/lovelace/cards/hui-recovery-mode-card.tssrc/panels/lovelace/cards/hui-humidifier-card.tssrc/panels/lovelace/cards/hui-shopping-list-card.tssrc/panels/lovelace/cards/hui-statistic-card.tssrc/panels/lovelace/cards/hui-cover-tilt-card-feature.tssrc/panels/lovelace/card-features/hui-alarm-modes-card-feature.tssrc/panels/lovelace/card-features/hui-light-brightness-card-feature.tssrc/panels/lovelace/card-features/hui-light-color-temp-card-feature.tssrc/panels/lovelace/card-features/hui-target-temperature-card-feature.tssrc/panels/lovelace/card-features/hui-target-humidity-card-feature.tssrc/panels/lovelace/card-features/hui-select-options-card-feature.tssrc/panels/lovelace/card-features/hui-fan-speed-card-feature.tssrc/panels/lovelace/card-features/hui-fan-preset-modes-card-feature.tssrc/panels/lovelace/card-features/hui-climate-preset-modes-card-feature.tssrc/panels/lovelace/card-features/hui-climate-hvac-modes-card-feature.tssrc/panels/lovelace/card-features/hui-climate-swing-modes-card-feature.tssrc/panels/lovelace/card-features/hui-humidifier-modes-card-feature.tssrc/panels/lovelace/card-features/hui-humidifier-toggle-card-feature.tssrc/panels/lovelace/card-features/hui-water-heater-operation-modes-card-feature.tssrc/panels/lovelace/card-features/hui-cover-open-close-card-feature.tssrc/panels/lovelace/card-features/hui-cover-position-card-feature.tssrc/panels/lovelace/card-features/hui-cover-tilt-position-card-feature.tssrc/panels/lovelace/card-features/hui-lawn-mower-commands-card-feature.tssrc/panels/lovelace/card-features/hui-vacuum-commands-card-feature.tssrc/panels/lovelace/card-features/hui-update-actions-card-feature.tssrc/panels/lovelace/card-features/hui-numeric-input-card-feature.tssrc/panels/lovelace/card-features/hui-humidifier-modes-card-feature.tssrc/panels/lovelace/card-features/hui-humidifier-modes-card-feature.tssrc/panels/lovelace/card-features/hui-humidifier-modes-card-feature.tssrc/panels/lovelace/card-features/hui-humidifier-modes-card-feature.tssrc/panels/lovelace/card-features/hui-humidifier-modes-card-feature.tssrc/panels/lovelace/card-features/hui-humidifier-modes-card-feature.tssrc/panels/lovelace/card-features/hui-humidifier-modes-card-feature.tssrc/panels/lovelace/card-features/hui-humidifier-modes-card-feature.tssrc/panels/lovelace/card-features/hui-humidifier-modes-card-feature.tssrc/panels/lovelace/card-features/hui-humidifier-modes-card-feature.tssrc/panels/lovelace/card-features/hui-humidifier-modes-card-feature.tssrc/panels/lovelace/card-features/hui-humidifier-modes-card-feature.tssrc/panels/lovelace/card-features/hui-humidifier-modes-card-feature.tssrc/panels/lovelace/card-features/hui-humidifier-modes-card-feature.tssrc/panels/lovelace/card-features/hui-humidifier-modes-card-feature.tssrc/panels/lovelace/card-features/hui-humidifier-modes-card-feature.tssrc/panels/lovelace/card-features/hui-humidifier-modes-card-feature.tssrc/panels/lovelace/card-features/hui-humidifier-modes-card-feature.tssrc/panels/lovelace/card-features/hui-humidifier-modes-card-feature.tssrc/panels/lovelace/card-features/hui-humidifier-modes-card-feature.tssrc/panels/lovelace/card-features/hui-humidifier-modes-card-feature.tssrc/panels/lovelace/card-features/hui-humidifier-modes-card-feature.tssrc/panels/lovelace/card-features/hui-humidifier-modes-card-feature.tssrc/panels/lovelace/card-features/hui-humidifier-modes-card-feature.tsPlease address these occurrences by replacing non-null assertions with optional chaining where appropriate.
Analysis chain
Line range hint
73-113: Enhancements to column properties improve interactivity and user experience.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify removal of non-null assertions and replacement with optional chaining. # Test: Search for non-null assertions. Expect: No occurrences. rg --type typescript '!'Length of output: 61
Script:
#!/bin/bash # Description: Verify removal of non-null assertions and replacement with optional chaining. # Test: Search for non-null assertions. Expect: No occurrences. rg '!\.' --glob '*.ts'Length of output: 273458
src/panels/config/blueprint/ha-blueprint-overview.ts (4)
Line range hint
116-116: Remove unnecessary type annotation.The type annotation on this line is inferred by TypeScript and is therefore redundant.
- private _filter: string = ""; + private _filter = "";
Line range hint
123-123: Specify a more explicit type instead ofany.Using
anycan disable TypeScript's type checking, reducing the benefits of static type checking. If possible, replaceanywith a more specific type to enhance code reliability and maintainability.
Line range hint
376-379: Replace non-null assertions with optional chaining.Non-null assertions can lead to runtime errors if not handled carefully. It's safer to use optional chaining (
?.) which includes runtime checks.- this.hass!.localize + this.hass?.localize - result!.blueprint.metadata.name + result?.blueprint.metadata.name - this._activeSorting!.detail + this._activeSorting?.detail - this._activeGrouping!.value + this._activeGrouping?.value - this._activeCollapsed!.value + this._activeCollapsed?.valueAlso applies to: 407-407, 435-435, 450-450, 469-469, 504-504, 527-527, 528-528
Line range hint
157-157: Remove or utilize the unused variablenarrow.The variable
narrowis declared but not used within the scope of its declaration. If it's not needed, consider removing it to clean up the codebase.- narrow,src/panels/config/voice-assistants/ha-config-voice-assistants-expose.ts (2)
Line range hint
298-298: Refactor to reduce complexity.The
_filteredEntitiesfunction is complex, making it hard to understand and maintain. Consider breaking it down into smaller, more focused functions or using more descriptive variable names to improve readability.
Line range hint
361-361: Replace non-null assertions with optional chaining for safety.Using non-null assertions can lead to runtime errors if the objects are actually null. Use optional chaining (
?.) to safely access properties and methods.- this.hass!.localize + this.hass?.localize - this.exposedEntities![entityId] + this.exposedEntities?.[entityId] - this._extEntities![entityId] + this._extEntities?.[entityId] - this.cloudStatus!.prefs.alexa_enabled + this.cloudStatus?.prefs.alexa_enabled - this._searchParms!.get("assistants")!.split(",") + this._searchParms?.get("assistants")?.split(",")Also applies to: 414-414, 632-632, 636-637, 659-659, 668-668, 701-701, 740-740
src/layouts/hass-tabs-subpage-data-table.ts (1)
Line range hint
223-223: Refactor Required Due to High Complexity.The function around line 223 is flagged for excessive complexity. Consider breaking it down into smaller, more manageable functions to improve readability and maintainability.
- [Original complex function code] + [Refactored code with smaller functions]src/panels/config/devices/ha-config-devices-dashboard.ts (1)
Line range hint
451-570: Enhanced data table columns configuration.The modifications to the
_columnsmethod introduce significant improvements in terms of interactivity and data representation. Properties likesortable,filterable, andgroupableare crucial for a dynamic user experience.Consider refactoring for better readability.
- const columnsConfig = { + const iconColumn = { + ... + }; + const nameColumn = { + ... + }; + return { iconColumn, nameColumn, ... };This refactoring breaks down the complex method into smaller, more manageable parts, improving readability and maintainability.
Tools
Biome
[error] 528-529: Change to an optional chain. (lint/complexity/useOptionalChain)
Unsafe fix: Change to an optional chain.
[error] 535-536: Change to an optional chain. (lint/complexity/useOptionalChain)
Unsafe fix: Change to an optional chain.
[error] 540-540: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
[error] 540-540: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead. (lint/suspicious/noGlobalIsNan)
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.src/panels/config/scene/ha-scene-dashboard.ts (3)
Line range hint
407-407: Refactor_applyFiltersto reduce complexity.This method has a high cognitive complexity due to multiple nested conditions and loops. Consider breaking it down into smaller, more manageable functions. Each filter type could potentially be handled in its own method, improving readability and maintainability.
- private _applyFilters() { + private _applyFilters() { + this._applyCategoryFilters(); + this._applyLabelFilters(); + // Additional filter methods + }
Line range hint
813-813: Refactor_bulkCreateCategoryto reduce complexity.This method handles multiple operations related to category creation and bulk update. Consider separating the creation logic from the bulk update logic into separate methods. This can make the code easier to understand and maintain.
- private async _bulkCreateCategory() { + private async _createCategory() { + // Category creation logic + } + private async _bulkUpdateCategory(categoryId: string) { + // Bulk update logic + }
Line range hint
1025-1025: Specify a more accurate type thanany.The use of
anytype for the event parameter in_handleSortingChangedreduces type safety. Specify a more accurate type to enhance code quality and maintainability.- private _handleSortingChanged(ev: CustomEvent) { + private _handleSortingChanged(ev: HASSDomEvent<SortingChangedEvent>) {src/panels/config/automation/ha-automation-picker.ts (3)
Line range hint
216-216: Replace non-null assertions with optional chaining for safety.Using non-null assertions can lead to runtime errors if the object is indeed null or undefined. It's safer to use optional chaining (
?.) which will gracefully handle these cases.- this._overflowMenu!.open + this._overflowMenu?.open - this._overflowMenu!.close() + this._overflowMenu?.close() - this._overflowAutomation!.state + this._overflowAutomation?.state - this._overflowAutomation!.category + this._overflowAutomation?.categoryAlso applies to: 237-237, 947-947, 958-958, 972-972, 995-995, 1120-1120, 1121-1121
Line range hint
388-388: Refactor functions to reduce complexity.The functions
_applyFiltersand_bulkLabelare flagged for excessive complexity. Consider breaking these functions into smaller, more manageable parts or simplifying the logic to enhance maintainability and readability.Also applies to: 934-934
Line range hint
1130-1130: Specify explicit types instead of usingany.Using
anydisables TypeScript's type checking, which can lead to potential bugs. It's better to specify explicit types for better code safety and clarity.Also applies to: 1154-1154
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range comments (2)
src/panels/config/tags/ha-config-tags.ts (2)
Line range hint
209-223: Specify more precise types instead ofany.Using
anytype disables TypeScript's type checking, which can lead to bugs that are hard to detect. Specify more precise types to improve code safety and maintainability.- private _handleWriteClick = (ev: Event) => this._openWrite((ev.currentTarget as any).tag); + private _handleWriteClick = (ev: Event) => this._openWrite((ev.currentTarget as HTMLElement).tag); // Specify HTMLElement or a more appropriate type
Line range hint
235-309: Replace non-null assertions with safer alternatives.Using non-null assertions can lead to runtime errors if the assumptions about non-nullability are incorrect. Use optional chaining or explicit null checks instead.
- this.hass.auth.external!.fireMessage({ type: "tag/write", payload: { name: tag.name || null, tag: tag.id } }); + this.hass.auth.external?.fireMessage({ type: "tag/write", payload: { name: tag.name || null, tag: tag.id } }); // Use optional chaining
bramkragten
force-pushed
the
column-settings
branch
from
8336f9d to
25b8993
Compare
frenck
added
the
Noteworthy
There was a problem hiding this comment.
Actionable comments posted: 15
Outside diff range comments (11)
src/panels/config/labels/ha-config-labels.ts (1)
Line range hint
87-127: Refactor Template Code for ClarityThe template code for rendering icons and overflow menus is complex and repeated across different columns. Consider extracting this into a dedicated method or using a custom component.
- template: (label) => html`<ha-icon .icon=${label.icon}></ha-icon>` + template: this._renderIconAnd define
_renderIconas:_renderIcon(label) { return html`<ha-icon .icon=${label.icon}></ha-icon>`; }src/panels/config/lovelace/dashboards/ha-config-lovelace-dashboards.ts (1)
Line range hint
118-215: Improve Template Readability and PerformanceThe templates for dashboard items are complex and could be optimized for better readability and performance. Consider breaking down complex templates into simpler, reusable components.
- template: (dashboard) => html`...` + template: this._renderDashboardItemAnd define
_renderDashboardItemas:_renderDashboardItem(dashboard) { return html`...`; // Simplified version of the original template }src/layouts/hass-tabs-subpage-data-table.ts (1)
Line range hint
223-223: Reduce cognitive complexity as indicated by static analysis.Consider refactoring the code to simplify complex logic, possibly by breaking down large methods or simplifying conditional statements.
src/components/data-table/ha-data-table.ts (2)
Line range hint
461-576: Improve code readability and maintainability in row rendering.The method
_renderRowuses deeply nested conditions and loops, making it hard to read and maintain.Refactor to use helper functions to handle specific parts of the row rendering, such as handling templates or conditional rendering.
Tools
Biome
[error] 556-557: Change to an optional chain. (lint/complexity/useOptionalChain)
Unsafe fix: Change to an optional chain.
Line range hint
511-570: Reduce complexity in cell rendering logic.The cell rendering logic in the data table is overly complex and could benefit from simplification or breaking down into smaller components.
Consider extracting parts of this logic into separate, well-named functions or methods to improve readability and testability.
Tools
Biome
[error] 556-557: Change to an optional chain. (lint/complexity/useOptionalChain)
Unsafe fix: Change to an optional chain.
src/panels/config/scene/ha-scene-dashboard.ts (4)
Line range hint
242-338: Ensure consistency and add documentation for column properties.The properties
moveable,showNarrow, andhideablehave been added to various columns. It's important to maintain consistency across the implementation of these properties and add inline documentation to explain their behavior and impact on the user interface.
Line range hint
215-215: Replace non-null assertions with safer alternatives.Several parts of the code use non-null assertions (
!). These can lead to runtime errors if the values are actually null or undefined. Consider using optional chaining (?.) or adding checks to ensure the values are not null before accessing them.- const labels = labelReg && entityRegEntry?.labels; + const labels = labelReg && entityRegEntry?.labels ? entityRegEntry.labels : undefined;Also applies to: 233-233, 843-843, 854-854, 867-867, 890-890, 1086-1086, 1089-1089, 1093-1093, 1094-1094
Line range hint
421-421: Reduce complexity in complex functions.The functions handling bulk actions have excessive cognitive complexity. Consider breaking them down into smaller, more manageable functions or using simpler logic to improve readability and maintainability.
Also applies to: 830-830
Line range hint
1042-1042: Specify a type instead of usingany.The use of
anytype can lead to potential type-safety issues. Consider specifying a more explicit type to enhance type safety and clarity of the code.src/panels/config/entities/ha-config-entities.ts (2)
Line range hint
439-439: Reduce complexity of therendermethod.The complexity of this method exceeds the recommended limit, which could impact maintainability. Consider refactoring to simplify the logic, possibly by breaking it down into smaller, more manageable functions.
- protected render() { + protected render() { + // Refactor suggestion: Break down into smaller methods + }
Line range hint
516-516: Replace non-null assertions with optional chaining.Non-null assertions are unsafe as they bypass TypeScript's strict null checks. Replace them with optional chaining to ensure runtime safety.
- this.hass! + this.hass? - this._entries! + this._entries? - this._labels! + this._labels? - this._entitySources! + this._entitySources? - this._filters.config_entry!.value![0] + this._filters.config_entry?.value?[0] - this._filters.config_entry!.value![0] + this._filters.config_entry?.value?[0] - this._searchParms! + this._searchParms?Also applies to: 537-537, 546-546, 570-570, 577-577, 857-857, 857-857, 1304-1305
| private _sortedColumns = memoizeOne( | ||
| (columns: DataTableColumnContainer, columnOrder?: string[]) => { | ||
| if (!columnOrder || !columnOrder.length) { | ||
| return columns; | ||
| } | ||
|
|
||
| return Object.keys(columns) | ||
| .sort((a, b) => { | ||
| const orderA = columnOrder!.indexOf(a); | ||
| const orderB = columnOrder!.indexOf(b); | ||
| if (orderA !== orderB) { | ||
| if (orderA === -1) { | ||
| return 1; | ||
| } | ||
| if (orderB === -1) { | ||
| return -1; | ||
| } | ||
| } | ||
| return orderA - orderB; | ||
| }) | ||
| .reduce((obj, key) => { | ||
| obj[key] = columns[key]; | ||
| return obj; | ||
| }, {}) as DataTableColumnContainer; | ||
| } | ||
| ); |
There was a problem hiding this comment.
Refactor to reduce complexity and improve maintainability.
The method _sortedColumns has a complex sorting logic that could be simplified or broken down into smaller functions.
Consider breaking down the sorting logic into more manageable functions or utilizing built-in JavaScript sorting capabilities with custom comparators.
Tools
Biome
[error] 300-300: 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
[error] 301-301: 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
| ${Object.entries(columns).map(([key, column]) => { | ||
| if ( | ||
| column.hidden || | ||
| (this.columnOrder && this.columnOrder.includes(key) | ||
| ? this.hiddenColumns?.includes(key) ?? column.defaultHidden | ||
| : column.defaultHidden) | ||
| ) { | ||
| return nothing; |
There was a problem hiding this comment.
Complex conditional logic in rendering headers.
The rendering logic in the header uses complex conditional statements to determine visibility. This could be refactored for clarity.
Consider using a helper function to determine if a column should be hidden based on the various conditions.
Tools
Biome
[error] 376-376: Change to an optional chain. (lint/complexity/useOptionalChain)
Unsafe fix: Change to an optional chain.
[error] 373-373: Excessive complexity detected. (lint/complexity/noExcessiveCognitiveComplexity)
Please refactor this function to reduce its complexity score from 26 to the max allowed complexity 15.
| @@ -235,6 +246,7 @@ | |||
| (column: ClonedDataTableColumnData) => { | |||
| delete column.title; | |||
| delete column.template; | |||
| delete column.extraTemplate; | |||
There was a problem hiding this comment.
Avoid using the delete operator for performance reasons.
Using delete on properties can lead to performance issues due to changes in object shape, which can affect optimization.
- delete column.extraTemplate;
+ column.extraTemplate = undefined;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.
| delete column.extraTemplate; | |
| column.extraTemplate = undefined; |
Tools
Biome
[error] 249-249: Avoid the delete operator which can impact performance. (lint/performance/noDelete)
Unsafe fix: Use an undefined assignment instead.
| @storage({ | ||
| key: "devices-table-hidden-columns", | ||
| state: false, | ||
| subscribe: false, | ||
| }) | ||
| private _activeHiddenColumns?: string[]; |
There was a problem hiding this comment.
Consider initializing _activeHiddenColumns with a default value
Similar to _activeColumnOrder, initializing _activeHiddenColumns with a default value could improve the robustness of the component.
- private _activeHiddenColumns?: string[];
+ private _activeHiddenColumns: string[] = [];This ensures that the property is always in a defined state, potentially simplifying checks and operations on this array.
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.
| @storage({ | |
| key: "devices-table-hidden-columns", | |
| state: false, | |
| subscribe: false, | |
| }) | |
| private _activeHiddenColumns?: string[]; | |
| @storage({ | |
| key: "devices-table-hidden-columns", | |
| state: false, | |
| subscribe: false, | |
| }) | |
| private _activeHiddenColumns: string[] = []; |
| moveable?: boolean; | ||
| hideable?: boolean; | ||
| defaultHidden?: boolean; | ||
| showNarrow?: boolean; |
There was a problem hiding this comment.
Add type annotations instead of any to enhance type safety.
The interface DataTableSortColumnData uses any for the generic type T. Using any can lead to potential bugs and less maintainable code by bypassing TypeScript's type checking.
- export interface DataTableSortColumnData<T = any> {
+ export interface DataTableSortColumnData<T = unknown> {Committable suggestion was skipped due to low confidence.
| const settingsButton = html`<ha-assist-chip | ||
| class="has-dropdown select-mode-chip" | ||
| @click=${this._openSettings} | ||
| .title=${localize("ui.components.subpage-data-table.settings")} | ||
| > | ||
| <ha-svg-icon slot="icon" .path=${mdiCog}></ha-svg-icon> | ||
| </ha-assist-chip>`; | ||
|
|
There was a problem hiding this comment.
Enhance accessibility of the settings button.
Consider adding an aria-label attribute to improve accessibility for screen readers.
- <ha-assist-chip
+ <ha-assist-chip aria-label=${localize("ui.components.subpage-data-table.settings")}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 settingsButton = html`<ha-assist-chip | |
| class="has-dropdown select-mode-chip" | |
| @click=${this._openSettings} | |
| .title=${localize("ui.components.subpage-data-table.settings")} | |
| > | |
| <ha-svg-icon slot="icon" .path=${mdiCog}></ha-svg-icon> | |
| </ha-assist-chip>`; | |
| const settingsButton = html`<ha-assist-chip | |
| class="has-dropdown select-mode-chip" | |
| @click=${this._openSettings} | |
| .title=${localize("ui.components.subpage-data-table.settings")} | |
| aria-label=${localize("ui.components.subpage-data-table.settings")} | |
| > | |
| <ha-svg-icon slot="icon" .path=${mdiCog}></ha-svg-icon> | |
| </ha-assist-chip>`; |
| this.hass.language, | ||
| this.hass.localize | ||
| )} | ||
| .columns=${this._columns(this.hass.localize)} |
There was a problem hiding this comment.
Optimize Event Subscription Handling
Consider unsubscribing from events when the component is disconnected to prevent potential memory leaks.
+ disconnectedCallback() {
+ if (this._tagScannedUnsub) {
+ this._tagScannedUnsub();
+ this._tagScannedUnsub = null;
+ }
+ }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.
| .columns=${this._columns(this.hass.localize)} | |
| .columns=${this._columns(this.hass.localize)} | |
| + disconnectedCallback() { | |
| + if (this._tagScannedUnsub) { | |
| + this._tagScannedUnsub(); | |
| + this._tagScannedUnsub = null; | |
| + } | |
| + } |
| private _columns = memoizeOne((localize: LocalizeFunc) => { | ||
| const columns: DataTableColumnContainer<TagRowData> = { | ||
| icon: { | ||
| title: "", | ||
| type: "icon-button", | ||
| template: (tag) => | ||
| html` <ha-icon-button | ||
| .tag=${tag} | ||
| @click=${this._handleAutomationClick} | ||
| .label=${this.hass.localize( | ||
| "ui.panel.config.tag.create_automation" | ||
| )} | ||
| .path=${mdiRobot} | ||
| ></ha-icon-button>`, | ||
| }; | ||
| columns.edit = { | ||
| moveable: false, | ||
| showNarrow: true, | ||
| label: localize("ui.panel.config.tag.headers.icon"), | ||
| type: "icon", | ||
| template: (tag) => html`<tag-image .tag=${tag}></tag-image>`, | ||
| }, | ||
| display_name: { | ||
| title: localize("ui.panel.config.tag.headers.name"), | ||
| main: true, | ||
| sortable: true, | ||
| filterable: true, | ||
| grows: true, | ||
| }, | ||
| last_scanned_datetime: { | ||
| title: localize("ui.panel.config.tag.headers.last_scanned"), | ||
| sortable: true, | ||
| direction: "desc", | ||
| width: "20%", | ||
| template: (tag) => html` | ||
| ${tag.last_scanned_datetime | ||
| ? html`<ha-relative-time | ||
| .hass=${this.hass} | ||
| .datetime=${tag.last_scanned_datetime} | ||
| capitalize | ||
| ></ha-relative-time>` | ||
| : this.hass.localize("ui.panel.config.tag.never_scanned")} | ||
| `, | ||
| }, | ||
| }; | ||
| if (this._canWriteTags) { | ||
| columns.write = { | ||
| title: "", | ||
| label: localize("ui.panel.config.tag.headers.write"), | ||
| type: "icon-button", | ||
| showNarrow: true, | ||
| template: (tag) => | ||
| html` <ha-icon-button | ||
| html`<ha-icon-button | ||
| .tag=${tag} | ||
| @click=${this._handleEditClick} | ||
| .label=${this.hass.localize("ui.panel.config.tag.edit")} | ||
| .path=${mdiCog} | ||
| @click=${this._handleWriteClick} | ||
| .label=${this.hass.localize("ui.panel.config.tag.write")} | ||
| .path=${mdiContentDuplicate} | ||
| ></ha-icon-button>`, | ||
| }; | ||
| return columns; | ||
| } | ||
| ); | ||
| columns.automation = { | ||
| title: "", | ||
| type: "icon-button", | ||
| showNarrow: true, | ||
| template: (tag) => | ||
| html`<ha-icon-button | ||
| .tag=${tag} | ||
| @click=${this._handleAutomationClick} | ||
| .label=${this.hass.localize("ui.panel.config.tag.create_automation")} | ||
| .path=${mdiRobot} | ||
| ></ha-icon-button>`, | ||
| }; | ||
| columns.edit = { | ||
| title: "", | ||
| type: "icon-button", | ||
| showNarrow: true, | ||
| hideable: false, | ||
| moveable: false, | ||
| template: (tag) => | ||
| html`<ha-icon-button | ||
| .tag=${tag} | ||
| @click=${this._handleEditClick} | ||
| .label=${this.hass.localize("ui.panel.config.tag.edit")} | ||
| .path=${mdiCog} | ||
| ></ha-icon-button>`, | ||
| }; | ||
| return columns; | ||
| }); |
There was a problem hiding this comment.
Refactor Column Definitions for Clarity and Maintainability
The column definitions are well-structured but could benefit from splitting into smaller, more manageable functions or methods, especially since some columns have complex templates. This will improve readability and maintainability.
- private _columns = memoizeOne((localize: LocalizeFunc) => { ... });
+ private _defineIconColumn(localize) { ... }
+ private _defineDisplayNameColumn(localize) { ... }
+ private _defineLastScannedColumn(localize) { ... }
+ private _columns = memoizeOne((localize: LocalizeFunc) => {
+ return {
+ icon: this._defineIconColumn(localize),
+ display_name: this._defineDisplayNameColumn(localize),
+ last_scanned_datetime: this._defineLastScannedColumn(localize),
+ };
+ });Committable suggestion was skipped due to low confidence.
| template: narrow | ||
| ? undefined | ||
| : (backup) => | ||
| html`${backup.name} | ||
| <div class="secondary">${backup.path}</div>`, | ||
| }, | ||
| path: { | ||
| title: localize("ui.panel.config.backup.path"), | ||
| hidden: !narrow, |
There was a problem hiding this comment.
Refactor to use template literals for better readability and performance.
The template rendering logic in the name column can be improved by using template literals instead of the ternary operator for conditional rendering. This aligns with best practices and improves code readability.
- template: narrow ? undefined : (backup) => html`${backup.name}<div class="secondary">${backup.path}</div>`,
+ template: (backup) => narrow ? undefined : html`${backup.name}<div class="secondary">${backup.path}</div>`,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.
| template: narrow | |
| ? undefined | |
| : (backup) => | |
| html`${backup.name} | |
| <div class="secondary">${backup.path}</div>`, | |
| }, | |
| path: { | |
| title: localize("ui.panel.config.backup.path"), | |
| hidden: !narrow, | |
| template: (backup) => narrow | |
| ? undefined | |
| : html`${backup.name} | |
| <div class="secondary">${backup.path}</div>`, |
| private _sortedColumns = memoizeOne( | ||
| ( | ||
| columns: DataTableColumnContainer, | ||
| columnOrder: string[] | undefined, | ||
| hiddenColumns: string[] | undefined | ||
| ) => | ||
| Object.keys(columns) | ||
| .filter((col) => !columns[col].hidden) | ||
| .sort((a, b) => { | ||
| const orderA = columnOrder?.indexOf(a) ?? -1; | ||
| const orderB = columnOrder?.indexOf(b) ?? -1; | ||
| const hiddenA = | ||
| hiddenColumns?.includes(a) ?? Boolean(columns[a].defaultHidden); | ||
| const hiddenB = | ||
| hiddenColumns?.includes(b) ?? Boolean(columns[b].defaultHidden); | ||
| if (hiddenA !== hiddenB) { | ||
| return hiddenA ? 1 : -1; | ||
| } | ||
| if (orderA !== orderB) { | ||
| if (orderA === -1) { | ||
| return 1; | ||
| } | ||
| if (orderB === -1) { | ||
| return -1; | ||
| } | ||
| } | ||
| return orderA - orderB; | ||
| }) | ||
| .reduce( | ||
| (arr, key) => { | ||
| arr.push({ key, ...columns[key] }); | ||
| return arr; | ||
| }, | ||
| [] as (DataTableColumnData & { key: string })[] | ||
| ) | ||
| ); |
There was a problem hiding this comment.
Refactor to simplify the column sorting logic.
The _sortedColumns method has a complex logic for sorting columns based on order and visibility. Consider refactoring to reduce complexity and improve maintainability.
- // Existing complex sorting logic
+ // Simplified sorting logic using modern JavaScript featuresCommittable suggestion was skipped due to low confidence.
Tools
Biome
[error] 47-47: Excessive complexity detected. (lint/complexity/noExcessiveCognitiveComplexity)
Please refactor this function to reduce its complexity score from 17 to the max allowed complexity 15.
Proposed change
Allow to hide and sort columns in data tables, change the way mobile data rows are created to respect hiding and sorting on the secondary line of the main field there.
Type of change
Example configuration
Additional information
Checklist
If user exposed functionality or configuration variables are added/changed:
Summary by CodeRabbit
New Features
Enhancements
User Interface
Translations