Skip to content

fix(ui): round-3 polish — container list, bundle upload, toast severity#35380

Open
hmoreras wants to merge 4 commits intomainfrom
issue-35274-ui-fixes-round-3
Open

fix(ui): round-3 polish — container list, bundle upload, toast severity#35380
hmoreras wants to merge 4 commits intomainfrom
issue-35274-ui-fixes-round-3

Conversation

@hmoreras
Copy link
Copy Markdown
Member

@hmoreras hmoreras commented Apr 18, 2026

Summary

Round 3 of UI consistency fixes for the post-Lara-theme portlet polish (part of #35274). Four related areas:

Container list (dot-container-list)

  • Right-click anywhere on a row now opens the action context menu — previously pContextMenuRow was set on each <tr> but no <p-contextMenu> was attached, so the directive was a no-op. Fixed by wiring a <p-contextMenu> via viewChild and a (contextmenu) handler that reuses the existing setContainerActions() entries, matching the pattern already used by the template list.
  • 3-dot action button is now hover-only (Tailwind group / group-hover:opacity-100), matching other list portlets. <dot-action-menu-button> replaced with a <p-button pi-ellipsis-v> that opens the same context menu, so right-click and the button share one source of truth.
  • Fixed a @ViewChild('actionsMenu') regression: once a signal-based viewChild was declared for the context menu, the legacy decorator started resolving to the wrong component at runtime (ContextMenu instead of Menu), breaking handleActionMenuOpen. Migrating actionsMenu to the signal API restores correct resolution.
  • Spec updated: added the window.matchMedia jsdom polyfill required by PrimeNG ContextMenu, and converted the action-menu DOM assertions to call setContainerActions() directly.

Bundle upload dialog (view_publish_tool.jsp)

  • Replaced the raw <input type="file"> with a dojox.form.Uploader widget so the dialog matches the dijit styling of the surrounding portlet, and added a localized filename label.
  • Worked around two bugs in the custom Dojo build's Uploader:
    1. Uploader._createInput() does not forward the accept attribute to its internal <input type="file">, so any file could be picked. applyBundleAcceptFilter() walks the widget's domNode and sets accept=".tar.gz,.gz,.tgz" on the real file input. Re-applied on every dialog open because uploader.reset() recreates the input.
    2. The widget only fires an onChange event — no visible element is updated — so the label kept reading "No file chosen" even after a pick. updateBundleFileNameDisplay() is wired to onChange in dojo.ready() and reflects the selection.
  • Spacing + action-bar margin tweaks so the new controls sit cleanly.

Before
Screenshot 2026-04-17 at 1 30 54 PM

After
image

Toast severity (dot-message-display)

  • PrimeNG Toast expects severity 'warn', but DotMessageSeverity emits 'WARNING'. Map 'warning' → 'warn' before calling MessageService.add() so warning toasts pick up the warn style. SCSS class renamed (.warning.warn) to stay in sync.

Before

Screenshot 2026-04-17 at 1 23 40 PM

After

Screenshot 2026-04-17 at 1 24 47 PM

Container service (dot-containers.service)

  • copy() switched from PUT to POST — the backend _copy endpoint is a POST, and the PUT was returning 405.

Related

Part of #35274

Test plan

  • yarn nx test dotcms-ui --testPathPattern=container-list.component.spec passes
  • Right-click on a container row opens the context menu with the correct actions (Publish/Unpublish/Duplicate for a working container; Unarchive/Delete for an archived one)
  • 3-dot button on a container row is hidden by default and fades in on hover; clicking it opens the same menu
  • Archived/system/file containers do not show the 3-dot or right-click menu (disableInteraction respected)
  • Upload Bundle dialog: the OS file picker only lets you select .tar.gz, .gz, .tgz files
  • After picking a file, the filename replaces "No file chosen" in the dialog
  • Reopening the dialog resets the filename label
  • Copying a container succeeds (previously returned 405)
  • Triggering a warning toast (e.g. DotMessageDisplayService.push({ severity: DotMessageSeverity.WARNING, ... })) displays with the warn style rather than the default

🤖 Generated with Claude Code

…rity

Groups four related UI fixes surfaced during issue 35274 review.

Container list (dot-container-list):
- Add right-click context menu that reuses the existing row actions.
  The row now listens to (contextmenu) and shows a <p-contextMenu>
  attached via viewChild, matching the standard used by the template
  list portlet.
- Hide the per-row 3-dot action button by default and reveal it on
  row hover via Tailwind group/group-hover utilities. Replaces
  <dot-action-menu-button> with a <p-button pi-ellipsis-v> that opens
  the same context menu, giving one source of truth for row actions.
- Migrate the @ViewChild('actionsMenu') decorator to the signal-based
  viewChild<Menu>('actionsMenu') API. Mixing the legacy decorator with
  the new signal viewChild for the context menu caused @ViewChild to
  resolve to the wrong component at runtime (ContextMenu instead of
  Menu), breaking handleActionMenuOpen.
- Spec: add a window.matchMedia jsdom polyfill (required once
  PrimeNG ContextMenu is present) and update action-menu tests to
  assert against setContainerActions() directly instead of querying
  the removed <dot-action-menu-button>.

Bundle upload dialog (view_publish_tool.jsp):
- Replace the raw <input type=\"file\"> with a dojox.form.Uploader
  widget so the dialog matches the dijit look-and-feel of the rest
  of the portlet. Adds a localized file-name label that updates on
  selection.
- Work around two dojox.form.Uploader limitations observed in the
  custom Dojo build:
    1. The widget does not forward the 'accept' attribute to its
       internal <input type=\"file\">, so any file could be picked.
       applyBundleAcceptFilter() walks the widget's domNode and sets
       accept=\".tar.gz,.gz,.tgz\" on the actual file input. Rerun on
       every dialog open because uploader.reset() recreates the input.
    2. The widget only fires an onChange event — it does not update
       any visible element. updateBundleFileNameDisplay() is wired to
       onChange in dojo.ready() so the label reflects the picked file
       (or falls back to the localized 'No file chosen' string).
- Add spacing and an action-bar margin so the new controls sit cleanly
  in the dialog.

Toast severity (dot-message-display):
- PrimeNG Toast expects severity 'warn', but the DotMessageSeverity
  enum emits 'WARNING'. Map 'warning' → 'warn' before handing it to
  MessageService.add() so warning toasts pick up the warn style. SCSS
  class rename (.warning → .warn) keeps the component stylesheet in
  sync.

Container service (dot-containers.service):
- Switch copy() from PUT to POST. The backend _copy endpoint is a
  POST, and the previous PUT returned 405 on the first copy attempt.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions github-actions bot added Area : Backend PR changes Java/Maven backend code Area : Frontend PR changes Angular/TypeScript frontend code labels Apr 18, 2026
@claude
Copy link
Copy Markdown
Contributor

claude bot commented Apr 18, 2026

Claude finished @hmoreras's task in 2m 34s —— View job


PR Review

  • Gather context
  • Read changed files
  • Analyze for bugs, risks, design issues
  • Post review

The core fixes (PUT→POST for _copy, WARNINGwarn severity mapping, viewChild migration, context-menu wiring) are correct. A few things worth flagging:

1. innerHTML with user-supplied filename (JSP — view_publish_tool.jsp:259)

display.innerHTML = names.join(", ");

names is populated from f.name (File objects from the OS picker). A filename like <img src=x onerror=alert(1)>.tar.gz would execute JS in the admin portal — self-XSS, but a real browser security finding. Use textContent instead:

display.textContent = names.join(", ");

Fix this →

2. Dead form action attribute (JSP — line 421)

<form action="/DotAjaxDirector/com.dotcms.publisher.ajax.RemotePublishAjaxAction/cmd/uploadBundle" ...>

Upload now goes through fetch("/api/bundle/sync", ...). The form's action attribute is dead code — it's never submitted. Low risk since there's no submit button, but it's misleading and points to a legacy endpoint that may have different auth/validation semantics. Consider removing.

3. Redundant (contextmenu) on the last <td> (container-list.component.html:132)

<td style="width: 5%" (contextmenu)="setContextMenu($event, rowData)">

The <tr> already has (contextmenu)="setContextMenu($event, rowData)". Since setContextMenu calls event.stopPropagation(), the td handler fires first and stops propagation — net behavior is identical, but it's a confusing duplicate. Remove the (contextmenu) from the <td>.

4. Duplicate preventDefault/stopPropagation in setContextMenu (container-list.component.ts:184)

setContextMenu(event: Event, container: DotContainer): void {
    if (container.disableInteraction) {
        event.preventDefault();   // ← duplicated
        event.stopPropagation();  // ← duplicated
        return;
    }
    event.preventDefault();
    event.stopPropagation();
    ...
}

These two calls are unconditional — move them before the guard to eliminate the duplication.

5. contextMenuItems is not a signal (container-list.component.ts:92)

Reviewer raised this in the previous round; it was noted as non-breaking (no OnPush yet). Still unaddressed. Not a bug, but the migration is already 90% done (actionsMenu and tableRows are signals); leaving this one plain is inconsistent.

6. warning icon test may not render inside <p-toast> (dot-message-display.component.spec.ts:132)

The new test at line 132 asserts icon.name === 'warning'. <p-toast> renders content inside its overlay/template lazily. If PrimeNG's NoopAnimations environment doesn't flush the toast message into the DOM, By.css('dot-icon') would return the ERROR message icon from the previous test's dotMessageDisplayServiceMock.messages$ emission (the stream is shared). The test should ensure only the WARNING message is in flight when it asserts. Worth verifying the test actually catches a regression.

No issues with:

  • PUT→POST fix for _copy is correct
  • WARNINGwarn severity mapping and SCSS .warning.warn rename are consistent
  • viewChild signal migration for actionsMenu/tableRows is correct
  • applyBundleAcceptFilter / getBundleUploadFile Dojo workarounds are well-documented
  • window.matchMedia polyfill in spec is necessary and correct

…n keys

- Route right-click on actions column through setContextMenu() instead of
  swallowing the event, so right-clicking anywhere on a row opens the menu
- Add event.stopPropagation() to setContextMenu() disabled guard for parity
  with the active path
- Expand doBundleUpload() suffix check to accept .gz and .tgz (matching the
  accept filter already set on the Uploader widget)
- Add missing i18n keys: Choose-File and No-file-chosen in Language.properties

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace p-tag (theme SCSS / p-tag-success) with p-chip using the same
Tailwind utility classes (bg-green-100 / text-green-700) already used by
dot-contentlet-status-chip. Swap TagModule for ChipModule, drop the
tag-padding workaround, and update the spec selector accordingly.

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

- migrate @ViewChildren tableRows to viewChildren() signal
- split static/conditional [class] binding in container-list template
- update spec to use signal array access (tableRows()[0]) instead of QueryList.get()
- add template-level test for warn severity icon in dot-message-display
- trim JSDoc boilerplate on setContextMenu, keep only the non-obvious why
@hmoreras hmoreras disabled auto-merge April 20, 2026 16:54
@hmoreras hmoreras enabled auto-merge April 20, 2026 19:33
@hmoreras hmoreras added this pull request to the merge queue Apr 20, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Apr 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI: Safe To Rollback Area : Backend PR changes Java/Maven backend code Area : Frontend PR changes Angular/TypeScript frontend code

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants