fix: Prevent unexpected vertical spacing when hidden

[ildar170975]

Replacing #185.
Fixes #184

[ildar170975]

@iantrich
Cannot pass "Claude Code Review" task, can you assist me with passing it?

[iantrich]

@ildar170975 rebase off beta. added a fix so forks can use the workflow.

[iantrich]

@ildar170975 I'd probably have to open up permissions on the repo, don't want to do that though.
@claude can you do a code review on this PR?

[claude]

Claude finished @iantrich's task in 2m 55s —— View job

---

Code Review — PR #195: fix: Prevent unexpected vertical spacing when hidden

• Gather context and understand request
• Read changed files and git diff
• Analyze the TypeScript changes in src/restriction-card.ts
• Analyze the workflow changes in .github/workflows/claude-code-review.yml
• Post review findings

---

Summary

This PR fixes issue #184 — unexpected vertical spacing when a card is hidden via the hide restriction. The root cause is that returning html``\`` from render()leaves the custom element host node in the DOM, still occupying layout space. The correct fix is to set thehiddenattribute on the host element itself, which appliesdisplay: none` via the browser's built-in hidden attribute behavior. The approach used here is the standard web component pattern for this problem and is sound.

---

src/restriction-card.ts — Core Fix

What changed:

• fireEvent added to the custom-card-helpers import
• New private field _prevHidden = false tracks the last reported visibility state
• New private method checkVisibilityChanged(hidden: boolean) handles the attribute toggle and event firing
• render() now calls checkVisibilityChanged(true) before returning empty HTML when hidden, and checkVisibilityChanged(false) otherwise

The checkVisibilityChanged implementation (src/restriction-card.ts:211-222):

private checkVisibilityChanged(hidden: boolean) {
  const visibilityChanged = this._prevHidden != hidden;
  if (visibilityChanged) {
    this._prevHidden = hidden;
    if (this._config!.row) {
      fireEvent(this, 'row-visibility-changed', { row: this, value: !hidden });
    } else {
      this.toggleAttribute('hidden', hidden);
      fireEvent(this, 'card-visibility-changed', { value: !hidden });
    }
  }
}

The logic is correct: _prevHidden prevents repeated toggleAttribute and fireEvent calls on every render where visibility hasn't changed. The row/non-row branching is also correct — row elements let the parent handle layout, so toggleAttribute is intentionally skipped there.

Findings

1. Loose equality operator (src/restriction-card.ts:212) — minor style nit

const visibilityChanged = this._prevHidden != hidden;  // !=

Both operands are boolean, so this is functionally equivalent to !==, but TypeScript and most ESLint configs mandate strict equality. This should be !==. Fix this →

2. Non-null assertion on _config (src/restriction-card.ts:215) — minor defensive style nit

if (this._config!.row) {

checkVisibilityChanged is only ever called from render(), which has an early-return guard (if (!this._config || ...)) so _config is guaranteed non-null at this call site. The ! assertion is therefore safe. However, this._config?.row would be more idiomatic TypeScript and would silently handle any future refactor that moves the call site outside the guard. Not blocking.

3. Early-bail path doesn't update visibility (src/restriction-card.ts:143-146)

if (!this._config || !this._hass || !this._config.card || !this._helpers) {
  return html``;  // checkVisibilityChanged is NOT called here
}

If the card bails early (e.g., during initial load before helpers resolve), the hidden state is not updated. In practice this is harmless: _prevHidden starts false, the hidden attribute is never set in this state, and once _helpers resolves the card re-renders through the correct path. Worth noting in case the early-bail behavior changes in the future.

---

.github/workflows/claude-code-review.yml — Workflow

The changes (pull-requests: write permission + explicit github_token) mirror the fix already applied in commit e9fdc94 on the main repo. These are required for the review action to post comments on PRs from forks. No issues here.

---

Verdict

Looks good overall. The core fix is correct, focused, and uses the right API (toggleAttribute('hidden')). The two findings above are minor nits — the != → !== change is the only one worth fixing before merge. No bugs or behavioral issues identified.