Skip to content

refactor: Extract Block Grid render-timing decision into a pure, tested module - #330

Open
rickbutterfield wants to merge 3 commits into
v5/devfrom
v5/refactor/frontend-render-scheduler
Open

refactor: Extract Block Grid render-timing decision into a pure, tested module#330
rickbutterfield wants to merge 3 commits into
v5/devfrom
v5/refactor/frontend-render-scheduler

Conversation

@rickbutterfield

Copy link
Copy Markdown
Owner

Summary

Extracts the Block Grid preview's "should I re-render yet" decision — previously four mutable fields read via inline conditionals at two call sites in block-grid-preview.custom-view.element.ts — into a new, pure, independently unit-tested module (render-scheduler.ts).

  • decideGridRenderTrigger / shouldDeferInitialGridRender — pure functions, no DOM, no this, no timers.
  • ResizeDebouncer — replaces the inline setTimeout/clearTimeout pair.
  • BlockGridPreviewCustomView is wired to call these instead of inlining the logic.

This is the render-timing decision behind issues #293/#294 (Block Grid area previews rendering blank or before layout data was available) — it had never had a dedicated unit test until this branch. A new element-level test (grid-render-timing.test.ts) drives real observable state through the element to prove the wiring reproduces the fix end-to-end, not just that the extracted functions are individually correct.

One subtle, deliberate behavior change surfaced during review: the pure function's single-return-value design makes the "layoutAreas arrived" and "resize" triggers mutually exclusive on the same observable emission, where the old code could fire both (an immediate render plus a redundant 300ms-later one). This is locked down with an explicit test and comment rather than left implicit — the likely effect is one fewer redundant preview request, not a behavior regression.

Scoped to Block Grid only — List/Single/RichText don't have this timing complexity and are untouched. A follow-on plan (collapsing all four preview elements onto shared descriptors) is intentionally out of scope here.

Test plan

  • npm test — 32/32 passing
  • Two rounds of automated code review (task-level + whole-branch), both approved with fixes applied
  • Manual backoffice smoke test (Block Grid with at least one area, resize behavior) — not run this cycle, no browser access available; recommend running before a release tag

🤖 Generated with Claude Code

Rick Butterfield and others added 3 commits August 4, 2026 17:32
…scheduler

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Replaces the element's inline layoutAreas/resize conditionals with calls to
decideGridRenderTrigger/shouldDeferInitialGridRender, and the ad-hoc
setTimeout/clearTimeout pair with ResizeDebouncer, both from Task 1's
render-scheduler.ts. Adds an element-level regression test
(grid-render-timing.test.ts) that drives real observable state through the
element to prove the wiring, not just the pure functions, for issues #293/#294.
…disconnect

Addresses two whole-branch review findings on the render-scheduler extraction:
document and test that decideGridRenderTrigger intentionally collapses the old
two-independent-if-blocks behavior (render always wins over debounce when both
conditions hold on the same emission), and cancel the pending ResizeDebouncer
timer in BlockGridPreviewCustomView.disconnectedCallback so it can't fire
renderBlockPreview() after the element is removed from the DOM.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant