Skip to content

feat(davinci-client): support single checkbox component #629

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.

Sign up for GitHub

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
ancheetah wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat(davinci-client): support single checkbox component #629

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ GEMINI.md

.claude/worktrees
.claude/settings.local.json
.claude/skills
.opensource

# Polaris
Expand Down
57 changes: 57 additions & 0 deletions e2e/davinci-app/components/boolean.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/*
* Copyright (c) 2026 Ping Identity Corporation. All rights reserved.
*
* This software may be modified and distributed under the terms
* of the MIT license. See the LICENSE file for details.
*/
import type { ValidatedBooleanCollector, Updater } from '@forgerock/davinci-client/types';

/**
* Creates a single checkbox and attaches it to the form
* @param {HTMLFormElement} formEl - The form element to attach the checkboxes to
* @param {ValidatedBooleanCollector} collector - Contains the configuration
* @param {Updater} updater - Function to call when selection changes
*/
export default function booleanComponent(
formEl: HTMLFormElement,
collector: ValidatedBooleanCollector,
updater: Updater<ValidatedBooleanCollector>,
) {
// Create a container for the checkboxes
const containerDiv = document.createElement('div');
containerDiv.className = 'single-checkbox-container';

// Create a heading/label for the checkbox group
const groupLabel = document.createElement('div');
groupLabel.textContent = collector.output.label || 'Single Checkbox';
groupLabel.className = 'single-checkbox-label';
containerDiv.appendChild(groupLabel);

// Create checkboxes for each option
const wrapper = document.createElement('div');
wrapper.className = 'checkbox-wrapper';

const checkbox = document.createElement('input');
checkbox.type = 'checkbox';
checkbox.id = collector.output.key;
checkbox.name = collector.output.key || 'single-checkbox-field';
checkbox.checked = collector.output.value as boolean;
checkbox.value = 'checked';
Comment on lines +34 to +39
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 11, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Minor: Consider adding required attribute support.

The SingleCheckboxField type includes a required property, but the component doesn't set the HTML required attribute on the checkbox element. This means required validation won't work as expected.

🛡️ Proposed fix 
   const checkbox = document.createElement('input');
   checkbox.type = 'checkbox';
   checkbox.id = collector.output.key;
   checkbox.name = collector.output.key || 'single-checkbox-field';
   checkbox.checked = collector.output.value as boolean;
   checkbox.value = 'checked';
+  if (collector.output.required) {
+    checkbox.required = true;
+  }
📝 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
const checkbox = document.createElement('input');
checkbox.type = 'checkbox';
checkbox.id = collector.output.key;
checkbox.name = collector.output.key || 'single-checkbox-field';
checkbox.checked = collector.output.value as boolean;
checkbox.value = 'checked';
const checkbox = document.createElement('input');
checkbox.type = 'checkbox';
checkbox.id = collector.output.key;
checkbox.name = collector.output.key || 'single-checkbox-field';
checkbox.checked = collector.output.value as boolean;
checkbox.value = 'checked';
if (collector.output.required) {
checkbox.required = true;
}
🤖 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 `@e2e/davinci-app/components/boolean.ts` around lines 34 - 39, The
SingleCheckboxField's required flag isn't applied to the created checkbox
element; update the element creation in the component that builds the checkbox
(where checkbox is created from collector.output) to set checkbox.required =
!!collector.output.required (and optionally
checkbox.setAttribute('aria-required','true') for accessibility) so the HTML
required validation is honored; ensure you reference collector.output.required
and the checkbox element in the SingleCheckboxField handling logic.


const label = document.createElement('label');
label.htmlFor = checkbox.id;
label.textContent = collector.output.label;

// Add event listener to handle single-select behavior
checkbox.addEventListener('change', (event) => {
const target = event.target as HTMLInputElement;
updater(target.value === 'checked');
});
Comment on lines +46 to +49
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 11, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Verify checkbox event handling in similar components

# Search for other checkbox change event handlers in the codebase
rg -n -A3 -B3 "addEventListener\s*\(\s*['\"]change['\"]" --glob "*.ts" --glob "*.js" -g '!node_modules'

Repository: ForgeRock/ping-javascript-sdk

Length of output: 7149

🏁 Script executed:

cat -n e2e/davinci-app/components/boolean.ts

Repository: ForgeRock/ping-javascript-sdk

Length of output: 2558

Use target.checked instead of target.value === 'checked'.

The checkbox value attribute (line 39) is a static string that never changes based on checkbox state. The expression target.value === 'checked' will always evaluate to true, regardless of whether the checkbox is checked or unchecked. This breaks the boolean field functionality. Use target.checked to get the actual checkbox state, which is also the pattern used in other checkbox handlers throughout the codebase (e.g., terms-and-conditions.ts, confirmation.ts).

Proposed fix 
   checkbox.addEventListener('change', (event) => {
     const target = event.target as HTMLInputElement;
-    updater(target.value === 'checked');
+    updater(target.checked);
   });
📝 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
checkbox.addEventListener('change', (event) => {
const target = event.target as HTMLInputElement;
updater(target.value === 'checked');
});
checkbox.addEventListener('change', (event) => {
const target = event.target as HTMLInputElement;
updater(target.checked);
});
🤖 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 `@e2e/davinci-app/components/boolean.ts` around lines 46 - 49, The change
handler for the checkbox uses the static value attribute and compares
target.value === 'checked', which is always true; update the event listener on
checkbox (the callback passed to checkbox.addEventListener('change', ...)) to
call updater with the actual boolean state using target.checked instead of
comparing target.value, i.e., replace the expression target.value === 'checked'
with target.checked so the updater receives the real checked state.


wrapper.appendChild(checkbox);
wrapper.appendChild(label);
containerDiv.appendChild(wrapper);

// Append the container to the form
formEl.appendChild(containerDiv);
}
3 changes: 3 additions & 0 deletions e2e/davinci-app/main.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import fidoComponent from './components/fido.js';
import qrCodeComponent from './components/qr-code.js';
import agreementComponent from './components/agreement.js';
import pollingComponent from './components/polling.js';
import booleanComponent from './components/boolean.js';

const loggerFn = {
error: () => {
Expand Down Expand Up @@ -294,6 +295,8 @@ const urlParams = new URLSearchParams(window.location.search);
singleValueComponent(formEl, collector, davinciClient.update(collector));
} else if (collector.type === 'MultiSelectCollector') {
multiValueComponent(formEl, collector, davinciClient.update(collector));
} else if (collector.type === 'ValidatedBooleanCollector') {
booleanComponent(formEl, collector, davinciClient.update(collector));
}
});

Expand Down
1 change: 1 addition & 0 deletions packages/davinci-client/src/lib/client.store.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,7 @@ export async function davinci<ActionType extends ActionTypes = ActionTypes>({
value:
| string
| string[]
| boolean
| PhoneNumberInputValue
| PhoneNumberExtensionInputValue
| FidoRegistrationInputValue
Expand Down
42 changes: 22 additions & 20 deletions packages/davinci-client/src/lib/client.types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,28 +44,30 @@ export type CollectorValueType<T> = T extends { type: 'PasswordCollector' }
? string
: T extends { type: 'MultiSelectCollector' }
? string[]
: T extends { type: 'DeviceRegistrationCollector' }
? string
: T extends { type: 'DeviceAuthenticationCollector' }
: T extends { type: 'ValidatedBooleanCollector' }
? boolean
: T extends { type: 'DeviceRegistrationCollector' }
? string
: T extends { type: 'PhoneNumberCollector' }
? PhoneNumberInputValue
: T extends { type: 'FidoRegistrationCollector' }
? FidoRegistrationInputValue
: T extends { type: 'FidoAuthenticationCollector' }
? FidoAuthenticationInputValue
: T extends { category: 'SingleValueCollector' }
? string
: T extends { category: 'ValidatedSingleValueCollector' }
: T extends { type: 'DeviceAuthenticationCollector' }
? string
: T extends { type: 'PhoneNumberCollector' }
? PhoneNumberInputValue
: T extends { type: 'FidoRegistrationCollector' }
? FidoRegistrationInputValue
: T extends { type: 'FidoAuthenticationCollector' }
? FidoAuthenticationInputValue
: T extends { category: 'SingleValueCollector' }
? string
: T extends { category: 'MultiValueCollector' }
? string[]
:
| string
| string[]
| PhoneNumberInputValue
| FidoRegistrationInputValue
| FidoAuthenticationInputValue;
: T extends { category: 'ValidatedSingleValueCollector' }
? string
: T extends { category: 'MultiValueCollector' }
? string[]
:
| string
| string[]
| PhoneNumberInputValue
| FidoRegistrationInputValue
| FidoAuthenticationInputValue;

/**
* Generic updater function that accepts values appropriate for the collector type.
Expand Down
24 changes: 13 additions & 11 deletions packages/davinci-client/src/lib/collector.types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import type { FidoAuthenticationOptions, FidoRegistrationOptions } from './davin
*/
export type SingleValueCollectorTypes =
| 'PasswordCollector'
| 'ValidatedBooleanCollector'
| 'SingleValueCollector'
| 'SingleSelectCollector'
| 'SingleSelectObjectCollector'
Expand Down Expand Up @@ -157,14 +158,16 @@ export type InferSingleValueCollectorType<T extends SingleValueCollectorTypes> =
? ValidatedTextCollector
: T extends 'PasswordCollector'
? PasswordCollector
: /**
: T extends 'ValidatedBooleanCollector'
? ValidatedBooleanCollector
: /**
* At this point, we have not passed in a collector type
* or we have explicitly passed in 'SingleValueCollector'
* So we can return either a SingleValueCollector with value
* or without a value.
**/
| SingleValueCollectorWithValue<'SingleValueCollector'>
| SingleValueCollectorNoValue<'SingleValueCollector'>;
| SingleValueCollectorNoValue<'SingleValueCollector'>;

/**
* SINGLE-VALUE COLLECTOR TYPES
Expand All @@ -178,12 +181,15 @@ export type SingleValueCollectors =
| SingleSelectCollectorWithValue<'SingleSelectCollector'>
| SingleValueCollectorWithValue<'SingleValueCollector'>
| SingleValueCollectorWithValue<'TextCollector'>
| ValidatedSingleValueCollectorWithValue<'TextCollector'>;
| ValidatedSingleValueCollectorWithValue<'TextCollector'>
| ValidatedSingleValueCollectorWithValue<'ValidatedBooleanCollector'>;

export type PasswordCollector = SingleValueCollectorNoValue<'PasswordCollector'>;
export type TextCollector = SingleValueCollectorWithValue<'TextCollector'>;
export type SingleSelectCollector = SingleSelectCollectorWithValue<'SingleSelectCollector'>;
export type ValidatedTextCollector = ValidatedSingleValueCollectorWithValue<'TextCollector'>;
export type ValidatedBooleanCollector =
ValidatedSingleValueCollectorWithValue<'ValidatedBooleanCollector'>;

/** *********************************************************************
* MULTI-VALUE COLLECTORS
Expand Down Expand Up @@ -625,10 +631,8 @@ export interface ProtectOutputValue {
universalDeviceIdentification: boolean;
}

export interface AttestationValue extends Omit<
PublicKeyCredential,
'rawId' | 'response' | 'getClientExtensionResults' | 'toJSON'
> {
export interface AttestationValue
extends Omit<PublicKeyCredential, 'rawId' | 'response' | 'getClientExtensionResults' | 'toJSON'> {
rawId: string;
response: {
// https://developer.mozilla.org/en-US/docs/Web/API/AuthenticatorAttestationResponse
Expand All @@ -646,10 +650,8 @@ export interface FidoRegistrationOutputValue {
trigger: string;
}

export interface AssertionValue extends Omit<
PublicKeyCredential,
'rawId' | 'response' | 'getClientExtensionResults' | 'toJSON'
> {
export interface AssertionValue
extends Omit<PublicKeyCredential, 'rawId' | 'response' | 'getClientExtensionResults' | 'toJSON'> {
rawId: string;
response: {
// https://developer.mozilla.org/en-US/docs/Web/API/AuthenticatorAssertionResponse
Expand Down
53 changes: 50 additions & 3 deletions packages/davinci-client/src/lib/collector.utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import type {
InferActionCollectorType,
NoValueCollectorTypes,
InferNoValueCollectorType,
ValidatedBooleanCollector,
ValidatedSingleValueCollectorWithValue,
ValidatedTextCollector,
InferValueObjectCollectorType,
Expand Down Expand Up @@ -45,6 +46,7 @@ import type {
PollingField,
ReadOnlyField,
RedirectField,
SingleCheckboxField,
SingleSelectField,
StandardField,
ValidatedField,
Expand Down Expand Up @@ -151,7 +153,7 @@ export function returnSubmitCollector(field: StandardField, idx: number) {
* @returns {SingleValueCollector} The constructed SingleValueCollector object.
*/
export function returnSingleValueCollector<
Field extends StandardField | SingleSelectField | ValidatedField,
Field extends StandardField | SingleSelectField | ValidatedField | SingleCheckboxField,
CollectorType extends SingleValueCollectorTypes = 'SingleValueCollector',
>(field: Field, idx: number, collectorType: CollectorType, data?: string) {
let error = '';
Expand Down Expand Up @@ -212,10 +214,40 @@ export function returnSingleValueCollector<
options: options,
},
} as InferSingleValueCollectorType<'SingleSelectCollector'>;
} else if (collectorType === 'ValidatedBooleanCollector') {
const validationArray = [];
if ('required' in field && field.required === true) {
validationArray.push({
type: 'required',
message:
('validation' in field && field.validation?.errorMessage) || 'Value cannot be empty',
rule: true,
});
}

return {
category: 'ValidatedSingleValueCollector',
error: error || null,
type: collectorType,
id: `${field.key}-${idx}`,
name: field.key,
input: {
key: field.key,
value: false,
type: field.type,
validation: validationArray,
},
output: {
key: field.key,
label: field.label,
type: field.type,
value: false,
},
} as ValidatedSingleValueCollectorWithValue<'ValidatedBooleanCollector'>;
} else if ('validation' in field || 'required' in field) {
const validationArray = [];

if ('validation' in field) {
if ('validation' in field && field.validation && 'regex' in field.validation) {
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 11, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Critical: Validation logic regression for required-only text fields.

The condition on line 250 has been changed from:

} else if ('validation' in field || 'required' in field) {

to:

} else if ('validation' in field && field.validation && 'regex' in field.validation) {

This is a breaking change that affects existing functionality:

Before: Text fields with either validation OR required: true would receive validation handling and return a ValidatedTextCollector.

After: Only text fields with regex validation enter this branch. Text fields with only required: true (no regex) will skip this branch entirely and fall through to the generic SingleValueCollector at line 285, losing their validation behavior.

Impact: Required-only text fields will no longer be validated, which is a regression.

Root cause: The change appears to be an unintended side effect of adding the ValidatedBooleanCollector branch above.

🔧 Proposed fix

Restore the original OR logic to preserve validation for required-only fields:

-  } else if ('validation' in field && field.validation && 'regex' in field.validation) {
+  } else if ('validation' in field || 'required' in field) {
     const validationArray = [];
 
     if ('validation' in field && field.validation && 'regex' in field.validation) {

The inner conditions at lines 250-256 and 257-263 already safely check for the presence of regex and required respectively, so the outer condition should use OR.

🤖 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 `@packages/davinci-client/src/lib/collector.utils.ts` at line 250, The new AND
check causes required-only text fields to skip validation; revert the outer
conditional in the text-field branch to use OR (i.e. change "else if
('validation' in field && field.validation && 'regex' in field.validation)" back
to "else if ('validation' in field || 'required' in field)") so that fields with
either validation or required:true are handled by the
ValidatedTextCollector/validation logic (the inner branches already distinguish
regex vs required), preventing them from falling through to
SingleValueCollector.

validationArray.push({
type: 'regex',
message: field.validation?.errorMessage || '',
Expand Down Expand Up @@ -464,6 +496,16 @@ export function returnSingleSelectCollector(field: SingleSelectField, idx: numbe
return returnSingleValueCollector(field, idx, 'SingleSelectCollector', data);
}

/**
* @function returnValidatedBooleanCollector - Creates a ValidatedBooleanCollector object based on the provided field and index.
* @param {SingleCheckboxField} field - The field object containing key, label, type, required, and validation.
* @param {number} idx - The index to be used in the id of the ValidatedBooleanCollector.
* @returns {ValidatedBooleanCollector} The constructed ValidatedBooleanCollector object.
*/
export function returnValidatedBooleanCollector(field: SingleCheckboxField, idx: number) {
return returnSingleValueCollector(field, idx, 'ValidatedBooleanCollector');
}

/**
* @function returnProtectCollector - Creates a ProtectCollector object based on the provided field and index.
* @param {DaVinciField} field - The field object containing key, label, type, and links.
Expand Down Expand Up @@ -846,7 +888,12 @@ export function returnAgreementCollector(field: AgreementField, idx: number): Ag
* @returns {function} - A "validator" function that validates the input value
*/
export function returnValidator(
collector: ValidatedTextCollector | ObjectValueCollectors | MultiValueCollectors | AutoCollectors,
collector:
| ValidatedTextCollector
| ValidatedBooleanCollector
| ObjectValueCollectors
| MultiValueCollectors
| AutoCollectors,
) {
const rules = collector.input.validation;
return (value: string | string[] | Record<string, unknown>) => {
Expand Down
33 changes: 24 additions & 9 deletions packages/davinci-client/src/lib/davinci.types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,17 @@ export type ValidatedField = {
};
};

export type SingleCheckboxField = {
type: 'SINGLE_CHECKBOX';
inputType: 'BOOLEAN';
key: string;
label: string;
required: boolean;
validation?: {
errorMessage: string;
};
};

export type SingleSelectField = {
inputType: 'SINGLE_SELECT';
key: string;
Expand Down Expand Up @@ -185,10 +196,11 @@ export type ProtectField = {
universalDeviceIdentification: boolean;
};

export interface FidoRegistrationOptions extends Omit<
PublicKeyCredentialCreationOptions,
'challenge' | 'user' | 'pubKeyCredParams' | 'excludeCredentials'
> {
export interface FidoRegistrationOptions
extends Omit<
PublicKeyCredentialCreationOptions,
'challenge' | 'user' | 'pubKeyCredParams' | 'excludeCredentials'
> {
challenge: number[];
user: {
id: number[];
Expand Down Expand Up @@ -216,10 +228,8 @@ export type FidoRegistrationField = {
required: boolean;
};

export interface FidoAuthenticationOptions extends Omit<
PublicKeyCredentialRequestOptions,
'challenge' | 'allowCredentials'
> {
export interface FidoAuthenticationOptions
extends Omit<PublicKeyCredentialRequestOptions, 'challenge' | 'allowCredentials'> {
challenge: number[];
allowCredentials?: {
id: number[];
Expand Down Expand Up @@ -260,7 +270,12 @@ export type ComplexValueFields =
export type MultiValueFields = MultiSelectField;
export type ReadOnlyFields = ReadOnlyField | QrCodeField | AgreementField;
export type RedirectFields = RedirectField;
export type SingleValueFields = StandardField | ValidatedField | SingleSelectField | ProtectField;
export type SingleValueFields =
| StandardField
| ValidatedField
| SingleCheckboxField
| SingleSelectField
| ProtectField;

export type DaVinciField =
| ComplexValueFields
Expand Down
Loading
Loading