Skip to content

Extend FormRenderer to support all 28 OpenStack standard field types #180

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

Merged
hotlong merged 5 commits into from
Jan 24, 2026
Merged

Extend FormRenderer to support all 28 OpenStack standard field types #180

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a4ba21c
Initial plan
Copilot Jan 24, 2026
6a8d8e5
Add support for all field types in FieldFactory
Copilot Jan 24, 2026
21c8952
Fix TypeScript types for extended field properties
Copilot Jan 24, 2026
ace48b3
Improve multiple select handling and update documentation
Copilot Jan 24, 2026
8668ee2
Refactor to use helper function and fix TypeScript types
Copilot Jan 24, 2026
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
283 changes: 278 additions & 5 deletions packages/react/src/components/form/FieldFactory.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,21 @@ import React from 'react';
import { UseFormReturn } from 'react-hook-form';
import type { FormField } from '@objectstack/spec/ui';

Comment on lines 10 to 12
Copy link

Copilot AI Jan 24, 2026

Choose a reason for hiding this comment

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

The PR title mentions "OpenStack" but the correct product name is "ObjectStack" (based on the package names @objectstack/spec and repository context). This appears to be a typo in the PR title/description, though not in the actual code.

Copilot uses AI. Check for mistakes.
/**
* Extended form field with additional properties for complex widgets
* These properties are not part of the standard FormField schema but may be
* provided by specific implementations
*/
export interface ExtendedFormField extends FormField {
multiple?: boolean;
accept?: string[];
options?: Array<{
label: string;
value: string;
disabled?: boolean;
}>;
}
Comment on lines +13 to +26
Copy link

Copilot AI Jan 24, 2026

Choose a reason for hiding this comment

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

The ExtendedFormField interface duplicates properties that already exist in the underlying field metadata types from @object-ui/types. For example, FileFieldMetadata, ImageFieldMetadata, LookupFieldMetadata, and SelectFieldMetadata already define multiple, accept, and options properties. This creates ambiguity about which definition takes precedence and could lead to inconsistencies. Consider either: 1) importing and reusing the existing metadata types from @object-ui/types, or 2) clearly documenting that ExtendedFormField is a bridge interface and explaining the relationship.

Copilot uses AI. Check for mistakes.

export interface FieldFactoryProps {
/**
* Field configuration from FormFieldSchema
Expand All @@ -36,11 +51,22 @@ export const FieldFactory: React.FC<FieldFactoryProps> = ({
}) => {
const { register, formState: { errors } } = methods;

// Cast to extended field for properties not in base schema
const extendedField = field as ExtendedFormField;

// Determine the widget type
const widgetType = field.widget || 'text';
const fieldName = field.field;
const error = errors[fieldName];

// Helper function to handle multiple select value conversion
const handleMultipleSelectValue = (value: any) => {
if (extendedField.multiple && value instanceof HTMLCollection) {
return Array.from(value as HTMLCollectionOf<HTMLOptionElement>).map((opt) => opt.value);
}
Comment on lines +64 to +66
Copy link

Copilot AI Jan 24, 2026

Choose a reason for hiding this comment

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

The handleMultipleSelectValue function checks if value is an instance of HTMLCollection, but when react-hook-form calls setValueAs, the value passed is typically the selected options, not an HTMLCollection. This logic may not work as intended. Consider testing this thoroughly or using a different approach, such as handling the conversion in an onChange handler.

Suggested change
if (extendedField.multiple && value instanceof HTMLCollection) {
return Array.from(value as HTMLCollectionOf<HTMLOptionElement>).map((opt) => opt.value);
}
// If the field is not configured for multiple values, return as-is
if (!extendedField.multiple) {
return value;
}
// If value is already an array (e.g., react-hook-form has normalized it), use it directly
if (Array.isArray(value)) {
return value;
}
if (value && typeof value === 'object') {
// If we were passed a select element or similar, prefer its selectedOptions/options collections
const collection =
(value as any).selectedOptions ||
(value as any).options ||
value;
// Treat any array-like collection with length and item() (HTMLCollection, HTMLOptionsCollection, NodeList)
if (
collection &&
typeof (collection as any).length === 'number' &&
typeof (collection as any).item === 'function'
) {
return Array.from(collection as ArrayLike<HTMLOptionElement>).map(
(opt) => (opt as HTMLOptionElement).value,
);
}
}

Copilot uses AI. Check for mistakes.
return value;
};

// Handle conditional visibility
// Note: visibleOn expression evaluation is not yet implemented
// Fields are always visible unless explicitly hidden
Expand Down Expand Up @@ -151,20 +177,23 @@ export const FieldFactory: React.FC<FieldFactoryProps> = ({

case 'select':
case 'dropdown':
// Note: This is a basic implementation without options support
// To properly support select fields, options would need to be passed
// via an extended FormField schema or external configuration
return renderField(
<select
id={fieldName}
disabled={disabled}
multiple={extendedField.multiple}
className="w-full px-3 py-2 border border-gray-300 rounded-md shadow-sm focus:outline-none focus:ring-2 focus:ring-blue-500 focus:border-blue-500 disabled:bg-gray-100 disabled:cursor-not-allowed"
{...register(fieldName, {
required: field.required ? `${field.label || fieldName} is required` : false,
setValueAs: handleMultipleSelectValue,
})}
>
<option value="">{field.placeholder || 'Select an option'}</option>
{/* TODO: Add options support - requires extending FormField schema or external options provider */}
{!extendedField.multiple && <option value="">{field.placeholder || 'Select an option'}</option>}
{extendedField.options?.map((option) => (
<option key={option.value} value={option.value} disabled={option.disabled}>
{option.label}
</option>
))}
</select>
);

Expand Down Expand Up @@ -211,6 +240,250 @@ export const FieldFactory: React.FC<FieldFactoryProps> = ({
/>
);

case 'currency':
return renderField(
<div className="relative">
<span className="absolute left-3 top-2 text-gray-500">$</span>
<input
id={fieldName}
type="number"
placeholder={field.placeholder}
disabled={disabled}
step="0.01"
className="w-full pl-8 pr-3 py-2 border border-gray-300 rounded-md shadow-sm focus:outline-none focus:ring-2 focus:ring-blue-500 focus:border-blue-500 disabled:bg-gray-100 disabled:cursor-not-allowed"
{...register(fieldName, {
required: field.required ? `${field.label || fieldName} is required` : false,
valueAsNumber: true,
})}
/>
</div>
);
Comment on lines +243 to +260
Copy link

Copilot AI Jan 24, 2026

Choose a reason for hiding this comment

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

The currency field hardcodes the dollar sign ($) symbol, but the field-types.ts definition includes a currency property to specify different currencies (e.g., EUR, GBP). Consider using the currency property from the field configuration to display the appropriate currency symbol dynamically, or at minimum document this limitation.

Copilot uses AI. Check for mistakes.

case 'percent':
return renderField(
<div className="relative">
<input
id={fieldName}
type="number"
placeholder={field.placeholder}
disabled={disabled}
step="0.01"
min="0"
max="100"
className="w-full pr-8 pl-3 py-2 border border-gray-300 rounded-md shadow-sm focus:outline-none focus:ring-2 focus:ring-blue-500 focus:border-blue-500 disabled:bg-gray-100 disabled:cursor-not-allowed"
{...register(fieldName, {
required: field.required ? `${field.label || fieldName} is required` : false,
valueAsNumber: true,
})}
/>
<span className="absolute right-3 top-2 text-gray-500">%</span>
</div>
);
Comment on lines +262 to +281
Copy link

Copilot AI Jan 24, 2026

Choose a reason for hiding this comment

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

The percent field hardcodes min="0" and max="100", but according to PercentFieldMetadata in field-types.ts, the min and max values should be configurable. Some use cases may require negative percentages or values above 100% (e.g., growth rates). Consider reading min/max from the field configuration instead of hardcoding these values.

Copilot uses AI. Check for mistakes.

case 'phone':
return renderField(
<input
id={fieldName}
type="tel"
placeholder={field.placeholder || '+1 (555) 000-0000'}
disabled={disabled}
className="w-full px-3 py-2 border border-gray-300 rounded-md shadow-sm focus:outline-none focus:ring-2 focus:ring-blue-500 focus:border-blue-500 disabled:bg-gray-100 disabled:cursor-not-allowed"
{...register(fieldName, {
required: field.required ? `${field.label || fieldName} is required` : false,
})}
/>
);

case 'markdown':
return renderField(
<textarea
id={fieldName}
placeholder={field.placeholder || 'Enter markdown text...'}
disabled={disabled}
rows={8}
className="w-full px-3 py-2 border border-gray-300 rounded-md shadow-sm focus:outline-none focus:ring-2 focus:ring-blue-500 focus:border-blue-500 disabled:bg-gray-100 disabled:cursor-not-allowed font-mono"
{...register(fieldName, {
required: field.required ? `${field.label || fieldName} is required` : false,
})}
/>
);

case 'html':
return renderField(
<textarea
id={fieldName}
placeholder={field.placeholder || 'Enter HTML...'}
disabled={disabled}
rows={8}
className="w-full px-3 py-2 border border-gray-300 rounded-md shadow-sm focus:outline-none focus:ring-2 focus:ring-blue-500 focus:border-blue-500 disabled:bg-gray-100 disabled:cursor-not-allowed font-mono"
{...register(fieldName, {
required: field.required ? `${field.label || fieldName} is required` : false,
})}
/>
);

case 'file':
return renderField(
<input
id={fieldName}
type="file"
disabled={disabled}
multiple={extendedField.multiple}
accept={extendedField.accept?.join(',')}
className="w-full px-3 py-2 border border-gray-300 rounded-md shadow-sm focus:outline-none focus:ring-2 focus:ring-blue-500 focus:border-blue-500 disabled:bg-gray-100 disabled:cursor-not-allowed file:mr-4 file:py-2 file:px-4 file:rounded-md file:border-0 file:text-sm file:font-semibold file:bg-blue-50 file:text-blue-700 hover:file:bg-blue-100"
{...register(fieldName, {
required: field.required ? `${field.label || fieldName} is required` : false,
})}
/>
);

case 'image':
return renderField(
<input
id={fieldName}
type="file"
disabled={disabled}
multiple={extendedField.multiple}
accept={extendedField.accept?.join(',') || 'image/*'}
className="w-full px-3 py-2 border border-gray-300 rounded-md shadow-sm focus:outline-none focus:ring-2 focus:ring-blue-500 focus:border-blue-500 disabled:bg-gray-100 disabled:cursor-not-allowed file:mr-4 file:py-2 file:px-4 file:rounded-md file:border-0 file:text-sm file:font-semibold file:bg-blue-50 file:text-blue-700 hover:file:bg-blue-100"
{...register(fieldName, {
required: field.required ? `${field.label || fieldName} is required` : false,
})}
/>
);
Comment on lines +325 to +353
Copy link

Copilot AI Jan 24, 2026

Choose a reason for hiding this comment

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

The file and image fields do not validate file sizes (max_size) or number of files (max_files) as defined in FileFieldMetadata and ImageFieldMetadata. This could allow users to upload excessively large files or more files than intended. Consider adding client-side validation using the maxSize and maxFiles properties from the extended field configuration.

Copilot uses AI. Check for mistakes.

case 'location':
return renderField(
<div className="space-y-2">
<input
id={`${fieldName}-lat`}
type="number"
placeholder="Latitude"
disabled={disabled}
step="any"
className="w-full px-3 py-2 border border-gray-300 rounded-md shadow-sm focus:outline-none focus:ring-2 focus:ring-blue-500 focus:border-blue-500 disabled:bg-gray-100 disabled:cursor-not-allowed"
{...register(`${fieldName}.lat`, {
required: field.required ? 'Latitude is required' : false,
valueAsNumber: true,
})}
/>
<input
id={`${fieldName}-lng`}
type="number"
placeholder="Longitude"
disabled={disabled}
step="any"
className="w-full px-3 py-2 border border-gray-300 rounded-md shadow-sm focus:outline-none focus:ring-2 focus:ring-blue-500 focus:border-blue-500 disabled:bg-gray-100 disabled:cursor-not-allowed"
{...register(`${fieldName}.lng`, {
required: field.required ? 'Longitude is required' : false,
valueAsNumber: true,
})}
/>
</div>
);
Comment on lines +355 to +383
Copy link

Copilot AI Jan 24, 2026

Choose a reason for hiding this comment

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

The location field registers two separate form fields (field.lat and field.lng) using nested paths, but this creates a structure where the value is stored as an object { lat: number, lng: number }. This may not match the expected data structure for location fields. Consider documenting this behavior clearly or ensuring it aligns with how location data is typically represented in the ObjectStack ecosystem.

Copilot uses AI. Check for mistakes.
Comment on lines +355 to +383
Copy link

Copilot AI Jan 24, 2026

Choose a reason for hiding this comment

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

The location field creates two input elements for latitude and longitude, but they lack proper labels. The inputs only have placeholder text, which is insufficient for screen readers. Each input should have an associated label element or aria-label attribute for proper accessibility. Consider wrapping each input with a label or adding aria-label attributes.

Copilot uses AI. Check for mistakes.

case 'lookup':
case 'master_detail':
return renderField(
<select
id={fieldName}
disabled={disabled}
multiple={extendedField.multiple}
className="w-full px-3 py-2 border border-gray-300 rounded-md shadow-sm focus:outline-none focus:ring-2 focus:ring-blue-500 focus:border-blue-500 disabled:bg-gray-100 disabled:cursor-not-allowed"
{...register(fieldName, {
required: field.required ? `${field.label || fieldName} is required` : false,
setValueAs: handleMultipleSelectValue,
})}
>
{!extendedField.multiple && <option value="">{field.placeholder || 'Select an option'}</option>}
{extendedField.options?.map((option) => (
<option key={option.value} value={option.value} disabled={option.disabled}>
{option.label}
</option>
))}
</select>
);

case 'user':
case 'owner':
return renderField(
<select
id={fieldName}
disabled={disabled}
multiple={extendedField.multiple}
className="w-full px-3 py-2 border border-gray-300 rounded-md shadow-sm focus:outline-none focus:ring-2 focus:ring-blue-500 focus:border-blue-500 disabled:bg-gray-100 disabled:cursor-not-allowed"
{...register(fieldName, {
required: field.required ? `${field.label || fieldName} is required` : false,
setValueAs: handleMultipleSelectValue,
})}
>
{!extendedField.multiple && <option value="">{field.placeholder || 'Select user'}</option>}
{extendedField.options?.map((option) => (
<option key={option.value} value={option.value} disabled={option.disabled}>
{option.label}
</option>
))}
</select>
);

case 'formula':
case 'summary':
case 'auto_number':
// Read-only computed fields - display as disabled text input
return renderField(
<input
id={fieldName}
type="text"
disabled={true}
className="w-full px-3 py-2 border border-gray-300 rounded-md shadow-sm bg-gray-50 cursor-not-allowed text-gray-600"
{...register(fieldName)}
/>
);

case 'object':
return renderField(
<textarea
id={fieldName}
placeholder={field.placeholder || 'Enter JSON object...'}
disabled={disabled}
rows={6}
className="w-full px-3 py-2 border border-gray-300 rounded-md shadow-sm focus:outline-none focus:ring-2 focus:ring-blue-500 focus:border-blue-500 disabled:bg-gray-100 disabled:cursor-not-allowed font-mono text-sm"
{...register(fieldName, {
required: field.required ? `${field.label || fieldName} is required` : false,
validate: (value) => {
if (!value) return true;
try {
JSON.parse(value);
return true;
} catch (e) {
return 'Invalid JSON format';
}
},
})}
/>
);
Comment on lines +443 to +464
Copy link

Copilot AI Jan 24, 2026

Choose a reason for hiding this comment

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

The object field validates JSON syntax but does not validate against the optional schema property defined in ObjectFieldMetadata. If a schema is provided in the field configuration, the JSON should be validated against it to ensure data integrity.

Copilot uses AI. Check for mistakes.

case 'vector':
// Vector fields are typically read-only or require specialized input
return renderField(
<input
id={fieldName}
type="text"
placeholder={field.placeholder || 'Vector data (read-only)'}
disabled={true}
className="w-full px-3 py-2 border border-gray-300 rounded-md shadow-sm bg-gray-50 cursor-not-allowed text-gray-600"
{...register(fieldName)}
/>
);

case 'grid':
// Grid fields require complex table/grid editor - placeholder for now
return renderField(
<div className="w-full px-3 py-2 border border-gray-300 rounded-md bg-gray-50 text-gray-600">
<p className="text-sm">Grid editor not yet implemented</p>
</div>
);

default:
// Default to text input
return renderField(
Expand Down
Loading
Loading