Skip to content

Commit e0cdac0

Browse files
committed
fix(davinci-client): address PR feedback
1 parent b0d790d commit e0cdac0

11 files changed

Lines changed: 63 additions & 51 deletions

File tree

‎.changeset/curly-wolves-swim.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,4 @@
22
'@forgerock/davinci-client': minor
33
---
44

5-
Add `ImageCollector` for rendering `IMAGE` form fields from DaVinci Forms.
5+
Add `ImageCollector` for rendering `IMAGE` form fields for PingOne Forms.

‎e2e/davinci-app/components/form-image.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,13 @@ export default function (formEl: HTMLFormElement, collector: ImageCollector) {
1717
const container = document.createElement('div');
1818

1919
const img = document.createElement('img');
20-
img.src = collector.output.imageUrl;
21-
img.alt = collector.output.description;
20+
img.src = collector.output.src;
21+
img.alt = collector.output.alt;
2222
img.setAttribute('data-testid', 'form-image');
2323

24-
if (collector.output.hyperlinkUrl) {
24+
if (collector.output.href) {
2525
const anchor = document.createElement('a');
26-
anchor.href = collector.output.hyperlinkUrl;
26+
anchor.href = collector.output.href;
2727
anchor.appendChild(img);
2828
container.appendChild(anchor);
2929
} else {

‎e2e/davinci-suites/src/form-fields.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ test('Should render form fields', async ({ page }) => {
7171
await expect(page.getByRole('button', { name: 'Flow Link' })).toBeVisible();
7272

7373
const requestPromise = page.waitForRequest(
74-
(request) => request.url().includes('showForm') && request.method() === 'POST',
74+
(request) => request.url().includes('customForm') && request.method() === 'POST',
7575
);
7676

7777
await page.getByRole('button', { name: 'Submit' }).click();

‎e2e/davinci-suites/src/form-image.test.ts‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,7 @@ test('Should render image collector in form', async ({ page }) => {
1919

2020
const formImage = page.getByTestId('form-image');
2121
await expect(formImage).toBeVisible();
22-
await expect(formImage).toHaveAttribute(
23-
'src',
24-
'https://travel.destinationcanada.com/_next/image?url=https%3A%2F%2Fadmin.destinationcanada.com%2Fsites%2Fdefault%2Ffiles%2F2023-06%2FQC-Montreal-Skyline_hero.jpg&w=1920&q=75',
25-
);
22+
await expect(formImage).toHaveAttribute('src', /QC-Montreal-Skyline_hero\.jpg/);
2623
await expect(formImage).toHaveAttribute('alt', 'New Image');
2724

2825
const formImageAnchor = page.locator('a:has([data-testid="form-image"])');

‎packages/davinci-client/api-report/davinci-client.api.md‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -959,9 +959,9 @@ export type IdpCollector = ActionCollectorWithUrl<'IdpCollector'>;
959959
export interface ImageCollector extends NoValueCollectorBase<'ImageCollector'> {
960960
// (undocumented)
961961
output: NoValueCollectorBase<'ImageCollector'>['output'] & {
962-
imageUrl: string;
963-
description: string;
964-
hyperlinkUrl?: string;
962+
src: string;
963+
alt: string;
964+
href?: string;
965965
};
966966
}
967967

‎packages/davinci-client/api-report/davinci-client.types.api.md‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -956,9 +956,9 @@ export type IdpCollector = ActionCollectorWithUrl<'IdpCollector'>;
956956
export interface ImageCollector extends NoValueCollectorBase<'ImageCollector'> {
957957
// (undocumented)
958958
output: NoValueCollectorBase<'ImageCollector'>['output'] & {
959-
imageUrl: string;
960-
description: string;
961-
hyperlinkUrl?: string;
959+
src: string;
960+
alt: string;
961+
href?: string;
962962
};
963963
}
964964

‎packages/davinci-client/src/lib/collector.types.test-d.ts‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -666,9 +666,27 @@ describe('Collector Types', () => {
666666
key: 'image1',
667667
label: '',
668668
type: 'IMAGE',
669-
imageUrl: 'https://example.com/image.png',
670-
description: 'A hero image',
671-
hyperlinkUrl: 'https://example.com',
669+
src: 'https://example.com/image.png',
670+
alt: 'A hero image',
671+
href: 'https://example.com',
672+
},
673+
};
674+
expectTypeOf(tCollector).toEqualTypeOf<ImageCollector>();
675+
});
676+
677+
it('should correctly infer ImageCollector Type without optional href', () => {
678+
const tCollector: InferNoValueCollectorType<'ImageCollector'> = {
679+
category: 'NoValueCollector',
680+
type: 'ImageCollector',
681+
name: 'ImageCollector',
682+
id: '1',
683+
error: null,
684+
output: {
685+
key: 'image1',
686+
label: '',
687+
type: 'IMAGE',
688+
src: 'https://example.com/image.png',
689+
alt: 'A hero image',
672690
},
673691
};
674692
expectTypeOf(tCollector).toEqualTypeOf<ImageCollector>();

‎packages/davinci-client/src/lib/collector.types.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -639,14 +639,14 @@ export interface QrCodeCollector extends NoValueCollectorBase<'QrCodeCollector'>
639639

640640
/**
641641
* @interface ImageCollector - Display-only collector for IMAGE fields. Extends the
642-
* generic `NoValueCollectorBase` with the image URL (`src`), an optional
643-
* hyperlink URL (`hyperlink.url`), and an optional `description` (alt text).
642+
* generic `NoValueCollectorBase` with `src` (image URL), `alt` (alternative text), and an optional
643+
* `href` (hyperlink URL).
644644
*/
645645
export interface ImageCollector extends NoValueCollectorBase<'ImageCollector'> {
646646
output: NoValueCollectorBase<'ImageCollector'>['output'] & {
647-
imageUrl: string;
648-
description: string;
649-
hyperlinkUrl?: string;
647+
src: string;
648+
alt: string;
649+
href?: string;
650650
};
651651
}
652652

‎packages/davinci-client/src/lib/collector.utils.test.ts‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1118,15 +1118,15 @@ describe('returnImageCollector', () => {
11181118
key: 'heroImage-1',
11191119
label: '',
11201120
type: 'IMAGE',
1121-
imageUrl: 'https://cdn.example.com/image.png',
1122-
description: 'Alt text',
1123-
hyperlinkUrl: 'https://example.com/install',
1121+
src: 'https://cdn.example.com/image.png',
1122+
alt: 'Alt text',
1123+
href: 'https://example.com/install',
11241124
},
11251125
});
11261126
});
11271127

11281128
// (b) Minimal payload — no hyperlinkUrl
1129-
it('should omit hyperlinkUrl when hyperlinkUrl is absent', () => {
1129+
it('should omit href when hyperlinkUrl is absent', () => {
11301130
const mockField: ImageField = {
11311131
type: 'IMAGE',
11321132
key: 'image',
@@ -1135,14 +1135,14 @@ describe('returnImageCollector', () => {
11351135
};
11361136
const result = returnImageCollector(mockField, 0);
11371137
expect(result.error).toBeNull();
1138-
expect(result.output.imageUrl).toBe('https://example.com/test-image.png');
1139-
expect(result.output.description).toBe('Alt text');
1138+
expect(result.output.src).toBe('https://example.com/test-image.png');
1139+
expect(result.output.alt).toBe('Alt text');
11401140
expect(result.output.label).toBe('');
1141-
expect(result.output).not.toHaveProperty('hyperlinkUrl');
1141+
expect(result.output).not.toHaveProperty('href');
11421142
});
11431143

1144-
// (c) hyperlinkUrl present — hyperlink emitted; absent — omitted
1145-
it('should emit hyperlinkUrl when hyperlinkUrl is set and omit it when absent', () => {
1144+
// (c) hyperlinkUrl present — href emitted; absent — omitted
1145+
it('should emit href when hyperlinkUrl is set and omit it when absent', () => {
11461146
const withHyperlink: ImageField = {
11471147
type: 'IMAGE',
11481148
key: 'image',
@@ -1151,7 +1151,7 @@ describe('returnImageCollector', () => {
11511151
hyperlinkUrl: 'https://example.com/click-target',
11521152
};
11531153
const resultWith = returnImageCollector(withHyperlink, 0);
1154-
expect(resultWith.output.hyperlinkUrl).toBe('https://example.com/click-target');
1154+
expect(resultWith.output.href).toBe('https://example.com/click-target');
11551155

11561156
const withoutHyperlink: ImageField = {
11571157
type: 'IMAGE',
@@ -1160,19 +1160,19 @@ describe('returnImageCollector', () => {
11601160
imageUrl: 'https://example.com/test-image.png',
11611161
};
11621162
const resultWithout = returnImageCollector(withoutHyperlink, 0);
1163-
expect(resultWithout.output).not.toHaveProperty('hyperlinkUrl');
1163+
expect(resultWithout.output).not.toHaveProperty('href');
11641164
});
11651165

1166-
// (d) description passes through verbatim
1167-
it('should pass description through verbatim', () => {
1166+
// (d) description passes through verbatim as alt
1167+
it('should pass description through verbatim as alt', () => {
11681168
const mockField: ImageField = {
11691169
type: 'IMAGE',
11701170
key: 'image',
11711171
description: 'Friendly alt text',
11721172
imageUrl: 'https://example.com/test-image.png',
11731173
};
11741174
const result = returnImageCollector(mockField, 0);
1175-
expect(result.output.description).toBe('Friendly alt text');
1175+
expect(result.output.alt).toBe('Friendly alt text');
11761176
});
11771177

11781178
// (e) imageUrl absent — src defaults to empty string
@@ -1184,7 +1184,7 @@ describe('returnImageCollector', () => {
11841184
imageUrl: '',
11851185
};
11861186
const result = returnImageCollector(mockField, 0);
1187-
expect(result.output.imageUrl).toBe('');
1187+
expect(result.output.src).toBe('');
11881188
});
11891189

11901190
// (f) output.label === '' for any IMAGE payload

‎packages/davinci-client/src/lib/collector.utils.ts‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -990,10 +990,7 @@ export function returnQrCodeCollector(field: QrCodeField, idx: number): QrCodeCo
990990
*
991991
* Composes on top of `returnNoValueCollector` for `category`, `id`, `name`, and base
992992
* `output.{key, type}`. Overrides `output.label` (IMAGE has no wire `label` property;
993-
* set to empty string — consumers use `output.description` for alt-text).
994-
*
995-
* `hyperlink` is present when the server includes `hyperlinkUrl` on the wire.
996-
* `description` is present when the server includes it on the wire.
993+
* set to empty string).
997994
*
998995
* @param {ImageField} field - The IMAGE field from the API response.
999996
* @param {number} idx - The index used in the collector `id`/`name`.
@@ -1008,9 +1005,9 @@ export function returnImageCollector(field: ImageField, idx: number): ImageColle
10081005
output: {
10091006
...base.output,
10101007
label: '',
1011-
imageUrl: field.imageUrl,
1012-
description: field.description,
1013-
...(field.hyperlinkUrl ? { hyperlinkUrl: field.hyperlinkUrl } : {}),
1008+
src: field.imageUrl,
1009+
alt: field.description,
1010+
...(field.hyperlinkUrl ? { href: field.hyperlinkUrl } : {}),
10141011
},
10151012
};
10161013
}

0 commit comments

Comments
 (0)