fix(template-builder): stop echoing layout changes synchronously in templates portlet

[zJaaal]

Summary

Closes #35697.

• Drop the immediate updateTemplate.emit(item) in the templates portlet wrapper (dot-template-builder.component.ts). It was echoing layout changes synchronously back into the designer's [layout] input, triggering updateOldRows mid-cycle while state had gaps in y (e.g. [0, 2] right after removeRow, before grid auto-compact + moveRow collapse to [0, 1]). That mismatch silently produced rows with children: undefined and missing x/y/w/h — visible as columns reordering or rows duplicating after a row deletion.
• Now mirrors the EMA pattern (edit-ema-layout.component.ts): layout only flows back to the designer after a successful save (onSaveTemplate updates both working and original). Auto-save still fires via the existing 5s templateUpdate$ debounce; forbidRouteDeactivation() continues to block navigation on every edit.
• The template-builder library (libs/template-builder) is untouched — root cause was in the consumer.

Why this regressed now

Commit ab89c85e10 (PR #35277) switched @for row tracking from identify(row) (id+index) to row.id. Before that change, removing a row shifted indices and forced Angular to destroy/recreate every subsequent row, which incidentally reset GridStack state and masked this latent updateOldRows corruption. EMA was always immune because its layout signal only changes after save.

Test plan

Demo

Screen.Recording.2026-05-13.at.5.04.38.PM.mov

Manual

• Open a design template in /templates/edit/<id>, ≥ 3 rows with non-zero-x columns.
• Delete a middle row → only that row goes; other rows' columns stay put; no duplicates.
• Make a change, wait ~5s → auto-save fires.
• Make a change, immediately navigate away → page-leave prompt blocks navigation until save completes.
• EMA layout editor unchanged — repeat row delete there to confirm no regression.

Automated

• New regression test in dot-template-builder.component.spec.ts fires two rapid templateChange emissions (simulating removeRow → moveRow) and asserts updateTemplate.emit is never called synchronously, while a single debounced save.emit fires with the latest item.
• Existing should react to templateChange output updated to match the new contract.
• yarn nx test dotcms-ui — 2129 passed.

🤖 Generated with Claude Code

[claude]

Claude finished @zJaaal's task in 3m 4s —— View job

---

Review

The fix is targeted, the comment is excellent, and the EMA analogy is the right one. A few things worth flagging before merge — none are blockers but the first two are real and the PR currently soft-pedals them in prose only.

---

1. The bug isn't fixed, only narrowed — and the comment admits it

store.saveTemplate (store/dot-template.store.ts:201) still calls updateWorkingTemplate(template) synchronously inside the effect's tap before the HTTP call. So at debounce-fire time, vm.working mutates → [item]="vm.working" → [layout]="item.layout" → updateOldRows runs in the lib via the exact same echo path. The PR comment at dot-template-builder.component.ts:104-108 calls this out, but the argument that "the lib has been idle for 5s so state is settled" is timing-based, not structural.

Concrete edge case: pageLeaveRequest$ (line 122-124) emits save.emit(this.lastTemplate) without the 5s debounce. If a navigation request fires in the same macrotask as a templateChange from removeRow (e.g. user clicks a route-link button right after deleting a row, before GridStack's moveRow auto-compact has run its rows.changes subscriber), we land in the exact corrupt state the PR is trying to fix — only now with no recovery path because the save also persists it.

This is unlikely in practice (clicks and Angular CD are serialized) but it's a structural footgun that the prose argument doesn't actually rule out.

Suggestion: either (a) fix updateOldRows in the lib so it's robust to layout shape that doesn't match its internal rows.y, or (b) at minimum, file a follow-up issue. The PR closes #35697 but the root cause survives.

2. Regression test's "assertion (2)" is mostly a tautology

The spec at lines 245-299 asserts builderDe.componentInstance.layout === layoutBefore after each templateChange. But the only way layout could change in this isolated test is if setInput('item', ...) ran on the wrapper itself — which it doesn't, because the parent component (the one that would route the emitted updateTemplate back through the store and re-flow vm.working) is not present in this createComponentFactory setup. The comment at lines 240-244 claims (2) "would still pass assertion (1) but fail (2)" under a hypothetical refactor — but any refactor confined to the wrapper that doesn't synchronously call setInput('item', ...) on itself will also pass (2). The two assertions collapse to the same check in this test scope.

If you want (2) to actually be load-bearing, the test needs a host component that re-binds [item] in response to the wrapper's outputs (i.e., simulates the parent). Otherwise consider removing the second assertion + the long comment justifying it, since it overstates what's protected.

3. updateTemplate @Output is now dead for design templates

After this change, the wrapper's updateTemplate only ever fires from the advanced path (dot-template-builder.component.html:19, direct passthrough). The design path no longer emits it. The output is still declared (dot-template-builder.component.ts:58) and the parent still binds it (dot-template-create-edit.component.html:18), but for design that binding is a no-op. Not wrong, just worth knowing — and tests like dot-template-create-edit.component.spec.ts:624 ('should call updateWorkingTemplate from store when updateTemplate') still pass because they manually triggerEventHandler('updateTemplate', ...), which doesn't reflect real design-path behavior anymore.

4. vm.didTemplateChanged is now permanently false during design edits

Because working no longer updates on each change, working stays === original until a save lands. The store's didTemplateChanged$ therefore stays false during the entire edit session for design templates. The canRouteBeDesativated subscriber (store/dot-template.store.ts:413-421) consequently wouldn't forbidRouteDeactivation on its own — only the wrapper's explicit call at line 93 does. The wrapper covers it, so route protection still works, but the store-derived flag has semantically drifted: it no longer means "user has unsaved edits" for design. Worth a comment in the store, or just be aware of it if anything else starts depending on didTemplateChanged$ for design.

For design, didTemplateChanged isn't consumed in the design lib's input set — only forwarded to dot-template-advanced — so no visible UI regression today. But if anyone adds a "you have unsaved changes" indicator wired to vm.didTemplateChanged, it will lie on design templates.

5. The lib bug is the real problem

libs/template-builder/.../template-builder.component.ts updateOldRows produces silently-corrupt rows (children undefined, missing x/y/w/h) when fed a layout whose y values don't match its internal rows.y. That's a defensive-coding bug in the lib that any consumer can hit. The PR's choice to fix at the consumer is pragmatic, but leaving the lib as-is means the next consumer (or the save-time echo from #1) can re-trigger it. Worth a tracking issue.

---

Nits

• The block comment at dot-template-builder.component.ts:95-108 is long but load-bearing — fine to keep.
• The // (1) / // (2) numbered comments in the test (245-299) read as overclaim given Test Branch and Commit #2 above. Consider trimming.

---

Verdict

Ship it as the targeted bleeding-stopper — it does eliminate the user-visible regression from PR #35277 — but please open a follow-up for the lib's updateOldRows fragility (#1, #5) so the next consumer of the lib (or a future code path that bypasses the 5s buffer) doesn't rediscover this.

---

• Read changed files (component + spec)
• Inspect EMA reference pattern to validate the analogy
• Look for other consumers / call sites that depend on synchronous updateTemplate emission
• Verify the regression-test actually proves what the PR claims
• Post review
  · Branch