fix(uve): lock layout canvas during save to prevent container data loss (#36197) by gortiz-dotcms · Pull Request #37244 · dotCMS/core

gortiz-dotcms · 2026-08-26T18:59:20Z

Summary

Fixes #36197 — UVE Layout tab data loss caused by the GridStack canvas never being locked while a layout save is in flight.

Root cause

Every layout change emits templateChange → updateTemplate$ → 5-second debounce → POST /api/v1/page/{id}/layout. The canvas remained fully interactive during the entire debounce window, the in-flight POST, and the pageReload() + updateOldRows() re-hydration cycle. Any edit made in that window was silently discarded when the server response overwrote local state. Because the backend performs a destructive diff (deletes any container absent from the incoming payload), mid-flight edits could permanently remove content.

Changes

`TemplateBuilderComponent` (`template-builder.component.ts` / `.html`)

@Input() disabled = false — new input that locks the canvas.
applyGridDisabled(disabled) — called synchronously from ngOnChanges (before any render cycle):
- grid.disable() / grid.enable() on the main grid and every subgrid — blocks drag, resize, and external drop at the GridStack event level.
- Dispatches keydown Escape on document — closes any open PrimeNG dropdowns (container pickers, etc.) that are appended to <body> outside the [inert] subtree.
- Drag cancellation — if a drag is in progress when the canvas locks, dispatches a synthetic document mouseup to terminate it. suppressStoreUpdates is set for the synchronous cancel window so that GridStack's forced commit events (and the subsequent grid.load() restore) do not write to the store. Placeholder widgets committed by the cancelled drag are removed via removeWidget before the lock completes.
- preDragGrid tracking — records the specific GridStack instance (main grid or subgrid) that owns the dragged widget. Row drags restore the main grid; column/box drags restore only the affected subgrid via its own save(false) snapshot.
suppressStoreUpdates flag — silences moveRow, addRow, updateColumnGridStackData, and subGridOnDropped calls inside GridStack event handlers during the cancel window, keeping the store consistent with pre-drag state.
Visual overlay — @if (disabled) block rendered after the grid content with bg-white/50 backdrop-blur-[2px] and a centered p-progress-spinner. [attr.inert] on both the toolbar and the grid container blocks all keyboard/pointer interaction.

`EditEmaLayoutComponent` (`edit-ema-layout.component.ts` / `.html`)

Component-local #layoutSaveInFlight = signal(false) — replaces the computed(() => uveStore.uveStatus() === LOADING) approach. Decouples the canvas-disabled state from the global UVE status so that unrelated LOADING events (workflow lock toggles, page-api re-fetches via withWorkflow.ts) cannot unlock the canvas mid-POST or trigger a spurious spinner.
nextTemplateUpdate guard — returns early when #layoutSaveInFlight is true, dropping any templateChange events that fire after the canvas is frozen (in-progress drags, open dropdown selections).
tap before debounceTime — gates the Page Properties nav item immediately on any edit (setUveStatus(LOADING)) so users cannot navigate to page settings while layout changes are pending, while the actual canvas lock (#layoutSaveInFlight) only engages when the POST is actually sent.
catchError → EMPTY — keeps the switchMap subscription alive on HTTP error so a failed save doesn't permanently disable autosave.

Lock lifecycle

User edits → tap fires → setUveStatus(LOADING)   [Page Properties nav gated]
  → 5 s debounce settles
  → #layoutSaveInFlight = true                    [canvas locks + overlay appears]
  → POST /api/v1/page/{id}/layout
    (any new edit dropped by nextTemplateUpdate guard)
  → handleSuccessSaveTemplate()
  → #layoutSaveInFlight = false                   [canvas unlocks]
  → pageReload()

Test plan

Edit layout (move a row) → wait 5 s → "Saving…" toast appears → canvas shows overlay + spinner → cannot drag/add/resize during save + reload cycle → canvas unlocks after reload
Start dragging a row → hold during save → row disappears on lock, no ghost row created, no row added to layout → canvas unlocks cleanly
Start dragging toolbar "Add Box" → hold during save → box disappears on lock, no ghost box created → canvas unlocks cleanly
Start dragging toolbar "Add Row" → hold during save → Row widget disappears, no row added to layout → canvas unlocks cleanly
Edit layout → open "+" container dropdown in a box → wait 5 s → dropdown closes (Escape), overlay appears → canvas is frozen
Edit layout → save fails (disconnect network) → canvas unlocks, uveStatus set to ERROR, user can retry
pnpm nx test portlets-edit-ema-portlet --testPathPattern=edit-ema-layout
pnpm nx test template-builder --testPathPattern=template-builder.component

Grabacion.de.pantalla.2026-08-24.a.la.s.5.41.44.p.m.mov

🤖 Generated with Claude Code

claude · 2026-08-26T19:04:35Z

Claude finished @gortiz-dotcms's task in 2m 20s —— View job

PR Review — canvas lock during layout save

Reviewed the current branch state (10 commits) against origin/main, rechecking each finding from rjvelazco's earlier review. The blocking findings have been genuinely addressed, not papered over.

Resolved

✅ template-builder.component.ts:309-372 — The Escape "no-op" cancel is gone. Drag cancellation is now real: dragstart snapshots preDragState/preDragGrid (grid.save(false)), and on lock the code dispatches a synthetic mouseup (terminating the drag before grid.disable() can commit it), then restores via preDragGrid.load(preDragState) under a suppressStoreUpdates window that silences moveRow/addRow/subGridOnDropped/updateColumnGridStackData. The Escape dispatch is retained only for closing PrimeNG panels, and the comment (template-builder.component.ts:343) now correctly states GridStack 8.4.0 doesn't honor Escape for drag cancel. Column drags restore the correct subgrid via preDragGrid.
✅ edit-ema-layout.component.ts:66-87 — uveStatus is no longer the lock. A component-local #layoutSaveInFlight signal now drives $isSaving, decoupled from global LOADING. The #handleReloadComplete effect keeps the canvas locked through the pageReload() re-hydration window and unlocks on LOADED or ERROR (so a failed reload can't leave a permanent lock).
✅ edit-ema-layout.component.ts:179-183 — Page Properties gate restored. The pre-debounceTime tap setting setUveStatus(LOADING) is back, gating the nav item on any edit, while the actual canvas lock only engages in the switchMap when the POST is sent (requirement 2 preserved).
✅ edit-ema-layout.component.ts:204-210 — catchError → EMPTY + finalize added to the inner save() pipe, keeping the outer updateTemplate$ subscription alive after an HTTP error so retry actually works.
✅ edit-ema-layout.component.spec.ts:161 — Test interdependence fixed. jest.clearAllMocks() in the file-level beforeEach (plus an explicit save mock reset for mockReturnValue overrides); the inline mockClear() workaround is gone and assertions now use toHaveBeenCalledTimes(1).
✅ template-builder.component.html:2,36,120-124 — Keyboard bypass closed. [attr.inert] on both the p-toolbar and #templateContainerRef, plus role="status" aria-busy="true" on the overlay.
✅ template-builder.component.html:129 — aria-label now goes through | dm (dot.template.builder.saving.layout).
✅ template-builder.component.ts:486-490 — Init race fixed. applyGridDisabled(this.disabled) is now called at the end of setUpGridStack(), so a grid created while disabled is already true boots disabled.
✅ edit-ema-layout.component.ts:126-129 — The lastTemplate drop is now explicitly documented as intentional (dropped edits are changes the server never saw).
✅ template-builder.component.spec.ts:491-538 — New tests assert the drag-cancel store-consistency path (grid.load, preDragGrid.load, suppressStoreUpdates reset), and edit-ema-layout.component.spec.ts:323-369 cover unlock-on-ERROR and lock-survives-pageReload.

New Issues

🟡 Medium: edit-ema-layout.component.ts:80-87 — The #handleReloadComplete effect clears the lock on any transition to LOADED/ERROR, so a cross-feature store writer (e.g. withWorkflow completing a lock toggle) landing LOADED during the in-flight window still unlocks the canvas early. This is explicitly documented as an accepted narrow race (requires a concurrent banner-button click inside a <2s window), and a proper fix needs pageReload() to expose a completion observable. Calling it out for visibility — not blocking given the documented rationale.
🟡 Medium: template-builder.component.ts:352-354 — The document-level Escape dispatch also dismisses an open PrimeNG DynamicDialog ("Add style classes"). Documented as a known limitation with low practical impact (any edit in the dialog resets the debounce). Non-blocking, but if you want to be precise you could guard the dispatch on there being an open overlay panel rather than firing unconditionally.

Notes

The remaining mock-grid assertions in template-builder.component.spec.ts:472-601 still verify method invocation on a hand-rolled mockGrid, but the newly added setSubGridEvent — dropped handler block (:604-661) exercises the real store-suppression logic, which is what the earlier review was actually asking for. Acceptable.
bg-white/50 (overlay, .html:124) remains light-theme-only — cosmetic, worth a follow-up against theme tokens but not blocking.

Verdict: All four blocking findings from the prior review are resolved with real behavioral fixes and matching tests. The two remaining items are documented, accepted trade-offs with narrow impact. No new blocking issues. Note: this is a static review — I did not run the two Jest targets in the test plan.
· issue-36197-canvas-lock-v2

@input

…ss (#36197) The GridStack canvas in the UVE Layout tab was never disabled while a save was in flight, allowing edits during the debounce window, the in-flight POST, and the post-save pageReload() re-hydration to be silently dropped when the server response overwrote local state. Changes: - Add `disabled` @input to TemplateBuilderComponent. When true, ngOnChanges calls grid.disable() + subgrid.disable() synchronously (before any render cycle) and dispatches keydown Escape to cancel any in-progress drag or close any open PrimeNG dropdown. - Add a visible overlay (semi-transparent white + backdrop-blur + PrimeNG ProgressSpinner) rendered after the grid content so it sits on top in DOM order, giving users clear saving feedback. - Add $isSaving computed signal in EditEmaLayoutComponent that reads uveStatus === LOADING and binds it to [disabled] on the template builder. - Move setUveStatus(LOADING) from the tap() before debounceTime into switchMap, so the canvas only locks when the POST is actually sent — not on every keystroke during the 5-second debounce window. - Guard nextTemplateUpdate() to discard templateChange events that arrive while uveStatus === LOADING, covering the edge case where an in-progress action (held drag, open dropdown) completes after the freeze activates. The lock spans the full lifecycle: POST fired → pageReload() re-fetch → updateOldRows() re-hydration → uveStatus reset to LOADED → canvas unlocked. Refs: #36197 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Cover the disabled input, overlay rendering, grid API interactions, nextTemplateUpdate LOADING guard, and debounce-before-lock behaviour. Refs: #36197 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

MockDotRouterJestService is a singleton across the test suite; without clearing forbidRouteDeactivation before the guard assertion, accumulated calls from prior tests cause a false failure. Refs: #36197 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

rjvelazco

Review — canvas lock during layout save

Thanks for the clear write-up in the description; the root-cause analysis is correct and made this reviewable. Full-file review of both components plus the UVE store features, cross-checked against the pinned gridstack@8.4.0 and primeng@21.1.3 sources in node_modules.

Verdict: needs changes before merge. Requirement 2 (don't lock during the debounce) is precisely implemented, but requirement 3 (cancel in-progress interactions) is unmet in a way that reproduces the original bug, and coupling the lock to uveStatus leaves it unlockable by unrelated store writers.

What's well done

The diagnosis is right. The debounce → POST → re-hydration window is exactly where the data loss lives.
Requirement 2 is met precisely. Moving setUveStatus(LOADING) out of the pre-debounceTime tap and into the switchMap is the minimal correct change.
ngOnChanges ordering is correct. updateOldRows (layout branch) runs before the grid is re-enabled, so re-hydration happens under the lock.
The store cooperates. withPageApi.ts:340-348 patches uveStatus: LOADED and setPageAsset in one synchronous tap, so layout and disabled reach ngOnChanges in a single CD pass — no interim "unlocked with stale layout" frame.
Defence in depth is sound. updateOldRows already sets shouldEmit: false (template-builder.store.ts:434-439); the new nextTemplateUpdate guard is a correct second barrier.
Overlay placement is correct. It sits outside the @if (vm$ | async) block, and :host { @apply relative } gives it the right containing block — so it covers the toolbar (the external drag-in source), not just the canvas.
Backward compatible. disabled = false leaves the other consumer (apps/dotcms-ui/.../dot-template-builder.component.html) untouched.
Test hygiene is generally good — byTestId, spectator.setInput, the real UVEStore rather than a stub, fakeAsync/tick with an explicit flush comment.

Blocking (details inline)

#	Where	Issue
1	`template-builder.component.ts:296-303`	🔴 The Escape dispatch is a no-op — GridStack 8.4.0 has no keyboard handling (added in v10+). What actually ends the drag is `grid.disable()`, and it commits the move instead of cancelling it → the row moves locally, the emit is dropped by the new guard, and `updateOldRows` merges over it. Same divergence class the PR fixes.
2	`edit-ema-layout.component.ts:57`	🟠 `uveStatus` is a shared flag — `withWorkflow.ts:179` (page lock/unlock, reachable from the shell banner on this route) can patch `LOADED` mid-POST and unlock the canvas early.
3	`edit-ema-layout.component.ts` (removed `tap`)	🟠 The deleted line was gating the Page Properties nav item (`dot-ema-shell.component.ts:188`). That gate is now gone for the whole 5s pending-save window.
4	`edit-ema-layout.component.ts:148-150`	🟠 No `catchError` in the `switchMap` — one HTTP error kills `updateTemplate$` for the component's lifetime, so requirement 4 advertises a retry that silently no-ops.

Issues 2 and 3 both dissolve if the lock gets its own component-local signal instead of borrowing uveStatus — one flag serving two purposes is the pattern that produced the original bug.

Also worth addressing

Keyboard bypasses the overlay — no inert/aria-busy; tab order still reaches the toolbar and box controls.
Escape closes PrimeNG dialogs — the dispatch's one real effect, and unintended: it discards an in-progress "Add style classes" edit.
Test interdependence in the new layout specs (shared router mock, worked around with an inline mockClear()) — a Critical 🔴 per docs/frontend/TESTING_REVIEW_RULES.md.
Missing coverage for unlock-on-ERROR and for the lock surviving pageReload() — the requirement most central to the fix.
Minor: init race when disabled is already true before the grid exists; hardcoded aria-label (needs | dm); bg-white/50 is light-theme-only.

Note on `@Input()` over `input()`

docs/frontend/ANGULAR_STANDARDS.md:51 prefers signal inputs, but this is a justified deviation — the JSDoc correctly explains the state must apply synchronously before render, which a post-CD effect() can't do, and the component's four existing inputs are all decorators. Consistent locally, no change requested. (An @Input() set disabled(v: boolean) setter would express the intent more directly than the ngOnChanges branch, if you want it.)

Follow-up (not this PR)

apps/dotcms-ui/.../dot-template-builder is the other consumer of this component and has the same debounce-save shape without a lock. It won't regress from this PR, but it carries the same bug — worth a ticket.

Note: review is static — the two Jest targets in the test plan were not run.

Suppresses GridStack store writes (suppressStoreUpdates flag) during the synchronous cancel window triggered by applyGridDisabled(true), so that the forced mouseup commit and the subsequent grid.load() restore do not corrupt the Angular store. Adds preDragGrid tracking so column drags restore the correct subgrid instead of the main grid. Removes orphaned placeholder widgets from subgrids (toolbar Box drop) and the main grid (toolbar Row drop) when cancelled mid-flight. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

The canvas was unlocking the moment the POST returned 200, before pageReload() finished its re-fetch. During that gap the store guard was open and user edits would be merged over by updateOldRows(). Moves the unlock to a #handleReloadComplete effect that fires only when uveStatus reaches LOADED — i.e. after the full reload cycle. Also: - aria-label on the saving spinner now goes through | dm - applyGridDisabled() is called at the end of setUpGridStack() so a grid that initialises while disabled=true cannot boot enabled behind the overlay - Documents why the manual subgrid querySelectorAll loop is needed Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

pageReload() sets uveStatus=ERROR (not LOADED) when the re-fetch fails. #handleReloadComplete now clears the in-flight flag on either LOADED or ERROR so a failed reload can never leave the canvas permanently locked. Documents the known workflow-lock LOADED race and the Escape/DynamicDialog limitation so future reviewers understand the trade-offs. Documents why lastTemplate is intentionally not updated when an edit is dropped mid-flight. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…te field template-builder: z-[9999] → z-9999 (better-tailwindcss/enforce-canonical-classes) edit-ema-layout: #handleReloadComplete → $handleReloadComplete to match the $handleCanEditLayout convention and avoid no-unused-private-class-members Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…ation and wrong injector in spec Two test failures fixed: 1. portlets-edit-ema-portlet: jest.clearAllMocks() does not reset mockReturnValue/mockImplementation overrides, so the error test's throwError() persisted into the next tests, triggering the error path and clearing #layoutSaveInFlight prematurely. Fix: explicitly reset dotPageLayoutService.save to the default implementation in beforeEach after injecting the service. 2. template-builder: setSubGridEvent describe block injected DotTemplateBuilderStore without `true`, getting the TestBed root instance instead of the component-scoped instance (the component declares providers: [DotTemplateBuilderStore]). Spying on the wrong instance made subGridOnDropped appear uncalled. Fix: use spectator.inject(DotTemplateBuilderStore, true). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

github-project-automation Bot added this to dotCMS - Product Planning Aug 26, 2026

github-actions Bot mentioned this pull request Aug 26, 2026

Layout: Lose containers/content because the editing canvas is never locked during save #36197

Open

6 tasks

github-actions Bot added the Area : Frontend PR changes Angular/TypeScript frontend code label Aug 26, 2026

gortiz-dotcms and others added 4 commits August 26, 2026 16:06

style(uve): fix prettier formatting on layout canvas lock files

fe50abc

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

test(uve): add unit tests for layout canvas lock and disabled binding

f7c896b

Cover the disabled input, overlay rendering, grid API interactions, nextTemplateUpdate LOADING guard, and debounce-before-lock behaviour. Refs: #36197 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

gortiz-dotcms force-pushed the issue-36197-canvas-lock-v2 branch from a4f41f5 to 7dbb9b8 Compare August 26, 2026 19:06

gortiz-dotcms mentioned this pull request Aug 26, 2026

fix(uve): lock layout canvas during save to prevent container data loss (#36197) #37168

Closed

6 tasks

claude Bot added the AI: Safe To Rollback label Aug 26, 2026

gortiz-dotcms requested review from rjvelazco and zJaaal August 26, 2026 20:42

rjvelazco reviewed Aug 27, 2026

View reviewed changes

rjvelazco previously approved these changes Aug 27, 2026

View reviewed changes

danielsolis-dotcms previously approved these changes Aug 27, 2026

View reviewed changes

gortiz-dotcms dismissed rjvelazco’s stale review via 91e89fc August 28, 2026 18:31

github-actions Bot removed the AI: Safe To Rollback label Aug 28, 2026

claude Bot added the AI: Safe To Rollback label Aug 28, 2026

github-actions Bot removed the AI: Safe To Rollback label Aug 28, 2026

gortiz-dotcms dismissed danielsolis-dotcms’s stale review via 10dd03c August 28, 2026 19:36

claude Bot added the AI: Safe To Rollback label Aug 28, 2026

style(uve): prettier format template-builder spec

371d77b

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

github-actions Bot removed the AI: Safe To Rollback label Aug 28, 2026

claude Bot added the AI: Safe To Rollback label Aug 28, 2026

github-actions Bot removed the AI: Safe To Rollback label Aug 28, 2026

Navigation Menu

Uh oh!

fix(uve): lock layout canvas during save to prevent container data loss (#36197) - #37244

fix(uve): lock layout canvas during save to prevent container data loss (#36197)#37244
gortiz-dotcms wants to merge 10 commits into
mainfrom
issue-36197-canvas-lock-v2

gortiz-dotcms commented Aug 26, 2026 •

edited

Loading

Uh oh!

claude Bot commented Aug 26, 2026 •

edited

Loading

Uh oh!

rjvelazco left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

gortiz-dotcms commented Aug 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Root cause

Changes

TemplateBuilderComponent (template-builder.component.ts / .html)

EditEmaLayoutComponent (edit-ema-layout.component.ts / .html)

Lock lifecycle

Test plan

Uh oh!

claude Bot commented Aug 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review — canvas lock during layout save

Resolved

New Issues

Notes

Uh oh!

rjvelazco left a comment

Choose a reason for hiding this comment

Review — canvas lock during layout save

What's well done

Blocking (details inline)

Also worth addressing

Note on @Input() over input()

Follow-up (not this PR)

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

gortiz-dotcms commented Aug 26, 2026 •

edited

Loading

`TemplateBuilderComponent` (`template-builder.component.ts` / `.html`)

`EditEmaLayoutComponent` (`edit-ema-layout.component.ts` / `.html`)

claude Bot commented Aug 26, 2026 •

edited

Loading

Note on `@Input()` over `input()`