Skip to content

Commit 6ee6fbf

Browse files
Patch76claude
Patch76
and
claude
authored
fix: add name attributes to generated settings-UI form controls (a11y) (#1497)
* fix: add name attributes to generated settings-UI form controls (a11y) The settings UI generates its form controls (tool toggles, group masters, feature-flag / advanced / backup / code-mode fields, policy inputs) in JS without an id or name, tripping the accessibility rule "form field should have an id or name attribute" (~100 warnings on a populated page). Add a derived, additive `name` to each generator (e.g. `tool:<tool>:enabled`, `adv:<field>`, `feature:<field>`, `backup:<field>`, `policy:<purpose>`). No selector or event handler reads `name` (selection is via `data-*`), so behaviour — including the toggle / disabled / gate logic — is unchanged. A holistic JS-harness test asserts every rendered <input> carries a name or id. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix: address PR #1497 Gemini review — <select> a11y + format - Add `name` to the 4 generated <select> controls the input-only pass missed: the 3 policy predicate selects (path / op / value) and the advanced choices dropdown. The "id or name" a11y rule covers <select> too. - Expand the holistic a11y guard to scan <select> as well as <input>, and add a choices field to its fixture so the advanced dropdown actually renders and is verified. - Drop the redundant local `import re` from the new tests (re is already imported at module level). - Run `ruff format` — the red CI step was `ruff format --check`, not the lint check; the new test file was not canonically formatted. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: cover tool-group + policy form-control names (#1497 step-5.5) Pre-mark-ready adversarial pass surfaced two generators the holistic guard didn't reach: - group-master toggle: added a `name="tool-group:"` sanity assert to the holistic test (it already renders in that fixture). - policy predicate controls: new test drives policyLoadConfig() with one rule so renderPolicyCard emits the predicate form, asserting name= on the path/op/custom selects + the remember-minutes input. The predicate value control is op-change/async-gated; its name is set at both generator sites and covered by review. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: also cover the int feature-flag input in the holistic a11y guard tool_search_max_results is an int-typed feature flag, so it exercises the number-input branch of the feature-flag generator. Add it to the holistic fixture (+ sanity assert) so that generator's name= is render-tested too. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: close predicate-value render gap + address PR #1497 review (4 items) Adds a render test for the policy predicate *value* control — the one touched surface that had no coverage — and applies the three optional review nits. - New test_policy_predicate_value_control_carries_name_attribute drives both generator sites through the JS harness: a tool-schema 503 -> the renderFreeTextValue <input> branch; a schema enum on the chosen path -> the renderChoiceSelect <select> branch. Each asserts the control carries name="policy:predicate-value". - Reword the test_policy_predicate_controls_carry_name_attribute docstring: the value control renders on form-open (not only op-change) and now has its own dedicated test. - Element-scope the name asserts in test_backup_config_inputs_* and test_policy_predicate_controls_* (regex against a single <input>/<select> tag instead of a whole-DOM substring). - Soften the code_mode_max_duration sanity comment: the name is emitted by both the advanced-field generator and renderCodeModeSubRows, so the assert only proves a control with that name rendered. - Precise the holistic-guard docstring: "every form surface present at default page init" (the ?tab= deep-link can auto-activate backup/policy). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 9443446 commit 6ee6fbf

2 files changed

Lines changed: 510 additions & 25 deletions

File tree

‎src/ha_mcp/settings_ui.py‎

Lines changed: 21 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1618,7 +1618,7 @@ def apply_tool_visibility(
16181618
`<span class="group-count">${groupEnabled}/${tools.length} enabled</span>` +
16191619
`</div>` +
16201620
`<label class="switch group-master" title="Enable/disable all tools in this group">` +
1621-
`<input type="checkbox" ${anyEnabled ? 'checked' : ''} ${toggleable.length === 0 ? 'disabled' : ''}>` +
1621+
`<input type="checkbox" name="tool-group:${escapeHtml(tag)}" ${anyEnabled ? 'checked' : ''} ${toggleable.length === 0 ? 'disabled' : ''}>` +
16221622
`<span class="slider"></span>` +
16231623
`</label>`;
16241624
@@ -1722,20 +1722,20 @@ def apply_tool_visibility(
17221722
`</div>` +
17231723
`<div class="tool-toggles">` +
17241724
`<div class="toggle-group">` +
1725-
`<label class="switch"><input type="checkbox" data-tool="${escapeHtml(t.name)}" data-field="enabled" ` +
1725+
`<label class="switch"><input type="checkbox" name="tool:${escapeHtml(t.name)}:enabled" data-tool="${escapeHtml(t.name)}" data-field="enabled" ` +
17261726
`${isEnabled ? 'checked' : ''} ${lockEnabled ? 'disabled' : ''}>` +
17271727
`<span class="slider"></span></label>` +
17281728
`<span>enabled</span>` +
17291729
`</div>` +
17301730
`<div class="toggle-group ${!isEnabled ? 'disabled-toggle' : ''}">` +
1731-
`<label class="switch"><input type="checkbox" data-tool="${escapeHtml(t.name)}" data-field="pinned" ` +
1731+
`<label class="switch"><input type="checkbox" name="tool:${escapeHtml(t.name)}:pinned" data-tool="${escapeHtml(t.name)}" data-field="pinned" ` +
17321732
`${isPinned ? 'checked' : ''} ${lockPinned ? 'disabled' : ''}>` +
17331733
`<span class="slider"></span></label>` +
17341734
`<span>pinned</span>` +
17351735
`</div>` +
17361736
`<div class="toggle-group ${(policyState.enabled && isEnabled) ? '' : 'disabled-toggle'}" ` +
17371737
`title="${policyState.enabled ? '' : 'Enable Tool Security Policies in addon config first.'}">` +
1738-
`<label class="switch"><input type="checkbox" data-tool="${escapeHtml(t.name)}" data-field="gated" ` +
1738+
`<label class="switch"><input type="checkbox" name="tool:${escapeHtml(t.name)}:gated" data-tool="${escapeHtml(t.name)}" data-field="gated" ` +
17391739
`${policyState.gatedTools.has(t.name) ? 'checked' : ''} ` +
17401740
`${(policyState.enabled && isEnabled) ? '' : 'disabled'}>` +
17411741
`<span class="slider"></span></label>` +
@@ -1926,16 +1926,16 @@ def apply_tool_visibility(
19261926
row.className = 'backup-field';
19271927
let controlHtml;
19281928
if (typeof f.value === 'boolean') {
1929-
controlHtml = `<input type="checkbox" data-field="${escapeHtml(f.field)}" ${f.value ? 'checked' : ''} ${f.editable ? '' : 'disabled'}>`;
1929+
controlHtml = `<input type="checkbox" name="backup:${escapeHtml(f.field)}" data-field="${escapeHtml(f.field)}" ${f.value ? 'checked' : ''} ${f.editable ? '' : 'disabled'}>`;
19301930
} else if (typeof f.value === 'string') {
19311931
// Path / freeform string fields (auto_backup_dir).
1932-
controlHtml = `<input type="text" data-field="${escapeHtml(f.field)}" value="${escapeHtml(String(f.value ?? ''))}" ${f.editable ? '' : 'disabled'}>`;
1932+
controlHtml = `<input type="text" name="backup:${escapeHtml(f.field)}" data-field="${escapeHtml(f.field)}" value="${escapeHtml(String(f.value ?? ''))}" ${f.editable ? '' : 'disabled'}>`;
19331933
} else {
19341934
let min = 1;
19351935
let max = 10000;
19361936
if (f.field === 'auto_backup_throttle_minutes') { min = 0; max = 1440; }
19371937
else if (f.field === 'auto_backup_calendar_lookahead_days') { min = 1; max = 365; }
1938-
controlHtml = `<input type="number" data-field="${escapeHtml(f.field)}" value="${Number(f.value)}" min="${min}" max="${max}" ${f.editable ? '' : 'disabled'}>`;
1938+
controlHtml = `<input type="number" name="backup:${escapeHtml(f.field)}" data-field="${escapeHtml(f.field)}" value="${Number(f.value)}" min="${min}" max="${max}" ${f.editable ? '' : 'disabled'}>`;
19391939
}
19401940
let originMsg;
19411941
if (f.origin === 'env') {
@@ -2396,6 +2396,7 @@ def apply_tool_visibility(
23962396
label.className = 'switch';
23972397
const input = document.createElement('input');
23982398
input.type = 'checkbox';
2399+
input.name = 'feature:' + fieldName;
23992400
input.checked = !!f.value;
24002401
input.disabled = !f.editable || lockedByMaster;
24012402
input.addEventListener('change', () => {
@@ -2444,6 +2445,7 @@ def apply_tool_visibility(
24442445
} else if (f.type === 'int') {
24452446
const input = document.createElement('input');
24462447
input.type = 'number';
2448+
input.name = 'feature:' + fieldName;
24472449
input.value = f.value;
24482450
if (typeof f.min === 'number') input.min = f.min;
24492451
if (typeof f.max === 'number') input.max = f.max;
@@ -2514,6 +2516,7 @@ def apply_tool_visibility(
25142516
label.className = 'switch';
25152517
const input = document.createElement('input');
25162518
input.type = 'checkbox';
2519+
input.name = 'feature:' + fieldName;
25172520
input.checked = !!f.value;
25182521
input.disabled = !f.editable || lockedByGate;
25192522
input.addEventListener('change', () => {
@@ -2605,6 +2608,7 @@ def apply_tool_visibility(
26052608
}
26062609
inputEl.disabled = disabled;
26072610
inputEl.dataset.advField = f.field;
2611+
inputEl.name = 'adv:' + f.field;
26082612
inputEl.addEventListener('change', () => {
26092613
let v;
26102614
if (f.type === 'int') v = parseInt(inputEl.value, 10);
@@ -2817,15 +2821,15 @@ def apply_tool_visibility(
28172821
'<div class="policy-predicate-form" style="display:none;">' +
28182822
'<div class="policy-form-row">' +
28192823
'<label class="policy-form-label">Argument:</label>' +
2820-
'<select class="policy-predicate-path-select">' +
2824+
'<select name="policy:predicate-path" class="policy-predicate-path-select">' +
28212825
'<option value="">(loading...)</option>' +
28222826
'</select>' +
2823-
'<input type="text" class="policy-predicate-path-custom" ' +
2827+
'<input type="text" name="policy:predicate-path-custom" class="policy-predicate-path-custom" ' +
28242828
'placeholder="e.g. args.color_temp" style="display:none">' +
28252829
'</div>' +
28262830
'<div class="policy-form-row">' +
28272831
'<label class="policy-form-label">Match when:</label>' +
2828-
'<select class="policy-predicate-op">' +
2832+
'<select name="policy:predicate-op" class="policy-predicate-op">' +
28292833
'<option value="exists">is present (any value)</option>' +
28302834
'<option value="eq">equals</option>' +
28312835
'<option value="neq">does NOT equal</option>' +
@@ -2850,7 +2854,7 @@ def apply_tool_visibility(
28502854
'</div>' +
28512855
'<div class="policy-rule-lifetime">' +
28522856
'<label>Remember approval for:' +
2853-
'<input type="number" min="0" max="1440" class="policy-remember-minutes" ' +
2857+
'<input type="number" name="policy:remember-minutes" min="0" max="1440" class="policy-remember-minutes" ' +
28542858
'value="' + (rule.remember_minutes || 0) + '">' +
28552859
'minutes (0 = single-shot)' +
28562860
'</label>' +
@@ -3084,7 +3088,7 @@ def apply_tool_visibility(
30843088
const existingArr = Array.isArray(existingValue)
30853089
? existingValue
30863090
: (existingValue !== undefined && existingValue !== null ? [existingValue] : []);
3087-
let html = '<select class="policy-predicate-value-control"' +
3091+
let html = '<select name="policy:predicate-value" class="policy-predicate-value-control"' +
30883092
(isMulti ? ' multiple size="6" style="min-width:220px"' : '') +
30893093
'>';
30903094
if (!isMulti) {
@@ -3114,7 +3118,7 @@ def apply_tool_visibility(
31143118
const initial = (existingValue === undefined || existingValue === null)
31153119
? ''
31163120
: JSON.stringify(existingValue);
3117-
valueSlotEl.innerHTML = '<input type="text" ' +
3121+
valueSlotEl.innerHTML = '<input type="text" name="policy:predicate-value" ' +
31183122
'class="policy-predicate-value-control policy-predicate-value" ' +
31193123
'placeholder="' + escapeHtml(placeholder) + '" ' +
31203124
'value="' + escapeHtml(initial) + '">';
@@ -3643,22 +3647,22 @@ def apply_tool_visibility(
36433647
const meta = ADVANCED_FIELD_META[f.field] || { label: f.field, help: '' };
36443648
let controlHtml;
36453649
if (f.choices) {
3646-
controlHtml = `<select data-adv-field="${escapeHtml(f.field)}" ${f.editable ? '' : 'disabled'}>` +
3650+
controlHtml = `<select name="adv:${escapeHtml(f.field)}" data-adv-field="${escapeHtml(f.field)}" ${f.editable ? '' : 'disabled'}>` +
36473651
f.choices.map(c =>
36483652
`<option value="${escapeHtml(c)}" ${String(f.value) === c ? 'selected' : ''}>${escapeHtml(c)}</option>`
36493653
).join('') +
36503654
'</select>';
36513655
} else if (f.type === 'bool') {
3652-
controlHtml = `<input type="checkbox" data-adv-field="${escapeHtml(f.field)}" ${f.value ? 'checked' : ''} ${f.editable ? '' : 'disabled'}>`;
3656+
controlHtml = `<input type="checkbox" name="adv:${escapeHtml(f.field)}" data-adv-field="${escapeHtml(f.field)}" ${f.value ? 'checked' : ''} ${f.editable ? '' : 'disabled'}>`;
36533657
} else if (f.type === 'int' || f.type === 'float') {
3654-
controlHtml = `<input type="number" data-adv-field="${escapeHtml(f.field)}" value="${Number(f.value)}" ` +
3658+
controlHtml = `<input type="number" name="adv:${escapeHtml(f.field)}" data-adv-field="${escapeHtml(f.field)}" value="${Number(f.value)}" ` +
36553659
(f.min !== undefined ? `min="${f.min}" ` : '') +
36563660
(f.max !== undefined ? `max="${f.max}" ` : '') +
36573661
(f.type === 'float' ? 'step="0.1" ' : '') +
36583662
(f.editable ? '' : 'disabled') + '>';
36593663
} else {
36603664
// str
3661-
controlHtml = `<input type="text" data-adv-field="${escapeHtml(f.field)}" value="${escapeHtml(String(f.value ?? ''))}" ${f.editable ? '' : 'disabled'}>`;
3665+
controlHtml = `<input type="text" name="adv:${escapeHtml(f.field)}" data-adv-field="${escapeHtml(f.field)}" value="${escapeHtml(String(f.value ?? ''))}" ${f.editable ? '' : 'disabled'}>`;
36623666
}
36633667
let originMsg = '';
36643668
if (f.origin === 'env') {

0 commit comments

Comments
 (0)