Skip to content

fix(edit-content): side panel feedback — click-outside, 80% width, sidebar polish (#36631) - #36965

Draft
adrianjm-dotCMS wants to merge 1 commit into
mainfrom
36631-feedback
Draft

fix(edit-content): side panel feedback — click-outside, 80% width, sidebar polish (#36631)#36965
adrianjm-dotCMS wants to merge 1 commit into
mainfrom
36631-feedback

Conversation

@adrianjm-dotCMS

Copy link
Copy Markdown
Member

Parent Issue

#36631 — the panel itself shipped in #36732; this PR addresses the review/QA feedback on top of it.

What changed

1. Click outside closes the panel (through the unsaved-changes guard)

Clicking the area behind the panel now behaves exactly like ESC and the X button: it routes through the editor's unsaved-changes guard, so a dirty form prompts and "Keep editing" cancels the close.

PrimeNG's own dismissible is deliberately left off — it calls close() on mask click, which hides the drawer and emits visibleChange(false) immediately. That both bypasses the guard (unsaved edits lost silently) and desyncs the one-way [visible]="!!data()" binding. The mask click is matched at document level instead, since appendTo="body" moves the mask out of the component's subtree.

Edge cases covered: a click inside the panel is ignored; a drag that starts inside and ends on the mask resolves to a different target and is ignored; with stacked panels (relationship field) only the frontmost reacts, same as ESC.

2. Default width 70% → 80%

3. Nav collapse threshold 1800px → 2560px

At 80% the leftover strip is 20% of the viewport. Against the 270px expanded nav that left ~90px of content at 1800px — a useless sliver, so the nav stayed expanded for no benefit. At 2560 the strip is ~242px.

4. Binary field preview: narrow-layout buttons no longer paint over the sticky tab bar

.dot-file-field-preview__actions--responsive went from z-index: 100 to 2, in line with its siblings (1 for the wide variant, 2 for remove — nothing in the component needs more than 2). 100 was the only value in the whole edit-content lib able to beat the form's sticky .p-tablist (z-index: 10), which carries the status chip and the permissions/references actions. Only reproduced on narrow containers, because that is the only layout where these filled blue/red variants render.

5. Sidebar toggle icon → dock_to_left

Replaces the two mirrored UVE SVGs (left_panel_open.svg / left_panel_close.svg) with a single Material Symbol. Since one glyph now covers both states, the open/closed cue moved to the button's aria-label and the icon is aria-hidden so screen readers don't announce the ligature text.

6. Actions tab section order → Details, Locales, Workflow

The shared .dot-section bottom-border override moved to Workflow (now last) so there is no divider dangling at the end of the tab. Persistence keys are unchanged, so each user's saved expand/collapse state carries over.

Testing

Updated specs alongside each change:

  • dot-edit-content-side-panel.component.spec.ts — 4 new tests for click-outside (closes via guard; does not close when the guard cancels; ignored when not frontmost; ignored on a click inside), plus the width assertions.
  • dot-side-panel-nav.service.spec.tssetWideViewport() pinned to the 2560 threshold (the comparison is exclusive, so the boundary must count as wide; bumping the threshold fails here on purpose).
  • dot-edit-content-form.component.spec.ts — replaced the two-SVG test with one for the single dock_to_left glyph.
  • dot-edit-content-sidebar.component.spec.ts — section-key order updated; noted that queryAll returns DOM order, so that test guards the order too.

Manual verification of the visual changes was done by the author.

🤖 Generated with Claude Code

…debar polish (#36631)

Follow-up feedback on the Edit Content slide-in panel:

- Click outside the panel now closes it through the same unsaved-changes guard
  as ESC and the X button. PrimeNG's `dismissible` is deliberately left off: it
  tears the drawer down immediately, bypassing the guard and desyncing the
  one-way `[visible]` binding. The mask click is matched at document level
  (the drawer is `appendTo="body"`) and only the frontmost stacked panel reacts.
- Default panel width 70% -> 80%.
- Nav collapse threshold 1800px -> 2560px: at 80% the leftover strip left only
  ~90px of content beside the expanded nav, so the nav stayed expanded for no
  benefit.
- Binary field preview: the narrow-layout action buttons dropped from
  `z-index: 100` to `2`, matching their siblings. 100 was the only value in the
  lib able to paint over the form's sticky tab list (z-index 10), which carries
  the status chip and permissions/references.
- Sidebar toggle now uses the `dock_to_left` Material Symbol instead of the two
  mirrored UVE SVGs. Single glyph, so the open/closed cue moved to the button's
  aria-label and the icon is `aria-hidden`.
- Actions tab section order is now Details, Locales, Workflow. The shared
  bottom-border override moved to Workflow, which is last.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Claude finished @adrianjm-dotCMS's task in 2m 45s —— View job


Code Review

Reviewed the full diff against the described changes. The click-outside logic is coherent, and the routing through requestClose() correctly preserves the unsaved-changes guard contract. Comments/JSDoc are thorough and the specs cover the meaningful new paths (guard-proceeds, guard-cancels, not-frontmost, click-inside).

New Issues

  • 🟡 Medium: dot-edit-content-side-panel.component.ts:45 — The entire click-outside feature hinges on DRAWER_MASK_CLASS = 'p-drawer-mask' matching the class PrimeNG actually puts on the drawer's modal mask. If PrimeNG 21 uses a different class, onMaskClick silently never fires and click-outside is dead — and no test would catch it, because clickOutside() in the spec fabricates its own <div class="p-drawer-mask"> rather than letting PrimeNG build the mask. The theme SCSS in this repo still styles the old .p-sidebar-mask (libs/dotcms-scss/angular/dotcms-theme/components/_sidebar.scss:42), and apps/dotcms-binary-field-builder/src/style.css:2 notes PrimeNG 21 renamed the overlay mask class — so the naming is genuinely in flux here.
    Assumption: PrimeNG 21's p-drawer renders its mask with p-drawer-mask (consistent with the v18 Sidebar→Drawer rename).
    What to verify: Confirm against the running app / PrimeNG 21 source that the mask element carries p-drawer-mask. If it does, consider adding one integration-level check that opens a real drawer and asserts the rendered mask class, so a future PrimeNG bump that renames it fails loudly instead of silently disabling click-to-close. Fix this →

Everything else checks out:

  • Click-outside logic (:203-213) — event.target class filter correctly ignores clicks inside the panel and drags whose common ancestor isn't the mask; isTop(this) correctly scopes to the frontmost stacked panel, matching the existing onEscape() contract. requestClose() routes through layout.confirmClose(proceed) so a dirty form still prompts.
  • dismissible left false — the rationale (PrimeNG's dismissible tears down immediately, bypassing the guard and desyncing the one-way [visible] binding) is correct and well-documented.
  • z-index 100 → 2 (dot-file-field-preview.component.scss:126) — brings the responsive actions in line with siblings (1/2) and below the sticky .p-tablist (z-index: 10); no new stacking conflict introduced.
  • Section reorder (dot-edit-content-sidebar.component.html) — border-b-0 override correctly moved from the old-last (details) to the new-last (workflow); persistence keys unchanged; spec updated to assert DOM order.
  • Single dock_to_left glypharia-hidden on the icon with the accessible name moved to the button's aria-label is the right a11y pattern; spec asserts the exact ligature text (guards against a typo rendering as plain words).
  • Nav threshold 1800 → 2560 — spec pins setWideViewport() to the exact boundary and documents the exclusive (<) comparison, so a future threshold bump fails on purpose.

No blocking issues. The Medium above is worth confirming before merge since it's a silent-failure risk, but it doesn't block.
· branch 36631-feedback

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area : Frontend PR changes Angular/TypeScript frontend code

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

1 participant