diff --git a/src/Umbraco.Community.BlockPreview.UI/src/blockEditor/block-grid-preview.custom-view.element.ts b/src/Umbraco.Community.BlockPreview.UI/src/blockEditor/block-grid-preview.custom-view.element.ts index 313256e..d368954 100644 --- a/src/Umbraco.Community.BlockPreview.UI/src/blockEditor/block-grid-preview.custom-view.element.ts +++ b/src/Umbraco.Community.BlockPreview.UI/src/blockEditor/block-grid-preview.custom-view.element.ts @@ -5,6 +5,7 @@ import { css, customElement, property } from "@umbraco-cms/backoffice/external/l import { UMB_BLOCK_GRID_ENTRY_CONTEXT, UMB_BLOCK_GRID_MANAGER_CONTEXT, UmbBlockGridLayoutModel, UmbBlockGridValueModel, UmbBlockGridLayoutAreaItemModel } from "@umbraco-cms/backoffice/block-grid"; import { UMB_CONTENT_WORKSPACE_CONTEXT } from "@umbraco-cms/backoffice/content"; import { observeMultiple } from "@umbraco-cms/backoffice/observable-api"; +import { decideGridRenderTrigger, shouldDeferInitialGridRender, ResizeDebouncer } from './render-scheduler'; const elementName = "block-grid-preview"; @@ -59,6 +60,11 @@ export class BlockGridPreviewCustomView extends BlockPreviewBaseElement 0 && this.#managerObserved && !this._isLoading) { + const trigger = decideGridRenderTrigger( + { layoutAreas: prevLayoutAreas, layout: { columnSpan: prevColumnSpan, rowSpan: prevRowSpan } }, + { areas, layoutAreas, layout }, + { hasMarkup: !!this._htmlMarkup, isLoading: this._isLoading, managerObserved: this.#managerObserved }, + ); + + if (trigger.kind === 'render') { this.blockGridValue = { ...this._blockGridValue, layout: { ['Umbraco.BlockGrid']: this.#filterLayouts() } }; this.renderBlockPreview(); - } - - // Re-render when layout dimensions change (resize) - if (this._htmlMarkup && layout && ( - layout.columnSpan !== prevColumnSpan || - layout.rowSpan !== prevRowSpan - )) { + } else if (trigger.kind === 'debounce') { this.blockGridValue = { ...this._blockGridValue, layout: { ['Umbraco.BlockGrid']: this.#filterLayouts() } }; - clearTimeout(this.#layoutResizeTimer); - this.#layoutResizeTimer = setTimeout(() => { - this.renderBlockPreview(); - }, 300); + this.#resizeDebouncer.schedule(trigger.delayMs, () => this.renderBlockPreview()); } } ); @@ -155,7 +154,7 @@ export class BlockGridPreviewCustomView extends BlockPreviewBaseElement; + #resizeDebouncer = new ResizeDebouncer(); async #observeBlockPropertyValue() { this.consumeContext(UMB_BLOCK_GRID_MANAGER_CONTEXT, (context) => { @@ -177,9 +176,7 @@ export class BlockGridPreviewCustomView extends BlockPreviewBaseElement x.key === this._blockContext.contentUdi); if (!this._htmlMarkup && !this._isLoading) { - // Defer render if areas are expected but layoutAreas haven't arrived yet; - // observeBlockValue will trigger the render once layoutAreas are available. - if ((this._blockContext.areas?.length ?? 0) > 0 && !this._blockContext.layoutAreas) { + if (shouldDeferInitialGridRender(this._blockContext.areas, this._blockContext.layoutAreas)) { return; } this.renderBlockPreview(); diff --git a/src/Umbraco.Community.BlockPreview.UI/src/blockEditor/grid-render-timing.test.ts b/src/Umbraco.Community.BlockPreview.UI/src/blockEditor/grid-render-timing.test.ts new file mode 100644 index 0000000..c693946 --- /dev/null +++ b/src/Umbraco.Community.BlockPreview.UI/src/blockEditor/grid-render-timing.test.ts @@ -0,0 +1,166 @@ +import { expect, fixture, defineCE } from '@open-wc/testing'; +import { UmbLitElement } from '@umbraco-cms/backoffice/lit-element'; +import { UmbStringState, UmbArrayState, UmbBasicState } from '@umbraco-cms/backoffice/observable-api'; +import { UMB_CONTENT_WORKSPACE_CONTEXT } from '@umbraco-cms/backoffice/content'; +import { + UMB_BLOCK_GRID_ENTRY_CONTEXT, + UMB_BLOCK_GRID_MANAGER_CONTEXT, + type UmbBlockGridLayoutAreaItemModel, + type UmbBlockGridLayoutModel, +} from '@umbraco-cms/backoffice/block-grid'; +import { BLOCK_PREVIEW_CONTEXT } from '../context/block-preview.context-token'; +import { BlockGridPreviewCustomView } from './block-grid-preview.custom-view.element'; + +/** + * Element-level regression test for issues #293/#294. render-scheduler.test.ts + * covers the decision in isolation; this proves BlockGridPreviewCustomView is + * actually wired to it — that a block with areas defers its first render until + * layoutAreas arrives, and that a resize after the first render debounces. + */ + +const DOC_KEY = 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa'; +const DOC_TYPE_KEY = 'bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb'; +const CONTENT_KEY = 'cccccccc-cccc-cccc-cccc-cccccccccccc'; +const ELEMENT_TYPE_KEY = 'dddddddd-dddd-dddd-dddd-dddddddddddd'; +const AREA_KEY = 'eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee'; + +class TestHostElement extends UmbLitElement {} +const testHostTag = defineCE(TestHostElement); + +class SpyGridPreview extends BlockGridPreviewCustomView { + public renderCalls: string[] = []; + protected override async callPreviewApi() { + this.renderCalls.push(JSON.stringify(this.blockGridValue.layout)); + return { data: '
preview
' }; + } + protected override async fetchStylesheets() { + return []; + } +} +const previewTag = defineCE(SpyGridPreview); + +function createBlockPreviewContextFake(host: HTMLElement) { + return { + getUnique: () => '', + setUnique: (_u: string) => {}, + getDocumentTypeUnique: () => '', + setDocumentTypeUnique: (_d: string) => {}, + requestQueue: { enqueue: (fn: () => Promise) => fn() }, + getOrCreateStylesheet: (_href: string) => Promise.resolve(new CSSStyleSheet()), + getHostElement: () => host, + }; +} + +function createContentWorkspaceFake(host: HTMLElement) { + return { + IS_CONTENT_WORKSPACE_CONTEXT: true, + getEntityType: () => 'document', + unique: new UmbStringState(DOC_KEY).asObservable(), + structure: { + contentTypeUniques: new UmbArrayState([DOC_TYPE_KEY], (x) => x).asObservable(), + }, + getHostElement: () => host, + }; +} + +function createGridEntryFake(host: HTMLElement, layoutAreasState: UmbBasicState, hasAreas: boolean) { + return { + contentKey: new UmbStringState(CONTENT_KEY).asObservable(), + settingsKey: new UmbBasicState(undefined).asObservable(), + workspaceEditContentPath: new UmbStringState('/edit/path').asObservable(), + contentElementTypeAlias: new UmbStringState('myElement').asObservable(), + contentElementTypeKey: new UmbStringState(ELEMENT_TYPE_KEY).asObservable(), + areas: new UmbArrayState(hasAreas ? [{ key: AREA_KEY }] : [], (x: { key: string }) => x.key).asObservable(), + layout: new UmbBasicState({ contentKey: CONTENT_KEY, columnSpan: 6, rowSpan: 1, areas: [] }).asObservable(), + layoutAreas: layoutAreasState.asObservable(), + getHostElement: () => host, + }; +} + +function createGridManagerFake(host: HTMLElement) { + return { + contents: new UmbArrayState([{ key: CONTENT_KEY, contentTypeKey: ELEMENT_TYPE_KEY }], (x: { key: string }) => x.key).asObservable(), + settings: new UmbArrayState<{ key: string }>([], (x) => x.key).asObservable(), + exposes: new UmbArrayState<{ contentKey: string }>([], (x) => x.contentKey).asObservable(), + propertyAlias: new UmbStringState('myBlockGrid').asObservable(), + getHostElement: () => host, + }; +} + +async function waitForCondition(fn: () => boolean, timeout = 2000, interval = 20): Promise { + const start = performance.now(); + while (performance.now() - start < timeout) { + if (fn()) return true; + await new Promise((r) => setTimeout(r, interval)); + } + return fn(); +} + +describe('block grid preview render timing (issue #293/#294)', () => { + let root: TestHostElement; + + afterEach(() => root?.remove()); + + it('defers the initial render until layoutAreas arrives for a block with areas', async () => { + root = await fixture(`<${testHostTag}>`); + root.provideContext(BLOCK_PREVIEW_CONTEXT, createBlockPreviewContextFake(root) as never); + root.provideContext(UMB_CONTENT_WORKSPACE_CONTEXT, createContentWorkspaceFake(root) as never); + + const layoutAreasState = new UmbBasicState(undefined); + root.provideContext(UMB_BLOCK_GRID_ENTRY_CONTEXT, createGridEntryFake(root, layoutAreasState, true) as never); + root.provideContext(UMB_BLOCK_GRID_MANAGER_CONTEXT, createGridManagerFake(root) as never); + + const el = document.createElement(previewTag) as SpyGridPreview; + root.appendChild(el); + await el.updateComplete; + await new Promise((r) => setTimeout(r, 50)); + + expect(el.renderCalls.length, 'should not render before layoutAreas arrives').to.equal(0); + + layoutAreasState.setValue([{ key: AREA_KEY, items: [] }]); + const rendered = await waitForCondition(() => el.renderCalls.length > 0); + + expect(rendered, 'should render once layoutAreas arrives').to.be.true; + expect(el.renderCalls.length).to.equal(1); + }); + + it('renders immediately for a block with no areas, regardless of layoutAreas', async () => { + root = await fixture(`<${testHostTag}>`); + root.provideContext(BLOCK_PREVIEW_CONTEXT, createBlockPreviewContextFake(root) as never); + root.provideContext(UMB_CONTENT_WORKSPACE_CONTEXT, createContentWorkspaceFake(root) as never); + + root.provideContext(UMB_BLOCK_GRID_ENTRY_CONTEXT, createGridEntryFake(root, new UmbBasicState(undefined), false) as never); + root.provideContext(UMB_BLOCK_GRID_MANAGER_CONTEXT, createGridManagerFake(root) as never); + + const el = document.createElement(previewTag) as SpyGridPreview; + root.appendChild(el); + await el.updateComplete; + + const rendered = await waitForCondition(() => el.renderCalls.length > 0); + + expect(rendered).to.be.true; + }); + + it('debounces a re-render when the block is resized after the first render', async () => { + root = await fixture(`<${testHostTag}>`); + root.provideContext(BLOCK_PREVIEW_CONTEXT, createBlockPreviewContextFake(root) as never); + root.provideContext(UMB_CONTENT_WORKSPACE_CONTEXT, createContentWorkspaceFake(root) as never); + + const entryFake = createGridEntryFake(root, new UmbBasicState(undefined), false); + const layoutState = new UmbBasicState({ contentKey: CONTENT_KEY, columnSpan: 6, rowSpan: 1, areas: [] }); + entryFake.layout = layoutState.asObservable(); + root.provideContext(UMB_BLOCK_GRID_ENTRY_CONTEXT, entryFake as never); + root.provideContext(UMB_BLOCK_GRID_MANAGER_CONTEXT, createGridManagerFake(root) as never); + + const el = document.createElement(previewTag) as SpyGridPreview; + root.appendChild(el); + await waitForCondition(() => el.renderCalls.length === 1); + + layoutState.setValue({ contentKey: CONTENT_KEY, columnSpan: 12, rowSpan: 1, areas: [] }); + await new Promise((r) => setTimeout(r, 100)); + expect(el.renderCalls.length, 'debounce window should not have elapsed yet').to.equal(1); + + const rerendered = await waitForCondition(() => el.renderCalls.length === 2, 1000); + expect(rerendered, 'should render again after the debounce window').to.be.true; + }); +}); diff --git a/src/Umbraco.Community.BlockPreview.UI/src/blockEditor/render-scheduler.test.ts b/src/Umbraco.Community.BlockPreview.UI/src/blockEditor/render-scheduler.test.ts new file mode 100644 index 0000000..b2d562b --- /dev/null +++ b/src/Umbraco.Community.BlockPreview.UI/src/blockEditor/render-scheduler.test.ts @@ -0,0 +1,115 @@ +import { expect } from '@open-wc/testing'; +import { decideGridRenderTrigger, shouldDeferInitialGridRender, ResizeDebouncer, GRID_RESIZE_DEBOUNCE_MS } from './render-scheduler'; + +/** + * Unit tests for the Block Grid preview's render-timing decision, extracted from + * block-grid-preview.custom-view.element.ts to make issues #293/#294 (previews + * rendering blank or before layoutAreas is available) provable by assertion. + */ +describe('decideGridRenderTrigger (issue #293/#294)', () => { + const noAreas = { areas: [] as { key: string }[], layoutAreas: undefined, layout: undefined }; + const readyState = { hasMarkup: true, isLoading: false, managerObserved: true }; + + it('renders when layoutAreas arrives for the first time and the manager is observed', () => { + const prev = { layoutAreas: undefined, layout: undefined }; + const next = { areas: [{ key: 'a' }], layoutAreas: [{ key: 'a', items: [] }], layout: undefined }; + + expect(decideGridRenderTrigger(prev, next, readyState)).to.deep.equal({ kind: 'render', reason: 'layout-areas-arrived' }); + }); + + it('does nothing when layoutAreas arrives but the manager has not been observed yet', () => { + const prev = { layoutAreas: undefined, layout: undefined }; + const next = { areas: [{ key: 'a' }], layoutAreas: [{ key: 'a', items: [] }], layout: undefined }; + + expect(decideGridRenderTrigger(prev, next, { ...readyState, managerObserved: false })).to.deep.equal({ kind: 'none' }); + }); + + it('does nothing when the block has no areas', () => { + const prev = { layoutAreas: undefined, layout: undefined }; + const next = { areas: [] as { key: string }[], layoutAreas: [{ key: 'a', items: [] }], layout: undefined }; + + expect(decideGridRenderTrigger(prev, next, readyState)).to.deep.equal({ kind: 'none' }); + }); + + it('debounces a re-render when columnSpan changes after the first render', () => { + const prev = { layoutAreas: undefined, layout: { columnSpan: 6, rowSpan: 1 } }; + const next = { ...noAreas, layout: { columnSpan: 12, rowSpan: 1 } }; + + expect(decideGridRenderTrigger(prev, next, readyState)).to.deep.equal({ kind: 'debounce', reason: 'resized', delayMs: GRID_RESIZE_DEBOUNCE_MS }); + }); + + it('does not debounce a resize before the first render has happened', () => { + const prev = { layoutAreas: undefined, layout: { columnSpan: 6, rowSpan: 1 } }; + const next = { ...noAreas, layout: { columnSpan: 12, rowSpan: 1 } }; + + expect(decideGridRenderTrigger(prev, next, { ...readyState, hasMarkup: false })).to.deep.equal({ kind: 'none' }); + }); + + it('does nothing when neither layoutAreas nor layout span changed', () => { + const prev = { layoutAreas: [{ key: 'a', items: [] }], layout: { columnSpan: 6, rowSpan: 1 } }; + const next = { areas: [{ key: 'a' }], layoutAreas: [{ key: 'a', items: [] }], layout: { columnSpan: 6, rowSpan: 1 } }; + + expect(decideGridRenderTrigger(prev, next, readyState)).to.deep.equal({ kind: 'none' }); + }); + + it('renders (not debounce) when layoutAreas arrives on the same emission as a span change', () => { + // Both conditions are true here: layoutAreas just arrived (prev undefined -> + // next defined, block has areas, manager observed, not loading) AND the layout + // span changed (hasMarkup true, prev.layout undefined -> next.layout has a + // defined columnSpan/rowSpan, so both differ from prev). The old two-`if`-block + // caller would have fired an immediate render AND scheduled a 300ms debounce; + // the pure function must report only 'render'. + const prev = { layoutAreas: undefined, layout: undefined }; + const next = { areas: [{ key: 'a' }], layoutAreas: [{ key: 'a', items: [] }], layout: { columnSpan: 6, rowSpan: 1 } }; + + expect(decideGridRenderTrigger(prev, next, readyState)).to.deep.equal({ kind: 'render', reason: 'layout-areas-arrived' }); + }); +}); + +describe('shouldDeferInitialGridRender', () => { + it('defers when the block has areas but layoutAreas has not arrived yet', () => { + expect(shouldDeferInitialGridRender([{ key: 'a' }], undefined)).to.be.true; + }); + + it('does not defer when the block has no areas', () => { + expect(shouldDeferInitialGridRender([], undefined)).to.be.false; + }); + + it('does not defer once layoutAreas has arrived', () => { + expect(shouldDeferInitialGridRender([{ key: 'a' }], [{ key: 'a', items: [] }])).to.be.false; + }); +}); + +describe('ResizeDebouncer', () => { + it('invokes the callback once after the delay', async () => { + const debouncer = new ResizeDebouncer(); + let calls = 0; + debouncer.schedule(20, () => { calls++; }); + + await new Promise((r) => setTimeout(r, 50)); + + expect(calls).to.equal(1); + }); + + it('cancels a pending call when scheduled again before it fires', async () => { + const debouncer = new ResizeDebouncer(); + let calls = 0; + debouncer.schedule(20, () => { calls++; }); + debouncer.schedule(20, () => { calls++; }); + + await new Promise((r) => setTimeout(r, 50)); + + expect(calls).to.equal(1); + }); + + it('does not invoke the callback if cancelled', async () => { + const debouncer = new ResizeDebouncer(); + let calls = 0; + debouncer.schedule(20, () => { calls++; }); + debouncer.cancel(); + + await new Promise((r) => setTimeout(r, 50)); + + expect(calls).to.equal(0); + }); +}); diff --git a/src/Umbraco.Community.BlockPreview.UI/src/blockEditor/render-scheduler.ts b/src/Umbraco.Community.BlockPreview.UI/src/blockEditor/render-scheduler.ts new file mode 100644 index 0000000..f9d33a2 --- /dev/null +++ b/src/Umbraco.Community.BlockPreview.UI/src/blockEditor/render-scheduler.ts @@ -0,0 +1,70 @@ +/** + * Extracted from BlockGridPreviewCustomView so the render-timing decision behind + * issues #293/#294 (Block Grid area previews rendering blank or before layoutAreas + * was available) is a pure function instead of four fields read at two call sites. + */ + +export interface GridLayoutSpan { + columnSpan?: number; + rowSpan?: number; +} + +export const GRID_RESIZE_DEBOUNCE_MS = 300; + +export type GridRenderTrigger = + | { kind: 'none' } + | { kind: 'render'; reason: 'layout-areas-arrived' } + | { kind: 'debounce'; reason: 'resized'; delayMs: number }; + +/** + * Decides whether an already-rendered (or render-pending) Block Grid preview should + * re-render in response to an observed layout/layoutAreas update. + */ +export function decideGridRenderTrigger( + prev: { layoutAreas: unknown[] | undefined; layout: GridLayoutSpan | undefined }, + next: { areas: unknown[] | undefined; layoutAreas: unknown[] | undefined; layout: GridLayoutSpan | undefined }, + state: { hasMarkup: boolean; isLoading: boolean; managerObserved: boolean }, +): GridRenderTrigger { + const hasAreas = (next.areas?.length ?? 0) > 0; + const layoutAreasJustArrived = !prev.layoutAreas && !!next.layoutAreas; + + // Render takes precedence over debounce when both conditions hold on the same + // emission. Before this function existed, the caller ran two independent `if` + // blocks, so an emission where layoutAreas arrived *and* the layout span changed + // fired an immediate render plus a redundant 300ms-later debounced render. This + // intentionally collapses that into a single immediate render: the freshly-updated + // layout/areas data used here is the same data the debounced branch would have used + // moments later, so the second render added nothing but a delayed duplicate. + if (hasAreas && layoutAreasJustArrived && state.managerObserved && !state.isLoading) { + return { kind: 'render', reason: 'layout-areas-arrived' }; + } + + const resized = state.hasMarkup && !!next.layout && ( + next.layout.columnSpan !== prev.layout?.columnSpan || + next.layout.rowSpan !== prev.layout?.rowSpan + ); + if (resized) { + return { kind: 'debounce', reason: 'resized', delayMs: GRID_RESIZE_DEBOUNCE_MS }; + } + + return { kind: 'none' }; +} + +/** Should the first render be deferred until layoutAreas is known? */ +export function shouldDeferInitialGridRender(areas: unknown[] | undefined, layoutAreas: unknown[] | undefined): boolean { + return (areas?.length ?? 0) > 0 && !layoutAreas; +} + +/** Replaces an inline `setTimeout`/`clearTimeout` pair with a named, testable debounce. */ +export class ResizeDebouncer { + #timer?: ReturnType; + + schedule(delayMs: number, fn: () => void): void { + clearTimeout(this.#timer); + this.#timer = setTimeout(fn, delayMs); + } + + cancel(): void { + clearTimeout(this.#timer); + } +}