refactor(ui): redesign the shared key/value field across its three consumers (#37191) by adrianjm-dotCMS · Pull Request #37284 · dotCMS/core

adrianjm-dotCMS · 2026-08-28T19:09:20Z

Proposed Changes

Reworks the one shared Key/Value editor in libs/ui to the new design and rolls it out to all three
of its consumers — the Edit Content field, the Content Type Field Variables tab, and the
Apps custom-properties panel — so they finally look and behave the same.

Built from stock PrimeNG 21. No new component enters the workspace; the net component count for
this feature is zero.

What changed for the user

Values render as plain text at rest and become an input when activated — Enter commits, Escape restores.
Row actions (drag handle, remove) are revealed on hover instead of competing for attention on every row.
Hidden values move from a toggle switch in its own column to an eye inside the value field. It is a button in both states, so hiding a value is reversible — previously it was a one-way trip.
The hidden indicator is never hover-gated: which values are secret has to be readable at a glance.
Empty state gains an icon and reuses the existing copy, which already matched the design.
Reordering is offered in all three consumers rather than only one.

Three findings worth a reviewer's attention

The missing table styling was a selector problem, not a CSS one. PrimeNG themes its table with direct-child combinators (.p-datatable-tbody > tr > td). The row components used element selectors with display: contents, so the component element sat between the tbody and the tr and no table style from the theme ever applied — no borders, no header band. They now attach by attribute selector (tr[dotKeyValueTableRow]), making the host the tr itself.
onRowReorder must not re-apply the move. PrimeNG's onRowDrop already calls reorderArray on the array bound to [value]. Recomputing from dragIndex/dropIndex applies it twice. The handler only re-publishes the array with a fresh reference so the signal notifies.
Hover-hidden actions use opacity-0, never display:none or an @if. Those remove the control from the tab order and strand keyboard and touch users with no visible symptom for anyone on a mouse. A test asserts the mechanism, not merely that the icon is invisible.

Removed along the way

DotKeyValueUtil — 87 lines nothing imported.
A stale [autoFocus] binding in the Apps template, plus the spec stub that made it look real. It compiled only because strictTemplates is off in that app.
A duplicated copy of the row template that the tests were asserting against instead of the real one.
The dragAndDrop input, once reordering became universal and it had nothing left to gate.
Both SCSS files — styling is Tailwind against PrimeNG theme tokens, so no colour is hardcoded.

Checklist

Tests
Translations — none needed; the design's copy already existed as keyValue.value_no_rows.label
Security Implications Contemplated — value masking is a UI concern only, not a security boundary. It keeps a secret off a shoulder-surfer's screen; the underlying value is transmitted exactly as before. Stated in the spec so nobody mistakes it for protection.

Additional Info

Tests: 61 in libs/ui (from 31), 24 in edit-content, 70 in dotcms-ui, plus Playwright smokes for each consumer. Lint and build clean.

Not yet run: the Playwright specs have a clean typecheck but have not been executed, and the manual keyboard-only pass is still pending. Flagging it rather than implying green.

Scope: frontend only. Order persistence for Field Variables and Apps was scoped out of this issue — it would mean database, REST and encrypted-storage work.

spec.md, data-model.md and contracts/ are included as the reviewed contract; the Spec-Kit process artifacts are gitignored by design.

Screenshots

Original	Updated
original screenshot	updated screenshot

🤖 Generated with Claude Code

This PR fixes: #37191

…nsumers (#37191) Rework the one shared Key/Value editor in `libs/ui` to the new design and roll it out to all three of its consumers: the Edit Content field, the Content Type Field Variables tab, and the Apps custom-properties panel. The redesign is assembled from stock PrimeNG 21 — no new component enters the workspace, and the net component count for the feature is zero. What changed for the user: - Values render as plain text at rest and become an input when activated; Enter commits, Escape restores. - Row actions (drag handle, remove) are revealed on hover instead of competing for attention on every row. - Hidden values move from a toggle switch in its own column to an eye inside the value field. It is a button in both states, so hiding a value is reversible. - The hidden indicator is never hover-gated: which values are secret has to be readable at a glance. - Empty state gains an icon and reuses the existing `keyValue.value_no_rows.label` copy, which already matched the design. - Reordering is offered in all three consumers rather than only one. Implementation notes worth knowing: - The row components attach by attribute selector (`tr[dotKeyValueTableRow]`), so their host IS the `tr`. PrimeNG themes its table with direct-child combinators (`.p-datatable-tbody > tr > td`), and an element wrapper between the tbody and the tr silently defeats every table style the theme provides. This is why the previous markup rendered with no borders or header band. - `onRowReorder` must not re-apply the move: PrimeNG's `onRowDrop` already calls `reorderArray` on the array bound to `[value]`. The handler only re-publishes it with a fresh reference so the signal notifies. - Hover-hidden actions use `opacity-0`, never `display:none` or an `@if` — those remove the control from the tab order and strand keyboard and touch users. A test asserts the mechanism, not merely that the icon is invisible. Also removed along the way: `DotKeyValueUtil` (87 lines nothing imported), a stale `[autoFocus]` binding in the Apps template together with the spec stub that made it look real, a duplicated copy of the row template that tests were asserting against instead of the real one, the now-unused `dragAndDrop` input, and both SCSS files. Icons follow the repo standard (Material Symbols); PrimeIcons are gone from this component's authored markup. No new i18n key was needed. Tests: 61 in `libs/ui` (from 31), 24 in `edit-content`, 70 in `dotcms-ui`, plus Playwright smokes for each consumer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

claude · 2026-08-28T19:10:44Z

Claude finished @adrianjm-dotCMS's task in 8m 0s —— View job

Code review — key/value field redesign (#37191)

Review shared editor (dot-key-value-ng + row/header components)
Review key-order recovery util + edit-content wiring
Review Field Variables deferred-save logic
Review Apps + paste util
Post findings

Re-reviewed at the current commit (35c580a4). The prior-round findings are all addressed in code; verified below. One non-blocking behavioral inconsistency remains.

New Issues

🟡 Reorder is offered where it can't be saved, and Save silently loses it — dot-content-type-fields-variables.component.ts:88 / :163

The shared editor renders a drag handle for every non-read-only consumer, so Field Variables (and the Apps panel) get row reordering. $hasChanges compares by ordered [key, value] JSON, so a pure reorder flips it to true and enables the dialog's Save. But saveChanges() only sends removed + written (changed values); a reorder produces neither, so it hits the if (!removed.length && !written.length) early return and save.emit()s without any server call. The dialog closes as if saved, and the order reverts on reload.

Assumption: field-variable order is not persisted server-side — confirmed: dot-field-variables.service.ts exposes only load/save/delete, no reorder/order endpoint.
What to verify: whether reordering should be offered at all for Field Variables/Apps (i.e. pass readOnly for the handle, or exclude reorder-only diffs from $hasChanges), so Save doesn't promise a change it discards. In Edit Content, where order is carried as JSON text, this works correctly.

Existing

None open.

Resolved (prior-round findings, confirmed fixed)

Read-only lists were reorderable — rows now bind [pReorderableRowDisabled]="readOnly", the handle isn't rendered, and onRowReorder() early-returns on read-only (dot-key-value-ng.component.ts:247).
Paste bypassed the blank-value rule — parseKeyValueBlock now skips KEY= / KEY=" " via if (!value.trim()) (dot-key-value-paste.util.ts:76).
Recovered key order trusted without matching the source — orderedKeyValueText now requires both equal count and key membership before using the recovered order, else returns null (key-value-order.util.ts:166).
Field-variable add recorded the request, not the response — partial-failure retry could DELETE .../id/undefined; now records { ...variable, ...saved } and #storedAfter reads landed pairs from succeeded via a Map (dot-content-type-fields-variables.component.ts:213,:257).
$-prefixed outputs — changeControls/save renamed (outputs aren't signals); consumer binding in dot-edit-field-dialog.component.html updated to match, and its spec updated. No stale references remain (other settings panels legitimately keep their own $-named outputs).
In-place edit closed silently on an invalid/blank commit — commitEdit now keeps the input open and dirty on invalid; key and value share the entry row's required/duplicatedKey validators and messages (dot-key-value-table-row.component.ts:157).
Hardcoded English labels — delete/add now use keyValue.action.*; all referenced keyValue.* keys exist in Language.properties.
Deleted dot-key-value-util.ts and the .scss have no remaining references.

…coverage This update refines the key/value field component across various consumers, ensuring consistent behavior and improved user experience. Key changes include: - Implemented a new method for closing the field dialog using the Escape key, enhancing accessibility. - Updated tests to utilize unique keys for entries, preventing collisions and ensuring reliable test outcomes. - Enhanced the visibility control for hidden values, making it clearer when values are masked. - Introduced pagination for long lists in the key/value component, allowing users to load more entries as needed. These changes aim to streamline interactions and maintain data integrity across the application.

erickgonzalez

Spec looks good! Let's continue with the implementation.

Recovering a Key/Value field's key order picked its target by shape — "a plain object whose values are all primitives". A binary field's `metaData` is exactly that, so it was rewritten into JSON text, `contentlet.metaData.name` stopped being readable, and the file preview came back empty after a reload (`binary-field.spec.ts`, CI). `getContentById` serves every field type, so a guess there reaches all five of its callers. The decision moves to the one place that knows a field's type: the KEY_VALUE entry in the form's resolution map. `getContentById` now hands back the contentlet untouched and parks the order-preserving parse alongside it, under a key of its own; the resolver reads its own variable out of that and nothing else can be mistaken for it. Also fixes two E2E specs that only passed on a machine seeded a particular way: - Apps custom properties drove SSO — SAML, which requires nine parameters including certificates. Save is bound to the whole form's validity, so it is disabled on a fresh instance. Now uses an app that requires one string, filled by the spec, and waits for the navigation a save triggers instead of racing it. - The field dialog was closed with Escape, which depends on where focus is. Uses the dialog's own close button, which is always present.

The Field Variables persistence check closed the dialog to reopen it. In CI the close button was found, visible and stable, but every click was intercepted by a modal mask sitting above it, so the test spent its whole timeout retrying. Reloading the builder gets at what the test is actually for — that each row was written against the field rather than held in the dialog's state — and does it more strictly, since nothing at all survives a reload. It also has no modal in its way. `closeFieldDialog` had no other caller and could not be relied on, so it goes rather than sitting there for someone to reach for.

The previous commit meant to reload but called `goToBuilder`, which navigates to the URL already in the address bar. That is a same-document fragment navigation: nothing is torn down, so the field dialog stayed open and its header intercepted the click meant for the field underneath. `reloadBuilder` does a real reload and waits for the builder to come back. With the local instance up again this reproduced the CI failure exactly, and the four specs now pass twice over.

The Field Variables persistence check reloaded straight after deleting a row. On screen the row was already gone — the shared editor drops it from its own list on click, while the consumer only commits once the request returns — so `expectKeyAbsent` passed while the DELETE was still in flight. The reload overtook it and the row came back, failing in CI and passing here. Each mutation now waits for the response from the per-row endpoint and asserts it succeeded, so the reload can only happen after the write has landed. Verified by holding that endpoint for 3s locally, which reproduces CI: without the wait the row is back after the reload, with it the delete survives.

This update introduces a new method for handling pasted blocks of key/value pairs in the DotKeyValue component. The `saveVariables` method allows users to paste multiple entries at once, maintaining their order and emitting individual saves for each pair. Additionally, the component now includes a `handleKeyInputPaste` method to parse and process pasted `.env`-style blocks, ensuring that only valid pairs are accepted and preventing duplicates from being added. Tests have been added to verify the correct behavior of the new paste functionality, including handling ordinary pastes and ensuring that existing keys are not overwritten. This enhancement improves the user experience by streamlining the input process for key/value pairs.

…ble key Five things asked for while reviewing the redesigned Key/Value editor, plus the spacing pass on the table. - Pasting `KEY=VALUE` text into the key input creates one pair per line, in the order pasted. Comments, blank lines, `export` prefixes and quoted values are handled; text with no assignment in it pastes normally. Multi-line values stay out of scope, and the parser says so. - Clear All empties the field behind the host's own confirmation dialog, styled like the unsaved-changes prompt rather than a dialog of the editor's own. It is not offered when there is nothing to clear. - A field the content type marks `readOnly` renders with nothing that could change it. Nothing new decides this: the declaration, the disabled form control and `setDisabledState` were all already there, and the editor was simply ignoring them. A file asset's `metaData`, regenerated on every save, is the case that prompted it. Reading affordances stay. - A pair's key is editable in place, like its value. This reverses FR-008, on request. A rename refuses an empty key or one another pair holds, and carries the pair's value across. - Field Variables holds its edits until Save instead of writing each keystroke, so Cancel means something. The dialog footer is supplied the way the Settings tabs already supply theirs, Save is disabled until something differs, and only what changed is written. The table now uses a fixed layout: a long key used to widen its own column and squeeze the value's (measured 366/250 against 308/308). Column gutters are even at 28px, and the key cell truncates with its full text in `title`. Spec, contract and data model are updated, including the two decisions this reverses.

Renaming a field stopped saving. The dialog has one Save button, and the Field Variables tab hands its own over through `changesDialogActions` — but it did so from `initTableData`, which runs when the dialog opens, whatever tab is showing. The Overview Save was replaced before it was ever used, so accepting the dialog ran the variables save instead of the field's and no `PUT .../fields` was sent (`content-type-fields.spec.ts`, CI). The tab now hands its buttons over only while it is the one on screen. Missed locally because the dialog's own spec stubs the variables component, so nothing there emits. The regression test drives the real component with the tab hidden, and the flag is set before the first change-detection cycle — a later `detectChanges` does not update the binding. Also moves `DotDialogActions` back to its models import in that spec.

…ent duplicate operations This update addresses a regression where partial failures during save operations could lead to incorrect retries for already processed writes. The component now tracks each operation individually, ensuring that only outstanding actions are retried after a failure. Additionally, tests have been added to verify that successful adds are not retried unnecessarily. This change enhances the reliability of the save functionality in the DotContentTypeFieldsVariablesComponent.

…rows Three defects, two of them raised in review and confirmed against the running instance before being fixed. Clear All did nothing in Field Variables or Apps. The editor asked `ConfirmationService` for a confirmation, but the two hosts do not agree on where a dialog lives: Edit Content's layout renders one unconditionally, while dotcms-ui's only exists while `DotAlertConfirmService` holds a model. With no dialog listening, the request went nowhere and the button appeared inert. The editor now carries its own `p-confirmDialog`, keyed so it answers only its own Clear All and Edit Content's dialog keeps ignoring it. Revealing rows no longer survives only until the next edit. `$visibleCount` was a `linkedSignal` sourced from `$variables`, and Field Variables and Apps hand back a fresh array on every change, so the table collapsed from 45 rows to 40 on a single delete. It is the editor's own state now; a field opened afresh still starts at page one because each consumer builds a new editor for it. FR-041 said the reset was required — it described the defect, and is corrected in place. A partial failure when saving Field Variables could wedge the tab. Writes ran in one `forkJoin`, so a single error left the recorded state untouched even though some deletes had already gone through; the next Save re-issued a DELETE for a variable that was gone, which the endpoint answers with 404 (checked against the API). Each operation is tracked on its own now and the recorded state advances with whatever landed, so a retry sends only what is still outstanding — including not re-sending an add that already succeeded. The Clear All spec drives all three consumers, because the first defect was invisible to a unit test: only mounting each host shows it.

Copilot

🟡 Changes recommended

Moderate persistence, metadata-collision, paste-validation, and keyboard-accessibility issues remain unresolved.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Redesigns the shared key/value editor across Edit Content, Field Variables, and Apps.

Changes:

Adds inline editing, reordering, paging, paste, clear-all, and masking.
Preserves key ordering and adds deferred Field Variables persistence.
Expands unit and E2E coverage.

File summaries

File	Description
`specs/37191-key-value-field-redesign/data-model.md`	Documents the pair data model.
`specs/37191-key-value-field-redesign/contracts/dot-key-value-ng.component.md`	Defines the component contract.
`dotCMS/src/main/webapp/WEB-INF/messages/Language.properties`	Adds paging and clear-all messages.
`core-web/libs/ui/src/lib/components/dot-key-value-ng/util/dot-key-value-util.ts`	Removes an unused utility.
`core-web/libs/ui/src/lib/components/dot-key-value-ng/dot-key-value-table-row/dot-key-value-table-row.component.ts`	Implements editable table rows.
`core-web/libs/ui/src/lib/components/dot-key-value-ng/dot-key-value-table-row/dot-key-value-table-row.component.spec.ts`	Tests row behavior.
`core-web/libs/ui/src/lib/components/dot-key-value-ng/dot-key-value-table-row/dot-key-value-table-row.component.html`	Renders redesigned rows.
`core-web/libs/ui/src/lib/components/dot-key-value-ng/dot-key-value-table-header-row/dot-key-value-table-header-row.component.ts`	Implements entry and paste handling.
`core-web/libs/ui/src/lib/components/dot-key-value-ng/dot-key-value-table-header-row/dot-key-value-table-header-row.component.spec.ts`	Tests entry-row behavior.
`core-web/libs/ui/src/lib/components/dot-key-value-ng/dot-key-value-table-header-row/dot-key-value-table-header-row.component.html`	Renders entry controls.
`core-web/libs/ui/src/lib/components/dot-key-value-ng/dot-key-value-paste.util.ts`	Parses pasted assignments.
`core-web/libs/ui/src/lib/components/dot-key-value-ng/dot-key-value-paste.util.spec.ts`	Tests assignment parsing.
`core-web/libs/ui/src/lib/components/dot-key-value-ng/dot-key-value-ng.component.ts`	Coordinates editor state and actions.
`core-web/libs/ui/src/lib/components/dot-key-value-ng/dot-key-value-ng.component.scss`	Removes obsolete styling.
`core-web/libs/ui/src/lib/components/dot-key-value-ng/dot-key-value-ng.component.html`	Builds the redesigned editor table.
`core-web/libs/edit-content/src/lib/utils/key-value-order.util.ts`	Preserves numeric-key ordering.
`core-web/libs/edit-content/src/lib/utils/key-value-order.util.spec.ts`	Tests order recovery.
`core-web/libs/edit-content/src/lib/services/dot-edit-content.service.ts`	Parses raw content responses.
`core-web/libs/edit-content/src/lib/fields/dot-edit-content-key-value/dot-edit-content-key-value.component.spec.ts`	Updates consumer assertions.
`core-web/libs/edit-content/src/lib/fields/dot-edit-content-key-value/components/key-value-field/key-value-field.component.ts`	Serializes ordered JSON text.
`core-web/libs/edit-content/src/lib/fields/dot-edit-content-key-value/components/key-value-field/key-value-field.component.spec.ts`	Tests content-field round trips.
`core-web/libs/edit-content/src/lib/fields/dot-edit-content-key-value/components/key-value-field/key-value-field.component.html`	Connects read-only state.
`core-web/libs/edit-content/src/lib/components/dot-edit-content-side-panel/dot-edit-content-side-panel.component.html`	Adjusts panel title sizing.
`core-web/libs/edit-content/src/lib/components/dot-edit-content-form/dot-edit-content-form-resolutions.ts`	Resolves ordered key/value fields.
`core-web/libs/edit-content/src/lib/components/dot-edit-content-form/dot-edit-content-form-resolutions.spec.ts`	Tests ordered resolution.
`core-web/apps/dotcms-ui/src/app/portlets/shared/dot-content-types-edit/components/fields/dot-edit-field-dialog/dot-edit-field-dialog.component.ts`	Keeps Variables footer actions visible.
`core-web/apps/dotcms-ui/src/app/portlets/shared/dot-content-types-edit/components/fields/dot-edit-field-dialog/dot-edit-field-dialog.component.spec.ts`	Tests dialog actions.
`core-web/apps/dotcms-ui/src/app/portlets/shared/dot-content-types-edit/components/fields/dot-edit-field-dialog/dot-edit-field-dialog.component.html`	Wires Variables save actions.
`core-web/apps/dotcms-ui/src/app/portlets/shared/dot-content-types-edit/components/fields/dot-content-type-fields-variables/dot-content-type-fields-variables.component.ts`	Defers and batches variable persistence.
`core-web/apps/dotcms-ui/src/app/portlets/shared/dot-content-types-edit/components/fields/dot-content-type-fields-variables/dot-content-type-fields-variables.component.spec.ts`	Tests deferred persistence.
`core-web/apps/dotcms-ui/src/app/portlets/shared/dot-content-types-edit/components/fields/dot-content-type-fields-variables/dot-content-type-fields-variables.component.html`	Uses whole-list updates.
`core-web/apps/dotcms-ui/src/app/portlets/dot-apps/components/dot-apps-configuration-detail/dot-apps-configuration-detail.component.spec.ts`	Tests custom-property integration.
`core-web/apps/dotcms-ui/src/app/portlets/dot-apps/components/dot-apps-configuration-detail/dot-apps-configuration-detail.component.html`	Removes a stale input binding.
`core-web/apps/dotcms-ui-e2e/src/tests/edit-content/fields/key-value-field/key-value-paging.spec.ts`	Covers paging behavior.
`core-web/apps/dotcms-ui-e2e/src/tests/edit-content/fields/key-value-field/key-value-field.spec.ts`	Covers cancellation and persistence.
`core-web/apps/dotcms-ui-e2e/src/tests/edit-content/fields/key-value-field/key-edit.spec.ts`	Covers key renaming.
`core-web/apps/dotcms-ui-e2e/src/tests/edit-content/fields/key-value-field/helpers/key-value-field.ts`	Extends the shared E2E helper.
`core-web/apps/dotcms-ui-e2e/src/tests/edit-content/fields/key-value-field/clear-all.spec.ts`	Covers clear-all behavior.
`core-web/apps/dotcms-ui-e2e/src/tests/content-types/field-variables.spec.ts`	Covers Field Variables persistence.
`core-web/apps/dotcms-ui-e2e/src/tests/apps/custom-properties.spec.ts`	Covers Apps custom properties.
`core-web/apps/dotcms-ui-e2e/src/pages/contentTypeBuilder.page.ts`	Adds Field Variables navigation helpers.

Review details

Suppressed comments (7)

core-web/libs/edit-content/src/lib/services/dot-edit-content.service.ts:80

getContentById now owns raw-text parsing and metadata attachment, but its service specs still only assert the URL and never flush a text response. Add a service-level test that flushes raw JSON and verifies both the normal entity and preserved numeric-key order; otherwise this integration can break while the isolated utility tests remain green.

        return this.#http
            .get(`/api/v1/content/${id}`, { params: httpParams, responseType: 'text' })
            .pipe(
                map((raw) => {
                    const { entity } = JSON.parse(raw) as DotCMSAPIResponse<DotCMSContentlet>;

core-web/libs/ui/src/lib/components/dot-key-value-ng/dot-key-value-ng.component.scss:1

Removing this stylesheet leaves an updated component without the required .ts/.html/.scss three-file structure or a styleUrls reference. Repository standards require all updated components to retain a separate stylesheet (docs/frontend/ANGULAR_STANDARDS.md:179-207), even when most styling is utility-based.
core-web/libs/ui/src/lib/components/dot-key-value-ng/dot-key-value-ng.component.ts:202
This comment says the component brings no dialog and uses a host-owned one, but the template now declares its own keyed p-confirmDialog and the CLEAR_ALL_KEY documentation explains why. Update this method contract to match the actual component-local confirmation flow.

     * Confirmed through `ConfirmationService`, so the dialog each host already renders
     * is the one that appears — this component brings none of its own. Its options
     * match the unsaved-changes prompt in Edit Content: a header and no icons. The
     * reject is a plain text button, as `dot-tags-list` does.

core-web/libs/ui/src/lib/components/dot-key-value-ng/dot-key-value-table-row/dot-key-value-table-row.component.html:44

Because this span exposes role="button", it must activate with Space as well as Enter. Currently a keyboard user pressing Space only scrolls the page instead of entering key-edit mode; add a keydown.space handler that starts editing and prevents the default scroll.
specs/37191-key-value-field-redesign/contracts/dot-key-value-ng.component.md:55
This rationale is stale after the deferred-save change: Field Variables now binds only updatedList and no longer consumes save/update/delete per row. Update the contract and matching component comments to explain why these legacy outputs remain public without claiming consumer 2 still depends on them.
specs/37191-key-value-field-redesign/contracts/dot-key-value-ng.component.md:164
This section says no translation key is required and later says only one is added, but Language.properties adds six keys: Load more plus five Clear All strings. List the actual additions so the contract matches the localization surface.
specs/37191-key-value-field-redesign/spec.md:364
FR-053 calls reordering a read-only reading affordance, but reordering mutates and emits the field value; this conflicts with FR-051 above and SC-011, which require no way to change a read-only field. Resolve whether read-only rows are reorderable, then align the implementation and contract with that single decision.

Files reviewed: 43/43 changed files
Comments generated: 7
Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

…s reordering (#37191) Addresses the review on #37284. An in-place edit used to refuse silently: an emptied value was saved anyway, an emptied key and a rename onto a taken key closed the input and discarded what was typed. They now share one rule — an invalid edit keeps its input open and shows the entry row's message for the same refusal. Blank is refused on every path as a result, the pasted `KEY=` included. A value commits exactly as typed; trimming only decides whether it counts as blank. Read-only no longer reorders: the row is bound `pReorderableRowDisabled`, the handle is not rendered, and `onRowReorder()` returns early. The gutter cell stays, since the header renders its column unconditionally. Field Variables records the save response rather than the request, so the id the server assigns a new pair reaches the stored snapshot — without it, removing that pair after a partial failure went out as `.../variables/id/undefined`. The drag handle drops `role` and `tabindex`: PrimeNG's reorder is pointer-only, so it was a focus stop announcing a button that could not be operated. Space now activates the editable spans. Three labels reached the message pipe as English literals, which `DotMessageService` echoes rather than failing on; they are bundle keys now. Also: `DotKeyValueFieldValue` names the union the control actually carries, and the spec, data model and contract are corrected where they described the reverse of what was built (key immutability, the host-owned dialog, keyboard reordering, read-only reordering). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

This update introduces new tests and functionality for the key/value editor, ensuring that edits are properly managed. Users can now abandon edits by clicking away, which restores the original text, while clicking the delete button during an edit removes the entry immediately. Additionally, UI adjustments have been made to maintain consistent alignment and spacing across the table rows and headers. The gutter column remains visible, and validation messages are displayed correctly when edits are refused. These changes improve the overall user experience and adhere to the specified requirements for the key/value field redesign.

…r consistency This commit refactors the key/value components by replacing the `$` prefix with standard input and output bindings. The changes enhance code readability and maintain consistency across the components. Additionally, tests have been updated to reflect these changes, ensuring that the functionality remains intact. This update contributes to a cleaner and more maintainable codebase.

This commit refines the alignment behavior of cells in the DotKeyValueTableRow component. The changes clarify the rationale behind cell alignment during editing and when cells are open, ensuring consistent visual presentation. Additionally, the related tests have been updated to reflect these adjustments, confirming that the alignment remains stable during various states of cell interaction. This enhances the overall user experience and maintains the integrity of the component's functionality.

… tests This commit modifies the event handler names in the DotEditFieldDialogComponent tests to remove the `$` prefix, aligning with recent refactoring for consistency. Additionally, type annotations have been added to improve code clarity in the key-value field tests. These changes enhance the maintainability of the test suite and ensure it accurately reflects the current implementation.

github-project-automation Bot added this to dotCMS - Product Planning Aug 28, 2026

github-actions Bot mentioned this pull request Aug 28, 2026

Refactor shared Key/Value field to new design across edit content, field variables, and apps portlet #37191

Open

18 tasks

github-actions Bot added the Area : Frontend PR changes Angular/TypeScript frontend code label Aug 28, 2026

erickgonzalez previously approved these changes Aug 31, 2026

View reviewed changes

adrianjm-dotCMS dismissed erickgonzalez’s stale review via a1ae23e September 1, 2026 03:11

adrianjm-dotCMS added 3 commits September 1, 2026 09:08

claude Bot added the AI: Safe To Rollback label Sep 1, 2026

adrianjm-dotCMS added 2 commits September 2, 2026 10:58

github-actions Bot removed the AI: Safe To Rollback label Sep 2, 2026

claude Bot added the AI: Safe To Rollback label Sep 2, 2026

github-actions Bot removed the AI: Safe To Rollback label Sep 2, 2026

claude Bot added the AI: Safe To Rollback label Sep 2, 2026

github-actions Bot removed the AI: Safe To Rollback label Sep 3, 2026

adrianjm-dotCMS marked this pull request as ready for review September 3, 2026 14:15

claude Bot added the AI: Safe To Rollback label Sep 3, 2026

nicobytes requested a balanced review from Copilot September 3, 2026 20:11

Copilot started reviewing on behalf of nicobytes September 3, 2026 20:11 View session

Copilot AI reviewed Sep 3, 2026

View reviewed changes

oidacra reviewed Sep 3, 2026

View reviewed changes

Comment thread ...b/components/dot-key-value-ng/dot-key-value-table-row/dot-key-value-table-row.component.html Outdated

oidacra reviewed Sep 3, 2026

View reviewed changes

Comment thread core-web/libs/ui/src/lib/components/dot-key-value-ng/dot-key-value-ng.component.ts

oidacra reviewed Sep 3, 2026

View reviewed changes

Comment thread ...key-value-ng/dot-key-value-table-header-row/dot-key-value-table-header-row.component.spec.ts

oidacra reviewed Sep 3, 2026

View reviewed changes

Comment thread ...omponents/dot-key-value-ng/dot-key-value-table-row/dot-key-value-table-row.component.spec.ts

oidacra reviewed Sep 3, 2026

View reviewed changes

Comment thread ...t-types-edit/components/fields/dot-edit-field-dialog/dot-edit-field-dialog.component.spec.ts

oidacra reviewed Sep 3, 2026

View reviewed changes

Comment thread core-web/libs/edit-content/src/lib/utils/key-value-order.util.spec.ts

github-actions Bot removed the AI: Safe To Rollback label Sep 4, 2026

claude Bot added the AI: Safe To Rollback label Sep 4, 2026

oidacra previously approved these changes Sep 4, 2026

View reviewed changes