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
8 changes: 8 additions & 0 deletions javascript/reactjs-todo-davinci/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,20 @@ This sample code is provided "as is" and is not a supported product of Ping Iden
- SingleSelectCollector
- ReadOnlyCollector
- PhoneNumberCollector
- PhoneNumberExtensionCollector
- DeviceRegistrationCollector
- DeviceAuthenticationCollector
- FidoRegistrationCollector
- FidoAuthenticationCollector
- IdpCollector
- SubmitCollector
- FlowCollector
- ProtectCollector
- QrCodeCollector
- RichTextCollector
- BooleanCollector
- ValidatedBooleanCollector
- PollingCollector

## Requirements

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
* ping-sample-web-react-davinci
*
* boolean.js
*
* 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 React, { useState } from 'react';
import { interpolateRichContent } from '../utilities/rich-content';

export default function BooleanComponent({ collector, inputName, updater }) {
const [isChecked, setIsChecked] = useState(collector.output.value ?? false);

const fieldId = collector.output.key || `${inputName}-checkbox-field`;

const { richContent } = collector.output;
const label = richContent?.replacements?.length
? interpolateRichContent(richContent)
: collector.output.label || '';

const required = collector.input.validation?.some(
(validation) => validation.type === 'required' && validation.rule === true,
);
Comment on lines +24 to +26

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Guard collector.input before accessing validation.

This can throw at runtime when collector.input is absent. Add optional chaining at input level and default to false to keep rendering resilient.

Proposed fix
-  const required = collector.input.validation?.some(
+  const required = collector.input?.validation?.some(
     (validation) => validation.type === 'required' && validation.rule === true,
-  );
+  ) ?? false;
🤖 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 `@javascript/reactjs-todo-davinci/client/components/davinci-client/boolean.js`
around lines 24 - 26, The code accesses collector.input.validation without
verifying that collector.input exists first, which can cause a runtime error
when collector.input is absent. Add optional chaining at the input level before
accessing validation, and provide a default value of false to ensure the
required constant has a safe fallback when the input chain is broken. Update the
assignment to the required constant to use optional chaining syntax with a
logical OR to default to false.


const handleChange = (event) => {
const value = event.target.checked;
setIsChecked(value);
updater(value);
};

return (
<div className={'mb-5 form-check'}>
<label htmlFor={fieldId} className="form-label">
{label}
</label>
<input
type="checkbox"
id={fieldId}
name={fieldId}
checked={isChecked}
onChange={handleChange}
className="form-check-input"
required={required}
/>
</div>
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/*
* ping-sample-web-react-davinci
*
* device.js
*
* 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 React, { useEffect, useContext, useState } from 'react';
import { ThemeContext } from '../../context/theme.context.js';

export default function DeviceComponent({ collector, updater }) {
const [selectedDevice, setSelectedDevice] = useState(
collector.output.options?.[0]?.value ?? null,
);
const theme = useContext(ThemeContext);

useEffect(() => {
if (selectedDevice) {
const result = updater(selectedDevice);
if (result && 'error' in result) {
console.error('Failed to update device', result.error);
}
}
}, [selectedDevice, updater]);

return (
<div className="d-flex flex-column align-items-center mt-2 mb-2">
<label
htmlFor="device-select"
className={`form-label cstm_subhead-text mb-4 fw-bold text-center ${theme.textMutedClass}`}
>
{collector.output.label || 'select an option'}
</label>
<select
id="device-select"
className="form-select form-select-lg w-100"
Comment on lines +32 to +39

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 | 🟡 Minor | ⚡ Quick win

Use a unique id per DeviceComponent instance.

Line 32 and Line 38 hard-code device-select; multiple rendered collectors will duplicate DOM ids and break label targeting/accessibility. Generate an instance-specific id (e.g., from collector metadata or useId()).

Suggested fix
 import React, { useEffect, useContext, useState } from 'react';
+import { useId } from 'react';
@@
 export default function DeviceComponent({ collector, updater }) {
+  const deviceSelectId = useId();
@@
       <label
-        htmlFor="device-select"
+        htmlFor={deviceSelectId}
@@
       <select
-        id="device-select"
+        id={deviceSelectId}
🤖 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 `@javascript/reactjs-todo-davinci/client/components/davinci-client/device.js`
around lines 32 - 39, The label and select element in the DeviceComponent both
hard-code the id "device-select", which creates duplicate DOM IDs when multiple
instances are rendered and breaks accessibility. Generate a unique id per
component instance using React's useId() hook, then replace the hard-coded
"device-select" string in both the label's htmlFor attribute and the select
element's id attribute with this generated unique identifier.

value={selectedDevice}
onChange={(event) => setSelectedDevice(event.target.value)}
>
{collector.output.options?.map((option) => (
<option key={option.value + option.label} value={option.value}>
{option.label}
</option>
))}
</select>
<button type="submit" className="mt-5 justify-content w-100 btn btn-primary">
Next
</button>
</div>
);
}
Original file line number Diff line number Diff line change
@@ -1,3 +1,13 @@
/*
* ping-sample-web-react-davinci
*
* error.js
*
* Copyright (c) 2025 - 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 React, { useEffect, useState } from 'react';

export default function Error({ getError }) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,21 @@
*/
import React, { Fragment, useContext, useEffect, useState } from 'react';
import { useNavigate } from 'react-router-dom';
import Readonly from './readonly.js';
import ReadOnly from './readonly.js';
import Text from './text.js';
import Error from './error.js';
import Password from './password.js';
import SocialLoginButton from './social-login-button.js';
import SubmitButton from './submit-button.js';
import Protect from './protect.js';
import ObjectValueComponent from './object-value.js';
import PhoneNumberComponent from './phone-number.js';
import DeviceComponent from './device.js';
import SingleSelect from './single-select.js';
import FlowLink from './flow-link.js';
import FidoComponent from './fido.js';
import PollingComponent from './polling.js';
import BooleanComponent from './boolean.js';
import QrCode from './qr-code.js';
import Unknown from './unknown.js';
import Alert from './alert.js';
import KeyIcon from '../icons/key-icon';
Expand All @@ -30,7 +35,6 @@ import { ProtectContext } from '../../context/protect.context.js';
import { ThemeContext } from '../../context/theme.context.js';
import useDavinci from './hooks/davinci.hook.js';
import useOAuth from './hooks/oauth.hook.js';
import FidoComponent from './fido.js';

/**
* @function Form - React view for managing the user authentication journey
Expand Down Expand Up @@ -58,7 +62,7 @@ export default function Form() {
const [user, setCode] = useOAuth();
const [
{ formName, formAction, node, collectors },
{ getError, setNext, startNewFlow, updater, externalIdp },
{ getError, setNext, startNewFlow, updater, pollStatus, externalIdp },
] = useDavinci();

/**
Expand Down Expand Up @@ -169,17 +173,35 @@ export default function Form() {
case 'ERROR_DISPLAY':
return <Error key={idx + 'err'} getError={getError} />;
case 'ReadOnlyCollector':
return <Readonly key={idx + collectorName} collector={collector} />;
case 'RichTextCollector':
return <ReadOnly key={idx + collectorName} collector={collector} />;
case 'BooleanCollector':
case 'ValidatedBooleanCollector':
return (
<BooleanComponent

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We aren't passing in the collectorName here but we use the prop in boolean.js

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@ancheetah I think this is the only outstanding thing from my review.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nevermind, I see we removed the prop

key={idx + collectorName}
collector={collector}
updater={updater(collector)}
inputName={idx + collectorName}
/>
);
case 'PhoneNumberCollector':
case 'PhoneNumberExtensionCollector':
return (
<PhoneNumberComponent
inputName={idx + collectorName}
collector={collector}
updater={updater(collector)}
key={idx + collectorName}
/>
);
case 'DeviceRegistrationCollector':
case 'DeviceAuthenticationCollector':
return (
<ObjectValueComponent
inputName={collectorName}
<DeviceComponent
collector={collector}
updater={updater(collector)}
key={collectorName}
submitForm={setNext}
key={idx + collectorName}
/>
);
case 'IdpCollector':
Expand All @@ -201,8 +223,21 @@ export default function Form() {
submitForm={setNext}
/>
);
case 'PollingCollector':
return (
<PollingComponent
collector={collector}
updater={updater(collector)}
pollStatus={pollStatus(collector)}
cacheKey={node?.cache?.key}
key={collectorName}
Comment on lines +232 to +233

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Does the component use key and cacheKey? If not, can we remove them?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

key is on every component and not necessarily used in the component. It's just for assigning a unique key to components so React doesn't complain. Ideally each of these keys should probably also have the idx appended to them to truly make them unique.

cacheKey is needed off the node to trigger a re-render of the polling component in continue mode. Otherwise the collector component will look exactly the same and not trigger a re-poll.

submitForm={setNext}
/>
);
case 'ProtectCollector':
return <Protect collector={collector} key={collectorName} />;
case 'QrCodeCollector':
return <QrCode collector={collector} key={collectorName} />;
case 'SubmitCollector':
return <SubmitButton collector={collector} isLoading={isLoading} key={collectorName} />;
case 'FlowCollector':
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,14 @@ export default function useDavinci() {
return davinciClient.update(collector);
}

/**
* @function pollStatus - Gets the DaVinci client pollStatus function for a collector
* @returns {function} - A function to start challenge or continue polling
*/
function pollStatus(collector) {
return davinciClient.pollStatus(collector);
}

/**
* @function setNext - Get the next node in the DaVinci flow
* @returns {Promise<void>}
Expand Down Expand Up @@ -148,6 +156,7 @@ export default function useDavinci() {
setNext,
startNewFlow,
updater,
pollStatus,
externalIdp: davinciClient && davinciClient.externalIdp(),
getError: davinciClient && davinciClient.getError,
},
Expand Down

This file was deleted.

Loading
Loading