Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
<div class="collection" data-testid="resource-collection-field">
@for (entry of entries(); track $index; let index = $index) {
@let expanded = expandedIndex() === index;
<div
class="card"
[class.card--expanded]="expanded"
[attr.data-testid]="'resource-collection-item-' + index"
>
<button
type="button"
class="card__header"
[attr.aria-expanded]="expanded"
[attr.data-testid]="'resource-collection-item-' + index + '-toggle'"
(click)="toggle(index, $event)"
>
<ui5-icon
class="card__chevron"
[name]="expanded ? 'navigation-down-arrow' : 'navigation-right-arrow'"
/>
<span class="card__preview">{{ previewFor(entry, index) }}</span>
</button>

@if (expanded) {
<div class="card__body" (click)="$event.stopPropagation()">
@for (row of rows(); track row.field.property) {
<div class="row">
<span class="row__label">{{ row.label }}:</span>
<mfp-resource-field
[fieldDefinition]="row.field"
[resource]="entry"
/>
</div>
}
</div>
}
</div>
}
</div>
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
:host {
display: block;
width: 100%;
}

.collection {
display: flex;
flex-direction: column;
gap: 0.5rem;
width: 100%;
padding-top: 0.25rem;
}

.card {
border: 1px solid var(--sapGroup_ContentBorderColor, #d9d9d9);
border-radius: 0.5rem;
background: var(--sapGroup_TitleBackground, #fff);
overflow: hidden;

&--expanded {
box-shadow: 0 1px 2px rgba(0, 0, 0, 0.04);
}

&__header {
display: flex;
align-items: center;
gap: 0.5rem;
width: 100%;
padding: 0 0.75rem;
background: transparent;
border: 0;
text-align: left;
cursor: pointer;
font: inherit;
min-height: 1.5rem;
color: var(--sapTextColor, #1d2d3e);

&:hover {
background: var(--sapList_Hover_Background, rgba(0, 0, 0, 0.03));
}
}

&__chevron {
flex: 0 0 auto;
color: var(--sapContent_IconColor, #6a6d70);
}

&__preview {
font-weight: 500;
color: var(--sapTextColor, #1d2d3e);
overflow: hidden;
text-overflow: ellipsis;
white-space: nowrap;
}

&__body {
display: flex;
flex-direction: column;
gap: 0.5rem;
padding: 0.75rem 1rem 1rem;
border-top: 1px solid var(--sapGroup_ContentBorderColor, #d9d9d9);
}
}

.row {
display: flex;
gap: 0.25rem;
align-items: baseline;

&__label {
font-weight: 500;
color: var(--sapContent_LabelColor, #556b82);
white-space: nowrap;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
import { FieldDefinition, GenericResource } from '../../models';
import { ResourceCollectionField } from './resource-collection-field.component';
import { ComponentFixture, TestBed } from '@angular/core/testing';

type Fixture = ComponentFixture<
ResourceCollectionField<GenericResource, FieldDefinition>
>;
type Comp = ResourceCollectionField<GenericResource, FieldDefinition>;

const CONDITIONS_FIELD: FieldDefinition = {
label: 'Conditions',
property: 'status.conditions',
propertyCollection: [
{ label: 'Type', property: 'status.conditions.type' },
{ label: 'Status', property: 'status.conditions.status' },
{ label: 'Reason', property: 'status.conditions.reason' },
],
};

function setup(
field: FieldDefinition,
resource?: Partial<GenericResource>,
): { fixture: Fixture; component: Comp } {
const fixture: Fixture = TestBed.createComponent(
ResourceCollectionField as unknown as typeof ResourceCollectionField<
GenericResource,
FieldDefinition
>,
);
const component = fixture.componentInstance;
fixture.componentRef.setInput('fieldDefinition', field);
if (resource !== undefined)
fixture.componentRef.setInput('resource', resource as GenericResource);
fixture.detectChanges();
return { fixture, component };
}

function root(fixture: Fixture): ParentNode {
return fixture.nativeElement.shadowRoot ?? fixture.nativeElement;
}

function all(fixture: Fixture, selector: string): Element[] {
return Array.from(root(fixture).querySelectorAll(selector));
}

function el(fixture: Fixture, testId: string): Element | null {
return root(fixture).querySelector(`[data-testid="${testId}"]`);
}

describe('ResourceCollectionField', () => {
beforeEach(async () => {
await TestBed.configureTestingModule({
imports: [ResourceCollectionField],
}).compileComponents();
});

it('renders one collapsed card per array entry', () => {
const { fixture } = setup(CONDITIONS_FIELD, {
status: {
conditions: [
{ type: 'Ready', status: 'True', reason: 'OK' },
{ type: 'Progressing', status: 'False', reason: 'Retry' },
],
},
} as unknown as GenericResource);

expect(all(fixture, '.card').length).toBe(2);
expect(all(fixture, '.card__body').length).toBe(0);
});

it('shows the first non-empty sub-field as the header preview', () => {
const { fixture } = setup(CONDITIONS_FIELD, {
status: { conditions: [{ type: 'Ready', status: 'True' }] },
} as unknown as GenericResource);

expect(
el(fixture, 'resource-collection-item-0-toggle')?.textContent,
).toContain('Type: Ready');
});

it('falls back to label and index when the entry is empty', () => {
const { fixture } = setup(CONDITIONS_FIELD, {
status: { conditions: [{}] },
} as unknown as GenericResource);

expect(
el(fixture, 'resource-collection-item-0-toggle')?.textContent,
).toContain('Conditions 1');
});

it('expands a single card and renders a resource-field per sub-field', () => {
const { fixture, component } = setup(CONDITIONS_FIELD, {
status: { conditions: [{ type: 'Ready', status: 'True', reason: 'OK' }] },
} as unknown as GenericResource);

component.toggle(0);
fixture.detectChanges();

const bodies = all(fixture, '.card__body');
expect(bodies.length).toBe(1);
expect(bodies[0].querySelectorAll('mfp-resource-field').length).toBe(3);
expect(bodies[0].textContent).toContain('Type:');
});

it('collapses an expanded card when toggled again', () => {
const { fixture, component } = setup(CONDITIONS_FIELD, {
status: { conditions: [{ type: 'Ready' }] },
} as unknown as GenericResource);

component.toggle(0);
fixture.detectChanges();
expect(all(fixture, '.card__body').length).toBe(1);

component.toggle(0);
fixture.detectChanges();
expect(all(fixture, '.card__body').length).toBe(0);
});

it('renders no cards when the collection path is missing or not an array', () => {
const { fixture } = setup(CONDITIONS_FIELD, {
status: {},
} as unknown as GenericResource);

expect(all(fixture, '.card').length).toBe(0);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
import { FieldDefinition, GenericResource } from '../../models';
import { getResourceValueByJsonPath } from '../../table/utils/resource-field-by-path';
import { ResourceField } from '../resource-field.component';
import {
CUSTOM_ELEMENTS_SCHEMA,
ChangeDetectionStrategy,
Component,
ViewEncapsulation,
computed,
forwardRef,
input,
signal,
} from '@angular/core';
import { Icon } from '@fundamental-ngx/ui5-webcomponents/icon';
import '@ui5/webcomponents-icons/dist/navigation-down-arrow.js';
import '@ui5/webcomponents-icons/dist/navigation-right-arrow.js';

/** One `label: value` row of an expanded collection card. */
interface CollectionRow {
label: string;
field: FieldDefinition;
}

/**
* Read-only display for a `propertyCollection` field. Renders the resolved
* array as a column of collapsed cards; expanding a card lists each sub-field
* as a `label: value` row rendered through `mfp-resource-field`, so per-
* sub-field `uiSettings` still apply. Mirrors the styling of the editable
* `mfp-form-collection-field`.
*/
@Component({
selector: 'mfp-resource-collection-field',
// `ResourceField` imports this component back, so the two form a module-init
// cycle. `forwardRef` defers resolution until the template is instantiated.
imports: [Icon, forwardRef(() => ResourceField)],
templateUrl: './resource-collection-field.component.html',
styleUrl: './resource-collection-field.component.scss',
changeDetection: ChangeDetectionStrategy.OnPush,
encapsulation: ViewEncapsulation.ShadowDom,
schemas: [CUSTOM_ELEMENTS_SCHEMA],
})
export class ResourceCollectionField<
T extends GenericResource,
F extends FieldDefinition,
> {
readonly fieldDefinition = input.required<F>();
readonly resource = input<T>();

protected readonly expandedIndex = signal<number | null>(null);

private readonly collectionPath = computed(() => {
const property = this.fieldDefinition().property;
return typeof property === 'string' ? property : undefined;
});

protected readonly entries = computed<Record<string, unknown>[]>(() => {
const value = getResourceValueByJsonPath(this.resource() ?? {}, {
property: this.collectionPath(),
});
return Array.isArray(value) ? (value as Record<string, unknown>[]) : [];
});
Comment on lines +56 to +61

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

entries computed ignores jsonPathExpression from the field definition.

getResourceValueByJsonPath checks field.jsonPathExpression || field.property, but only property is passed here. If a collection field uses jsonPathExpression instead of (or alongside) property, the array will silently resolve to [] with no error logged.

🔧 Proposed fix
 protected readonly entries = computed<Record<string, unknown>[]>(() => {
-    const value = getResourceValueByJsonPath(this.resource() ?? {}, {
-      property: this.collectionPath(),
-    });
+    const fd = this.fieldDefinition();
+    const value = getResourceValueByJsonPath(this.resource() ?? {}, {
+      jsonPathExpression: fd.jsonPathExpression,
+      property: this.collectionPath(),
+    });
     return Array.isArray(value) ? (value as Record<string, unknown>[]) : [];
   });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
protected readonly entries = computed<Record<string, unknown>[]>(() => {
const value = getResourceValueByJsonPath(this.resource() ?? {}, {
property: this.collectionPath(),
});
return Array.isArray(value) ? (value as Record<string, unknown>[]) : [];
});
protected readonly entries = computed<Record<string, unknown>[]>(() => {
const fd = this.fieldDefinition();
const value = getResourceValueByJsonPath(this.resource() ?? {}, {
jsonPathExpression: fd.jsonPathExpression,
property: this.collectionPath(),
});
return Array.isArray(value) ? (value as Record<string, unknown>[]) : [];
});
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@projects/ngx/declarative-ui/resource-field/resource-collection-field/resource-collection-field.component.ts`
around lines 56 - 61, Update the entries computed in the resource collection
field component to pass the field definition’s jsonPathExpression along with
collectionPath() to getResourceValueByJsonPath, preserving the fallback to
property so both path configurations resolve correctly.


protected readonly rows = computed<CollectionRow[]>(() => {
const parent = this.collectionPath();
return (this.fieldDefinition().propertyCollection ?? []).map((sub) => {
const property =
typeof sub.property === 'string'
? stripParentPath(sub.property, parent)
: sub.property;
return {
label: sub.label ?? leafOf(property),
field: { ...sub, property } as FieldDefinition,
};
});
});

toggle(index: number, event?: Event): void {
event?.stopPropagation();
this.expandedIndex.set(this.expandedIndex() === index ? null : index);
}

previewFor(entry: Record<string, unknown>, index: number): string {
for (const row of this.rows()) {
const raw = getResourceValueByJsonPath(entry, {
property: row.field.property,
});
if (raw !== undefined && raw !== null && String(raw).trim() !== '') {
return `${row.label}: ${String(raw)}`;
}
}
const prefix = (this.fieldDefinition().label ?? '').trim() || 'Item';
return `${prefix} ${index + 1}`;
}
}

function stripParentPath(name: string, parent: string | undefined): string {
if (!parent) return name;
if (name.startsWith(parent + '.')) {
return name.slice(parent.length + 1);
}
return name;
}

function leafOf(property: string | string[] | undefined): string {
if (typeof property !== 'string') return '';
const segments = property.split('.');
return segments[segments.length - 1];
}
Loading