v0.3.1a1: code-review fixes + rule editor UX - #14
Merged
Merged
Conversation
Sweep across the backend and the non-panel frontend after a
project-wide review. No user-visible behavior changes; the panel
UX work in the next commit is separate.
Backend hardening:
- __init__: wire hass.data[DOMAIN] and the store/injector pointers
before injector.async_start() and WS command registration. A
client racing setup could see partial data otherwise.
- rule.py: validation size caps — mapping at 200, thresholds at 50,
source_attribute at 255 chars. Bounds the synchronous validation
loop against pathologically large payloads. Empty thresholds
list now rejected (was relying on a falsy-check that's better
made explicit). Non-string mapping keys rejected with a clear
message — YAML's bare `1:` was silently coerced via str(),
producing rules that didn't fire because the user expected `1`
as a state. Non-string source_attribute rejected (was vol.Any
which accepted weird types via voluptuous coercion).
- websocket_api: replace_all batch capped at 1000 rules.
INTEGRATION_VERSION now read from manifest.json at import time
instead of duplicated as a hardcoded string — single source of
truth. New test locks the two together.
- injector: state-update writes only allocate a new attrs dict
when at least one of color / icon / background actually changed.
Eliminates a per-event allocation for the common no-op case.
Switched 7 call sites from store.all() (list copy) to a new
all_view() that returns the live values view.
- store: all_view() method added — see above.
Frontend hardening:
- state-watcher: Listener signature now (id, newState,
oldAttributes, newAttributes). The watcher snapshots oldAttrs
before updating its cache and hands both to listeners so
transition detection ("decoration attr was here, now isn't")
can run without re-reading the cache.
- index.ts: painter.repaintAll() only fires when the smart_icons:
color or background attr was on the old state or is on the new.
Filters out the long tail of unrelated entity updates.
- painter: MutationObserver now processes removedNodes — descends
through light DOM and shadow DOM of removed subtrees and prunes
ha-state-icon hosts from knownHosts immediately. Fixes a slow
leak in busy Lovelace dashboards rotating cards in and out.
- evaluator: cosmetic cleanup of the field-claim counter to
mirror the Python side.
- rule-store: sources() filters empty source strings (matches
validate_rule's "" sentinel for per-target rules).
Tests:
- tests/test_rule_validation: empty-thresholds rejection, non-
string mapping-keys rejection, mapping size cap, thresholds
size cap, source_attribute length cap, source_attribute
non-string rejection.
- tests/test_websocket_api: INTEGRATION_VERSION matches manifest,
replace_all empty list works, replace_all oversized list rejected.
- tests/test_injector: target entity vanishing mid-runtime.
- frontend/test/painter: knownHosts pruned immediately on DOM
removal via the mutation observer (verifies shadow-pierce).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Two UX fixes from dogfooding the v0.3.0 GA, plus a small build banner for future cache-vs-rebuild debugging. YAML actions row visibility (smart-icons-panel.ts, styles.ts): ha-code-editor's internal .cm-editor is `height:100%!important;max-height:100%!important`. Setting max-height on the host alone doesn't constrain it — the percentage can't resolve against an auto-height parent, so the editor grew to fit its content and pushed the .panel-actions row (Save + "Show visual editor" toggle) off-screen. The user couldn't see the Save button without scrolling. Switched both the panel-level and per-rule code-editor styles to `--code-mirror-max-height`, the CSS variable HA's own automation editor uses; the editor scrolls internally and the action row stays visible. Verified by grepping hass_frontend for how other HA pages cap the editor height. Dirty-dismiss intercept (rule-editor.ts, smart-icons-panel.ts): Clicking off the rule-editor dialog or pressing ESC mid-edit silently threw away unsaved work. Now: 1. rule-editor takes a snapshot of (working + codeMode + codeText) right after hydrate. isDirty() compares the live state to that snapshot; a dirty-changed event fires on transitions. 2. The panel listens for dirty-changed and tracks editorDirty. 3. Document-capture pointerdown + keydown guards intercept backdrop click and ESC. When the rule-editor dialog is open AND the editor is dirty AND the Discard confirm isn't already showing, the guard preventDefault + stopImmediatePropagation to abort the close, and opens a Discard confirm dialog. 4. The confirm offers "Keep editing" (dialog stays open, working state preserved because the dialog never closed) or "Discard" (forces dialogOpen=false, real close, all state reset). Implementation notes worth keeping in the code: - HA no longer uses Material Web Components 2.x. ha-dialog now wraps a native <dialog> (scrim is the ::backdrop pseudo-element, not a DOM node). mwc-dialog's @closing event, scrimClickAction prop, and named action slots are all obsolete. - HA's own backdrop-close handler (`handleDialogPointerDown` in the bundle) listens on `pointerdown`, not `click`. A click intercept fires too late — HA has already called close() on pointerdown. The guard uses pointerdown. - e.target retargets to <home-assistant> at the document level (shadow-DOM composed retargeting). e.composedPath()[0] pierces every shadow boundary and gives the actual innermost element — <dialog> for a backdrop hit. The guard matches by composedPath, not target. - ESC: HA closes the dialog programmatically (not via the native dialog's cancel event), so @cancel on ha-dialog never fires. The keydown guard catches ESC at document capture. - The rule-editor's existing `cancel` event is renamed to `cancel-button` so it doesn't collide with native <dialog>'s `cancel` when this editor lives inside ha-dialog. Also code-review carry-over to the same file: - rule-editor serialize: source defaults to entities[0] only when the rule is a single literal target with no globs. Multi-target or mixed rules with blank source now serialize source as "", matching backend validate_rule's per-target-evaluation default (was collapsing to a single-source rule on save). Build banner (panel/index.ts, package.json, web-test-runner.config.mjs): esbuild --define injects a __BUILD_TIME__ constant into the panel bundle; the entry prints `[smart-icons] panel bundle build <iso8601>` to the console on mount. Future "did my rebuild actually land?" questions resolve in one console line instead of guessing. The wtr config mirrors the define so tests don't ReferenceError on the imported entry. Tests (rule-editor.test.ts): - Dirty tracking: clean on hydrate, flips after working-state change, resets on re-hydrate, fires dirty-changed once per transition (not per keystroke), cancel-button event carries the dirty flag. - Source-default parity (carried from code-review): pure single literal, glob-only, multi-target, and explicit source all serialize correctly. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Version bumps + rebuilt static bundles + changelog entry for the first v0.3.1 alpha. Two threads of work in this release: the code-review sweep and the rule-editor UX fixes — see the preceding commits for details, and CHANGELOG.md for the full upgrade summary. - manifest.json: 0.3.0 → 0.3.1a1 - frontend/package.json + package-lock.json: 0.3.0 → 0.3.1-alpha.1 (manifest uses HA-convention aN, npm uses semver-canonical prerelease tags — both refer to the same release) - INTEGRATION_VERSION (websocket_api): picks up the bump automatically since it now reads from manifest.json at import time (introduced in the code-review commit) - static/smart_icons.js + smart_icons_panel.js: rebuilt off current sources Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The `Editor & { working: ... }` intersection collapsed to `never`
on CI's stricter TypeScript pass because `working` is `private`
on the class — `private` members can't be widened through an
intersection. Switched the dirty-tracking tests to the same
`cast through unknown` pattern the file-level `priv()` helper
already uses for the same reason. Local typecheck wasn't
catching this before because I missed the stderr output on the
last green run.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The timestamp-based banner broke CI's "Verify committed bundles match a fresh build" check — every rebuild produced a different byte for the build stamp, so the committed bundle never matched. Switched to the package.json version (`0.3.1-alpha.1` for this alpha). Bundle is now byte-identical across rebuilds off the same source. The user-facing payoff is actually better: the banner tells you the release id directly, no separate version lookup needed. Local "did my rebuild land?" iteration is slightly less precise — the banner doesn't change between dev rebuilds — but the file mtime or a hard refresh tells you that just as well. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
First alpha of the v0.3.1 line. Two threads of work consolidated into one branch:
ha-code-editorhostmax-heightdoesn't constrain CodeMirror — switched to--code-mirror-max-height).<dialog>wrapper required document-capturepointerdown+keydownguards (the mwc-dialog intercept points are obsolete in current HA).Full upgrade summary in CHANGELOG.md.
Test plan
pytest -q— 128 passingnpm run test(web-test-runner) — 114 passing (5 new dirty-tracking tests)npm run typecheckcleannpm run buildcleanVersion
v0.3.1a1(manifest) /0.3.1-alpha.1(npm) — first alpha; tag after merge.🤖 Generated with Claude Code