fix(dot-roles): UI feedback round 2 — dialog buttons, node icons, submit errors, parent picker

[hmoreras]

Part of #36930.

Proposed Changes

UI feedback round 2 on the Roles & Tools Beta portlet.

• Tint the role detail tab strip — the p-tablist was visually flush with the panel below it, so the tabs read as floating over the content instead of heading it. Adds border-b border-gray-200 bg-surface-50!.
• Even out the Grant button spacing — the button row sat tight against whatever rendered above it. Adds first:pt-4, so the padding applies only when the row is the first child (the inherited-users notice above it is conditional) and no double gap appears when it is not.
• Consistent Cancel buttons — the three confirm dialogs already agreed on a text-styled reject; the two form dialogs used [outlined]. Add and Edit now use [text] too, so every Cancel in the portlet matches the one in the Remove User dialog.
• Remove the role node icons — design dropped the folder-vs-shield_person distinction, so the icon is gone from both places that drew it: the tree node label and the detail header.
• Delete Role is no longer red — drops severity="danger". The confirm dialog it opens already carried a comment that it uses primary styling per UX guidance "even though the action is destructive"; the trigger button was the only red left.
• Save is never disabled by validation — in both Add and Edit, an invalid submit now calls markAllAsTouched() and names the problem in the footer instead of leaving a dead button with nothing to act on. The message sits left of the buttons, and to the right of Delete in the Edit dialog.
• "None (Top Level)" is pinned first in the parent picker — and selected by default, so choosing a root role is an explicit pick rather than only the absence of one.

Checklist

• Tests
• Translations
• Security Implications Contemplated (add notes if applicable)

Additional Info

Parent picker — where the entry is added, and why it matters. It is prepended in each component's $parentTree computed rather than inside #toTreeNodes, so it survives the two paths that rebuild the tree: the server search replaces the whole list, and the Edit dialog strips the edited role and its descendants to prevent cycles. It is a choice, not a candidate, so neither should touch it.

The sentinel must not reach the API. The entry's key is __root__, and toParentRoleId in the shared models maps it — and an empty picker — back to null. Both onSave paths went through it: the previous parent?.key ?? null would have sent the sentinel as a real parent id and been answered with a 404 "parent role not found". There is a test pinning that mapping in each dialog.

Default only where there is nothing else to show. Add defaults to the root entry when opened from New, and leaves the parent alone when the row's + prefilled one — clobbering it would silently create a root role instead of the child that was asked for, which has its own regression test. Edit defaults to it when the role is already a root, and keeps the current parent otherwise. Save behaviour is unchanged in every case: an empty picker and the root entry both mean parentRoleId: null.

Dead code removed with the icons. DotRolesStore.selectedRoleIsParent existed only to feed the header's $icon computed — its docstring said as much ("folder icon in the header") — so it went, along with the @let isLeaf in the tree template and three specs that mocked or exercised it. The header skeleton's shape="circle" placeholder went too: it existed to mirror an icon that no longer renders.

node.leaf is deliberately untouched. It drives PrimeNG's chevron-vs-leaf rendering and the childCount logic that has its own specs — it never fed the icon.

Save button scope. readOnly still disables Save in the Edit dialog. That is a permission gate for system/locked roles, not a validation one, and it keeps its explanatory tooltip. Only the form.invalid condition was removed.

Open question, not addressed here. With the root entry selected by default, [showClear] changes what the field displays but not what is saved — clearing and picking "None (Top Level)" are the same outcome. It is left in place; removing it (and the now barely-reachable roles.form.parent.placeholder) is a one-line follow-up if design wants a single route to root.

Translations. Two new keys — roles.form.error.required and roles.form.parent.placeholder — plus a text change to roles.form.parent.root ("None (top level)" → "None (Top Level)") so it can label the option.

Screenshots

Original	Updated
** original screenshot **	** updated screenshot **

This PR fixes: #36930

[claude]

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

---

Code Review

• Read the diff
• Review changed components & specs
• Post review feedback

Reviewed both commits (84d0d2a1 UI feedback + 1418a650 pinned root option) against origin/main. This is well-executed, well-commented UI work with matching test coverage. The dead-code removal (selectedRoleIsParent, $icon, node-icon-*, the skeleton circle) is complete — grep confirms no dangling references remain, and the sentinel/toParentRoleId mapping is correctly the single guard keeping __root__ out of the API.

New Issues

• 🟡 Medium: core-web/libs/portlets/dot-roles/src/lib/dot-roles-add/dot-roles-add.component.ts:112-118 (and dot-roles-edit.component.ts:147-153) — the pinned root option is rebuilt as a fresh object literal on every $parentTree() recompute, but the form control is seeded once in the constructor with the object instance from the initial evaluation. After anything that re-runs the computed (a search via onFilter, or roleTree() resolving async), the tree's root node is a different reference than the one held in form.controls.parent.

  Assumption: p-treeSelect in single-select mode matches the highlighted node by object reference (there's no dataKey/key-based equality configured here).

  What to verify: open the parent picker after typing a 3+ char search and clearing it — does the "None (Top Level)" row still render as selected? The trigger label stays correct either way (it reads off the stored object), so this is cosmetic-only: the dropdown may just fail to highlight the current selection. Note this mirrors the pre-existing prefilled-parent pattern, so it's consistent with existing code rather than a regression. If it does drop the highlight, seeding from a stable singleton root node (module-level or a cached field) rather than this.$parentTree()[0] would fix it. Fix this →

Everything else checks out:

• Save-enabled + markAllAsTouched() + footer error — correct in both dialogs. onSave still guards $submitting() (no double-submit) and, in Edit, still gates on readOnly as a permission check with its tooltip preserved. $error is set on the invalid path and cleared on the valid path, matching the two new specs.
• toParentRoleId — handles empty picker, sentinel, and real ids correctly; return typing is sound. Both onSave paths route through it, and specs pin the null-not-sentinel mapping in Add and Edit.
• Translations — roles.form.error.required and roles.form.parent.placeholder added to Language.properties and both consumed; roles.form.parent.root relabel is consistent across properties and both specs.
• Styling changes (tab strip tint, first:pt-4, [text] cancels, dropped severity="danger", ml-auto button group) are self-consistent with the stated design intent.

No blocking issues.

_{• issue-36930-ui-feedback-round-2}