l-lessons-active-frame-border

Lessons from the multi-wave active-frame border bug (#1425, #1463, #1473, #1489). Use when: (1) Planning any work that touches frame border, focus ring, or active-leaf visual state, (2) Adding a new w...

Lessons: Active-Frame Border Regressions

Summary

The active-frame border required four separate attempts (#1425, #1463, #1473, #1489) before it stayed fixed. The root cause was architectural, not cosmetic: too many disconnected layers between the page and the leaf, with two competing render mechanisms and no single owner.

Each Prior Attempt

1425 — First attempt

• What it fixed: wired --color-active-frame as a CSS custom property; basic active leaf highlight appeared in default-dark.

• What it missed: the property resolved correctly in one theme but fell back silently in tokyo-night and default-light because Tailwind's JIT included a utility that referenced an unverified --theme-active-frame-border alias. The alias was only set in default-dark's applyColors() output.

• Lesson: do not chase a Tailwind utility route without verifying the CSS var exists in every theme's applyColors() call. Test all three themes before closing.

1463 — Second attempt

• What it fixed: corrected the applyColors() mapping for tokyo-night and default-light so all three themes emitted the var.

• What it missed: the frameset package's LeafRenderer wrapped the leaf with an additional <div> that had an outline-based focus ring. The outline painted on top of the inset box-shadow in certain WebKit circumstances, making the border appear doubled in the active state. The team closed the issue believing both render paths were patched, but the outline path was not noticed.

• Lesson: never use outline for visual focus indication on a frame leaf. The inset box-shadow is the single mechanism; outline is off the table.

1473 — Third attempt

• What it fixed: removed the outline from LeafRenderer's wrapper div; border regression looked resolved in spot checks.

• What it missed: by this point there were still two DOM nodes that could paint the active color: the frameset-level LeafRenderer wrapper and the FrameChrome wrapper. Depending on which node was active, the result was inconsistent. Tests written against the Tailwind-utility path gave false green because they checked for a CSS class, not the resolved box-shadow value.

• Lesson: if a test only checks for a CSS class string and does not verify the resolved style.boxShadow, it can give false confidence. Write tests that assert the canonical rendered property value.

1489 — Final structural fix (Frame Chrome Consolidation Wave 3)

• What it fixed: designated a single DOM node as the active-border owner: the outermost <div> inside FrameChrome (the [data-testid="frame-chrome"] element). This node always renders regardless of leaf state (normal, zoomed, collapsed, popped-out). The active border is expressed solely via boxShadow: "inset 0 0 0 1px var(--color-active-frame)". The neutral 1px border (border border-edge) is always present; the inset shadow layered on top adds the accent without changing the bounding box.

• Why it held: one DOM node, one CSS var, one render mechanism. Guard tests in packages/frameset/src/frame-chrome-active-border.test.tsx assert both the presence of the inset shadow and the absence of outer shadows (which would shift adjacent leaf bounding boxes).

Why Regressions Kept Happening

1. Multiple disconnected sources of truth. The CSS var was set in applyColors(), consumed in Tailwind utilities, and also sometimes set inline. Any one path going stale broke only some themes.

2. Wrong initial hypothesis. The team first chased the Tailwind JIT route (adding utility classes). The real fix required a DOM ownership decision — "which single node paints the border" — not a CSS-class tweak.

3. Two render mechanisms coexisting. outline (keyboard-focus ring in LeafRenderer) and box-shadow (active-frame indicator in FrameChrome) both painted around the same leaf. Each patch addressed one mechanism, leaving the other to regress later.

4. Architectural cause: too many wrapper layers. Before Wave 3 (#1485 page-chrome elimination) and Wave 3 (#1489 single DOM owner), there were page-level wrappers, frameset-level LeafRenderer wrappers, and provider-level wrappers, any of which could add a visual border. The fix required collapsing the hierarchy so FrameChrome is the sole owner.

The Structural Fix

Visual model: the table-cell metaphor

The active border follows a "table-cell" mental model: every leaf is like a table cell with exactly one 1px border painted by its own chrome (border-edge), and adjacent leaves share that edge visually. The active leaf paints an inset box-shadow over its border, highlighting it from the inside — no leaf ever adds a 2px "selected" border that would push against its neighbor, so there is zero layout shift. This is why outline (which paints outside the box) and extra wrapper divs (which would double the edge) are forbidden. (This is the canonical home for the metaphor; l-lessons-frame-component-architecture cross-links here.)

Mechanism

• One DOM node ([data-testid="frame-chrome"]) owns the active border.

• CSS mechanism: boxShadow: "inset 0 0 0 1px var(--color-active-frame)" inline style.

• The neutral border (border border-edge) is always present; the inset shadow is conditional on isActive.

• var(--color-active-frame) is set by applyColors() for every theme in packages/color-themes/; verified in guard tests.

• Guard tests: packages/frameset/src/frame-chrome-active-border.test.tsx asserts:

  • Active state → style.boxShadow contains "inset" and references var(--color-active-frame).

  • Inactive state → no boxShadow.

  • Same mechanism for collapsed strip, popped-out strip, and empty-leaf states.

  • No outer (non-inset) shadow on the same node.

• Page-chrome eliminated in #1485: route components (write-page.tsx, archives-page.tsx, search-page.tsx) are thin <Frameset> hosts with no <PageBar /> or page-level chrome layer.

Watch For Next Time

• New wrapper inside LeafRenderer. Any <div> added between <Frameset> and <FrameChrome> that carries a border or box-shadow will duplicate the active indicator.

• outline on a leaf. The outline CSS property is not allowed for visual focus in a frame leaf. Use the inset box-shadow mechanism on [data-testid="frame-chrome"] only.

• Tailwind utility based on an unverified theme var. Before shipping a Tailwind border-* or ring-* utility that references a CSS custom property, confirm the property is emitted by applyColors() in all three themes (default-dark, tokyo-night, default-light).

• New page-level chrome in a route component. Route files (*-page.tsx) must remain thin frameset hosts. If a toolbar or header is needed, it belongs inside the provider, not in the page.

Would-Skip-If-Redoing

The three prior attempts' incremental approach (patch the CSS var, then patch the theme, then remove the outline, then ...) each bought one week before the next regression. The correct first step would have been:

1. Audit every DOM node between <Frameset> and the leaf that could paint a border.

2. Designate a single owner.

3. Delete everything else.

4. Write guard tests before closing.

One structured audit + redesign session would have replaced four patch cycles.

Reference Files

• packages/frameset/src/frame-chrome.tsx — single-owner implementation (look for boxShadow: "inset 0 0 0 1px var(--color-active-frame)").

• packages/frameset/src/frame-chrome-active-border.test.tsx — guard tests.

• doc/src/content/docs/architecture/active-frame-border-strategy.mdx — decision record.

• packages/color-themes/ — applyColors() per theme (the var must be present here).

Fifth Attempt — Bug Fix 1512/1513 (Sub 4, #1512 §4 + #1513 fix1 + #1513 fix4)

Three additional bug reports that Attempt #4 did not cover

After the #1489 structural fix, three new failure modes appeared in the user's live workspace:

Bug 1 — Stale color.activeFrameBorder field in saved settings

Root cause: applyColors() in packages/color-themes/src/color-settings.ts previously iterated Object.entries(colorKeyToCssVar) and wrote every stored field, including activeFrameBorder → --theme-active-frame-border. This let an old, user-chosen value persist in .zudotext.settings.json and survive every theme or accent change. The user's actual saved value was #5CAAE9 while their current accent was #AE8556.

Why #1489 didn't catch it: #1489 fixed the DOM ownership (a single node, one CSS var) but left applyColors() and the ColorSettings type unchanged. If the stored blob contained a non-accent hex it was still written to --theme-active-frame-border on every settings load. The team had no test exercising "what happens when the stored value differs from the accent".

Fix: two-step removal.

1. packages/color-themes/src/color-settings.ts — the colorKeyToCssVar map no longer contains the activeFrameBorder key. Instead, applyColors() unconditionally writes root.style.setProperty("--theme-active-frame-border", colors.accent) (line 377) after the loop, ensuring the var physically equals the accent on every call regardless of what is (or was) in the stored blob.

2. packages/app-defaults/src/validate-settings.ts — validateSettings() now deletes s.color.activeFrameBorder on load (lines 192-193) so the stale field is not re-persisted.

Guard tests: frame-chrome-active-border.test.tsx Row 12 — "Active-frame border tracks accent at runtime" — verifies that applyColors() writes colors.accent to --theme-active-frame-border across all three shipping themes, and that an explicit non-default accent like #AE8556 is what gets stored.

Bug 2 — Nested suppression: outer chrome accent border visible during timeline-card selection (fix1)

Root cause: when the user clicked a card in timeline-vertical or timeline-horizontal layout, the card painted its own selection highlight AND the outer [data-testid="frame-chrome"] accent border remained visible, creating a double-highlight. There was no mechanism to tell the chrome that the provider had its own sufficient selection signal.

Fix: opted-in attribute suppression contract (introduced in Sub 4 / #1513 fix1):

• packages/frameset/src/frame-chrome.tsx lines 510-522 document the contract: any descendant of the chrome can set data-suppress-frame-active-border="true" to opt out of the outer accent border.

• ActiveFrameOverlay (lines 523-566) scans for the attribute via useLayoutEffect on every render and sets local suppressed state.

• When suppressed, the overlay renders a data-suppressed="true" / display: none marker instead of the inset shadow.

• The caller: tauri-app/renderer/components/inbox-layouts/inbox-timeline-layout.tsx line 117 — data-suppress-frame-active-border="true" on the outer wrapper of the card grid, active only when a card is selected.

Guard tests: frame-chrome-active-border.test.tsx Row 13 — "Nested suppression via data-suppress-frame-active-border" — three sub-cases: attribute present/active, attribute present/false, attribute toggling mid-lifecycle. All assert against [data-testid="frame-active-overlay"][data-suppressed].

Bug 3 — Wrap-around cutout: inset shadow clipped by floating children (fix4)

Root cause: before Sub 4, the active-frame inset shadow was applied as inline boxShadow on the chrome wrapper div. When a provider rendered absolutely-positioned children (e.g., the Archive button in the inbox, DraftNumberBadge), those children could paint over the shadow's right or bottom edge, producing a visual cutout on 1-2 sides.

Fix: the ActiveFrameOverlay component (frame-chrome.tsx line 557-566) renders the inset shadow as a sibling element of the content slot, placed AFTER it in the DOM so it stacks on top via CSS painting order. The overlay uses:

className="pointer-events-none absolute inset-0"
style={{ boxShadow: "inset 0 0 0 1px var(--color-active-frame)" }}

pointer-events: none keeps all children interactive. absolute inset-0 expands the overlay to fill the chrome wrapper on all four sides. inset 0 0 0 1px (four equal offsets) guarantees a continuous rectangle with no cutout regardless of what the content slot paints.

Guard tests: frame-chrome-active-border.test.tsx Row 14 — "Wrap-around overlay structural contract" — asserts: (a) overlay is a sibling of [data-testid="frame-content"], NOT nested inside it; (b) overlay boxShadow has four equal zero offsets; (c) pointer-events is none.

Why the prior four attempts didn't catch the stale #5CAAE9 color

Attempts #1–#4 fixed DOM ownership, CSS var propagation, and the outline collision — all code-side issues. None of them audited what happened when a user's existing .zudotext.settings.json contained a color.activeFrameBorder override value from a previous version of the settings schema. The stale value was written to --theme-active-frame-border on every applyColors() call, overriding the computed theme accent. Without a test that compared "stored override ≠ accent produces stale color" the regression was invisible in CI.

The fifth attempt's discovery protocol:

1. Read the user's actual .zudotext.settings.json field by field.

2. Noticed color.activeFrameBorder: "#5CAAE9" while color.accent: "#AE8556".

3. Traced applyColors() → confirmed the stored value was winning over the accent.

4. Removed the field from the schema, added the unconditional write, added Row 12 guard.

Verification protocol (what finally worked)

Run in order:

1. Audit applyColors() source — confirm activeFrameBorder key is absent from colorKeyToCssVar and the unconditional colors.accent write is present.

2. Check validateSettings.ts for the delete s.color.activeFrameBorder migration.

3. Run frame-chrome-active-border.test.tsx Rows 12, 13, 14 — all must green before and after accent changes.

4. For nested suppression: set data-suppress-frame-active-border="true" in a descendant and assert [data-testid="frame-active-overlay"][data-suppressed="true"].

5. For wrap-around: assert overlay is a sibling of [data-testid="frame-content"] and NOT a child; assert boxShadow matches inset 0 0 0 1px.

6. Verify all five leaf states (normal, zoomed, collapsed, popped-out, core.empty) each produce an overlay that satisfies steps 4-5.

Structural rules (single DOM owner, all-four-sides)

• Single DOM owner: [data-testid="frame-active-overlay"] — one element, rendered by ActiveFrameOverlay inside FrameChrome — is the only node that paints the active border. No provider, wrapper div, or route component may add a competing border or shadow.

• Attribute-based suppression: opt-in via data-suppress-frame-active-border="true" on any descendant of the chrome. The attribute triggers suppression; absence of the attribute is the default active state.

• All-four-sides verification: for each of the five leaf states (normal, zoomed, collapsed, popped-out, core.empty), focus the leaf and confirm the inset shadow forms a continuous unbroken rectangle. The guard tests check this via the inset 0 0 0 1px CSS value — four equal offsets are the mathematical guarantee of continuity.

2026-05-12 — Do NOT add a broad MutationObserver to ActiveFrameOverlay

What we set out to do

Catch suppression-attribute toggles that descendants make outside of React's render cycle (imperative DOM edits, third-party libraries). Existing code already used a no-deps useLayoutEffect to re-scan after every React render; commit da64d89b added a MutationObserver as defense-in-depth.

Approach we tried first

useLayoutEffect(() => {
  const observer = new MutationObserver(scanSuppression);
  observer.observe(parent, {
    attributes: true,
    subtree: true,
    attributeFilter: ["data-suppress-frame-active-border"],
    childList: true,
  });
  return () => observer.disconnect();
}, []);

Inside scanSuppression, setSuppressed((prev) => prev === next ? prev : next) — relying on the functional-updater bail to prevent infinite loops.

Why it went wrong (root cause)

subtree: true + childList: true means the observer fires on every descendant DOM mutation in the chrome's entire subtree — CodeMirror typing, virtualizer item insert/remove, hover state changes, focus chrome flicker, etc. Each fire enqueues a setSuppressed dispatch. The functional bail prevents the re-render but does not prevent the dispatch from incrementing React's update-depth counter. Combined with ancestor render pressure (write-page mount), the counter hit React's limit and surfaced as production error #185 "Maximum update depth exceeded" at app startup.

Same structural mistake as the no-deps useLayoutEffect it was meant to defend: trusting Object.is / functional-updater bail to make repeated setState calls "free." It isn't free under sustained pressure.

What worked instead

1. Remove the MutationObserver entirely. Descendants change the suppression attribute through React (or, at minimum, trigger a parent re-render that ripples down to the chrome). The no-deps useLayoutEffect runs on every chrome render and catches every transition that matters. The third-party-library / imperative-DOM-update case was hypothetical — no caller in the codebase actually needs it.

2. Gate the no-deps useLayoutEffect itself with a ref. Even without the observer, calling setSuppressed every render via the functional bail still pumps the update queue. Track the last scanned value in a useRef and only dispatch on an actual transition:

const lastScannedRef = useRef(false);

useLayoutEffect(() => {
  const next = parent.querySelector('[data-suppress-frame-active-border="true"]') !== null;
  if (lastScannedRef.current !== next) {
    lastScannedRef.current = next;
    setSuppressed(next);
  }
});

The inline comment in packages/frameset/src/frame-chrome.tsx ActiveFrameOverlay now warns against re-adding the observer.

Watch for next time

• MutationObserver({ subtree: true, childList: true }) inside a render-heavy chrome is a footgun. It will fire on every descendant DOM change — far more often than you think. If you need attribute-change detection only, omit childList: true and rely on attributes: true + attributeFilter.

• setState with a functional-updater bail is not free. (prev) => prev === next ? prev : next skips the re-render but the dispatch still counts. Pair every "scan on every render" pattern with a useRef value-gate so setState is only called on a real transition.

• The CSS :has() rule in tokens.css is the production backstop. The JS scan exists for jsdom-based guard tests; do not over-engineer it for production correctness.

• If you see React #185 at app startup with a stack pointing inside frame-chrome.tsx, the dev-mode unminified error usually lands at the most-recent setSuppressed call site, but the root cause may be either the observer (too broad) or a sibling provider's prop-sync useEffect (see l-lessons-frameset-persistence 2026-05-12 note). Check both before assuming one.

Would-skip-if-redoing

The MutationObserver addition in da64d89b was speculative defense-in-depth for a case no caller actually exercised. The commit message even noted that the no-deps useLayoutEffect was "a contributing factor to the StrictMode loop" — the right move would have been to gate that one useLayoutEffect with a ref, not to add a second mechanism on top. Adding broad DOM observation to defend against a hypothetical edge case made an existing tolerable pattern intolerable.