Refactor dot-ai-config-detail component for improved UI and functionality

[KevinDavilaDotCMS]

• Updated HTML structure for better layout and styling, including adjustments to class names and element properties.
• Enhanced loading and error handling with new components for better user feedback.
• Improved accessibility and readability by modifying text classes and labels.
• Updated icons to use Material Symbols for consistency across the application.
• Refactored dynamic field and settings card components for better organization and usability.
• Adjusted constants for AI capabilities to use more descriptive icon names.

Proposed Changes

• change 1
• change 2

Checklist

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

Additional Info

** any additional useful context or info **

Screenshots

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

[github-actions]

🐳 PR Docker test image

Latest build for commit e66870c pushed to dotcms/dotcms-test:

docker pull dotcms/dotcms-test:pr-37451-36826-dotai-provider-configuration-ui-fix-ui-issues
docker pull dotcms/dotcms-test:pr-37451-36826-dotai-provider-configuration-ui-fix-ui-issues_e66870c

[oidacra]

This PR adds a ❌ NEVER: hand-rolled "card" — this is p-card reinvented example to STYLING_STANDARDS.md, and the markup in that example is this triplet:

rounded-md border border-surface
rounded-t-md bg-surface-50 px-5 py-4.5
border-t border-surface px-5 pt-4.5 pb-5

Both card components here use it character-for-character — this file at L1/L3/L21, and dot-ai-settings-card.component.html at L1/L2/L14. p-card appears in none of the five templates.

Is the intent that these two are the grandfathered exception, or should they move to p-card before the anti-pattern lands in the doc? As it stands the standard and the code contradict each other in the same commit, which will be confusing for the next person who reads either one.

[KevinDavilaDotCMS]

Keeping the markup and fixing the doc instead — but you're right that the two can't ship contradicting each other.

Two things I checked before deciding. p-card isn't an equivalent here: it's overridden in libs/ui/src/lib/theme/theme.config.ts to shadow: none, 1rem body padding and a plain border, with no header band — I'd still be hand-building the header bar inside it. The component this markup actually reproduces is p-panel, almost token for token (Lara: surface.50 header background, 0 0 1px 0 header border, 1.125rem padding, content.border.radius).

I did try the p-panel conversion and wasn't happy with the result, so the screen stays as it is. The fix is the doc: I removed the ❌ example (it was a photograph of this PR's own code) and replaced it with what p-panel and p-card each actually give you, so the next person picks with the facts in front of them. A container migration can be its own ticket.

[oidacra]

Does p-field-hint actually resolve here? Searching core-web/apps and core-web/libs, the only rule for it is:

/* core-web/apps/dotcms-ui/src/style.css:85 */
.form .p-field-hint { @apply text-sm text-gray-500; }

It is descendant-scoped to .form, and nothing in this component tree carries class="form" — the <form> in dot-ai-settings-card.component.html is class="flex w-full flex-col gap-5", and this component has no <form> at all. So the class matches no CSS and the hint loses text-sm text-gray-500, rendering at default size/color.

This looks like a change in the wrong direction: main has no p-field-hint anywhere in this tree, and the line this replaces was <small class="text-gray-500">, which did style correctly. Same in dot-ai-settings-card.component.html at L27 and L111.

Worth noting the deleted doc section warned about exactly this failure mode for p-error ("renders as plain inherited text"), and that p-field-* "stay scoped to .form".

Would reverting these three to <small class="text-sm text-gray-500"> work? The `block min-h-5" on this one and L111 also looks like the reserved-hint-slot pattern the new doc marks ❌.

[KevinDavilaDotCMS]

Confirmed and fixed — this was a real regression, thanks for catching it.

.form .p-field-hint is descendant-scoped (style.css:85) and nothing in this component tree carries .form, so all three hints were matching no CSS and losing text-sm text-gray-500. All three are back to <small class="text-sm text-gray-500">, and the min-h-5 went with them.

Your note about the deleted doc section was the right call too — the .form/.field section is restored as an explicit Legacy subsection so that scoping caveat stays written down. Details in the STYLING_STANDARDS thread.

[oidacra]

Was dropping the site indicator intentional? The old header had an @if (siteName()) block rendering apps.ai.config.site ("Site: {0}") right below the subtitle, and this PR removes both it and the apps.ai.config.site key from Language.properties.

Two things make me think it may be an oversight rather than a decision:

1. PR feat(dotAI): Implement new dotAI config page #37048 (the PR that shipped this screen) lists under Fixes from review: "the page now shows the site it's configuring" — so this was added deliberately in response to review feedback, five days ago.
2. The siteName computed at dot-ai-config-detail.component.ts:79 is left in place with no remaining consumer. Usage deleted, source retained.

It matters a little more than a lost label here because every read and write on this screen is scoped by siteId (.ts:111 and .ts:182), so someone configuring two sites in one session has no in-page confirmation of which site's provider credentials they are about to save.

Either restoring the line next to the new subtitle, or removing siteName plus its now-inaccurate doc comment, would close the loop — which did you intend?

[KevinDavilaDotCMS]

Dropping the site line was intentional in the header redesign; leaving siteName behind was not. Removed the computed and its doc comment — which literally read "just never surfaced in the redesigned page", i.e. dead code documenting its own deadness.

Your point about in-page confirmation of which site is being configured is fair, but I'd rather not re-add a line to a header I just restructured as part of a UI-fix pass. Tracking it separately.

[oidacra]

Could this standards rewrite be split into its own PR?

This section replaces the repo-wide .form / .field / p-field-hint / p-field-error / dotFieldRequired convention, which is a much wider decision than the rest of the PR (a UI-fix pass on one portlet). Current usage of what is being de-documented:

• dotFieldRequired — 43 files
• class="form" — 27 files
• class="field" — 95 files
• p-field-hint / p-field-error — 11 files

None are touched here. The new text does grandfather them ("do not extend... to new features"), so nothing breaks at runtime — but the convention those files follow ends up undocumented, and the section being removed was written on Sep 4 specifically because a reviewer could not tell which p- classes were ours versus PrimeNG's.

There is also a concrete cost visible in this same diff: the removed section is what documented that p-field-hint only works inside .form, and this PR adds three p-field-hint usages with no .form ancestor (see my comment on dot-ai-dynamic-field.component.html).

Not asking for a code change — just that this gets its own review from whoever owns the frontend standards, rather than riding along with an AI-config restyle. Alternatively, keeping the .form/.field section marked legacy alongside the new guidance would avoid the documentation gap.

[KevinDavilaDotCMS]

Not splitting the PR, but taking your alternative, which is the better one: the .form/.field section is back as an explicit Legacy subsection alongside the new guidance, so those ~100 files don't end up following an undocumented convention.

You were also right about the concrete cost — the caveat I deleted is exactly the one this PR then tripped over. Fixed, see the dot-ai-dynamic-field thread.

While writing the section back I verified something worth having on record: .p-error is dead in the built app. It's defined only in libs/dotcms-scss/angular/dotcms-theme/utils/_validation.scss, reachable only through dotcms-theme/theme.scss, and that import is commented out in libs/dotcms-scss/angular/styles.scss. Nothing else the app builds defines it. There are 15 live usages in templates — 13 of them in dot-experiments — whose validation errors currently render as plain inherited text rather than red. That's now documented, with <p-message severity="error" variant="simple"> as the replacement.

[oidacra]

This label carries text-sm font-semibold, which the Form Fields section added in this same PR rules out:

No text-* size class on body copy or labels. Labels and normal UI text inherit the PrimeNG default size/weight — do not add text-sm, text-base, font-medium, etc. to make a label "look right." The only text allowed text-sm is hint/error text under a field.

Four places in the diff look affected:

• this line, and dot-ai-additional-properties.component.html:2 — text-sm font-semibold text-gray-950 on a group label
• dot-ai-capability-card.component.html:12 and dot-ai-settings-card.component.html:8 — text-sm text-gray-600 on card description copy

The bare font-semibold on the card titles (L9 here) matches the doc's own ✅ example, so those look fine.

Should these drop the size classes, or is the new rule meant to apply only to labels attached to a form control rather than section headings? Either answer is reasonable — worth pinning down since the rule is landing in this PR.

[KevinDavilaDotCMS]

Good question to pin down — the rule is meant for labels attached to a form control, not section headings.

I scoped it that way in the doc and spelled out that section headings, group labels over a set of controls, and card/section descriptions are a separate typographic level (text-sm font-semibold for a group label, text-sm text-gray-600 for secondary copy). So the four places you list stay as they are and no longer contradict the standard.

[oidacra]

Is the gap-1 here deliberate? Every other field wrapper in this file uses gap-2 (L16, L33, L44, L92), and the Form Fields rule added in this PR says:

Label-to-control gap is gap-2. A field wrapper is flex flex-col gap-2 — label, then control, nothing wider.

Looks like flex w-1/2 flex-col gap-2 was intended for the imageSize field.

[KevinDavilaDotCMS]

Typo — fixed, it's gap-2 now.

[oidacra]

Dropping bg-white from the host — is a token-based background meant to replace it somewhere?

I traced the ancestor chain looking for one and could not find it: this is a top-level routed component rendered into the router-outlet inside main-legacy.component.html (L16-18), whose wrapper is class="overflow-auto" with no background, and no background-color is declared on html/body in the global stylesheets. bg-white is also removed from the scroll container (.html:1) and the footer (.html:36).

So the screen should still look right today, but only because of the browser's default canvas white rather than anything explicit. For contrast, the sibling route in the same outlet slot (DotAppsConfigurationDetailComponent) sets its own :host background for what looks like this exact reason.

Given the rest of the PR moves to theme tokens (border-surface, bg-surface-50), was bg-surface on the host the intent here?

[KevinDavilaDotCMS]

Right, and it was only surviving on the browser's default canvas: no background-color on html/body, and the wrapper this route renders into (main-legacy.component.html:16-18) sets none either. Added it explicitly on the host.

One correction to the suggestion though: bg-surface isn't a utility. tailwindcss-primeui exposes the scale bg-surface-0..950, and border-surface is a separate @utility for border color only. So it's bg-surface-0 — the tokenized equivalent of the bg-white that was there.

[oidacra]

This file is the largest change in the PR (108 added / 104 deleted — a full structural rewrite of the field wrappers, the advanced grid from 2 to 3 columns, and the checkbox row), and dot-ai-settings-card has no .spec.ts at all. Same for dot-ai-additional-properties, which also changed materially here.

The missing specs are pre-existing rather than something this PR removed, so this is not a regression — but it does mean the biggest chunk of the diff has no automated verification behind it, and the icon swap in dot-ai-additional-properties.component.html moves the add/remove buttons from an icon="pi pi-..." attribute to projected <span class="material-symbols-outlined"> content, which is a different PrimeNG rendering path.

Would you be open to adding a spec for at least the settings card as part of this PR — enough to cover the checkbox vs text/number field branches and that add/remove reach addProperty()/removeProperty()? Adding data-testid to those two buttons would be needed first; neither has one.

[KevinDavilaDotCMS]

Agreed it's pre-existing rather than a regression, and I'd rather not grow this PR further — opening a follow-up for specs on dot-ai-settings-card and dot-ai-additional-properties (the checkbox vs text/number branches, the add/remove wiring, and the data-testids that need adding first).

One thing from your note I did fix here, because it turned out to be a live bug rather than just a different rendering path: the icon swap broke the buttons. p-button derives p-button-icon-only from hasIcon (primeng-button.mjs:768), which counts the icon input or an #icon template — bare projected content satisfies neither. So the remove button had silently lost its square icon-only shape and picked up the padding of a labelled button. Both buttons now project through <ng-template #icon>.

[oidacra]

The new <dot-spinner /> and the <p-message> on L20 have no assertion anywhere, and no data-testid.

dot-ai-config-detail.component.spec.ts contains zero spectator.query / byTestId calls and never renders the template (it invokes ngOnInit() on the unrendered component), so it asserts the loading() / loadFailed() signal values but nothing about what those branches actually render. That is also why the existing specs still pass despite this rewrite — they are DOM-blind, not verified against the new markup.

ANGULAR_STANDARDS.md asks for data-testid on exactly these two branches:

@if ($loading()) {
  <dot-spinner data-testid="loading-indicator" />
} @else if ($error()) {
  <dot-error ... data-testid="error-message" />
}

There are currently 0 data-testid attributes in this template. Adding them to the spinner and the message, plus one rendering assertion per branch, would make the two new components' wiring actually verified — worth it since both were newly added to imports in this PR.

[KevinDavilaDotCMS]

Done — data-testid on both, plus one rendering assertion per branch.

Your read of the existing spec is right: it runs with detectChanges: false and never queries the DOM, which is why it stayed green through the rewrite. The two new tests render the component, so the loading and error branches are now checked against the markup that actually ships.

[oidacra]

The icon migration itself checks out — I traced CAPABILITY_META[].icon to its render sites and there is exactly one consumer, dot-ai-capability-card.component.html:5, which interpolates it as text content inside a material-symbols-outlined span. No consumer still binds it as a CSS class, so the bare-name format is correct and complete.

Two follow-ups though:

1. dot-ai-capability-card.component.spec.ts:26 still uses the fixture icon: 'pi pi-comments'. It compiles and the test passes, but that value now renders as the literal string "pi pi-comments" through a Material Symbols span — a state unreachable in production. Worth updating to 'forum'.
2. dot-ai-config.constants.spec.ts never asserts on CAPABILITY_META, only on isFieldAlwaysVisible and requiredUnlessValidator. A small assertion that each icon is non-empty and not pi -prefixed would have caught the drift in point 1 and would catch the next one.

Also worth a check: does DotAiCapabilityMeta.icon still have a type or doc comment describing a PrimeIcons class?

[KevinDavilaDotCMS]

Thanks for tracing it through.

1. Fixed — the fixture is 'forum'.
2. Deferring the CAPABILITY_META assertion to the follow-up test ticket, along with the specs from the settings-card thread.
3. Checked: DotAiCapabilityMeta.icon is a bare icon: string with no doc comment, so there's no stale PrimeIcons reference left in the type.