Add dynamic dropdown/selector options from entity attributes and templates (Ref #137)#1
Add dynamic dropdown/selector options from entity attributes and templates (Ref #137)#1kristofferR wants to merge 8 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
📜 Recent review details🔇 Additional comments (1)
📝 WalkthroughWalkthroughThis PR adds dynamic option generation for dropdowns and selectors. Contracts extend dropdown/selector options to support arrays or template strings; utilities provide attribute heuristics, default-action inference, HTML decoding, and flexible list parsing. BaseCustomFeature resolves and caches options from static arrays, entity attributes, or rendered templates, building individual options with templates and default actions. Dropdown and selector components consume resolved options for rendering and state sync. The row editor gains option-template editing mode, options-source picker UI, and autofill logic that detects source types and pre-fills option templates. README documents the feature with Example 9. ChangesDynamic Option Generation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7549b9b9c6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (typeof config == 'string' && config.trim()) { | ||
| const rendered = String(this.renderTemplate(config)); | ||
| if (rendered == this.optionsSignature) { | ||
| return false; |
There was a problem hiding this comment.
When options is a template, the cache key only includes the rendered list text, so a config update that changes option_template but leaves the list unchanged keeps using the old cloned options. For example, editing a dynamic selector's option_template.label or tap_action in the UI will trigger a render, but this.options is not rebuilt because rendered == this.optionsSignature, so the old labels/actions remain until the options source changes or the card is remounted.
Useful? React with 👍 / 👎.
| switch (field) { | ||
| case 'option': | ||
| entries = this.activeEntry?.options ?? []; | ||
| entries = (this.activeEntry?.options as IOption[]) ?? []; |
There was a problem hiding this comment.
This commit makes primitive option arrays valid at runtime, but the GUI editor still casts the raw entries to IOption[]. When a user opens a config like options: [A, B] in the visual editor, each string is passed through the option-list/editor code as though it were an object, so the list cannot show the actual option value and editing the option attempts to deepSet fields on a string. Primitive entries need to be converted to { option: ... } (or otherwise handled) before they enter the object editor path.
Useful? React with 👍 / 👎.
| const autofill = this.renderTemplate( | ||
| (this.config.autofill_entity_id ?? AUTOFILL) as unknown as string, | ||
| ); |
There was a problem hiding this comment.
For dynamic select/input_select options, the default select_option action is controlled only by the parent feature's autofill_entity_id. If a user edits the option template and turns Autofill off (option_template.autofill_entity_id: false) to prevent generated options from getting an implicit tap action, this check still evaluates the parent and adds the default service call to every generated option.
Useful? React with 👍 / 👎.
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/classes/custom-feature-dropdown.ts (1)
172-266:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftSelected option appearance not re-evaluated when options change.
When
setOptions()returnstrue(indicating template-generated options changed), the code setsshould = truebut does not re-enter the block at lines 175-266 that re-evaluatesselectedIcon,selectedLabel, andselectedStyles. This means if a template-based option list changes the selected option's appearance fields without changinghass/stateObj/value, the select button will continue showing stale icon/label/styles until the next hass update.For example, if
option_templateincludes a time-dependent icon and the template output changes at midnight, the select button won't reflect the new icon until the next entity state change.🔧 Proposed fix
shouldUpdate(changedProperties: PropertyValues) { let should = super.shouldUpdate(changedProperties); - if (this.setOptions()) { + const optionsChanged = this.setOptions(); + if (optionsChanged) { should = true; } if ( + optionsChanged || changedProperties.has('hass') || changedProperties.has('stateObj') || changedProperties.has('value') ) {This ensures the selected option's appearance is re-evaluated whenever the options list changes, not just when entity state or value changes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/classes/custom-feature-dropdown.ts` around lines 172 - 266, The selected option appearance isn't re-evaluated when template-generated options change because setOptions() only sets should = true but doesn't trigger the block that computes selectedIcon/selectedLabel/selectedStyles; update the logic so option-list changes force that re-evaluation (either by incorporating the boolean result of setOptions() into the same condition that checks changedProperties.has('hass'|'stateObj'|'value') or by setting an optionsChanged flag and including it in that condition). Specifically modify the control flow around setOptions(), the changedProperties check, and the section that iterates this.options and assigns selectedIcon/selectedLabel/selectedStyles (referencing setOptions(), changedProperties, this.options, renderTemplate(), selectedIcon, selectedLabel, selectedStyles, value, thumbType) so template-driven option updates immediately recompute and apply the selected option appearance.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Line 235: Replace the vague link text "found [here]" with a descriptive phrase
that includes the linked resource and context (e.g., "in the Home Assistant
websocket HassEntity type definition") so readers know what the link points to;
update the sentence referencing stateObj to read something like "The structure
of stateObj can be found in the Home Assistant websocket HassEntity type
definition" and keep the existing URL target unchanged.
- Line 209: Reword the sentence about options to distinguish template
expressions from literal strings: explain that an options "template expression"
must be wrapped in {{ }} (e.g., options: "{{ myTemplate }}" or options: "{{
userChoices }}" ) whereas a plain literal string like options: "Cool, Warm, Hot"
is treated as a literal comma-separated list; update the paragraph so it
explicitly names "template expressions" vs "literal list strings" and clarifies
that templates are evaluated and cached while literal strings are used verbatim.
In `@src/classes/base-custom-feature.ts`:
- Around line 529-542: The assignment here can leak the original array
reference: when hasPrimitives is false avoid assigning the config array directly
to this.options; instead assign a shallow copy of the array (e.g., via slice or
Array.from) so external mutations to the passed config won't affect
this.options; keep the rest of the logic (hasPrimitives check, buildOption usage
and setting this.optionsSignature = undefined) unchanged and apply the change
where this.options is assigned inside the Array.isArray(config) branch.
In `@src/custom-features-row-editor.ts`:
- Around line 325-331: toggleDynamicOptions currently flips based only on the
existing type of this.activeEntry.options, which treats undefined as an array
and prevents enabling dynamic mode on new entries; change toggleDynamicOptions
to read the requested toggle state from the event (e.g. (e.target as
HTMLInputElement).checked) and set entry.options = '' when the toggle requests
dynamic/template mode and entry.options = [] when it requests explicit-list
mode, then keep the existing this.optionIndex = -1 and call
this.entryChanged(entry) so the new state is persisted (refer to
toggleDynamicOptions, this.activeEntry, this.optionIndex, and
this.entryChanged).
In `@test/options.test.ts`:
- Around line 109-116: Add a unit test in the existing describe('unescapeHtml')
block to cover doubly-encoded entities (e.g., assert that
unescapeHtml('&') returns '&'), so the test verifies only a single
decoding pass and prevents double-decoding regressions; target the test in
test/options.test.ts and reference the unescapeHtml function to add one or two
expectations for inputs like '&' and a mixed case such as '"'
-> '"' (or similar) to ensure the function decodes only once.
---
Outside diff comments:
In `@src/classes/custom-feature-dropdown.ts`:
- Around line 172-266: The selected option appearance isn't re-evaluated when
template-generated options change because setOptions() only sets should = true
but doesn't trigger the block that computes
selectedIcon/selectedLabel/selectedStyles; update the logic so option-list
changes force that re-evaluation (either by incorporating the boolean result of
setOptions() into the same condition that checks
changedProperties.has('hass'|'stateObj'|'value') or by setting an optionsChanged
flag and including it in that condition). Specifically modify the control flow
around setOptions(), the changedProperties check, and the section that iterates
this.options and assigns selectedIcon/selectedLabel/selectedStyles (referencing
setOptions(), changedProperties, this.options, renderTemplate(), selectedIcon,
selectedLabel, selectedStyles, value, thumbType) so template-driven option
updates immediately recompute and apply the selected option appearance.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 9aee0632-e398-4bc4-aa3b-92379ff46835
⛔ Files ignored due to path filters (2)
dist/custom-card-features.min.jsis excluded by!**/dist/**,!**/*.min.jspackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
README.mdpackage.jsonsrc/classes/base-custom-feature.tssrc/classes/custom-feature-dropdown.tssrc/classes/custom-feature-selector.tssrc/custom-features-row-editor.tssrc/models/interfaces/IConfig.tssrc/utils/index.tssrc/utils/options.tstest/options.test.ts
📜 Review details
🧰 Additional context used
🪛 LanguageTool
README.md
[grammar] ~199-~199: Use a hyphen to join words.
Context: ...a list in any of these forms: - A comma separated string, which is what the defa...
(QB_NEW_EN_HYPHEN)
[grammar] ~200-~200: Use a hyphen to join words.
Context: ... Solid,Rainbow,Fireworks). - A newline separated string, useful when values the...
(QB_NEW_EN_HYPHEN)
[grammar] ~209-~209: Use a hyphen to join words.
Context: ...a template is treated as a literal comma separated list (so `options: "Cool, Warm...
(QB_NEW_EN_HYPHEN)
🪛 markdownlint-cli2 (0.22.1)
README.md
[warning] 235-235: Link text should be descriptive
(MD059, descriptive-link-text)
🔇 Additional comments (23)
src/models/interfaces/IConfig.ts (2)
57-62: LGTM!
64-69: LGTM!src/utils/options.ts (3)
35-73: LGTM!
102-127: LGTM!
10-19: Re-check HTML entity decoding: currentunescapeHtmldoesn’t double-decode&
The chained.replace()calls (with&last) produce&for input&(the previously claimed&&result doesn’t occur here), so the critical “double-decoding” bug report and the single-pass regex refactor aren’t justified by this case.> Likely an incorrect or invalid review comment.src/utils/index.ts (1)
3-3: LGTM!src/classes/base-custom-feature.ts (4)
13-28: LGTM!
506-513: LGTM!
569-598: LGTM!
623-625: LGTM!package.json (1)
8-8: LGTM!Also applies to: 52-52
test/options.test.ts (2)
11-70: LGTM!
72-107: LGTM!src/classes/custom-feature-dropdown.ts (6)
69-69: LGTM!
104-104: LGTM!
208-215: LGTM!
217-228: LGTM!
230-245: LGTM!
285-285: LGTM!src/classes/custom-feature-selector.ts (4)
26-26: LGTM!
91-91: LGTM!
113-146: LGTM!
167-167: LGTM!
kristofferR
force-pushed
the
dynamic-dropdown-options
branch
from
7549b9b to
b8ffa4b
Compare
kristofferR
changed the title
kristofferR
force-pushed
the
dynamic-dropdown-options
branch
2 times, most recently
from
79592a4 to
13ba8bc
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
test/options.test.ts (1)
132-139:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd test coverage for doubly-encoded entities.
The past review comment is still unaddressed. Adding a test to verify that doubly-encoded entities like
&decode only once (to&, not&) would prevent regressions and document the expected behavior.🧪 Suggested test addition
it('decodes the entities introduced by autoescaping', () => { assert.equal( unescapeHtml('"a & b" <x> &`#39`;y&`#39`;'), '"a & b" <x> \'y\'', ); }); + + it('does not double-decode nested entities', () => { + // When original data contains "&", nunjucks escapes it to "&amp;" + // Expected: decode once to "&", not twice to "&" + assert.equal(unescapeHtml('&amp;'), '&'); + assert.equal(unescapeHtml('&lt;'), '<'); + assert.equal(unescapeHtml('&quot;'), '"'); + }); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/options.test.ts` around lines 132 - 139, Add a unit test inside the existing describe('unescapeHtml') block that asserts doubly-encoded entities decode exactly once: call unescapeHtml with inputs like '&amp;' (and optionally '&lt;' / '&quot;') and assert the output is '&' (not '&') to ensure double-encoded entities are not fully decoded; update the test to include this new case alongside the existing examples so behavior is documented and guarded against regressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@package.json`:
- Line 52: Replace the non-reproducible "latest" spec for the "tsx" script entry
in package.json with a pinned version or constrained semver range (e.g., a
specific version like "1.1.0" or a caret/tilde range) so installs are
deterministic; update the "tsx" value under the scripts/dependencies entry (the
string key "tsx") to the chosen version and run npm/yarn install (and optionally
lockfile update) to ensure the lockfile reflects the pinned version.
- Line 8: The test script uses the Node CLI flag "--import" (see "test": "node
--import tsx --test test/*.test.ts") which requires Node >=18.18.0; add an
engines entry to package.json (e.g., "engines": {"node": ">=18.18.0"}) to
document/enforce the minimum Node version and update CI configuration to fail
when the runner's Node version is older (or add an
.nvmrc/.node-version/volta.json aligning with the same version) so the test
script is always run on a compatible Node runtime.
In `@README.md`:
- Around line 215-216: Replace plain phrases "comma separated", "newline
separated", and "literal comma separated" with hyphenated compound adjectives
"comma-separated", "newline-separated", and "literal comma-separated" throughout
the README; specifically update the two occurrences shown in the diff (the list
items containing "`A comma separated string...`" and "`A newline separated
string...`") and also the additional occurrence noted (the "literal comma
separated" mention) so all three variants are consistently hyphenated.
In `@src/classes/base-custom-feature.ts`:
- Around line 581-582: The dynamic attribute-source detection currently only
checks this.config.options_attribute and isObjectSource, skipping cases where
only this.config.options_entity is set; update the conditional that builds the
source so it also treats configs with options_entity as an attribute-based
source (e.g., change the if-check to include this.config.options_entity and
ensure the computed source variable (currently assigned via const source =
(isObjectSource ? config : {}) as IOptionsSource) is populated/derived when
options_entity is present), so attribute-only options_entity configs will
resolve options; adjust any downstream uses in the same method in class
BaseCustomFeature to rely on the updated source logic.
- Around line 639-644: The guard that decides whether to inject a default
tap_action misses checking option.momentary_repeat_action, so options that only
define a repeat-momentary action get an implicit tap_action; update the
conditional in the method that injects default actions (the block referencing
option.tap_action, option.double_tap_action, option.hold_action,
option.momentary_start_action, option.momentary_end_action) to also check
!option.momentary_repeat_action so the default tap_action is not added when
momentary_repeat_action exists.
---
Duplicate comments:
In `@test/options.test.ts`:
- Around line 132-139: Add a unit test inside the existing
describe('unescapeHtml') block that asserts doubly-encoded entities decode
exactly once: call unescapeHtml with inputs like '&amp;' (and optionally
'&lt;' / '&quot;') and assert the output is '&' (not '&') to ensure
double-encoded entities are not fully decoded; update the test to include this
new case alongside the existing examples so behavior is documented and guarded
against regressions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 68601f9b-4a00-4b06-a204-39613e7a18ff
⛔ Files ignored due to path filters (2)
dist/custom-card-features.min.jsis excluded by!**/dist/**,!**/*.min.jspackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
README.mdpackage.jsonsrc/classes/base-custom-feature.tssrc/classes/custom-feature-dropdown.tssrc/classes/custom-feature-selector.tssrc/custom-features-row-editor.tssrc/models/interfaces/IConfig.tssrc/utils/index.tssrc/utils/options.tstest/options.test.ts
📜 Review details
🧰 Additional context used
🪛 LanguageTool
README.md
[grammar] ~215-~215: Use a hyphen to join words.
Context: ...ressed in any of these forms: - A comma separated string, which is what the defa...
(QB_NEW_EN_HYPHEN)
[grammar] ~216-~216: Use a hyphen to join words.
Context: ... Solid,Rainbow,Fireworks). - A newline separated string, useful when values the...
(QB_NEW_EN_HYPHEN)
[grammar] ~225-~225: Use a hyphen to join words.
Context: ...a template is treated as a literal comma separated list (so `options: "Cool, Warm...
(QB_NEW_EN_HYPHEN)
🪛 markdownlint-cli2 (0.22.1)
README.md
[warning] 269-269: Link text should be descriptive
(MD059, descriptive-link-text)
🔇 Additional comments (11)
README.md (2)
269-269: Use descriptive link text instead of “here”.This was already flagged previously and still appears unresolved at Line 269.
Source: Linters/SAST tools
56-57: LGTM!Also applies to: 78-79, 173-246, 2539-2584
test/options.test.ts (3)
12-71: LGTM!
73-108: LGTM!
110-130: LGTM!src/models/interfaces/IConfig.ts (1)
56-99: LGTM!src/utils/index.ts (1)
3-3: LGTM!src/utils/options.ts (1)
12-146: LGTM!src/classes/custom-feature-dropdown.ts (1)
69-70: LGTM!Also applies to: 104-105, 172-174, 208-245, 285-295
src/classes/custom-feature-selector.ts (1)
26-27: LGTM!Also applies to: 91-92, 115-116, 144-145, 167-175
src/custom-features-row-editor.ts (1)
72-77: LGTM!Also applies to: 119-132, 173-176, 190-197, 266-269, 284-287, 312-367, 385-389, 727-730, 1344-1594, 2606-2610, 2872-2876
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/classes/custom-feature-dropdown.ts (1)
172-179:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRe-sync option DOM when options change while dropdown is already open.
Line 172 can force a rerender on dynamic option changes, but Line 285 only reapplies
selected/tabindexstate whenopenchanges. If options refresh while open, new option nodes can lose focusability/highlight until close/reopen.Suggested fix
export class CustomFeatureDropdown extends BaseCustomFeature { + private optionsChangedSinceLastUpdate = false; + shouldUpdate(changedProperties: PropertyValues) { let should = super.shouldUpdate(changedProperties); - if (this.setOptions()) { + const optionsChanged = this.setOptions(); + this.optionsChangedSinceLastUpdate = optionsChanged; + if (optionsChanged) { should = true; } @@ updated(changedProperties: PropertyValues) { super.updated(changedProperties); - if (changedProperties.has('open')) { + if ( + changedProperties.has('open') || + (this.open && this.optionsChangedSinceLastUpdate) + ) { const options = this.options; @@ this.sizeAndPositionDropdown(); + this.optionsChangedSinceLastUpdate = false; } }Also applies to: 285-307
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/classes/custom-feature-dropdown.ts` around lines 172 - 179, When dynamic options change (setOptions() returns true) we must also reapply option node selection/focus state instead of only doing that when 'open' changes; after calling setOptions() in the update flow, invoke the same logic that the 'open' change branch uses to reapply selected/tabindex (extract that block into a helper like syncOptionSelection() if needed) so new option DOM nodes get proper selected and tabindex attributes immediately when options refresh while the dropdown is open; reference setOptions(), changedProperties and the existing "reapply selected/tabindex state" block currently gated on 'open'.
♻️ Duplicate comments (3)
README.md (2)
269-269:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReplace vague link text with descriptive link text.
Use descriptive anchor text instead of
hereso readers know the target before clicking.Proposed edit
-... The structure of `stateObj` can be found [here](https://github.com/home-assistant/home-assistant-js-websocket/blob/1d51737f6092b95e2bc98e85aca752771b97b760/lib/types.ts#L72-L96) as a `HassEntity` type and is listed below. +... The structure of `stateObj` can be found in the [Home Assistant websocket `HassEntity` type definition](https://github.com/home-assistant/home-assistant-js-websocket/blob/1d51737f6092b95e2bc98e85aca752771b97b760/lib/types.ts#L72-L96) and is listed below.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` at line 269, Replace the vague link anchor text "here" with a descriptive phrase that indicates the target (e.g., "HassEntity type definition" or "HassEntity type in home-assistant-js-websocket"), so update the sentence referencing stateObj to use that descriptive link text while keeping the same URL, and ensure surrounding text still reads naturally (look for the phrase "The structure of `stateObj` can be found here as a `HassEntity` type" and change only the anchor text).Source: Linters/SAST tools
215-216:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse hyphenated compound adjectives consistently.
Please change
comma separated,newline separated, andliteral comma separatedtocomma-separated,newline-separated, andliteral comma-separatedfor readability consistency.Proposed edit
-- A comma separated string, which is what the default `{{ state_attr(...) }}` output produces (e.g. `Solid,Rainbow,Fireworks`). -- A newline separated string, useful when values themselves contain commas (e.g. `{{ my_list | join('\n') }}`). +- A comma-separated string, which is what the default `{{ state_attr(...) }}` output produces (e.g. `Solid,Rainbow,Fireworks`). +- A newline-separated string, useful when values themselves contain commas (e.g. `{{ my_list | join('\n') }}`). -The options template, like all other templates, must be wrapped in `{{ }}`. A plain string without a template is treated as a literal comma separated list (so `options: "Cool, Warm, Hot"` also works). +The options template, like all other templates, must be wrapped in `{{ }}`. A plain string without a template is treated as a literal comma-separated list (so `options: "Cool, Warm, Hot"` also works).Also applies to: 225-225
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 215 - 216, Update the hyphenation of the compound adjectives in the README: replace occurrences of "comma separated", "newline separated", and "literal comma separated" with "comma-separated", "newline-separated", and "literal comma-separated" (e.g., in the list items showing `{{ state_attr(...) }}` and `{{ my_list | join('\n') }}` and the related line at 225). Ensure all three phrases are updated consistently across those lines.Source: Linters/SAST tools
package.json (1)
8-8:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDefine runtime floor and pin
tsxfor deterministic tests.
node --import tsxneeds a documented Node minimum, andtsx: "latest"makes test environments drift across installs.Proposed edit
{ "name": "custom-card-features", "version": "4.9.4", + "engines": { + "node": ">=18.18.0" + }, ... "devDependencies": { - "tsx": "latest", + "tsx": "^4.20.6", ... } }#!/bin/bash # Read-only verification: confirm runtime contract and deterministic tsx spec. jq -r '.engines // "missing engines"' package.json jq -r '.devDependencies.tsx // "missing tsx"' package.json rg -n '"test"\s*:\s*"node --import tsx --test' package.jsonAlso applies to: 52-52
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@package.json` at line 8, Add a Node runtime floor and pin the tsx devDependency to a deterministic semver: update package.json to include an "engines" entry (e.g., "engines": {"node": ">=18.16.0"} or whichever minimum the project supports) and replace any "tsx": "latest" in devDependencies with a fixed version (e.g., "tsx": "1.10.0" or the tested release). Ensure the existing test script ("test": "node --import tsx --test test/*.test.ts") remains but is documented by the engines field and the pinned "tsx" in devDependencies so CI and local installs are deterministic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/classes/custom-feature-dropdown.ts`:
- Around line 221-227: The optionContext currently replaces the whole config
with selectedOption (in optionContext.config), which drops parent dropdown
settings; change it to merge the parent config first (e.g., ...this.config) and
then spread selectedOption, finally setting entity: this.entityId and attribute:
this.valueAttribute so templates retain parent config fields while allowing
option-specific overrides; update the config construction in optionContext
accordingly.
---
Outside diff comments:
In `@src/classes/custom-feature-dropdown.ts`:
- Around line 172-179: When dynamic options change (setOptions() returns true)
we must also reapply option node selection/focus state instead of only doing
that when 'open' changes; after calling setOptions() in the update flow, invoke
the same logic that the 'open' change branch uses to reapply selected/tabindex
(extract that block into a helper like syncOptionSelection() if needed) so new
option DOM nodes get proper selected and tabindex attributes immediately when
options refresh while the dropdown is open; reference setOptions(),
changedProperties and the existing "reapply selected/tabindex state" block
currently gated on 'open'.
---
Duplicate comments:
In `@package.json`:
- Line 8: Add a Node runtime floor and pin the tsx devDependency to a
deterministic semver: update package.json to include an "engines" entry (e.g.,
"engines": {"node": ">=18.16.0"} or whichever minimum the project supports) and
replace any "tsx": "latest" in devDependencies with a fixed version (e.g.,
"tsx": "1.10.0" or the tested release). Ensure the existing test script ("test":
"node --import tsx --test test/*.test.ts") remains but is documented by the
engines field and the pinned "tsx" in devDependencies so CI and local installs
are deterministic.
In `@README.md`:
- Line 269: Replace the vague link anchor text "here" with a descriptive phrase
that indicates the target (e.g., "HassEntity type definition" or "HassEntity
type in home-assistant-js-websocket"), so update the sentence referencing
stateObj to use that descriptive link text while keeping the same URL, and
ensure surrounding text still reads naturally (look for the phrase "The
structure of `stateObj` can be found here as a `HassEntity` type" and change
only the anchor text).
- Around line 215-216: Update the hyphenation of the compound adjectives in the
README: replace occurrences of "comma separated", "newline separated", and
"literal comma separated" with "comma-separated", "newline-separated", and
"literal comma-separated" (e.g., in the list items showing `{{ state_attr(...)
}}` and `{{ my_list | join('\n') }}` and the related line at 225). Ensure all
three phrases are updated consistently across those lines.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ba10a5b5-513f-4464-9e25-7963f30e3c65
⛔ Files ignored due to path filters (2)
dist/custom-card-features.min.jsis excluded by!**/dist/**,!**/*.min.jspackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
README.mdpackage.jsonsrc/classes/base-custom-feature.tssrc/classes/custom-feature-dropdown.tssrc/classes/custom-feature-selector.tssrc/custom-features-row-editor.tssrc/models/interfaces/IConfig.tssrc/utils/index.tssrc/utils/options.tstest/options.test.ts
📜 Review details
🧰 Additional context used
🪛 LanguageTool
README.md
[grammar] ~215-~215: Use a hyphen to join words.
Context: ...ressed in any of these forms: - A comma separated string, which is what the defa...
(QB_NEW_EN_HYPHEN)
[grammar] ~216-~216: Use a hyphen to join words.
Context: ... Solid,Rainbow,Fireworks). - A newline separated string, useful when values the...
(QB_NEW_EN_HYPHEN)
[grammar] ~225-~225: Use a hyphen to join words.
Context: ...a template is treated as a literal comma separated list (so `options: "Cool, Warm...
(QB_NEW_EN_HYPHEN)
🪛 markdownlint-cli2 (0.22.1)
README.md
[warning] 269-269: Link text should be descriptive
(MD059, descriptive-link-text)
🔇 Additional comments (30)
test/options.test.ts (1)
12-139: LGTM!src/custom-features-row-editor.ts (14)
72-77: LGTM!
117-132: LGTM!
169-177: LGTM!
188-197: LGTM!
232-252: LGTM!
262-270: LGTM!
282-289: LGTM!
312-367: LGTM!
385-388: LGTM!
553-553: LGTM!
1340-1594: LGTM!
2606-2611: LGTM!
2872-2876: LGTM!
727-730: LGTM!src/classes/base-custom-feature.ts (7)
531-544: Defensive copy suggestion already raised.The direct array assignment at line 541 was flagged in a previous review. The suggestion to use
[...(config as IOption[])]remains valid for immutability safety.
581-583: Missingoptions_entitycheck still present.The conditional at line 583 still doesn't include
this.config.options_entity !== undefined, so configs with onlyoptions_entity(relying on the defaultoptionsattribute for select/input_select) won't resolve options.
639-646: Missingmomentary_repeat_actionguard still present.The autofill guard at lines 639-645 still doesn't check for
option.momentary_repeat_action, so options defining only repeat behavior still get an implicittap_action.
13-30: LGTM!
584-624: LGTM!
647-659: LGTM!
684-686: LGTM!src/models/interfaces/IConfig.ts (1)
56-99: LGTM!src/utils/index.ts (1)
1-3: LGTM!src/utils/options.ts (5)
12-21: LGTM!
29-38: LGTM!
54-92: LGTM!
94-108: LGTM!
121-146: LGTM!src/classes/custom-feature-selector.ts (1)
26-27: LGTM!Also applies to: 91-99, 115-116, 144-145, 167-175
kristofferR
force-pushed
the
dynamic-dropdown-options
branch
from
13ba8bc to
1491774
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1491774d4a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
kristofferR
force-pushed
the
dynamic-dropdown-options
branch
from
1491774 to
1160e9e
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1160e9ece0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
kristofferR
force-pushed
the
dynamic-dropdown-options
branch
2 times, most recently
from
08028d4 to
e08a751
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e08a751977
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/custom-features-row-editor.ts`:
- Around line 1544-1550: The current predicate that builds hideAttributes
incorrectly hides arrays of numbers (e.g., [1,2,3]), so update the filter in
hideAttributes (which iterates sourceAttributes and checks value) to stop
excluding numeric arrays: replace the final clause value.every((item) => typeof
item == 'number') with a check that hides only arrays whose items are neither
strings nor numbers (e.g., value.every(item => typeof item !== 'string' &&
typeof item !== 'number')), so numeric option lists remain visible; keep the
other conditions (!Array.isArray(value) and value.length < 2) as-is.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a703c750-cfcb-4238-bf67-fff836a332b1
⛔ Files ignored due to path filters (2)
dist/custom-card-features.min.jsis excluded by!**/dist/**,!**/*.min.jspackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
README.mdpackage.jsonsrc/classes/base-custom-feature.tssrc/classes/custom-feature-dropdown.tssrc/classes/custom-feature-selector.tssrc/custom-features-row-editor.tssrc/models/interfaces/IConfig.tssrc/utils/index.tssrc/utils/options.tstest/options.test.ts
📜 Review details
🔇 Additional comments (34)
src/models/interfaces/IConfig.ts (1)
56-99: LGTM!src/utils/index.ts (1)
3-3: LGTM!src/utils/options.ts (5)
1-21: LGTM!
23-38: LGTM!
40-98: LGTM!
100-116: LGTM!
118-158: LGTM!src/classes/base-custom-feature.ts (4)
508-555: LGTM!
557-628: LGTM!
630-673: LGTM!
698-700: LGTM!package.json (3)
55-55: Pin the tsx version instead of using "latest".Using
"latest"for dependency versions leads to non-reproducible builds and potential breaking changes when the package updates. Different developers and CI runs may install different versions, causing inconsistent test results.
7-9: LGTM!
11-11: LGTM!README.md (4)
56-57: LGTM!
78-79: LGTM!
173-246: LGTM!
2538-2647: LGTM!test/options.test.ts (5)
1-11: LGTM!
12-88: LGTM!
90-140: LGTM!
142-162: LGTM!
164-179: LGTM!src/classes/custom-feature-dropdown.ts (7)
227-235: LGTM!
69-69: LGTM!
104-122: LGTM!
172-182: LGTM!
212-219: LGTM!
237-252: LGTM!
292-311: LGTM!src/classes/custom-feature-selector.ts (4)
26-48: LGTM!
91-102: LGTM!
115-115: LGTM!Also applies to: 144-144
167-192: LGTM!
kristofferR
force-pushed
the
dynamic-dropdown-options
branch
3 times, most recently
from
9125c26 to
b2859b1
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b2859b12fe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
kristofferR
force-pushed
the
dynamic-dropdown-options
branch
from
b2859b1 to
a7a478c
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a7a478c975
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
package.json (1)
55-55:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPin
tsxto a stable semver instead oflatest.Using
lateston Line 55 makes test runs non-reproducible across environments and time. Please pin to a concrete version (or at least a bounded range).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@package.json` at line 55, The package.json npm script entry "tsx" is set to the unpinned value "latest" which makes installs non-reproducible; update the "tsx" value in package.json (the "tsx" script key) to a concrete semver (e.g., a specific version like "1.1.0" or a bounded range like "^1.1.0") so CI and local environments install a stable, reproducible version; commit the updated package.json and run npm/yarn install to update the lockfile accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Line 261: The README row for select/input_select is incorrect: it lists the
action as `select.select_option` for both domains but the implementation uses
domain-prefixed actions like `<domain>.select_option`; update that row so
`select` maps to `select.select_option` and `input_select` maps to
`input_select.select_option` (reference symbols: select, input_select,
select.select_option, input_select.select_option) to match the actual behavior.
---
Duplicate comments:
In `@package.json`:
- Line 55: The package.json npm script entry "tsx" is set to the unpinned value
"latest" which makes installs non-reproducible; update the "tsx" value in
package.json (the "tsx" script key) to a concrete semver (e.g., a specific
version like "1.1.0" or a bounded range like "^1.1.0") so CI and local
environments install a stable, reproducible version; commit the updated
package.json and run npm/yarn install to update the lockfile accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: de25aae9-1f13-4e68-a662-7613a7c7e087
⛔ Files ignored due to path filters (2)
dist/custom-card-features.min.jsis excluded by!**/dist/**,!**/*.min.jspackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
README.mdpackage.jsonsrc/classes/base-custom-feature.tssrc/classes/custom-feature-dropdown.tssrc/classes/custom-feature-selector.tssrc/custom-features-row-editor.tssrc/models/interfaces/IConfig.tssrc/utils/index.tssrc/utils/options.tstest/options.test.ts
📜 Review details
🔇 Additional comments (41)
test/options.test.ts (1)
13-230: LGTM!src/models/interfaces/IConfig.ts (1)
56-99: LGTM!src/utils/index.ts (1)
1-4: LGTM!src/utils/options.ts (5)
1-21: LGTM!
23-115: LGTM!
117-132: LGTM!
134-210: LGTM!
212-252: LGTM!src/classes/base-custom-feature.ts (8)
13-31: LGTM!
380-385: LGTM!
510-557: LGTM!
559-597: LGTM!
599-616: LGTM!
618-667: LGTM!
669-714: LGTM!
739-741: LGTM!src/classes/custom-feature-dropdown.ts (5)
69-69: LGTM!
104-104: LGTM!Also applies to: 172-175, 179-182
212-235: LGTM!
237-252: LGTM!
292-292: LGTM!src/classes/custom-feature-selector.ts (4)
26-26: LGTM!
91-91: LGTM!
115-115: LGTM!Also applies to: 144-144
167-167: LGTM!src/custom-features-row-editor.ts (16)
1550-1556: Attribute filtering still hides valid numeric option lists.This concern was previously raised and remains unaddressed. The filter hides any array where every item is a number, which excludes potentially valid option sources like
[1, 2, 3, 4, 5]even though numeric primitives are supported by the dynamic option pipeline.
50-60: LGTM!
78-85: LGTM!
127-140: LGTM!
181-181: LGTM!Also applies to: 198-198, 246-246, 274-274, 292-292, 566-566
320-333: LGTM!
336-349: LGTM!
352-380: LGTM!
398-401: LGTM!
740-743: LGTM!
1356-1368: LGTM!
1506-1549: LGTM!Also applies to: 1559-1630
1632-1674: LGTM!
2651-2711: LGTM!
2748-2757: LGTM!
3019-3023: LGTM!
kristofferR
force-pushed
the
dynamic-dropdown-options
branch
from
a7a478c to
fbc5771
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fbc577153e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
kristofferR
force-pushed
the
dynamic-dropdown-options
branch
from
fbc5771 to
6cf105d
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6cf105d893
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
kristofferR
force-pushed
the
dynamic-dropdown-options
branch
2 times, most recently
from
c6fa90f to
f7ead59
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a4c5ba4b17
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
kristofferR
force-pushed
the
dynamic-dropdown-options
branch
from
a4c5ba4 to
e92a640
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/classes/base-custom-feature.ts (1)
531-538:⚠️ Potential issue | 🟠 Major | ⚡ Quick winManual primitive
optionsarrays bypass option normalization.When
config.optionscontains primitive values (e.g., YAMLoptions: [Solid, Rainbow]), line 536 copies them directly asIOption[]without normalization. These primitives lack theoptionproperty that dropdown/selector rendering and template context depend on, which can cause undefined access or broken selection state.Proposed fix: normalize primitives via buildOption
if (Array.isArray(config)) { + const hasPrimitives = config.some( + (item) => item == null || typeof item != 'object', + ); const changed = this.optionsSignature != undefined || config.length != this.options.length || - config.some((option, index) => option !== this.options[index]); - this.options = [...(config as IOption[])]; + (hasPrimitives + ? config.some((item, index) => + item != null && typeof item == 'object' + ? item !== this.options[index] + : String(item ?? '') != this.options[index]?.option, + ) + : config.some((option, index) => option !== this.options[index])); + this.options = hasPrimitives + ? config.map((item) => + item != null && typeof item == 'object' + ? (item as IOption) + : this.buildOption(item), + ) + : [...(config as IOption[])]; this.optionsSignature = undefined; return changed; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/classes/base-custom-feature.ts` around lines 531 - 538, When config is an array, the code currently copies primitives into this.options (bypassing normalization) which breaks consumers expecting IOption objects; update the block handling Array.isArray(config) in the class (where this.options and this.optionsSignature are set) to normalize each entry by calling buildOption(...) for primitive or shorthand entries and produce a normalized IOption[] before comparing and assigning to this.options, and compute the changed boolean against the normalized array (not the raw config) so optionsSignature is reset and change detection remains correct.src/custom-features-row-editor.ts (1)
341-351:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve legacy object-based
optionssources before mode/autofill normalization.
entry.optionsobjects (legacy{ entity_id, attribute }) are currently treated as manual mode, which can both crash the manual options list path and be silently rewritten to[]during autofill. Normalize/classify the legacy object as attribute mode first, then continue with existing logic.💡 Suggested fix
@@ get optionsMode(): 'manual' | 'attribute' | 'template' { + if ( + this.activeEntry?.options && + typeof this.activeEntry.options == 'object' && + !Array.isArray(this.activeEntry.options) + ) { + return 'attribute'; + } if ( this.activeEntry?.options_attribute !== undefined || this.activeEntry?.options_entity !== undefined @@ buildOptionsSection(type: 'dropdown' | 'selector') { const mode = this.optionsMode; const noun = type == 'dropdown' ? 'dropdown' : 'selector'; + const legacySource = + this.activeEntry?.options && + typeof this.activeEntry.options == 'object' && + !Array.isArray(this.activeEntry.options) + ? (this.activeEntry.options as { + entity_id?: string; + attribute?: string; + }) + : {}; @@ const sourceEntity = this.renderTemplate( - (this.activeEntry?.options_entity || + (this.activeEntry?.options_entity || + legacySource.entity_id || this.activeEntry?.entity_id || '') as string, @@ ${this.buildSelector( 'Source entity (optional)', 'options_entity', { entity: {} }, - this.activeEntry?.entity_id, + legacySource.entity_id ?? this.activeEntry?.entity_id, )} ${this.buildSelector('Source attribute', 'options_attribute', { attribute: { entity_id: - this.activeEntry?.options_entity || this.activeEntry?.entity_id, + this.activeEntry?.options_entity || + legacySource.entity_id || + this.activeEntry?.entity_id, hide_attributes: hideAttributes, }, - })}` + }, legacySource.attribute)}` @@ case 'dropdown': case 'selector': { + const legacySource = + entry.options && + typeof entry.options == 'object' && + !Array.isArray(entry.options) + ? (entry.options as { + entity_id?: string; + attribute?: string; + }) + : undefined; + if (legacySource) { + entry.options_entity ??= legacySource.entity_id; + entry.options_attribute ??= legacySource.attribute ?? ''; + delete entry.options; + } const isAttributeMode = entry.options_attribute !== undefined || entry.options_entity !== undefined;Also applies to: 1544-1548, 2783-2787, 2820-2821
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/custom-features-row-editor.ts` around lines 341 - 351, The optionsMode getter currently treats legacy object-shaped activeEntry.options as manual and misclassifies/re-writes them; update the logic in the optionsMode getter (and other similar mode-detection code paths) to first detect if activeEntry.options is a plain object with legacy keys (e.g., hasOwnProperty('entity_id') or hasOwnProperty('attribute')) and return 'attribute' in that case, then fall through to the existing string/template check and finally 'manual'; locate and change the same pattern where options normalization or autofill occurs (references: optionsMode getter and other mode/autofill checks) so legacy {entity_id, attribute} objects are preserved and handled as attribute mode.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@src/classes/base-custom-feature.ts`:
- Around line 531-538: When config is an array, the code currently copies
primitives into this.options (bypassing normalization) which breaks consumers
expecting IOption objects; update the block handling Array.isArray(config) in
the class (where this.options and this.optionsSignature are set) to normalize
each entry by calling buildOption(...) for primitive or shorthand entries and
produce a normalized IOption[] before comparing and assigning to this.options,
and compute the changed boolean against the normalized array (not the raw
config) so optionsSignature is reset and change detection remains correct.
In `@src/custom-features-row-editor.ts`:
- Around line 341-351: The optionsMode getter currently treats legacy
object-shaped activeEntry.options as manual and misclassifies/re-writes them;
update the logic in the optionsMode getter (and other similar mode-detection
code paths) to first detect if activeEntry.options is a plain object with legacy
keys (e.g., hasOwnProperty('entity_id') or hasOwnProperty('attribute')) and
return 'attribute' in that case, then fall through to the existing
string/template check and finally 'manual'; locate and change the same pattern
where options normalization or autofill occurs (references: optionsMode getter
and other mode/autofill checks) so legacy {entity_id, attribute} objects are
preserved and handled as attribute mode.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7ed4232a-4713-4fca-ab83-65f99943b63e
⛔ Files ignored due to path filters (1)
dist/custom-card-features.min.jsis excluded by!**/dist/**,!**/*.min.js
📒 Files selected for processing (8)
README.mdsrc/classes/base-custom-feature.tssrc/classes/custom-feature-dropdown.tssrc/classes/custom-feature-selector.tssrc/custom-features-row-editor.tssrc/models/interfaces/IConfig.tssrc/utils/index.tssrc/utils/options.ts
📜 Review details
🔇 Additional comments (30)
src/models/interfaces/IConfig.ts (1)
56-87: LGTM!src/utils/index.ts (1)
1-3: LGTM!src/utils/options.ts (9)
22-29: LGTM!
37-46: LGTM!
53-113: LGTM!
120-145: LGTM!
150-183: LGTM!
191-200: LGTM!
216-260: LGTM!
291-320: LGTM!
262-278: LGTM!src/classes/base-custom-feature.ts (9)
13-30: LGTM!
379-384: LGTM!
509-516: LGTM!
541-560: LGTM!
568-592: LGTM!
599-611: LGTM!
619-664: LGTM!
672-717: LGTM!
742-744: LGTM!src/classes/custom-feature-dropdown.ts (5)
212-272: LGTM!The past review concern about preserving parent dropdown config in
optionContext.confighas been addressed — the implementation now correctly mergesthis.configfirst before spreadingselectedOption, ensuring templates can reference parent config fields while option-specific fields override.
69-74: LGTM!
104-122: LGTM!
170-183: LGTM!
291-328: LGTM!src/classes/custom-feature-selector.ts (4)
26-53: LGTM!
96-108: LGTM!
118-161: LGTM!
170-198: LGTM!README.md (1)
56-57: LGTM!Also applies to: 78-79, 173-206, 229-229, 2499-2544
kristofferR
force-pushed
the
dynamic-dropdown-options
branch
from
e92a640 to
2426382
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 24263828a5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
kristofferR
force-pushed
the
dynamic-dropdown-options
branch
from
2426382 to
9a139d7
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
README.md (1)
2532-2538:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winExample contradicts the “omit
options_attribute” guidance.This section says the field can be omitted for
input_select, but the sample still includes a blankoptions_attribute:key. Align the example with the stated minimal config to avoid confusion.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 2532 - 2538, The example YAML contradicts the guidance to omit options_attribute for select/input_select; update the example that shows the dropdown for entity_id: input_select.scene to remove the blank options_attribute key (and omit option_template) so it reflects the minimal config that relies on the entity's options attribute and auto-generated select_option action.src/custom-features-row-editor.ts (1)
341-351:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve legacy object-based option sources instead of coercing them to manual arrays.
A legacy source shape (
options: { entity_id, attribute }) is not recognized byoptionsMode, so it falls into manual mode and later gets coerced to[]in autofill (Line 2823), silently dropping source configuration.Suggested fix
+const legacySource = + entry.options && + typeof entry.options === 'object' && + !Array.isArray(entry.options) + ? (entry.options as { entity_id?: string; attribute?: string }) + : undefined; const isAttributeMode = entry.options_attribute !== undefined || - entry.options_entity !== undefined; + entry.options_entity !== undefined || + !!legacySource; const isTemplateMode = typeof entry.options == 'string'; if (isAttributeMode || isTemplateMode) { + if (legacySource) { + entry.options_entity ??= legacySource.entity_id; + entry.options_attribute ??= legacySource.attribute; + delete entry.options; + } ... } -const options = Array.isArray(entry.options) ? entry.options : []; +const options = Array.isArray(entry.options) ? entry.options : [];Also applies to: 2783-2824
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/custom-features-row-editor.ts` around lines 341 - 351, The optionsMode getter currently treats legacy object-shaped option sources (where activeEntry.options is an object like { entity_id, attribute }) as 'manual', causing later autofill code to coerce them to [] and drop configuration; update optionsMode to also detect when this.activeEntry?.options is an object with entity_id and/or attribute and return 'attribute' in that case (in addition to the existing checks for options_attribute/options_entity and string templates), and update the autofill/code path that currently coerces options to [] so it preserves and handles the legacy object-shaped source instead of replacing it.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/classes/base-custom-feature.ts`:
- Around line 662-666: The code currently stringifies the template result before
parsing which loses structured data; change the logic in base-custom-feature.ts
so renderTemplate(config) is assigned to a variable without coercing to String
(e.g., const rendered = this.renderTemplate(config)) and pass that raw rendered
value into parseOptionsList(rendered) so parseOptionsList/buildTemplatedOption
receive the original array/object structure; also update the signature
construction to use a stable serialization (e.g., JSON.stringify(rendered))
instead of the flattened string so optionSignature remains unique and
consistent.
- Around line 531-537: The current Array branch in setOptions only checks length
and element identity so in-place edits to an option (label/icon/styles) don't
set optionsChanged; compute a stable signature of the new config (e.g.
JSON.stringify of the config or a hash of the option fields you care about) and
compare it to this.optionsSignature to set optionsChanged accordingly, then
update this.options and assign the new signature to this.optionsSignature so
custom-feature-dropdown.ts sees the change and recomputes the cached selected
thumb; update the logic inside setOptions (referencing setOptions, this.options,
this.optionsSignature, and optionsChanged) to use this signature comparison.
In `@src/classes/custom-feature-dropdown.ts`:
- Around line 170-183: The selected-option rendering can use stale this.entityId
/ this.valueAttribute because setValue() is only run on hass/value updates; when
optionsChanged is true during config-only updates you must refresh derived
entity/attribute before recomputing the selected thumb. Modify shouldUpdate so
that if optionsChanged is true but neither 'hass' nor 'value' are in
changedProperties you call this.setValue() (or another existing
refreshDerivedEntity method) before continuing, ensuring setValue is
safe/idempotent; apply the same change to the analogous block around lines
227-234 where optionsChanged triggers recompute. Ensure references to
shouldUpdate, setOptions, setValue, entityId, valueAttribute, and
BaseCustomFeature.shouldUpdate are used to locate the edits.
In `@src/custom-features-row-editor.ts`:
- Around line 1665-1668: The selector option-template editor is using
defaultUiActions (which omits 'repeat') for the 'hold_action' field, so the GUI
can't configure repeat-hold; update the call that creates the select editor for
'hold_action' (the createSelectEditor invocation referencing 'Hold behavior
(optional)' / 'hold_action') to use the actions list that includes 'repeat'
(either replace defaultUiActions with the full actions array or add 'repeat'
into the passed array) so repeat-hold appears in the generated selector options.
---
Duplicate comments:
In `@README.md`:
- Around line 2532-2538: The example YAML contradicts the guidance to omit
options_attribute for select/input_select; update the example that shows the
dropdown for entity_id: input_select.scene to remove the blank options_attribute
key (and omit option_template) so it reflects the minimal config that relies on
the entity's options attribute and auto-generated select_option action.
In `@src/custom-features-row-editor.ts`:
- Around line 341-351: The optionsMode getter currently treats legacy
object-shaped option sources (where activeEntry.options is an object like {
entity_id, attribute }) as 'manual', causing later autofill code to coerce them
to [] and drop configuration; update optionsMode to also detect when
this.activeEntry?.options is an object with entity_id and/or attribute and
return 'attribute' in that case (in addition to the existing checks for
options_attribute/options_entity and string templates), and update the
autofill/code path that currently coerces options to [] so it preserves and
handles the legacy object-shaped source instead of replacing it.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 60733eed-00df-4c35-8799-242b7dd5aa3e
⛔ Files ignored due to path filters (1)
dist/custom-card-features.min.jsis excluded by!**/dist/**,!**/*.min.js
📒 Files selected for processing (8)
README.mdsrc/classes/base-custom-feature.tssrc/classes/custom-feature-dropdown.tssrc/classes/custom-feature-selector.tssrc/custom-features-row-editor.tssrc/models/interfaces/IConfig.tssrc/utils/index.tssrc/utils/options.ts
📜 Review details
🔇 Additional comments (1)
src/classes/base-custom-feature.ts (1)
568-591: Don't letoptions_attribute/options_entityshadow template-modeoptions.
resolveDynamicOptions()consultsresolveAttributeSource()first, and this resolver now returns a source whenever either attribute field is present. That means a mixed or migrated config withoptions: "{{ ... }}"plus a staleoptions_attribute/options_entitysilently skips the template path entirely, anddefaultValueAttribute()follows the wrong branch for the same reason.
| if (typeof config == 'string' && config.trim()) { | ||
| const rendered = String(this.renderTemplate(config)); | ||
| return { | ||
| items: parseOptionsList(rendered), | ||
| signature: `tmpl:${rendered}:${optionSignature}`, |
There was a problem hiding this comment.
Pass the raw template result into parseOptionsList().
String(this.renderTemplate(config)) flattens any structured template result before parsing. A native array becomes a comma-joined string, and an array of objects degrades to "[object Object],...", so the structured-item support added in parseOptionsList()/buildTemplatedOption() never runs for template-backed sources.
Suggested fix
// Template source.
if (typeof config == 'string' && config.trim()) {
- const rendered = String(this.renderTemplate(config));
+ const rendered = this.renderTemplate(config as string) as unknown;
return {
items: parseOptionsList(rendered),
- signature: `tmpl:${rendered}:${optionSignature}`,
+ signature: `tmpl:${JSON.stringify(rendered ?? null)}:${optionSignature}`,
};
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/classes/base-custom-feature.ts` around lines 662 - 666, The code
currently stringifies the template result before parsing which loses structured
data; change the logic in base-custom-feature.ts so renderTemplate(config) is
assigned to a variable without coercing to String (e.g., const rendered =
this.renderTemplate(config)) and pass that raw rendered value into
parseOptionsList(rendered) so parseOptionsList/buildTemplatedOption receive the
original array/object structure; also update the signature construction to use a
stable serialization (e.g., JSON.stringify(rendered)) instead of the flattened
string so optionSignature remains unique and consistent.
9a139d7 to
50c2048
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
♻️ Duplicate comments (5)
src/classes/custom-feature-selector.ts (1)
118-150:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRefresh the derived value when options rebuild on a config-only change.
Unlike the dropdown, this path never calls
setValue()aftersetOptions()changes the resolved list.updated()andonFocus()then compare against a stalethis.value, so changingentity_id,value_attribute, or a dynamic source can leave the wrong selector button highlighted until the next HA update.💡 Suggested fix
shouldUpdate(changedProperties: PropertyValues) { const should = super.shouldUpdate(changedProperties); const optionsChanged = this.setOptions(); + if ( + optionsChanged && + !changedProperties.has('hass') && + !changedProperties.has('stateObj') && + !changedProperties.has('value') + ) { + this.setValue(); + } if ( changedProperties.has('hass') || changedProperties.has('stateObj') || changedProperties.has('value')🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/classes/custom-feature-selector.ts` around lines 118 - 150, The options rebuild path in shouldUpdate (method shouldUpdate) can change the resolved options without updating the derived selection value, so when setOptions() returns true you must refresh the derived value (call the same logic that setValue() performs) so updated()/onFocus() compare against the new value; modify shouldUpdate to call this.setValue() (or the internal value-derivation helper) when optionsChanged is true (and before returning) and ensure this.value is updated accordingly so the highlighted selector stays correct after a config-only change.src/utils/options.ts (1)
25-31:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAllow numeric option lists through this filter.
parseOptionsList()andbuildTemplatedOption()already handle primitive numbers, but this predicate rejects any all-number array. That keeps numeric-valued attributes out ofoptionListAttributes()and prevents the editor from surfacing otherwise valid dynamic sources.♻️ Proposed fix
export function isOptionListAttribute(name: string, value: unknown): boolean { + const isLikelyNumericTuple = + Array.isArray(value) && + (value.length == 3 || value.length == 4) && + value.every((item) => typeof item == 'number'); return ( !NON_OPTION_ATTRIBUTES.has(name) && Array.isArray(value) && value.length >= 2 && - !value.every((item) => typeof item == 'number') + !isLikelyNumericTuple ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/utils/options.ts` around lines 25 - 31, The predicate in isOptionListAttribute() incorrectly rejects arrays composed entirely of numbers, which prevents numeric option lists from being recognized; update the function (referencing isOptionListAttribute and NON_OPTION_ATTRIBUTES) to remove the "!value.every((item) => typeof item == 'number')" exclusion so that any Array with length >= 2 passes (while still excluding names in NON_OPTION_ATTRIBUTES), and ensure related consumers like optionListAttributes(), parseOptionsList(), and buildTemplatedOption() continue to handle numeric primitives correctly (add/update tests for a numeric-only array case).src/classes/base-custom-feature.ts (2)
663-669:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't stringify template results before parsing them.
This erases structured outputs: a native array becomes a comma-joined string, and an array of objects becomes
"[object Object],...". The new JSON/YAML/object-item support inparseOptionsList()/buildTemplatedOption()never runs for those template results.💡 Suggested fix
// Template source. if (typeof config == 'string' && config.trim()) { - const rendered = String(this.renderTemplate(config)); + const rendered = this.renderTemplate(config) as unknown; return { items: parseOptionsList(rendered), - signature: `tmpl:${rendered}:${optionSignature}`, + signature: `tmpl:${JSON.stringify(rendered ?? null)}:${optionSignature}`, }; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/classes/base-custom-feature.ts` around lines 663 - 669, The code currently calls String(this.renderTemplate(config)) which destroys structured template results; change it to keep the raw rendered value (const rendered = this.renderTemplate(config)), pass that raw value into parseOptionsList(rendered) so JSON/YAML/object-item handling in parseOptionsList/buildTemplatedOption can run, and compute the signature from a stable serialization (e.g. use rendered as-is if typeof rendered === 'string' else JSON.stringify(rendered)) instead of String(...). Ensure references: renderTemplate, parseOptionsList, and any use in buildTemplatedOption are updated accordingly.
570-594:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep template-mode configs out of attribute resolution.
resolveAttributeSource()ignores the shape ofconfig.options, so any leftoveroptions_attribute/options_entityon a string-template config preempts the template path. On non-selectdomains that can collapse to no options; onselect/input_selectit silently reads the entity’soptionsinstead of the rendered template.💡 Suggested fix
private resolveAttributeSource(): | { entityId: string; attribute: string } | undefined { + if (typeof this.config.options == 'string') { + return undefined; + } if ( this.config.options_attribute === undefined && this.config.options_entity === undefined ) { return undefined;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/classes/base-custom-feature.ts` around lines 570 - 594, resolveAttributeSource currently uses options_attribute/options_entity even when config.options is a template string; change it to detect template-mode options and ignore options_attribute/options_entity in that case. In resolveAttributeSource, check this.config.options (and/or whether it's a string/template) and only use this.config.options_attribute / this.config.options_entity when options is a concrete options object/array (not a string/template); keep using renderTemplate, resolveOptionsAttribute, and fallback to this.config.entity_id as before, but short-circuit to return undefined or use only entity_id when options is template-mode so template-mode configs don't preempt the template path.src/custom-features-row-editor.ts (1)
2785-2894:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't coerce object-valued
entry.optionsto an empty manual list.This branch only treats split
options_attribute/options_entityfields and string templates as dynamic. Any object-valuedentry.optionstherefore falls through here, andentry.options = optionsrewrites that source to[]on the next autofill pass.💡 Suggested fix
const isAttributeMode = entry.options_attribute !== undefined || entry.options_entity !== undefined; const isTemplateMode = typeof entry.options == 'string'; - if (isAttributeMode || isTemplateMode) { + const isObjectSource = + entry.options != null && + typeof entry.options == 'object' && + !Array.isArray(entry.options); + if (isAttributeMode || isTemplateMode || isObjectSource) { + if (isObjectSource) { + const source = entry.options as { + entity_id?: string; + attribute?: string; + }; + entry.options_entity ??= source.entity_id; + entry.options_attribute ??= source.attribute; + delete entry.options; + } if (isAttributeMode) { const sourceEntity = this.renderTemplate( (entry.options_entity || entry.entity_id || '') as string,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/custom-features-row-editor.ts` around lines 2785 - 2894, The code currently sets const options = Array.isArray(entry.options) ? entry.options : []; which coerces object-valued entry.options into an empty array and loses the original object on next autofill; update the logic around isAttributeMode/isTemplateMode and the options initialization so that if entry.options is an object (typeof entry.options === 'object' && entry.options !== null) you preserve it (use it directly) instead of replacing it with [] — keep using Array.isArray(entry.options) for array paths but do not overwrite object-valued entry.options when later assigning entry.options = options.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@src/classes/base-custom-feature.ts`:
- Around line 663-669: The code currently calls
String(this.renderTemplate(config)) which destroys structured template results;
change it to keep the raw rendered value (const rendered =
this.renderTemplate(config)), pass that raw value into
parseOptionsList(rendered) so JSON/YAML/object-item handling in
parseOptionsList/buildTemplatedOption can run, and compute the signature from a
stable serialization (e.g. use rendered as-is if typeof rendered === 'string'
else JSON.stringify(rendered)) instead of String(...). Ensure references:
renderTemplate, parseOptionsList, and any use in buildTemplatedOption are
updated accordingly.
- Around line 570-594: resolveAttributeSource currently uses
options_attribute/options_entity even when config.options is a template string;
change it to detect template-mode options and ignore
options_attribute/options_entity in that case. In resolveAttributeSource, check
this.config.options (and/or whether it's a string/template) and only use
this.config.options_attribute / this.config.options_entity when options is a
concrete options object/array (not a string/template); keep using
renderTemplate, resolveOptionsAttribute, and fallback to this.config.entity_id
as before, but short-circuit to return undefined or use only entity_id when
options is template-mode so template-mode configs don't preempt the template
path.
In `@src/classes/custom-feature-selector.ts`:
- Around line 118-150: The options rebuild path in shouldUpdate (method
shouldUpdate) can change the resolved options without updating the derived
selection value, so when setOptions() returns true you must refresh the derived
value (call the same logic that setValue() performs) so updated()/onFocus()
compare against the new value; modify shouldUpdate to call this.setValue() (or
the internal value-derivation helper) when optionsChanged is true (and before
returning) and ensure this.value is updated accordingly so the highlighted
selector stays correct after a config-only change.
In `@src/custom-features-row-editor.ts`:
- Around line 2785-2894: The code currently sets const options =
Array.isArray(entry.options) ? entry.options : []; which coerces object-valued
entry.options into an empty array and loses the original object on next
autofill; update the logic around isAttributeMode/isTemplateMode and the options
initialization so that if entry.options is an object (typeof entry.options ===
'object' && entry.options !== null) you preserve it (use it directly) instead of
replacing it with [] — keep using Array.isArray(entry.options) for array paths
but do not overwrite object-valued entry.options when later assigning
entry.options = options.
In `@src/utils/options.ts`:
- Around line 25-31: The predicate in isOptionListAttribute() incorrectly
rejects arrays composed entirely of numbers, which prevents numeric option lists
from being recognized; update the function (referencing isOptionListAttribute
and NON_OPTION_ATTRIBUTES) to remove the "!value.every((item) => typeof item ==
'number')" exclusion so that any Array with length >= 2 passes (while still
excluding names in NON_OPTION_ATTRIBUTES), and ensure related consumers like
optionListAttributes(), parseOptionsList(), and buildTemplatedOption() continue
to handle numeric primitives correctly (add/update tests for a numeric-only
array case).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 602269f2-0fab-403e-ad0b-4310b2e18d0d
⛔ Files ignored due to path filters (1)
dist/custom-card-features.min.jsis excluded by!**/dist/**,!**/*.min.js
📒 Files selected for processing (8)
README.mdsrc/classes/base-custom-feature.tssrc/classes/custom-feature-dropdown.tssrc/classes/custom-feature-selector.tssrc/custom-features-row-editor.tssrc/models/interfaces/IConfig.tssrc/utils/index.tssrc/utils/options.ts
📜 Review details
🔇 Additional comments (2)
README.md (2)
206-206: LGTM!
56-57: LGTM!Also applies to: 78-79, 173-206, 229-229, 2498-2544
Let a dropdown or selector generate its options from a list instead of requiring every option to be hand-written, creating one option per item. Implements Discussion Nerwyn#137 ("Automatically populate dropdown options from effects / entity attribute lists"). Builds on the per-option config.option template variable from Nerwyn#198. Sources (all backwards compatible with the existing explicit list): - options_attribute / options_entity: read the list straight from an entity attribute, e.g. a light's effect_list, with no template. select/input_select default to their `options` attribute. - options as a template string: rendered to a list for computed sources, parsed from comma/newline strings or JSON/YAML arrays (including nunjucks `| dump`, HTML-escaped) and YAML block sequences; items may be {value, label, icon} objects. - option_template: per-item label/icon/action applied to every generated option, inheriting the parent entity; each item's value is exposed as the `option` variable (alias of config.option). - Recognized list attributes (effect_list, source_list, sound_mode_list, activity_list, hvac/fan/swing/preset modes, available_modes, operation_list, fan_speed_list, select options) get a sensible default action and tracked attribute, so they work with no option template. - Resolved lists are cached and re-render when the source changes; the selected option is highlighted using the option's own context. - Editor: an "Options source" picker (manual / entity attribute / template), with the source-attribute picker limited to usable list attributes, the single qualifying attribute auto-selected, and a focused option-template editor pre-filled with the default label and action. - Docs: Dropdowns/Selectors sections, a Dynamic Options section, and Example 9. Ref Nerwyn#137
kristofferR
force-pushed
the
dynamic-dropdown-options
branch
from
50c2048 to
3d7a69e
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/classes/base-custom-feature.ts (1)
673-677:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPass the raw template result into
parseOptionsList().Line 674 flattens any structured template result before parsing. That turns template-backed arrays into comma-joined strings and
{ value, label, icon }[]into"[object Object],...", so the structured-item support added inparseOptionsList()/buildTemplatedOption()never runs for template sources.Suggested fix
// Template source. if (typeof config == 'string' && config.trim()) { - const rendered = String(this.renderTemplate(config)); + const rendered = this.renderTemplate(config as string) as unknown; return { items: parseOptionsList(rendered), - signature: `tmpl:${rendered}:${optionSignature}`, + signature: `tmpl:${JSON.stringify(rendered ?? null)}:${optionSignature}`, }; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/classes/base-custom-feature.ts` around lines 673 - 677, The code is calling renderTemplate(config) and immediately converting to String before passing to parseOptionsList, which flattens structured template results and prevents parseOptionsList/buildTemplatedOption from recognizing arrays/objects; instead, pass the raw renderTemplate result into parseOptionsList (i.e., remove String(...) / .trim() conversion path) so that when renderTemplate returns an array or objects the structured-item parsing is preserved, while still using the existing signature computation (optionSignature) and returning the same shape (items and signature).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Around line 2532-2538: The example shows an empty `options_attribute:` which
is valid YAML but confusing; update the snippet for clarity by removing the
`options_attribute` line entirely (since select/input_select default to the
`options` attribute) or replace it with an inline comment like `# uses default
"options" attribute` so readers understand this signals the default behavior for
`select`/`input_select` (reference `options_attribute`, `option_template`,
`options`, and the example entity `input_select.scene`).
In `@src/classes/custom-feature-dropdown.ts`:
- Around line 186-193: The cached thumb/selected visual state is only recomputed
when changedProperties has 'hass', 'stateObj', or 'value' (and optionsChanged),
but config-only updates bypass that and leave stale visuals; in the
shouldUpdate() implementation update the invalidation condition to also include
changedProperties.has('config') (or otherwise detect super.shouldUpdate(...)
config-only renders) alongside optionsChanged so the cached thumb/selected state
is recomputed on config changes; adjust the same check in both components'
shouldUpdate() methods (the block that currently tests
changedProperties.has('hass'|'stateObj'|'value' and optionsChanged) so that
config-driven refreshes trigger the recompute path for the thumb/selected visual
state.
---
Duplicate comments:
In `@src/classes/base-custom-feature.ts`:
- Around line 673-677: The code is calling renderTemplate(config) and
immediately converting to String before passing to parseOptionsList, which
flattens structured template results and prevents
parseOptionsList/buildTemplatedOption from recognizing arrays/objects; instead,
pass the raw renderTemplate result into parseOptionsList (i.e., remove
String(...) / .trim() conversion path) so that when renderTemplate returns an
array or objects the structured-item parsing is preserved, while still using the
existing signature computation (optionSignature) and returning the same shape
(items and signature).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: eb1af553-f55e-4534-985d-ecade5383bcd
⛔ Files ignored due to path filters (1)
dist/custom-card-features.min.jsis excluded by!**/dist/**,!**/*.min.js
📒 Files selected for processing (8)
README.mdsrc/classes/base-custom-feature.tssrc/classes/custom-feature-dropdown.tssrc/classes/custom-feature-selector.tssrc/custom-features-row-editor.tssrc/models/interfaces/IConfig.tssrc/utils/index.tssrc/utils/options.ts
📜 Review details
🔇 Additional comments (5)
README.md (5)
56-57: LGTM!
78-79: LGTM!
2498-2531: LGTM!
2540-2544: LGTM!
173-206: README “recognized attributes” list matchesdefaultOptionActionauto-fill logic:src/utils/options.tsprovides defaults foreffect_list,source_list,sound_mode_list,activity_list,hvac_modes,fan_modes,swing_modes,available_modes,operation_list,fan_speed_list;preset_modesis special-cased (only for domains that supportset_preset_mode), andselect/input_selectdefaults tooptionswhenoptions_attributeis omitted or set tooptions.
| The same pattern works for any entity with a list attribute, for example a media player's `source_list` (paired with `media_player.select_source`) or a climate entity's `preset_modes`. For `select` and `input_select` entities you can leave the `options_attribute` value blank and omit `option_template`, since the `options` attribute is used and a `select_option` action is generated for you: | ||
|
|
||
| ```yaml | ||
| - type: dropdown | ||
| entity_id: input_select.scene | ||
| options_attribute: | ||
| ``` |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Clarify the blank options_attribute syntax in the example.
Line 2537 shows options_attribute: with no value after the colon. While this is valid YAML (parses as null), it may confuse readers. Consider either:
- Adding a comment explaining this means "use the default
optionsattribute" - Showing the example without the
options_attributekey at all (since the docs state select/input_select default tooptions)
The second option would be clearer and more consistent with the "you can leave the value blank and omit option_template" phrasing in the text above.
📝 Clearer example
```yaml
- type: dropdown
entity_id: input_select.scene
- options_attribute:
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @README.md around lines 2532 - 2538, The example shows an empty
options_attribute: which is valid YAML but confusing; update the snippet for
clarity by removing the options_attribute line entirely (since
select/input_select default to the options attribute) or replace it with an
inline comment like # uses default "options" attribute so readers understand
this signals the default behavior for select/input_select (reference
options_attribute, option_template, options, and the example entity
input_select.scene).
</details>
<!-- fingerprinting:phantom:triton:puma -->
<!-- cr-comment:v1:ca6d818d6ce0ce6782566cbb -->
<!-- This is an auto-generated comment by CodeRabbit -->
| if ( | ||
| changedProperties.has('hass') || | ||
| changedProperties.has('stateObj') || | ||
| changedProperties.has('value') | ||
| changedProperties.has('value') || | ||
| // Rebuilt options can change which option is selected and how its | ||
| // thumb renders, so recompute even on config-only updates. | ||
| optionsChanged | ||
| ) { |
There was a problem hiding this comment.
Derived thumb-state caching is too narrowly invalidated in both components. Both shouldUpdate() implementations allow config-only renders via super.shouldUpdate(...), but the cached thumb/selected visual state is only recomputed on a smaller subset of changes. Include config-driven refreshes in the same invalidation path so dropdowns/selectors do not render stale classes or selected appearance after editor/runtime config updates.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/classes/custom-feature-dropdown.ts` around lines 186 - 193, The cached
thumb/selected visual state is only recomputed when changedProperties has
'hass', 'stateObj', or 'value' (and optionsChanged), but config-only updates
bypass that and leave stale visuals; in the shouldUpdate() implementation update
the invalidation condition to also include changedProperties.has('config') (or
otherwise detect super.shouldUpdate(...) config-only renders) alongside
optionsChanged so the cached thumb/selected state is recomputed on config
changes; adjust the same check in both components' shouldUpdate() methods (the
block that currently tests changedProperties.has('hass'|'stateObj'|'value' and
optionsChanged) so that config-driven refreshes trigger the recompute path for
the thumb/selected visual state.
- Add explicit optionType config field (list/attribute/template) instead
of inferring the options source from field shapes; centralize the
remaining inference in a single inferOptionType() fallback for legacy
configs
- Reuse the existing dropdown-option/button editors for the option
template editor, gaining the selector's momentary/hold/double-tap
actions
- Simplify the Option Template editor entry to a header and edit button
- Trim the attribute/template source descriptions and drop the
{{ option }} guidance outside the option template page
- Use || for option label aliases so blank strings fall through
- Use .filter(Boolean) and consolidate the resolveAttributeSource guard
- Drop the config-only setValue() path in dropdown/selector shouldUpdate
- Remove the unnecessary option_template deprecated-field upgrade
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/classes/custom-feature-dropdown.ts (1)
170-183:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRefresh cached thumb/selected state on config-only updates.
super.shouldUpdate()already permits config-only renders, but this recompute path still runs only forhass,stateObj,value, oroptionsChanged. Changingthumb,entity_id,value_attribute, or selected-option label/icon/styles templates can therefore leavethumbType,selectedIcon/selectedLabel/selectedStyles, and theoptionContextentity/attribute stale until a laterhassorvalueupdate.Suggested fix
shouldUpdate(changedProperties: PropertyValues) { let should = super.shouldUpdate(changedProperties); const optionsChanged = this.setOptions(); if (optionsChanged) { should = true; } + if ( + changedProperties.has('config') && + !changedProperties.has('hass') && + !changedProperties.has('value') + ) { + this.setValue(); + } if ( changedProperties.has('hass') || changedProperties.has('stateObj') || changedProperties.has('value') || + changedProperties.has('config') || // Rebuilt options can change which option is selected and how its // thumb renders, so recompute when they do. optionsChanged ) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/classes/custom-feature-dropdown.ts` around lines 170 - 183, The shouldUpdate path currently only recomputes selection/thumbnail when hass, stateObj, value, or optionsChanged change; add checks for config-only properties that affect option rendering so thumbType, selectedIcon/selectedLabel/selectedStyles and optionContext are refreshed on config updates. In shouldUpdate (which calls setOptions()), extend the condition to also trigger the recompute when changedProperties.has('thumb'), changedProperties.has('entity_id'), changedProperties.has('value_attribute') and any template-related config props used to build selected labels/icons/styles (e.g. selectedLabelTemplate, selectedIconTemplate, selectedStylesTemplate) so that after setOptions() you set should = true and re-run the selection/thumbnail update code paths that compute thumbType, selectedIcon/selectedLabel/selectedStyles and optionContext.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/classes/base-custom-feature.ts`:
- Around line 558-560: The options assignment in BaseCustomFeature (the
this.options = source.items.filter(Boolean).map(...)) incorrectly drops numeric
(0) and false values; change the filter to only remove null/undefined (e.g.,
filter(item => item !== null && item !== undefined)) so that
isOptionListAttribute()-allowed numeric lists like [0,1,2] preserve 0 and still
pass each item into buildOption(item, source.attribute).
- Around line 570-580: resolveAttributeSource currently infers source solely
from leftover fields and can be overridden by a stale options string; update the
function resolveAttributeSource to honor the persisted optionType selector
(config.optionType): if config.optionType === 'template' always treat the source
as template (return undefined), if config.optionType === 'attribute' always read
and return { entityId: config.options_entity, attribute:
config.options_attribute } even when config.options is a non-empty string, and
if config.optionType is undefined keep the current heuristic (fall back to
existing checks of config.options, config.options_attribute,
config.options_entity); ensure you reference and use the properties
config.optionType, config.options, config.options_attribute, and
config.options_entity inside resolveAttributeSource.
In `@src/custom-features-row-editor.ts`:
- Around line 2639-2651: The autofill guard that prevents adding a default
tap_action should be extended to also detect any momentary_*_action keys so
templates that only declare momentary actions don't get an implicit tap_action;
update the logic that currently checks for double_tap_action or hold_action to
also scan the resolved attribute/template (the value computed via
renderTemplate(...) and resolveOptionsAttribute(...) assigned to attribute)
and/or the entry object (entry.double_tap_action, entry.hold_action) for any
keys matching /^momentary_.*_action$/ and suppress the default tap_action when
found; mirror the same change at the other equivalent site (the block around
lines 2676-2680) so the editor autofill matches
BaseCustomFeature.buildOption()'s runtime guard.
---
Duplicate comments:
In `@src/classes/custom-feature-dropdown.ts`:
- Around line 170-183: The shouldUpdate path currently only recomputes
selection/thumbnail when hass, stateObj, value, or optionsChanged change; add
checks for config-only properties that affect option rendering so thumbType,
selectedIcon/selectedLabel/selectedStyles and optionContext are refreshed on
config updates. In shouldUpdate (which calls setOptions()), extend the condition
to also trigger the recompute when changedProperties.has('thumb'),
changedProperties.has('entity_id'), changedProperties.has('value_attribute') and
any template-related config props used to build selected labels/icons/styles
(e.g. selectedLabelTemplate, selectedIconTemplate, selectedStylesTemplate) so
that after setOptions() you set should = true and re-run the selection/thumbnail
update code paths that compute thumbType,
selectedIcon/selectedLabel/selectedStyles and optionContext.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b7f9a544-b621-45ac-aa26-c99f42876652
⛔ Files ignored due to path filters (1)
dist/custom-card-features.min.jsis excluded by!**/dist/**,!**/*.min.js
📒 Files selected for processing (6)
src/classes/base-custom-feature.tssrc/classes/custom-feature-dropdown.tssrc/classes/custom-feature-selector.tssrc/custom-features-row-editor.tssrc/models/interfaces/IConfig.tssrc/utils/options.ts
💤 Files with no reviewable changes (1)
- src/classes/custom-feature-selector.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c3c0f7eed5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| this.optionsSignature = source.signature; | ||
| this.options = source.items | ||
| .filter(Boolean) |
There was a problem hiding this comment.
When an attribute or JSON/YAML template source returns primitive options like 0 or false, this truthiness filter drops them before buildOption() runs. Numeric lists are explicitly accepted by isOptionListAttribute(), and selected-state comparison stringifies values later, so a valid first option such as 0 disappears from the dropdown/selector and can never be selected; filter only null/undefined/blank items instead of all falsy values.
Useful? React with 👍 / 👎.
| this.optionsSignature = signature; | ||
| // A shallow copy avoids aliasing the config array. | ||
| this.options = [...(config as IOption[])]; | ||
| return true; |
There was a problem hiding this comment.
When an explicit options array contains primitive entries such as options: [A, B], this stores the raw strings in this.options; the dropdown/selector render paths then clone each entry and assign fields like option.haptics/option.label, which throws for primitives in module strict mode (or leaves option.option undefined). Normalize primitive array items to option objects before caching them, matching the dynamic source path.
Useful? React with 👍 / 👎.
…runtime, guard momentary templates - Keep numeric 0/false option values instead of dropping them with a plain truthiness filter - Make resolveAttributeSource/resolveDynamicOptions honor the explicit optionType contract, falling back to field-shape inference only when it is absent - Suppress the autofilled default tap_action when the option template uses momentary actions, mirroring the runtime buildOption guard
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/custom-features-row-editor.ts (2)
325-331:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPreserve the momentary tab when reopening the option-template editor.
editOptionTemplate()always resetsactionsTabIndexto0. If the template only hasmomentary_*_actionconfigured, reopening the editor lands on the default tab and makes the active behavior look missing until the user switches tabs manually.Proposed fix
editOptionTemplate(_e: Event) { this.yamlStringsCache = {}; this.yamlString = undefined; - this.actionsTabIndex = 0; + const template = this.config.entries[this.entryIndex]?.option_template; + this.actionsTabIndex = + template && + (this.renderTemplate( + template.momentary_start_action?.action ?? 'none', + this.getEntryContext(template as IEntry), + ) != 'none' || + this.renderTemplate( + template.momentary_repeat_action?.action ?? 'none', + this.getEntryContext(template as IEntry), + ) != 'none' || + this.renderTemplate( + template.momentary_end_action?.action ?? 'none', + this.getEntryContext(template as IEntry), + ) != 'none') + ? 1 + : 0; this.activeEntryType = 'option_template'; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/custom-features-row-editor.ts` around lines 325 - 331, The editOptionTemplate method currently forces actionsTabIndex = 0 which loses the user's momentary tab selection; in src/custom-features-row-editor.ts modify editOptionTemplate(_) so it does not unconditionally reset actionsTabIndex to 0 (either remove that line or only reset when truly needed), preserving the existing actionsTabIndex (or explicitly set it to the momentary-action tab when the template has only momentary_*_action configured) so reopening the option-template editor keeps the momentary tab active.
942-975:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPersist repeat timing under
momentary_repeat_action, nothold_action.This branch still binds the timing controls to
hold_action.hold_time/hold_action.repeat_delayeven whenactionTypeismomentary_repeat_action. With the new option-template editor reusing this path, saving repeat timing on generated selector options writes the config under the wrong key, so runtime never sees it.Proposed fix
${action != 'none' && actionType == 'double_tap_action' ? this.buildSelector( 'Double tap window', 'double_tap_action.double_tap_window', @@ : ['hold_action', 'momentary_repeat_action'].includes(actionType) && this.activeEntry?.[actionType] ? html`<div class="form"> ${this.buildSelector( 'Hold time', - 'hold_action.hold_time', + `${actionType}.hold_time`, { number: { min: 0, @@ }, HOLD_TIME, )} ${this.renderTemplate( - this.activeEntry?.hold_action?.action as string, + this.activeEntry?.[actionType]?.action as string, context, ) == 'repeat' || actionType == 'momentary_repeat_action' ? this.buildSelector( 'Repeat delay', - 'hold_action.repeat_delay', + `${actionType}.repeat_delay`, { number: { min: 0,
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/custom-features-row-editor.ts`:
- Around line 325-331: The editOptionTemplate method currently forces
actionsTabIndex = 0 which loses the user's momentary tab selection; in
src/custom-features-row-editor.ts modify editOptionTemplate(_) so it does not
unconditionally reset actionsTabIndex to 0 (either remove that line or only
reset when truly needed), preserving the existing actionsTabIndex (or explicitly
set it to the momentary-action tab when the template has only momentary_*_action
configured) so reopening the option-template editor keeps the momentary tab
active.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 369c11b1-1cf4-4f4a-8006-0749ea8a9b1b
⛔ Files ignored due to path filters (1)
dist/custom-card-features.min.jsis excluded by!**/dist/**,!**/*.min.js
📒 Files selected for processing (2)
src/classes/base-custom-feature.tssrc/custom-features-row-editor.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6180aa3d8c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| sourceState && | ||
| entry.options_attribute && | ||
| !hasTemplate(entry.options_attribute) && | ||
| !(entry.options_attribute in sourceState.attributes) |
There was a problem hiding this comment.
When a hand-written dynamic source uses a nested attribute path such as options_attribute: supported.options, the runtime resolves it with deepGet() in BaseCustomFeature.resolveDynamicOptions(), but this stale-attribute check only tests the raw dotted path against top-level sourceState.attributes. Opening the visual editor/autofill therefore treats the valid nested source as missing and clears options_attribute, so the dropdown/selector loses its configured dynamic list.
Useful? React with 👍 / 👎.
| this.config.option_template ?? null, | ||
| this.config.autofill_entity_id ?? null, |
There was a problem hiding this comment.
When autofill_entity_id is templated on the parent feature or inside option_template, buildOption() renders it to decide whether to add default tap actions, but this signature only includes the raw template/config object. If the rendered autofill value changes while the option list itself stays the same, the generated options are not rebuilt and keep stale default actions until the source list changes or the card remounts.
Useful? React with 👍 / 👎.
Drop the feature-list row wrapper so the Option Template entry is just the header with the edit icon button.
Rename the manual options source value from 'list' to 'default' to match the thumb type convention, and define OptionTypes/OptionType as a const array and derived type like the thumb types.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/classes/base-custom-feature.ts (1)
595-599:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTemplate-empty
options_entitydoes not fall back toentity_id.At Line 595, fallback is applied before rendering. If
options_entityis a template string that renders to'', resolution uses an empty entity id instead of the feature entity.Suggested fix
- const entityId = String( - this.renderTemplate( - (this.config.options_entity || this.config.entity_id || '') as string, - ), - ); + const renderedOptionsEntity = String( + this.renderTemplate((this.config.options_entity ?? '') as string), + ); + const entityId = + renderedOptionsEntity || + String(this.renderTemplate((this.config.entity_id ?? '') as string));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/classes/base-custom-feature.ts` around lines 595 - 599, The fallback logic in renderTemplate for the entityId assignment is evaluated before template rendering, causing template strings that render to empty values to bypass the fallback to entity_id. Restructure the code to first render the options_entity template if it exists, then check if the rendered result is empty, and only then fall back to rendering entity_id. This ensures that a template string in options_entity that renders to an empty string will properly trigger fallback to entity_id rather than resulting in an empty entity id.
♻️ Duplicate comments (2)
src/custom-features-row-editor.ts (2)
327-332:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRestore the momentary-actions tab when reopening an option template.
editOptionTemplate()always resetsactionsTabIndexto0, so a selector template that only usesmomentary_*_actionreopens on the default tab and looks unconfigured until the user switches tabs manually.💡 Suggested fix
editOptionTemplate(_e: Event) { this.yamlStringsCache = {}; this.yamlString = undefined; - this.actionsTabIndex = 0; + const template = this.config.entries[this.entryIndex]?.option_template ?? {}; + const context = this.getEntryContext(template as IEntry); + this.actionsTabIndex = + this.renderTemplate( + template.momentary_start_action?.action ?? 'none', + context, + ) != 'none' || + this.renderTemplate( + template.momentary_repeat_action?.action ?? 'none', + context, + ) != 'none' || + this.renderTemplate( + template.momentary_end_action?.action ?? 'none', + context, + ) != 'none' + ? 1 + : 0; this.activeEntryType = 'option_template'; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/custom-features-row-editor.ts` around lines 327 - 332, The editOptionTemplate() method unconditionally resets actionsTabIndex to 0, causing option templates that use only momentary_*_action to reopen on the default tab instead of the appropriate tab for momentary actions. Instead of always setting actionsTabIndex to 0, determine the correct tab index based on the template's configuration—if the template contains momentary actions, set actionsTabIndex to the appropriate tab index that displays those actions, otherwise default to 0.
352-366:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep legacy
{ entity_id, attribute }option sources on the attribute path.A non-array object in
entry.optionsnow falls through to'default'. Opening one of those legacy configs sends the editor down the manual-list branch, wherebuildEntryList('option')tries to.map()the object, and the attribute UI still ignoresoptions.entity_id/options.attributeif it does render.💡 Suggested fix
inferOptionType(entry?: IEntry): OptionType { const options = entry?.options; + if (options && typeof options == 'object' && !Array.isArray(options)) { + return 'attribute'; + } if (typeof options == 'string' && options.trim()) { return 'template'; } @@ buildOptionsSection(type: 'dropdown' | 'selector') { const mode = this.optionsMode; const noun = type == 'dropdown' ? 'dropdown' : 'selector'; + const legacySource = + this.activeEntry?.options && + typeof this.activeEntry.options == 'object' && + !Array.isArray(this.activeEntry.options) + ? (this.activeEntry.options as { + entity_id?: string; + attribute?: string; + }) + : {}; const sourceEntity = this.renderTemplate( - (this.activeEntry?.options_entity || + (this.activeEntry?.options_entity || + legacySource.entity_id || this.activeEntry?.entity_id || '') as string, this.getEntryContext(this.activeEntry ?? {}), ) as string; @@ ${this.buildSelector( 'Source entity (optional)', 'options_entity', { entity: {} }, - this.activeEntry?.entity_id, + legacySource.entity_id ?? this.activeEntry?.entity_id, )} - ${this.buildSelector('Source attribute', 'options_attribute', { - attribute: { - entity_id: - this.activeEntry?.options_entity || this.activeEntry?.entity_id, - hide_attributes: hideAttributes, - }, - })}` + ${this.buildSelector( + 'Source attribute', + 'options_attribute', + { + attribute: { + entity_id: + this.activeEntry?.options_entity || + legacySource.entity_id || + this.activeEntry?.entity_id, + hide_attributes: hideAttributes, + }, + }, + legacySource.attribute, + )}`Also applies to: 1571-1602
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/custom-features-row-editor.ts` around lines 352 - 366, The inferOptionType method does not recognize legacy option sources that are non-array objects with entity_id and/or attribute properties. Add a check in the method to detect when entry.options is a non-array object containing entity_id or attribute properties, and return 'attribute' for these cases. This check should be positioned before the final return 'default' statement to ensure legacy configs are routed to the attribute path instead of falling through to the default manual-list branch where they cannot be properly handled.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/classes/base-custom-feature.ts`:
- Around line 595-599: The fallback logic in renderTemplate for the entityId
assignment is evaluated before template rendering, causing template strings that
render to empty values to bypass the fallback to entity_id. Restructure the code
to first render the options_entity template if it exists, then check if the
rendered result is empty, and only then fall back to rendering entity_id. This
ensures that a template string in options_entity that renders to an empty string
will properly trigger fallback to entity_id rather than resulting in an empty
entity id.
---
Duplicate comments:
In `@src/custom-features-row-editor.ts`:
- Around line 327-332: The editOptionTemplate() method unconditionally resets
actionsTabIndex to 0, causing option templates that use only momentary_*_action
to reopen on the default tab instead of the appropriate tab for momentary
actions. Instead of always setting actionsTabIndex to 0, determine the correct
tab index based on the template's configuration—if the template contains
momentary actions, set actionsTabIndex to the appropriate tab index that
displays those actions, otherwise default to 0.
- Around line 352-366: The inferOptionType method does not recognize legacy
option sources that are non-array objects with entity_id and/or attribute
properties. Add a check in the method to detect when entry.options is a
non-array object containing entity_id or attribute properties, and return
'attribute' for these cases. This check should be positioned before the final
return 'default' statement to ensure legacy configs are routed to the attribute
path instead of falling through to the default manual-list branch where they
cannot be properly handled.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 698dd090-1cb8-46c8-928f-f0caa3f10051
⛔ Files ignored due to path filters (1)
dist/custom-card-features.min.jsis excluded by!**/dist/**,!**/*.min.js
📒 Files selected for processing (3)
src/classes/base-custom-feature.tssrc/custom-features-row-editor.tssrc/models/interfaces/IConfig.ts
📜 Review details
🔇 Additional comments (1)
src/models/interfaces/IConfig.ts (1)
60-67: LGTM!Also applies to: 101-102
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3a39c57dab
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Explicit list of options (backwards compatible, unchanged behavior). The | ||
| // signature snapshot detects in-place edits to option objects, which a | ||
| // reference comparison would miss. | ||
| if (Array.isArray(config)) { |
There was a problem hiding this comment.
If a user converts an existing manual dropdown/selector by adding options_attribute: effect_list or setting optionType: attribute but leaves the old options: []/array in place, this early array branch returns before resolveDynamicOptions() can honor the dynamic source. The editor infers attribute mode from options_attribute, yet the runtime keeps rendering the stale manual array or an empty list, so the new source appears broken until the user manually deletes options; check the explicit source fields before accepting the array.
Useful? React with 👍 / 👎.
| if (template.label == null) { | ||
| template.label = '{{ option }}'; |
There was a problem hiding this comment.
When a template source emits the documented object items like { value: 'eco', label: 'Eco mode' } and the user edits the card in the visual editor, this autofill writes option_template.label = '{{ option }}'. buildTemplatedOption() only copies an item's label when the template label is undefined, so the saved template forces every generated option to display the raw value instead of the provided label; leave the label unset or treat this default as a fallback so object-provided labels survive editor autofill.
Useful? React with 👍 / 👎.
| config.trim() | ||
| ) { | ||
| const rendered = String(this.renderTemplate(config)); | ||
| return { |
There was a problem hiding this comment.
When an options template uses the documented parent context, e.g. {{ state_attr(config.entity, 'effect_list') }}, changing the dropdown/selector entity_id in the editor is a config-only update and super.shouldUpdate() does not refresh this.entityId. This render therefore still evaluates config.entity as the previous entity (while the cache key is already based on the new one), so the generated options can keep showing the old entity's list until a later hass update or remount; render the source template with the freshly resolved feature entity instead of the cached context.
Useful? React with 👍 / 👎.
| (this.config.value_attribute as string) ?? | ||
| this.defaultValueAttribute(), |
There was a problem hiding this comment.
When a dynamic dropdown/selector omits value_attribute, this new default can change with the selected options source, but setValue() is only run from super.shouldUpdate() on hass/state/value changes. If the editor changes options_attribute from one recognized list to another (for example effect_list to source_list) without an HA state update, setOptions() rebuilds the choices while this.valueAttribute and this.value still come from the old source, so the current option is not highlighted until a later state update or remount.
Useful? React with 👍 / 👎.
| const SELECT_DOMAINS = ['select', 'input_select']; | ||
|
|
||
| // Domains that provide a set_preset_mode service for the preset_modes attribute. | ||
| const PRESET_MODE_DOMAINS = ['climate', 'fan', 'humidifier']; |
There was a problem hiding this comment.
Including humidifier here makes a humidifier feature sourced from a preset_modes list generate humidifier.set_preset_mode, but Home Assistant humidifier entities expose modes via available_modes/mode and humidifier.set_mode rather than a preset-mode service. In that configuration, every generated option gets an invalid default action; keep humidifiers on the existing available_modes mapping instead of treating them as preset-mode entities.
Useful? React with 👍 / 👎.
Humidifier entities expose modes via available_modes/mode and humidifier.set_mode, not a preset-mode service, so drop humidifier from the preset_modes domain mapping to avoid generating an invalid humidifier.set_preset_mode default action.
setOptions() returned early on any options array, so a config that switched to an attribute/template source (or set optionType) but still carried the old array rendered the stale array instead of the dynamic list. Skip the array branch when a dynamic source is configured, matching the editor's source classification.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/custom-features-row-editor.ts (2)
352-366:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep legacy object-based option sources on the attribute path.
inferOptionType()no longer recognizes the oldoptions: { entity_id, attribute }shape, so those entries now fall into'default'mode. That sends a plain object into the manual-list path, wherebuildEntryList('option')and the option mutators treat it asIOption[]and will blow up on.map()/.splice(). Even if the entry survives that, Lines 1571-1602 never read the legacyentity_id/attribute, so opening and saving the editor can silently rewrite the source.💡 Suggested fix
inferOptionType(entry?: IEntry): OptionType { const options = entry?.options; if (typeof options == 'string' && options.trim()) { return 'template'; } + if (options && typeof options == 'object' && !Array.isArray(options)) { + return 'attribute'; + } if ( entry?.options_attribute !== undefined || entry?.options_entity !== undefined ) { return 'attribute'; @@ buildOptionsSection(type: 'dropdown' | 'selector') { const mode = this.optionsMode; const noun = type == 'dropdown' ? 'dropdown' : 'selector'; + const legacySource = + this.activeEntry?.options && + typeof this.activeEntry.options == 'object' && + !Array.isArray(this.activeEntry.options) + ? (this.activeEntry.options as { + entity_id?: string; + attribute?: string; + }) + : undefined; @@ const sourceEntity = this.renderTemplate( - (this.activeEntry?.options_entity || + (this.activeEntry?.options_entity || + legacySource?.entity_id || this.activeEntry?.entity_id || '') as string, this.getEntryContext(this.activeEntry ?? {}), ) as string; @@ ${this.buildSelector( 'Source entity (optional)', 'options_entity', { entity: {} }, - this.activeEntry?.entity_id, + legacySource?.entity_id ?? this.activeEntry?.entity_id, )} ${this.buildSelector('Source attribute', 'options_attribute', { attribute: { entity_id: - this.activeEntry?.options_entity || this.activeEntry?.entity_id, + this.activeEntry?.options_entity || + legacySource?.entity_id || + this.activeEntry?.entity_id, hide_attributes: hideAttributes, }, - })}` + }, legacySource?.attribute)}`Also applies to: 1571-1602
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/custom-features-row-editor.ts` around lines 352 - 366, The inferOptionType() method does not recognize the legacy object-based option sources that have entity_id and attribute properties, causing them to incorrectly fall into 'default' mode instead of 'attribute' mode. This leads to these legacy entries being treated as plain objects in the manual-list path where they fail on .map()/.splice() operations. Add a check in inferOptionType() to detect when the options parameter is an object containing entity_id or attribute properties and return 'attribute' for those cases before the final return statement, preserving backward compatibility with the legacy option source format.
327-332:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winInitialize the option-template action tab from the template’s momentary actions.
A selector
option_templatethat only configuresmomentary_*_actionalways reopens on the default tab, so the configured behavior looks missing until the user switches tabs manually.editOptionTemplate()should mirror the tab-selection logic already used ineditEntry()/editOption().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/custom-features-row-editor.ts` around lines 327 - 332, The editOptionTemplate() method unconditionally sets actionsTabIndex to 0, which causes the UI to always open on the default tab regardless of which momentary action type is actually configured in the template. To fix this, examine the tab-selection logic already implemented in the editEntry() and editOption() methods (which determine actionsTabIndex based on the configured momentary action type), and apply that same logic to editOptionTemplate() so it correctly initializes the active tab based on the template's configured momentary actions instead of always defaulting to tab 0.
♻️ Duplicate comments (1)
src/classes/base-custom-feature.ts (1)
690-699:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep structured template results intact.
parseOptionsList()already accepts native arrays/objects, but this branch coerces the template result withString(...)first. If the renderer returns an actual array (especially an array of{ value, label, icon }objects), it gets flattened beforebuildTemplatedOption()can use it.Minimal fix
- const rendered = String(this.renderTemplate(config)); + const rendered = this.renderTemplate(config as string) as unknown; return { items: parseOptionsList(rendered), - signature: `tmpl:${rendered}:${optionSignature}`, + signature: `tmpl:${JSON.stringify(rendered ?? null)}:${optionSignature}`, };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/classes/base-custom-feature.ts` around lines 690 - 699, The template rendering result is being unnecessarily coerced to a string with the String() wrapper, which destroys any structured data (like arrays of objects with value, label, icon properties) that renderTemplate() might return. Since parseOptionsList() already accepts native arrays and objects, remove the String() coercion around this.renderTemplate(config) so that the rendered result maintains its original data structure when passed to parseOptionsList(). Update the const rendered assignment to pass the raw rendered value directly without stringification.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/custom-features-row-editor.ts`:
- Around line 352-366: The inferOptionType() method does not recognize the
legacy object-based option sources that have entity_id and attribute properties,
causing them to incorrectly fall into 'default' mode instead of 'attribute'
mode. This leads to these legacy entries being treated as plain objects in the
manual-list path where they fail on .map()/.splice() operations. Add a check in
inferOptionType() to detect when the options parameter is an object containing
entity_id or attribute properties and return 'attribute' for those cases before
the final return statement, preserving backward compatibility with the legacy
option source format.
- Around line 327-332: The editOptionTemplate() method unconditionally sets
actionsTabIndex to 0, which causes the UI to always open on the default tab
regardless of which momentary action type is actually configured in the
template. To fix this, examine the tab-selection logic already implemented in
the editEntry() and editOption() methods (which determine actionsTabIndex based
on the configured momentary action type), and apply that same logic to
editOptionTemplate() so it correctly initializes the active tab based on the
template's configured momentary actions instead of always defaulting to tab 0.
---
Duplicate comments:
In `@src/classes/base-custom-feature.ts`:
- Around line 690-699: The template rendering result is being unnecessarily
coerced to a string with the String() wrapper, which destroys any structured
data (like arrays of objects with value, label, icon properties) that
renderTemplate() might return. Since parseOptionsList() already accepts native
arrays and objects, remove the String() coercion around
this.renderTemplate(config) so that the rendered result maintains its original
data structure when passed to parseOptionsList(). Update the const rendered
assignment to pass the raw rendered value directly without stringification.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: da661981-d324-4fa2-b4a7-f3a11137f10f
⛔ Files ignored due to path filters (1)
dist/custom-card-features.min.jsis excluded by!**/dist/**,!**/*.min.js
📒 Files selected for processing (4)
src/classes/base-custom-feature.tssrc/custom-features-row-editor.tssrc/models/interfaces/IConfig.tssrc/utils/options.ts
Drop the JSON/YAML parsing branch (and the js-yaml import). Attribute
sources already hand back arrays, and template sources render to a comma-
or newline-separated string, so the structured-parsing path was only used
for the niche case of objects emitted from a template — which option_template
with {{ option }} already covers for most uses.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f343f587f8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const separator = unescaped.includes('\n') ? '\n' : ','; | ||
| return unescaped | ||
| .split(separator) | ||
| .map((item) => item.trim()) | ||
| .filter((item) => item.length); |
There was a problem hiding this comment.
When an options template uses the documented | dump JSON/YAML array form, especially for { value, label, icon } objects or values containing commas, this path never attempts to load the structured array and instead splits the rendered string on commas/newlines. A rendered array such as [{"value":"eco","label":"Eco"}] is turned into string fragments rather than objects, so generated option values, labels, and default actions are wrong; parse JSON/YAML arrays before falling back to delimiter splitting.
Useful? React with 👍 / 👎.
| entity_id: | ||
| this.activeEntry?.options_entity || this.activeEntry?.entity_id, |
There was a problem hiding this comment.
When options_entity or the feature entity_id is templated, sourceEntity above is rendered for computing hidden attributes, but the actual ha-selector still receives the raw template text here. In that GUI context the Source attribute picker is bound to something like {{ states('input_text.source') }} instead of the current entity id, so users cannot pick valid attributes even though the runtime resolves the same template correctly; pass the rendered sourceEntity to the attribute selector.
Useful? React with 👍 / 👎.
1 participant
Summary
Dropdowns and selectors can now build their options from a list automatically, instead of every option having to be written out by hand. One option is generated per item at render time.
Implements discussion Nerwyn#137 (Nerwyn#137). Builds on the per-option
config.optiontemplate variable from Nerwyn#198.Ref Nerwyn#137
Why
Right now
optionshas to be a hand-written list of option objects. That doesn't work well for entities with long or changing attribute lists — a WLED light'seffect_list(200+ effects), a media player'ssource_list, theoptionsof aselect/input_select. With this change you can just point at the list and the options are generated for you.How it works
There are three ways to provide the list. An explicit list of option objects still works exactly as before.
Entity attribute — read the list straight from an attribute, no template needed:
For
select/input_selectthe attribute defaults tooptions, so the value can be left blank.Template — set
optionsto a template that renders a list. Comma- or newline-separated strings work, and so do JSON/YAML arrays (e.g. via| dump), including{ value, label, icon }objects:Option template —
option_templateis applied to every generated option and can set the label, icon, actions, and styles. Each item's value is available asoption(andconfig.option):Defaults
For known list attributes, the matching action and tracked attribute are filled in automatically, so most cases need no
option_templateat all:effect_list→light.turn_on,source_list/sound_mode_list→media_player.select_*,activity_list→remote.turn_on,hvac_modes/fan_modes/swing_modes/preset_modes→climate.*/<domain>.set_preset_mode,available_modes→humidifier.set_mode,operation_list→water_heater.set_operation_mode,fan_speed_list→vacuum.set_fan_speed, and a select'soptions→<domain>.select_option.Behavior
Editor
The editor gets an "Options source" picker with three modes: manual list, entity attribute, and template. The attribute picker only lists attributes that contain usable lists, and auto-selects when there's only one candidate. The option template gets its own editor, pre-filled with the default label and action.
Backward compatibility
optionslist of option objects is untouched.config.optionbehavior from {{option}} template variable on dropdown options Nerwyn/custom-card-features#198 is preserved;optionis added as an alias.Verification
npx tsc --noemit,npx eslint, andnpm run buildall pass.dist/is rebuilt and included, per repo convention. Tested in Home Assistant against WLED/Nanoleafeffect_list,input_select,climatemodes, and aremote.Summary by CodeRabbit
optionTypemodes plus per-option template shaping, with optional default action autofill.option/config.option, and generated options drive menu rendering and selection.