fix(davinci-client): address PR feedback

Author: vatsalparikh

.changeset/curly-wolves-swim.md

@@ -2,4 +2,4 @@
 '@forgerock/davinci-client': minor
 ---
 
-Add `ImageCollector` for rendering `IMAGE` form fields from DaVinci Forms.
+Add `ImageCollector` for rendering `IMAGE` form fields for PingOne Forms.

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

@@ -17,13 +17,13 @@ export default function (formEl: HTMLFormElement, collector: ImageCollector) {
   const container = document.createElement('div');
 
   const img = document.createElement('img');
-  img.src = collector.output.imageUrl;
-  img.alt = collector.output.description;
+  img.src = collector.output.src;
+  img.alt = collector.output.alt;
   img.setAttribute('data-testid', 'form-image');
 
-  if (collector.output.hyperlinkUrl) {
+  if (collector.output.href) {
     const anchor = document.createElement('a');
-    anchor.href = collector.output.hyperlinkUrl;
+    anchor.href = collector.output.href;
     anchor.appendChild(img);
     container.appendChild(anchor);
   } else {

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

@@ -71,7 +71,7 @@ test('Should render form fields', async ({ page }) => {
   await expect(page.getByRole('button', { name: 'Flow Link' })).toBeVisible();
 
   const requestPromise = page.waitForRequest(
-    (request) => request.url().includes('showForm') && request.method() === 'POST',
+    (request) => request.url().includes('customForm') && request.method() === 'POST',
   );
 
   await page.getByRole('button', { name: 'Submit' }).click();

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

@@ -19,10 +19,7 @@ test('Should render image collector in form', async ({ page }) => {
 
   const formImage = page.getByTestId('form-image');
   await expect(formImage).toBeVisible();
-  await expect(formImage).toHaveAttribute(
-    'src',
-    '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',
-  );
+  await expect(formImage).toHaveAttribute('src', /QC-Montreal-Skyline_hero\.jpg/);
   await expect(formImage).toHaveAttribute('alt', 'New Image');
 
   const formImageAnchor = page.locator('a:has([data-testid="form-image"])');

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

@@ -959,9 +959,9 @@ export type IdpCollector = ActionCollectorWithUrl<'IdpCollector'>;
 export interface ImageCollector extends NoValueCollectorBase<'ImageCollector'> {
     // (undocumented)
     output: NoValueCollectorBase<'ImageCollector'>['output'] & {
-        imageUrl: string;
-        description: string;
-        hyperlinkUrl?: string;
+        src: string;
+        alt: string;
+        href?: string;
     };
 }
 

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

@@ -956,9 +956,9 @@ export type IdpCollector = ActionCollectorWithUrl<'IdpCollector'>;
 export interface ImageCollector extends NoValueCollectorBase<'ImageCollector'> {
     // (undocumented)
     output: NoValueCollectorBase<'ImageCollector'>['output'] & {
-        imageUrl: string;
-        description: string;
-        hyperlinkUrl?: string;
+        src: string;
+        alt: string;
+        href?: string;
     };
 }
 

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

@@ -664,11 +664,29 @@ describe('Collector Types', () => {
         error: null,
         output: {
           key: 'image1',
-          label: '',
+          label: 'A hero image',
+          type: 'IMAGE',
+          src: 'https://example.com/image.png',
+          alt: 'A hero image',
+          href: 'https://example.com',
+        },
+      };
+      expectTypeOf(tCollector).toEqualTypeOf<ImageCollector>();
+    });
+
+    it('should correctly infer ImageCollector Type without optional href', () => {
+      const tCollector: InferNoValueCollectorType<'ImageCollector'> = {
+        category: 'NoValueCollector',
+        type: 'ImageCollector',
+        name: 'ImageCollector',
+        id: '1',
+        error: null,
+        output: {
+          key: 'image1',
+          label: 'A hero image',
           type: 'IMAGE',
-          imageUrl: 'https://example.com/image.png',
-          description: 'A hero image',
-          hyperlinkUrl: 'https://example.com',
+          src: 'https://example.com/image.png',
+          alt: 'A hero image',
         },
       };
       expectTypeOf(tCollector).toEqualTypeOf<ImageCollector>();

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

@@ -639,14 +639,14 @@ export interface QrCodeCollector extends NoValueCollectorBase<'QrCodeCollector'>
 
 /**
  * @interface ImageCollector - Display-only collector for IMAGE fields. Extends the
- * generic `NoValueCollectorBase` with the image URL (`src`), an optional
- * hyperlink URL (`hyperlink.url`), and an optional `description` (alt text).
+ * generic `NoValueCollectorBase` with `src` (image URL), `alt` (alternative text), and an optional
+ * `href` (hyperlink URL).
  */
 export interface ImageCollector extends NoValueCollectorBase<'ImageCollector'> {
   output: NoValueCollectorBase<'ImageCollector'>['output'] & {
-    imageUrl: string;
-    description: string;
-    hyperlinkUrl?: string;
+    src: string;
+    alt: string;
+    href?: string;
   };
 }
 

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

@@ -1116,17 +1116,17 @@ describe('returnImageCollector', () => {
       name: 'heroImage-1',
       output: {
         key: 'heroImage-1',
-        label: '',
+        label: 'Alt text',
         type: 'IMAGE',
-        imageUrl: 'https://cdn.example.com/image.png',
-        description: 'Alt text',
-        hyperlinkUrl: 'https://example.com/install',
+        src: 'https://cdn.example.com/image.png',
+        alt: 'Alt text',
+        href: 'https://example.com/install',
       },
     });
   });
 
   // (b) Minimal payload — no hyperlinkUrl
-  it('should omit hyperlinkUrl when hyperlinkUrl is absent', () => {
+  it('should omit href when hyperlinkUrl is absent', () => {
     const mockField: ImageField = {
       type: 'IMAGE',
       key: 'image',
@@ -1135,14 +1135,14 @@ describe('returnImageCollector', () => {
     };
     const result = returnImageCollector(mockField, 0);
     expect(result.error).toBeNull();
-    expect(result.output.imageUrl).toBe('https://example.com/test-image.png');
-    expect(result.output.description).toBe('Alt text');
-    expect(result.output.label).toBe('');
-    expect(result.output).not.toHaveProperty('hyperlinkUrl');
+    expect(result.output.src).toBe('https://example.com/test-image.png');
+    expect(result.output.alt).toBe('Alt text');
+    expect(result.output.label).toBe('Alt text');
+    expect(result.output).not.toHaveProperty('href');
   });
 
-  // (c) hyperlinkUrl present — hyperlink emitted; absent — omitted
-  it('should emit hyperlinkUrl when hyperlinkUrl is set and omit it when absent', () => {
+  // (c) hyperlinkUrl present — href emitted; absent — omitted
+  it('should emit href when hyperlinkUrl is set and omit it when absent', () => {
     const withHyperlink: ImageField = {
       type: 'IMAGE',
       key: 'image',
@@ -1151,7 +1151,7 @@ describe('returnImageCollector', () => {
       hyperlinkUrl: 'https://example.com/click-target',
     };
     const resultWith = returnImageCollector(withHyperlink, 0);
-    expect(resultWith.output.hyperlinkUrl).toBe('https://example.com/click-target');
+    expect(resultWith.output.href).toBe('https://example.com/click-target');
 
     const withoutHyperlink: ImageField = {
       type: 'IMAGE',
@@ -1160,19 +1160,20 @@ describe('returnImageCollector', () => {
       imageUrl: 'https://example.com/test-image.png',
     };
     const resultWithout = returnImageCollector(withoutHyperlink, 0);
-    expect(resultWithout.output).not.toHaveProperty('hyperlinkUrl');
+    expect(resultWithout.output).not.toHaveProperty('href');
   });
 
-  // (d) description passes through verbatim
-  it('should pass description through verbatim', () => {
+  // (d) description passes through verbatim as alt and label
+  it('should pass description through verbatim as alt and label', () => {
     const mockField: ImageField = {
       type: 'IMAGE',
       key: 'image',
       description: 'Friendly alt text',
       imageUrl: 'https://example.com/test-image.png',
     };
     const result = returnImageCollector(mockField, 0);
-    expect(result.output.description).toBe('Friendly alt text');
+    expect(result.output.alt).toBe('Friendly alt text');
+    expect(result.output.label).toBe('Friendly alt text');
   });
 
   // (e) imageUrl absent — src defaults to empty string
@@ -1184,19 +1185,26 @@ describe('returnImageCollector', () => {
       imageUrl: '',
     };
     const result = returnImageCollector(mockField, 0);
-    expect(result.output.imageUrl).toBe('');
+    expect(result.output.src).toBe('');
   });
 
-  // (f) output.label === '' for any IMAGE payload
-  it('should set output.label to empty string regardless of payload shape', () => {
-    const mockField: ImageField = {
+  // (f) output.label mirrors description
+  it('should set output.label to description when present, empty string when absent', () => {
+    const withDescription: ImageField = {
       type: 'IMAGE',
       key: 'image',
       description: 'Alt text',
       imageUrl: 'https://example.com/test-image.png',
     };
-    const result = returnImageCollector(mockField, 0);
-    expect(result.output.label).toBe('');
+    expect(returnImageCollector(withDescription, 0).output.label).toBe('Alt text');
+
+    const withoutDescription: ImageField = {
+      type: 'IMAGE',
+      key: 'image',
+      description: '',
+      imageUrl: 'https://example.com/test-image.png',
+    };
+    expect(returnImageCollector(withoutDescription, 0).output.label).toBe('');
   });
 });
 

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

@@ -989,11 +989,8 @@ export function returnQrCodeCollector(field: QrCodeField, idx: number): QrCodeCo
  * @function returnImageCollector - Creates an ImageCollector object for displaying IMAGE fields.
  *
  * Composes on top of `returnNoValueCollector` for `category`, `id`, `name`, and base
- * `output.{key, type}`. Overrides `output.label` (IMAGE has no wire `label` property;
- * set to empty string — consumers use `output.description` for alt-text).
- *
- * `hyperlink` is present when the server includes `hyperlinkUrl` on the wire.
- * `description` is present when the server includes it on the wire.
+ * `output.{key, type}`. Overrides `output.label` with `field.description` (IMAGE has no
+ * wire `label` property; falls back to empty string).
  *
  * @param {ImageField} field - The IMAGE field from the API response.
  * @param {number} idx - The index used in the collector `id`/`name`.
@@ -1007,10 +1004,10 @@ export function returnImageCollector(field: ImageField, idx: number): ImageColle
     error: null,
     output: {
       ...base.output,
-      label: '',
-      imageUrl: field.imageUrl,
-      description: field.description,
-      ...(field.hyperlinkUrl ? { hyperlinkUrl: field.hyperlinkUrl } : {}),
+      label: field.description ?? '',
+      src: field.imageUrl,
+      alt: field.description,
+      ...(field.hyperlinkUrl ? { href: field.hyperlinkUrl } : {}),
     },
   };
 }