-
Notifications
You must be signed in to change notification settings - Fork 0
feat: support list values in resource field component #202
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
gkrajniak
wants to merge
6
commits into
main
Choose a base branch
from
feat/support-list-values
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
88310c4
Add support vor generic form collection of object
gkrajniak b068e09
After review
gkrajniak b4f8f73
simplify solution with the reuse of the property field adn renaming t…
gkrajniak 8d2dcc9
simplify solution with the reuse of the property field adn renaming t…
gkrajniak 21ce8cb
revert label in form field as required
gkrajniak db0e8dd
Add collection display to the resource filed component
gkrajniak File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
38 changes: 38 additions & 0 deletions
38
...tive-ui/resource-field/resource-collection-field/resource-collection-field.component.html
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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> |
75 changes: 75 additions & 0 deletions
75
...tive-ui/resource-field/resource-collection-field/resource-collection-field.component.scss
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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; | ||
| } | ||
| } |
126 changes: 126 additions & 0 deletions
126
...e-ui/resource-field/resource-collection-field/resource-collection-field.component.spec.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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); | ||
| }); | ||
| }); |
108 changes: 108 additions & 0 deletions
108
...rative-ui/resource-field/resource-collection-field/resource-collection-field.component.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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>[]) : []; | ||
| }); | ||
|
|
||
| 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]; | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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
entriescomputed ignoresjsonPathExpressionfrom the field definition.getResourceValueByJsonPathchecksfield.jsonPathExpression || field.property, but onlypropertyis passed here. If a collection field usesjsonPathExpressioninstead 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
🤖 Prompt for AI Agents