dotCMS / dotCMS/core

Extract the shared side-panel drawer shell used by Edit Content and the UVE experiments panel

Open
#37,552 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Team : Falcon Type : Refactoring
Dominant language
Java
Stars
970
Forks
486
Avg merge
3d 33m
Merged PRs (30d)
170

Description

Description

dot-edit-content-side-panel and dot-experiments-panel are two instances of the same UI: a right-hand PrimeNG drawer that overlays the editor without dimming the page behind it, is closed by the component rather than by PrimeNG, and carries a header with expand/collapse and close buttons.

The second was written as a deliberate copy of the first, in #37478 (UVE experiments panel). Both files say so in their own comments — dot-experiments-panel.component.html opens with "Mirrors dot-edit-content-side-panel.component.html on purpose".

What is duplicated is not styling. It is a set of non-obvious decisions, each of which had a reason that is now written out twice:

Duplicated Why it is not obvious
[maskStyle]="{ background: 'transparent !important' }" The !important is required: neither the theme preset nor the pt API can clear PrimeNG's dimmed mask. Reads like a leftover; it is not.
[dismissible]="false" plus a manual mask-click handler PrimeNG's own dismissible tears the drawer down immediately, bypassing the unsaved-changes guard.
onMaskClick matching the mask by identity, not by .p-drawer-mask class That class is shared by every modal drawer in the app, so a class check lets a foreign drawer's mask close this panel. The listener is document-wide because appendTo="body" moves the mask out of the component's DOM.
class: '!shadow-2xl' on pt.root Shadow cannot live in component SCSS — appendTo="body" puts the element beyond the reach of :host ::ng-deep.
80% / 100% width + transition: 'width 250ms ease' Two independent copies of the same magic numbers.
$expanded signal + localStorage read/write helpers + toggleExpanded() ~30 lines copied verbatim, differing only in the storage key.
Header markup: two p-buttons using [ariaLabel] (never [attr.aria-label]) [attr.aria-label] lands on the non-focusable <p-button> host, leaving the real <button> — an icon glyph — with no accessible name.

The two panels have already diverged on Escape handling: Edit Content sets [closeOnEscape]="false" and handles the key itself, while the experiments panel routes Escape through the shared shortcut registry. That divergence is the cost this issue is meant to stop paying — the next panel will copy whichever file its author happens to open.

Why PrimeNG global config does not solve this

Worth stating up front, because it is the first thing a reader will reach for:

  • appendTo does have a global default — providePrimeNG({ overlayAppendTo: 'body' }); the drawer reads appendTo() || config.overlayAppendTo(). But that default governs every overlay in the app (selects, dialogs, tooltips, popovers), and today it is 'self'. Flipping it app-wide to save one attribute is a stacking and clipping change everywhere.
  • Global pt (providePrimeNG({ pt: { drawer: … } })) reaches all 9 drawers in the repo, including the 7 legacy experiments dialogs that do want a dimmed mask. Only the shadow is generic, and its proper home would be the components.drawer.root.shadow token in CustomLaraPreset, which is likewise app-wide.
  • modal, dismissible, closeOnEscape, closable, position and maskStyle are plain @Input()s. pt writes DOM attributes and classes only — it cannot set inputs. There is no global mechanism for them at all.

The unit that can actually be shared is the panel, not the drawer defaults.

Acceptance Criteria

  • A reusable side-panel shell component exists under libs/ui (suggested: libs/ui/src/lib/components/dot-side-panel/), exported from the library's public barrel.
  • The shell owns every drawer binding listed above: modal, dismissible, closable, appendTo, position, maskStyle, the shadow, and the width/transition styles.
  • The shell owns the expand/collapse state, including its localStorage persistence, keyed by a per-consumer storage key supplied as an input.
  • The shell owns the mask-click detection (identity match, not class match) and surfaces it as a close request, so each consumer keeps the final say and can run its own unsaved-changes guard before the panel actually closes.
  • The shell renders the header — title plus expand/collapse and close buttons — with the title supplied by the consumer and both buttons named via [ariaLabel].
  • The consumer projects its own body content, and can add classes to the drawer's content element (the experiments panel needs flex flex-col min-h-0, Edit Content does not).
  • The consumer supplies its own data-testid for the drawer root; existing test ids (edit-content-side-panel, experiments-panel) are unchanged.
  • dot-edit-content-side-panel and dot-experiments-panel both use the shell, and the duplicated bindings, header markup, and localStorage helpers are deleted from both.
  • Escape handling is settled deliberately rather than inherited by accident: the two panels currently differ, and the chosen behaviour is documented in the shell.
  • The shell has its own unit tests covering the close request (mask click and close button), the expand/collapse toggle and its persistence, and the fact that a foreign drawer's mask does not close it.
  • Both panels' existing specs still pass without being rewritten around the shell's internals.
  • Visually verified in the browser: both panels open, expand, collapse, and close exactly as before, and the page behind stays undimmed.

Priority

Medium

Additional Context

Files involved

  • core-web/libs/edit-content/src/lib/components/dot-edit-content-side-panel/dot-edit-content-side-panel.component.{ts,html} (322 lines of TS)
  • core-web/libs/portlets/dot-experiments/portlet/src/lib/dot-experiments-panel/dot-experiments-panel.component.{ts,html} (197 lines of TS)

Scope

Only these two panels. The block editor sidebar (dot-block-editor-sidebar) and the 7 legacy experiments drawers under libs/portlets/dot-experiments/portlet/src/lib/old/ are modal dialogs with a dimmed mask, not side panels, and are deliberately out of scope.

Placement

libs/ui follows from the repo's own placement rule in core-web/CLAUDE.md: used by multiple portlets and domain-agnostic.

Origin

Surfaced while reviewing the drawer configuration in #37478 (UVE experiments panel), which introduced the second copy. Not a blocker for that PR — the duplication is intentional there and documented in the code.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the two existing panel components and templates listed under Files involved, then read core-web/CLAUDE.md for the libs/ui placement rule. Define the shared shell around the stated drawer, header, mask, persistence, projection, and Escape requirements, while preserving each panel's test id and close guard. Done means the shell has focused unit tests, both panels use it, existing specs pass, and browser behavior remains unchanged.

Written by the indexing model from the issue text.

Assessment

Tech stack
angular, typescript
Domain
frontend
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.