From 90268aa22786fa03211ea3303a929d075bd1e59f Mon Sep 17 00:00:00 2001 From: Freddy Montes Date: Fri, 9 Oct 2026 10:47:23 -0600 Subject: [PATCH 1/3] refactor(edit-content): share side-panel behaviour across panels (#37965) Escape, a click on the mask, the side-panel stack and the full-width preference were copied into each side panel. injectSidePanelChrome now gives every panel the same behaviour; the Edit Content panel and Content Drive's legacy-editor panel use it, so each only writes its own content and close guard. Co-Authored-By: Claude Opus 5.5 --- core-web/libs/edit-content/src/index.ts | 6 + .../dot-edit-content-side-panel.component.ts | 163 ++---------------- .../services/dot-side-panel-chrome.spec.ts | 148 ++++++++++++++++ .../src/lib/services/dot-side-panel-chrome.ts | 141 +++++++++++++++ .../dot-legacy-editor-side-panel.component.ts | 113 +++--------- 5 files changed, 336 insertions(+), 235 deletions(-) create mode 100644 core-web/libs/edit-content/src/lib/services/dot-side-panel-chrome.spec.ts create mode 100644 core-web/libs/edit-content/src/lib/services/dot-side-panel-chrome.ts diff --git a/core-web/libs/edit-content/src/index.ts b/core-web/libs/edit-content/src/index.ts index 040cb3ebcac5..c086c8c9b4e1 100644 --- a/core-web/libs/edit-content/src/index.ts +++ b/core-web/libs/edit-content/src/index.ts @@ -7,6 +7,12 @@ export * from './lib/models/dot-edit-content-dialog.interface'; export * from './lib/services/dot-edit-content.service'; export * from './lib/components/dot-edit-content-side-panel/dot-edit-content-side-panel.component'; export { DotSidePanelNavController } from './lib/services/dot-side-panel-nav.service'; +// Escape, mask click, side-panel stack and full width, for side panels outside this library too. +export { injectSidePanelChrome } from './lib/services/dot-side-panel-chrome'; +export type { + DotSidePanelChrome, + DotSidePanelChromeOptions +} from './lib/services/dot-side-panel-chrome'; export * from './lib/utils/functions.util'; export * from './lib/models/dot-edit-content-field.constant'; diff --git a/core-web/libs/edit-content/src/lib/components/dot-edit-content-side-panel/dot-edit-content-side-panel.component.ts b/core-web/libs/edit-content/src/lib/components/dot-edit-content-side-panel/dot-edit-content-side-panel.component.ts index 12f19fa8c774..88180643633e 100644 --- a/core-web/libs/edit-content/src/lib/components/dot-edit-content-side-panel/dot-edit-content-side-panel.component.ts +++ b/core-web/libs/edit-content/src/lib/components/dot-edit-content-side-panel/dot-edit-content-side-panel.component.ts @@ -10,7 +10,6 @@ import { inject, input, output, - signal, viewChild } from '@angular/core'; import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; @@ -21,48 +20,18 @@ import { DialogService, DynamicDialogConfig } from 'primeng/dynamicdialog'; import { DotCMSContentlet } from '@dotcms/dotcms-models'; import { popFormBridge, pushFormBridge } from '@dotcms/edit-content-bridge'; -import { - ASSET_PICKER_LAUNCHER, - AngularAssetPickerLauncher, - DotKeyboardShortcutService, - DotMessagePipe, - hasOverlayAbove -} from '@dotcms/ui'; +import { ASSET_PICKER_LAUNCHER, AngularAssetPickerLauncher, DotMessagePipe } from '@dotcms/ui'; import { AngularImageEditorLauncher, IMAGE_EDITOR_LAUNCHER } from '../../fields/shared/image-editor-launcher'; import { EditContentDialogData } from '../../models/dot-edit-content-dialog.interface'; -import { DotSidePanelNavController } from '../../services/dot-side-panel-nav.service'; +import { injectSidePanelChrome } from '../../services/dot-side-panel-chrome'; import { EDIT_CONTENT_HOST } from '../../services/host/edit-content-host.model'; import { OverlayEditContentHost } from '../../services/host/overlay-edit-content-host'; import { DotEditContentLayoutComponent } from '../dot-edit-content-layout/dot-edit-content.layout.component'; -/** localStorage key persisting the user's expanded (full-width) preference for the side panel. */ -const EXPANDED_STORAGE_KEY = 'dot-edit-content-side-panel-expanded'; - -/** - * Reads the persisted expanded preference. Best-effort: returns `false` when storage is - * unavailable (SSR/tests) or the value is missing, so a read failure never breaks the panel. - */ -function readExpandedPreference(): boolean { - try { - return localStorage.getItem(EXPANDED_STORAGE_KEY) === 'true'; - } catch { - return false; - } -} - -/** Persists the expanded preference; silently ignores storage failures. */ -function writeExpandedPreference(expanded: boolean): void { - try { - localStorage.setItem(EXPANDED_STORAGE_KEY, String(expanded)); - } catch { - // best-effort: quota errors / disabled storage must not break the panel. - } -} - /** * Renders the new Edit Content editor inside a right-to-left slide-in panel (`p-drawer`), as an * alternative to the full-screen route or the centered dialog. @@ -107,35 +76,29 @@ function writeExpandedPreference(expanded: boolean): void { } ], templateUrl: './dot-edit-content-side-panel.component.html', - changeDetection: ChangeDetectionStrategy.OnPush, - // Click-outside closes the panel through the unsaved-changes guard, bound at document level - // because `appendTo="body"` moves the drawer (and its mask) out of this component's DOM subtree, - // so a template listener would never receive it. - // - // ESC used to be bound the same way. It now goes through the shared shortcut registry instead — - // see the registration in the constructor for why. - host: { - '(document:click)': 'onMaskClick($event)' - } + changeDetection: ChangeDetectionStrategy.OnPush }) export class DotEditContentSidePanelComponent implements OnDestroy { readonly #injector = inject(Injector); readonly #destroyRef = inject(DestroyRef); - readonly #navController = inject(DotSidePanelNavController); - readonly #shortcuts = inject(DotKeyboardShortcutService); - - /** Withdraws the Escape claim; called from `ngOnDestroy` alongside the other teardown. */ - #withdrawShortcuts: () => void = () => undefined; /** The hosted editor; used to run its unsaved-changes guard before closing. */ protected readonly $layout = viewChild(DotEditContentLayoutComponent); + /** This panel's own drawer: its mask and container are what the shared chrome checks. */ + protected readonly $drawer = viewChild(Drawer); + /** - * This panel's own drawer. Needed for its `mask` element: the click-outside handler compares - * the clicked node against THIS mask, not against the shared `p-drawer-mask` class, so masks - * belonging to other drawers in the document never close this panel (see {@link onMaskClick}). + * Escape, a click on the mask, the side-panel stack and the full-width toggle, shared with the + * other side panels. Escape and the mask close through {@link requestClose}, so the editor's + * unsaved-changes guard runs first. */ - protected readonly $drawer = viewChild(Drawer); + readonly #chrome = injectSidePanelChrome({ + panel: this, + drawer: this.$drawer, + requestClose: () => this.requestClose(), + escapeLabel: 'edit.content.side-panel.shortcut.close' + }); /** Identity (and header title) of the content to create/edit, or `null` when closed. */ readonly data = input(null); @@ -154,10 +117,9 @@ export class DotEditContentSidePanelComponent implements OnDestroy { /** * Whether the panel is expanded to the full viewport width (vs the default ~80%). Seeded from - * the user's persisted preference so a panel opens in the mode last chosen (see - * {@link toggleExpanded}). + * the user's persisted preference so a panel opens in the mode last chosen. */ - protected readonly $expanded = signal(readExpandedPreference()); + protected readonly $expanded = this.#chrome.expanded; /** Last successfully-saved contentlet, forwarded to `data.onContentSaved` when the panel closes. */ #lastSaved: DotCMSContentlet | null = null; @@ -177,22 +139,6 @@ export class DotEditContentSidePanelComponent implements OnDestroy { // Give the editor a clean form-bridge slot; restore the previous one on close. pushFormBridge(); - // ESC goes through the shared registry rather than a document listener of this component's - // own. Two independent document listeners cannot arbitrate: `preventDefault` does not stop - // the other one, so the portlet behind would clear its selection at the same moment this - // panel closed. The registry gives the key to the most recent claim, and the panel opens - // after the portlet, so it wins for exactly as long as it is open. - this.#withdrawShortcuts = this.#shortcuts.register({ - combination: 'escape', - label: 'edit.content.side-panel.shortcut.close', - handler: () => this.#onEscape() - }); - - // Collapse the main navigation while the panel is open (restored on close), and register - // this panel on the stack so ESC only closes the frontmost one. In afterNextRender so the - // store mutation lands after the current render, not during it. - afterNextRender(() => this.#navController.acquire(this)); - // Forward each save to the opener so it can refresh its view. The overlay host is resolved // AFTER construction (afterNextRender) on purpose: resolving it in the constructor would // cycle through its `DynamicDialogConfig` factory, which depends on this component. @@ -218,71 +164,6 @@ export class DotEditContentSidePanelComponent implements OnDestroy { }); } - /** - * ESC handler, invoked by the shared shortcut registry rather than a document listener of this - * component's own. - * - * Always **consumes** the key while a panel is open, whether or not it closes. A panel is a modal - * surface, so ESC must never fall through to the portlet behind it and act on a listing the user - * cannot even see — which is exactly what would happen with two independent document listeners, - * since `preventDefault` does not stop other listeners on the same node. - * - * The `isTop` guard was originally needed because stacked panels shared one document listener and - * a single ESC would collapse the whole stack. The registry makes that structurally impossible — - * only the most recent claim on a combination is called, and the frontmost panel registered last. - * It is kept as a second line of defence, not because it is still load-bearing. - */ - #onEscape(): boolean { - // An overlay above owns the key. Consumed, not declined, so it cannot reach the portlet. - if (this.#hasOverlayAbove()) { - return true; - } - - if (this.#navController.isTop(this)) { - this.requestClose(); - } - - return true; - } - - /** - * Whether another PrimeNG overlay is stacked on top of this panel — the image editor dialog, a - * confirm popup, an open select panel, or a second side panel. - * - * Delegates to the shared helper in `@dotcms/ui`: the portlet shell asks the same question for - * its own dialogs, and two copies of a z-index comparison would drift. - */ - #hasOverlayAbove(): boolean { - return hasOverlayAbove(this.$drawer()?.container); - } - - /** - * Click-outside handler: clicking the area behind the panel closes it with the exact same - * semantics as ESC and the X button — i.e. through the unsaved-changes guard. - * - * The drawer's own `dismissible` is deliberately left off: it hides the drawer and emits - * `visibleChange(false)` the moment the mask is clicked, which both bypasses the guard (unsaved - * edits lost silently) and desyncs the one-way `[visible]` binding. Matching the mask ourselves - * keeps the panel on screen until the guard says it is safe to close. - * - * The listener is document-wide because the mask is built imperatively into `document.body`, so - * no template binding can reach it. It matches on the mask's IDENTITY rather than its - * `p-drawer-mask` class: that class is shared by every modal drawer in the app (the UVE block - * editor sidebar is one), and a class check would let a foreign drawer's mask close this panel - * and pop the unsaved-changes prompt. Comparing the element also means a click inside the panel, - * or a drag that starts inside and ends on the mask, resolves to a different node and is - * ignored. As with ESC, only the frontmost stacked panel reacts. - */ - protected onMaskClick(event: MouseEvent): void { - if (event.target !== this.$drawer()?.mask) { - return; - } - - if (this.#navController.isTop(this)) { - this.requestClose(); - } - } - /** * Close intent (X button, ESC, or browser Back routed by the opener). Routes through the * editor's unsaved-changes guard so the user is prompted when the form is dirty; only closes @@ -312,9 +193,7 @@ export class DotEditContentSidePanelComponent implements OnDestroy { * starts in the same mode. */ protected toggleExpanded(): void { - const next = !this.$expanded(); - this.$expanded.set(next); - writeExpandedPreference(next); + this.#chrome.toggleExpanded(); } /** Fires the `data` lifecycle callbacks on close, matching {@link DotEditContentDialogComponent}. */ @@ -329,11 +208,7 @@ export class DotEditContentSidePanelComponent implements OnDestroy { } ngOnDestroy(): void { + // The chrome releases its own registrations; only the form bridge is this panel's. popFormBridge(); - // Restore the main navigation once the last panel closes. - this.#navController.release(this); - // Withdrawn here rather than through `DestroyRef` so every teardown this component owns is - // in one place. `DestroyRef` stays for `takeUntilDestroyed` on the `saved$` subscription. - this.#withdrawShortcuts(); } } diff --git a/core-web/libs/edit-content/src/lib/services/dot-side-panel-chrome.spec.ts b/core-web/libs/edit-content/src/lib/services/dot-side-panel-chrome.spec.ts new file mode 100644 index 000000000000..f57b9a3d8b04 --- /dev/null +++ b/core-web/libs/edit-content/src/lib/services/dot-side-panel-chrome.spec.ts @@ -0,0 +1,148 @@ +import { createComponentFactory, Spectator } from '@openng/spectator/vitest'; +import { Mock, vi } from 'vitest'; + +import { Component, signal } from '@angular/core'; + +import { Drawer } from 'primeng/drawer'; + +import { injectSidePanelChrome } from './dot-side-panel-chrome'; +import { DotSidePanelNavController } from './dot-side-panel-nav.service'; + +const EXPANDED_STORAGE_KEY = 'dot-edit-content-side-panel-expanded'; + +/** A side panel reduced to what the chrome needs: a drawer with a mask, and a close intent. */ +@Component({ + selector: 'dot-test-side-panel', + template: '' +}) +class TestSidePanelComponent { + readonly mask = document.createElement('div'); + readonly drawer = signal({ mask: this.mask, container: null } as unknown as Drawer); + readonly requestClose = vi.fn(); + readonly chrome = injectSidePanelChrome({ + panel: this, + drawer: this.drawer, + requestClose: () => this.requestClose(), + escapeLabel: 'test.side-panel.close' + }); +} + +describe('injectSidePanelChrome', () => { + let spectator: Spectator; + let navController: { acquire: Mock; release: Mock; isTop: Mock }; + + const createComponent = createComponentFactory({ + component: TestSidePanelComponent, + // Read per creation, so each test's controller reaches the panel. + providers: [{ provide: DotSidePanelNavController, useFactory: () => navController }], + detectChanges: false + }); + + beforeEach(() => { + localStorage.clear(); + navController = { + acquire: vi.fn(), + release: vi.fn(), + isTop: vi.fn().mockReturnValue(true) + }; + spectator = createComponent(); + spectator.detectChanges(); + }); + + const pressEscape = (): KeyboardEvent => { + const event = new KeyboardEvent('keydown', { + key: 'Escape', + bubbles: true, + cancelable: true + }); + document.dispatchEvent(event); + + return event; + }; + + const clickOn = (element: HTMLElement): void => { + document.body.appendChild(element); + element.dispatchEvent(new MouseEvent('click', { bubbles: true })); + element.remove(); + }; + + describe('Escape', () => { + it('asks the frontmost panel to close, and consumes the key', () => { + const event = pressEscape(); + + expect(spectator.component.requestClose).toHaveBeenCalledTimes(1); + expect(event.defaultPrevented).toBe(true); + }); + + it('consumes the key but leaves a panel that is not frontmost open', () => { + navController.isTop.mockReturnValue(false); + + const event = pressEscape(); + + expect(spectator.component.requestClose).not.toHaveBeenCalled(); + expect(event.defaultPrevented).toBe(true); + }); + + it('is the same handling when a panel forwards an Escape the registry cannot see', () => { + expect(spectator.component.chrome.escape()).toBe(true); + expect(spectator.component.requestClose).toHaveBeenCalledTimes(1); + }); + }); + + describe('mask click', () => { + it('asks the panel to close on a click on its own mask', () => { + clickOn(spectator.component.mask); + + expect(spectator.component.requestClose).toHaveBeenCalledTimes(1); + }); + + it("ignores a click on another drawer's mask", () => { + const foreignMask = document.createElement('div'); + foreignMask.classList.add('p-drawer-mask'); + + clickOn(foreignMask); + + expect(spectator.component.requestClose).not.toHaveBeenCalled(); + }); + + it('ignores a click on its mask when another panel is in front', () => { + navController.isTop.mockReturnValue(false); + + clickOn(spectator.component.mask); + + expect(spectator.component.requestClose).not.toHaveBeenCalled(); + }); + }); + + describe('side-panel stack', () => { + it('joins the stack after the first render', () => { + expect(navController.acquire).toHaveBeenCalledWith(spectator.component); + }); + + it('leaves the stack, and gives Escape and the mask back, when the panel is destroyed', () => { + const { component } = spectator; + + spectator.fixture.destroy(); + + expect(navController.release).toHaveBeenCalledWith(component); + expect(pressEscape().defaultPrevented).toBe(false); + clickOn(component.mask); + expect(component.requestClose).not.toHaveBeenCalled(); + }); + }); + + describe('full width', () => { + it('starts narrow when nothing is stored', () => { + expect(spectator.component.chrome.expanded()).toBe(false); + }); + + it('remembers the choice for the next panel', () => { + spectator.component.chrome.toggleExpanded(); + + expect(spectator.component.chrome.expanded()).toBe(true); + expect(localStorage.getItem(EXPANDED_STORAGE_KEY)).toBe('true'); + + expect(createComponent().component.chrome.expanded()).toBe(true); + }); + }); +}); diff --git a/core-web/libs/edit-content/src/lib/services/dot-side-panel-chrome.ts b/core-web/libs/edit-content/src/lib/services/dot-side-panel-chrome.ts new file mode 100644 index 000000000000..ab1fee0ec16a --- /dev/null +++ b/core-web/libs/edit-content/src/lib/services/dot-side-panel-chrome.ts @@ -0,0 +1,141 @@ +import { DOCUMENT } from '@angular/common'; +import { DestroyRef, Signal, afterNextRender, inject, signal } from '@angular/core'; + +import { Drawer } from 'primeng/drawer'; + +import { DotKeyboardShortcutService, hasOverlayAbove } from '@dotcms/ui'; + +import { DotSidePanelNavController } from './dot-side-panel-nav.service'; + +/** + * Shared by every side panel, so the author's expanded (full-width) choice carries over between + * them. + */ +const EXPANDED_STORAGE_KEY = 'dot-edit-content-side-panel-expanded'; + +/** Best-effort read of the expanded preference; `false` when storage is unavailable. */ +function readExpandedPreference(): boolean { + try { + return localStorage.getItem(EXPANDED_STORAGE_KEY) === 'true'; + } catch { + return false; + } +} + +/** Persists the expanded preference; storage failures must not break the panel. */ +function writeExpandedPreference(expanded: boolean): void { + try { + localStorage.setItem(EXPANDED_STORAGE_KEY, String(expanded)); + } catch { + // best-effort: quota errors / disabled storage are ignored. + } +} + +/** What a side panel tells {@link injectSidePanelChrome} about itself. */ +export interface DotSidePanelChromeOptions { + /** The panel itself: its identity on the side-panel stack. */ + panel: object; + /** The panel's own drawer, for its mask and its container. */ + drawer: Signal; + /** The panel's close intent, guard included. Called on Escape and on a click on its mask. */ + requestClose: () => void; + /** i18n key naming the Escape shortcut, for the shortcut documentation. */ + escapeLabel: string; +} + +/** The behaviour every side panel shares, as {@link injectSidePanelChrome} returns it. */ +export interface DotSidePanelChrome { + /** Whether the panel spans the full width (vs 80%), seeded from the stored choice. */ + readonly expanded: Signal; + /** Flips the full-width state and remembers it for the next panel. */ + toggleExpanded(): void; + /** + * Escape, as the panel handles it. Exposed for keys the shortcut registry cannot see, such as + * Escape pressed inside an iframe the panel hosts. + * + * @returns `true`: the key is consumed. + */ + escape(): boolean; +} + +/** + * Gives a side panel the behaviour all side panels share, so each one only writes its own content + * and its own close guard: + * + * - **Escape** closes it, through the shortcut registry rather than a listener of its own: two + * document listeners cannot arbitrate (`preventDefault` does not stop the other one), so the + * portlet behind would act on the same key. Escape is always consumed while the panel is open, and + * only closes when no overlay sits above the panel and it is the frontmost one. + * - **A click on its own mask** closes it, through the same close intent. The drawer's + * `dismissible` must stay off: it hides the drawer the moment the mask is clicked, skipping the + * guard and desyncing the one-way `[visible]` binding. The mask is matched by identity, not by its + * `p-drawer-mask` class, which every modal drawer shares (the UVE block editor sidebar is one); a + * click inside the panel, or a drag that starts inside and ends on the mask, resolves to another + * node and is ignored. + * - **The side-panel stack**: joined after the first render (which collapses the main navigation) + * and left on destroy. + * - **Full width**: the expanded preference, shared by every side panel. + * + * Call it from the panel's constructor or a field initializer: it needs an injection context, and + * everything it registers is released when the panel is destroyed. + * + * @param options what the panel tells it about itself + */ +export function injectSidePanelChrome({ + panel, + drawer, + requestClose, + escapeLabel +}: DotSidePanelChromeOptions): DotSidePanelChrome { + const navController = inject(DotSidePanelNavController); + const shortcuts = inject(DotKeyboardShortcutService); + const document = inject(DOCUMENT); + const expanded = signal(readExpandedPreference()); + + const escape = (): boolean => { + // An overlay above owns the key. Consumed, not declined, so it cannot reach the portlet. + if (hasOverlayAbove(drawer()?.container)) { + return true; + } + + if (navController.isTop(panel)) { + requestClose(); + } + + return true; + }; + + // Document-wide because `appendTo="body"` moves the drawer and its mask out of the panel's DOM, + // so no template binding can hear the mask click. + const onDocumentClick = (event: MouseEvent): void => { + if (event.target === drawer()?.mask && navController.isTop(panel)) { + requestClose(); + } + }; + + const withdrawEscape = shortcuts.register({ + combination: 'escape', + label: escapeLabel, + handler: escape + }); + document.addEventListener('click', onDocumentClick); + + // After the current render, so the store mutation it causes does not land during it. + afterNextRender(() => navController.acquire(panel)); + + inject(DestroyRef).onDestroy(() => { + navController.release(panel); + withdrawEscape(); + document.removeEventListener('click', onDocumentClick); + }); + + return { + expanded: expanded.asReadonly(), + toggleExpanded: () => { + const next = !expanded(); + expanded.set(next); + writeExpandedPreference(next); + }, + escape + }; +} diff --git a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-legacy-editor-side-panel/dot-legacy-editor-side-panel.component.ts b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-legacy-editor-side-panel/dot-legacy-editor-side-panel.component.ts index 96a495df1101..c4cc45748af8 100644 --- a/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-legacy-editor-side-panel/dot-legacy-editor-side-panel.component.ts +++ b/core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-legacy-editor-side-panel/dot-legacy-editor-side-panel.component.ts @@ -4,7 +4,6 @@ import { ElementRef, NgZone, OnDestroy, - afterNextRender, computed, inject, input, @@ -34,8 +33,8 @@ import { DotFunctionInfo, DotPushPublishDialogData } from '@dotcms/dotcms-models'; -import { DotSidePanelNavController } from '@dotcms/edit-content'; -import { DotKeyboardShortcutService, DotMessagePipe, hasOverlayAbove } from '@dotcms/ui'; +import { injectSidePanelChrome } from '@dotcms/edit-content'; +import { DotMessagePipe } from '@dotcms/ui'; import { isSameOriginRelativeUrl } from '@dotcms/utils'; import { @@ -47,30 +46,6 @@ import { /** The admin route every legacy portlet screen is served from. */ const LAYOUT_URL = '/c/portal/layout'; -/** - * Shared with the new-editor side panel on purpose, so the author's expanded (full-width) choice - * carries over between the two panels. - */ -const EXPANDED_STORAGE_KEY = 'dot-edit-content-side-panel-expanded'; - -/** Best-effort read of the expanded preference; `false` when storage is unavailable. */ -function readExpandedPreference(): boolean { - try { - return localStorage.getItem(EXPANDED_STORAGE_KEY) === 'true'; - } catch { - return false; - } -} - -/** Persists the expanded preference; storage failures must not break the panel. */ -function writeExpandedPreference(expanded: boolean): void { - try { - localStorage.setItem(EXPANDED_STORAGE_KEY, String(expanded)); - } catch { - // best-effort: quota errors / disabled storage are ignored. - } -} - /** The shape of the `ng-event` CustomEvents the legacy editor dispatches on its own document. */ interface LegacyEditorEventDetail { name: string; @@ -83,7 +58,8 @@ interface LegacyEditorEventDetail { * Hosts the legacy (JSP) content editor in a side panel over Content Drive (#37759). * * It looks and behaves like the new-editor side panel (`DotEditContentSidePanelComponent`): the - * same drawer, width toggle, mask and close rules. It is kept separate from it so that removing the + * same drawer, and the same width toggle, mask and Escape rules, which both get from + * `injectSidePanelChrome`. It is kept separate from it so that removing the * legacy editor later never touches the new-editor panel. Remove with the legacy editor. Every other piece that goes * with it carries the same phrase. * @@ -98,16 +74,9 @@ interface LegacyEditorEventDetail { // another one mounted elsewhere on the page. providers: [ConfirmationService], templateUrl: './dot-legacy-editor-side-panel.component.html', - changeDetection: ChangeDetectionStrategy.OnPush, - // Bound at document level because `appendTo="body"` moves the drawer and its mask out of this - // component's DOM, so a template listener would never hear the mask click. - host: { - '(document:click)': 'onMaskClick($event)' - } + changeDetection: ChangeDetectionStrategy.OnPush }) export class DotLegacyEditorSidePanelComponent implements OnDestroy { - readonly #navController = inject(DotSidePanelNavController); - readonly #shortcuts = inject(DotKeyboardShortcutService); readonly #uiColors = inject(DotUiColorsService); readonly #sanitizer = inject(DomSanitizer); readonly #zone = inject(NgZone); @@ -147,9 +116,21 @@ export class DotLegacyEditorSidePanelComponent implements OnDestroy { */ readonly languageChanged = output(); - /** This panel's own drawer, so the mask-click check compares against ITS mask only. */ + /** This panel's own drawer: its mask and container are what the shared chrome checks. */ protected readonly $drawer = viewChild(Drawer); + /** + * Escape, a click on the mask, the side-panel stack and the full-width toggle, shared with the + * other side panels. Escape and the mask close through {@link requestClose}, so the + * unsaved-changes prompt runs first. + */ + readonly #chrome = injectSidePanelChrome({ + panel: this, + drawer: this.$drawer, + requestClose: () => this.requestClose(), + escapeLabel: 'edit.content.side-panel.shortcut.close' + }); + /** The iframe the legacy editor loads in. */ protected readonly $iframe = viewChild>('iframe'); @@ -166,7 +147,7 @@ export class DotLegacyEditorSidePanelComponent implements OnDestroy { readonly #languageId = linkedSignal(() => this.request()?.languageId ?? 0); /** Whether the panel is expanded to the full width (vs 80%), seeded from the stored choice. */ - protected readonly $expanded = signal(readExpandedPreference()); + protected readonly $expanded = this.#chrome.expanded; /** The legacy editor URL for the request, or `null` when there is nothing valid to load. */ protected readonly $url = computed(() => { @@ -186,9 +167,6 @@ export class DotLegacyEditorSidePanelComponent implements OnDestroy { */ protected readonly $loading = linkedSignal(() => this.$url() !== null); - /** Withdraws the Escape claim; called on destroy. */ - readonly #withdrawShortcuts: () => void; - /** * A Bring Back is under way: the editor was asked for `getVersionBack` and will reload with the * restored version. Reported once that reload lands. @@ -199,17 +177,6 @@ export class DotLegacyEditorSidePanelComponent implements OnDestroy { #detachFrame: () => void = () => undefined; constructor() { - // ESC goes through the shared shortcut registry, as in the new-editor panel, so the portlet - // behind does not act on the same key while the panel is open. - this.#withdrawShortcuts = this.#shortcuts.register({ - combination: 'escape', - label: 'edit.content.side-panel.shortcut.close', - handler: () => this.#onEscape() - }); - - // Join the side-panel stack (and collapse the main nav) after the current render. - afterNextRender(() => this.#navController.acquire(this)); - // Other admin code asks the legacy editor to run one of its functions through this service: // the workflow wizard hands its answer back with `saveAssignCallBackAngular`, which fills // the workflow fields and saves (FR-009). The full-page editor's iframe answers it the same @@ -294,7 +261,7 @@ export class DotLegacyEditorSidePanelComponent implements OnDestroy { // told apart by having consumed the key. const onKeydown = (event: KeyboardEvent) => { if (event.key === 'Escape' && !event.defaultPrevented) { - this.#zone.run(() => this.#onEscape()); + this.#zone.run(() => this.#chrome.escape()); } }; @@ -313,32 +280,14 @@ export class DotLegacyEditorSidePanelComponent implements OnDestroy { }; } - /** - * Click-outside handler. Matches this drawer's own mask by identity, not by class, so a click on - * another drawer's mask never closes this panel. Only the frontmost side panel reacts. - */ - protected onMaskClick(event: MouseEvent): void { - if (event.target !== this.$drawer()?.mask) { - return; - } - - if (this.#navController.isTop(this)) { - this.requestClose(); - } - } - /** Toggles the full-width state and remembers it for the next panel. */ protected toggleExpanded(): void { - const next = !this.$expanded(); - this.$expanded.set(next); - writeExpandedPreference(next); + this.#chrome.toggleExpanded(); } - /** Drops the iframe listeners, leaves the side-panel stack and withdraws the Escape claim. */ + /** Drops the iframe listeners. The chrome releases its own registrations. */ ngOnDestroy(): void { this.#detachFrame(); - this.#navController.release(this); - this.#withdrawShortcuts(); } /** @@ -458,24 +407,6 @@ export class DotLegacyEditorSidePanelComponent implements OnDestroy { }; } - /** - * ESC handler. Always consumes the key while the panel is open, so it never falls through to the - * portlet behind; only closes when no overlay sits above and this is the frontmost panel. - * - * @returns `true`: the key is consumed. - */ - #onEscape(): boolean { - if (hasOverlayAbove(this.$drawer()?.container)) { - return true; - } - - if (this.#navController.isTop(this)) { - this.requestClose(); - } - - return true; - } - /** * The create screen for a `new` request: the URL the opener resolved from the server, with the * folder pre-selected. The panel never asks the server itself, so a type the server refuses From 4325343829fc2e6fe94c28fe7b77d98e98c37ea1 Mon Sep 17 00:00:00 2001 From: Freddy Montes Date: Mon, 12 Oct 2026 12:08:22 -0600 Subject: [PATCH 2/3] refactor(edit-content): one dot-side-panel component for every side panel's drawer (#37965) The Edit Content and legacy editor panels each copied the same drawer and header markup. DotSidePanelComponent now owns it, with the shared Escape, mask-click, side-panel-stack and full-width rules that injectSidePanelChrome carried, and asks its owner to close through closeRequested so each panel keeps its own unsaved-changes guard. A panel passes its content, an optional , and its heading. injectSidePanelChrome is folded into it. Co-Authored-By: Claude Opus 5.5 --- core-web/libs/edit-content/src/index.ts | 8 +- ...dot-edit-content-side-panel.component.html | 73 +---- ...-edit-content-side-panel.component.spec.ts | 199 ++------------ .../dot-edit-content-side-panel.component.ts | 46 +--- .../dot-side-panel.component.html | 80 ++++++ .../dot-side-panel.component.spec.ts | 253 ++++++++++++++++++ .../dot-side-panel.component.ts | 159 +++++++++++ .../services/dot-side-panel-chrome.spec.ts | 148 ---------- .../src/lib/services/dot-side-panel-chrome.ts | 141 ---------- ...ot-legacy-editor-side-panel.component.html | 63 +---- ...legacy-editor-side-panel.component.spec.ts | 48 +--- .../dot-legacy-editor-side-panel.component.ts | 45 +--- 12 files changed, 550 insertions(+), 713 deletions(-) create mode 100644 core-web/libs/edit-content/src/lib/components/dot-side-panel/dot-side-panel.component.html create mode 100644 core-web/libs/edit-content/src/lib/components/dot-side-panel/dot-side-panel.component.spec.ts create mode 100644 core-web/libs/edit-content/src/lib/components/dot-side-panel/dot-side-panel.component.ts delete mode 100644 core-web/libs/edit-content/src/lib/services/dot-side-panel-chrome.spec.ts delete mode 100644 core-web/libs/edit-content/src/lib/services/dot-side-panel-chrome.ts diff --git a/core-web/libs/edit-content/src/index.ts b/core-web/libs/edit-content/src/index.ts index c086c8c9b4e1..bbcfcecc2a4c 100644 --- a/core-web/libs/edit-content/src/index.ts +++ b/core-web/libs/edit-content/src/index.ts @@ -7,12 +7,8 @@ export * from './lib/models/dot-edit-content-dialog.interface'; export * from './lib/services/dot-edit-content.service'; export * from './lib/components/dot-edit-content-side-panel/dot-edit-content-side-panel.component'; export { DotSidePanelNavController } from './lib/services/dot-side-panel-nav.service'; -// Escape, mask click, side-panel stack and full width, for side panels outside this library too. -export { injectSidePanelChrome } from './lib/services/dot-side-panel-chrome'; -export type { - DotSidePanelChrome, - DotSidePanelChromeOptions -} from './lib/services/dot-side-panel-chrome'; +// The drawer every side panel opens in, for side panels outside this library too. +export * from './lib/components/dot-side-panel/dot-side-panel.component'; export * from './lib/utils/functions.util'; export * from './lib/models/dot-edit-content-field.constant'; diff --git a/core-web/libs/edit-content/src/lib/components/dot-edit-content-side-panel/dot-edit-content-side-panel.component.html b/core-web/libs/edit-content/src/lib/components/dot-edit-content-side-panel/dot-edit-content-side-panel.component.html index cc73a439302b..e8883bab6ad7 100644 --- a/core-web/libs/edit-content/src/lib/components/dot-edit-content-side-panel/dot-edit-content-side-panel.component.html +++ b/core-web/libs/edit-content/src/lib/components/dot-edit-content-side-panel/dot-edit-content-side-panel.component.html @@ -1,73 +1,8 @@ - - - -
- - {{ data()?.title }} - -
- - - - {{ $expanded() ? 'close_fullscreen' : 'open_in_full' }} - - - - - - close - - -
-
- + [heading]="data()?.title ?? ''" + (closeRequested)="requestClose()"> @for (item of $items(); track item.contentletInode ?? item.contentTypeId) { } - + diff --git a/core-web/libs/edit-content/src/lib/components/dot-edit-content-side-panel/dot-edit-content-side-panel.component.spec.ts b/core-web/libs/edit-content/src/lib/components/dot-edit-content-side-panel/dot-edit-content-side-panel.component.spec.ts index ffbc38af5aed..cecc38dc73c1 100644 --- a/core-web/libs/edit-content/src/lib/components/dot-edit-content-side-panel/dot-edit-content-side-panel.component.spec.ts +++ b/core-web/libs/edit-content/src/lib/components/dot-edit-content-side-panel/dot-edit-content-side-panel.component.spec.ts @@ -1,15 +1,19 @@ -import { createComponentFactory, Spectator, byTestId } from '@openng/spectator/vitest'; -import { MockComponent, MockPipe } from 'ng-mocks'; +import { + createComponentFactory, + mockProvider, + Spectator, + byTestId +} from '@openng/spectator/vitest'; +import { MockComponent } from 'ng-mocks'; import { Subject } from 'rxjs'; import { Mock, vi } from 'vitest'; -import { ButtonModule } from 'primeng/button'; -import { Drawer, DrawerModule } from 'primeng/drawer'; +import { Drawer } from 'primeng/drawer'; import { DialogService } from 'primeng/dynamicdialog'; import { ZIndexUtils } from 'primeng/utils'; +import { DotMessageService } from '@dotcms/data-access'; import { DotCMSContentlet } from '@dotcms/dotcms-models'; -import { DotMessagePipe } from '@dotcms/ui'; import { DotEditContentSidePanelComponent } from './dot-edit-content-side-panel.component'; @@ -21,6 +25,7 @@ import { EditContentDialogData } from '../../models/dot-edit-content-dialog.inte import { DotSidePanelNavController } from '../../services/dot-side-panel-nav.service'; import { OverlayEditContentHost } from '../../services/host/overlay-edit-content-host'; import { DotEditContentLayoutComponent } from '../dot-edit-content-layout/dot-edit-content.layout.component'; +import { DotSidePanelComponent } from '../dot-side-panel/dot-side-panel.component'; describe('DotEditContentSidePanelComponent', () => { let spectator: Spectator; @@ -45,16 +50,15 @@ describe('DotEditContentSidePanelComponent', () => { { set: { imports: [ - DrawerModule, - ButtonModule, - MockComponent(DotEditContentLayoutComponent), - MockPipe(DotMessagePipe, (key: string) => key) + DotSidePanelComponent, + MockComponent(DotEditContentLayoutComponent) ], providers: [{ provide: OverlayEditContentHost, useValue: undefined }] } } ] - ] + ], + providers: [mockProvider(DotMessageService, { get: (key: string) => key })] }); beforeEach(() => { @@ -122,60 +126,8 @@ describe('DotEditContentSidePanelComponent', () => { spectator.click(button as HTMLElement); }; - /** - * Reads the expand button's icon glyph — the user-visible reflection of `$expanded` - * (`open_in_full` when collapsed, `close_fullscreen` when expanded). Asserting on this instead - * of the protected signal covers what the user actually sees. `{ root: true }` because - * `appendTo="body"` teleports the header out of the fixture. - */ - const expandIcon = (): string | undefined => - spectator - .query(byTestId('side-panel-expand'), { root: true }) - ?.querySelector('i') - ?.textContent?.trim(); - - /** - * Expand button aria-label (i18n key via MockPipe) — user-facing expanded/collapsed cue. - * - * Read from the DESCENDANT native ` + + `; + +describe('DotSidePanelComponent', () => { + let spectator: SpectatorHost; + let navController: { acquire: Mock; release: Mock; isTop: Mock }; + let closeRequested: Mock; + + const createHost = createHostFactory({ + component: DotSidePanelComponent, + providers: [ + mockProvider(DotMessageService, { get: (key: string) => key }), + // Read per creation, so each test's controller reaches the panel. + { provide: DotSidePanelNavController, useFactory: () => navController } + ], + detectChanges: false + }); + + const open = (template = PANEL): void => { + spectator = createHost(template); + closeRequested = vi.fn(); + spectator.output('closeRequested').subscribe(closeRequested); + spectator.detectChanges(); + }; + + beforeEach(() => { + localStorage.clear(); + navController = { + acquire: vi.fn(), + release: vi.fn(), + isTop: vi.fn().mockReturnValue(true) + }; + }); + + // `appendTo="body"` teleports the drawer out of the fixture, so DOM queries search from the + // document root. + const query = (testId: string): HTMLElement | null => + spectator.query(byTestId(testId), { root: true }); + + const clickButton = (testId: string): void => { + spectator.click(query(testId)?.querySelector('button') as HTMLElement); + }; + + const pressEscape = (): KeyboardEvent => { + const event = new KeyboardEvent('keydown', { + key: 'Escape', + bubbles: true, + cancelable: true + }); + document.dispatchEvent(event); + + return event; + }; + + /** + * Clicks a stand-in for a drawer's mask. PrimeNG creates the real one during the show + * animation, which jsdom never runs. + */ + const clickMask = ({ ownedByPanel }: { ownedByPanel: boolean }): HTMLElement => { + const mask = document.createElement('div'); + mask.classList.add('p-drawer-mask'); + document.body.appendChild(mask); + + if (ownedByPanel) { + spectator.query(Drawer).mask = mask; + } + + mask.dispatchEvent(new MouseEvent('click', { bubbles: true })); + mask.remove(); + + return mask; + }; + + describe('drawer', () => { + it('shows the header and the content it is given', () => { + open(); + + expect(query('side-panel-title')?.textContent?.trim()).toBe('header.vtl'); + expect(query('panel-body')?.textContent).toBe('The source'); + }); + + it('draws no footer bar without a footer', () => { + open(); + + expect(spectator.query('.p-drawer-footer', { root: true })).toBeNull(); + }); + + it("draws the owner's footer", () => { + open(PANEL_WITH_FOOTER); + + expect( + spectator.query('.p-drawer-footer', { root: true })?.contains(query('panel-save')) + ).toBe(true); + }); + + it('asks to close from the close button', () => { + open(); + + clickButton('side-panel-close'); + + expect(closeRequested).toHaveBeenCalledTimes(1); + }); + }); + + describe('Escape', () => { + it('asks the frontmost panel to close, and consumes the key', () => { + open(); + + const event = pressEscape(); + + expect(closeRequested).toHaveBeenCalledTimes(1); + expect(event.defaultPrevented).toBe(true); + }); + + it('consumes the key but leaves a panel that is not frontmost open', () => { + open(); + navController.isTop.mockReturnValue(false); + + const event = pressEscape(); + + expect(closeRequested).not.toHaveBeenCalled(); + expect(event.defaultPrevented).toBe(true); + }); + + it('leaves the key to an overlay above the panel', () => { + open(); + // Stands in for a dialog opened from inside the panel: every PrimeNG overlay registers + // itself in this shared stack when it opens. + const overlay = document.createElement('div'); + ZIndexUtils.set('modal', overlay, 2000); + + pressEscape(); + + expect(closeRequested).not.toHaveBeenCalled(); + // Shared global stack: leaving the entry behind would leak into later tests. + ZIndexUtils.clear(overlay); + }); + + it('is the same handling when a panel forwards an Escape the registry cannot see', () => { + open(); + + expect(spectator.component.escape()).toBe(true); + expect(closeRequested).toHaveBeenCalledTimes(1); + }); + }); + + describe('mask click', () => { + it('asks to close on a click on its own mask', () => { + open(); + + clickMask({ ownedByPanel: true }); + + expect(closeRequested).toHaveBeenCalledTimes(1); + }); + + it("ignores a click on another drawer's mask", () => { + open(); + + clickMask({ ownedByPanel: false }); + + expect(closeRequested).not.toHaveBeenCalled(); + }); + + it('ignores a click on its mask when another panel is in front', () => { + open(); + navController.isTop.mockReturnValue(false); + + clickMask({ ownedByPanel: true }); + + expect(closeRequested).not.toHaveBeenCalled(); + }); + }); + + describe('side-panel stack', () => { + it('joins the stack after the first render', () => { + open(); + + expect(navController.acquire).toHaveBeenCalledWith(spectator.component); + }); + + it('leaves the stack, and gives Escape and the mask back, when destroyed', () => { + open(); + const { component } = spectator; + const drawer = spectator.query(Drawer); + const mask = document.createElement('div'); + drawer.mask = mask; + + spectator.fixture.destroy(); + + expect(navController.release).toHaveBeenCalledWith(component); + expect(pressEscape().defaultPrevented).toBe(false); + document.body.appendChild(mask); + mask.dispatchEvent(new MouseEvent('click', { bubbles: true })); + mask.remove(); + expect(closeRequested).not.toHaveBeenCalled(); + }); + }); + + describe('full width', () => { + const width = (): string => + spectator.query('.p-drawer', { root: true })?.style.width ?? ''; + + // Read from the native `