Issue #PS-1249 feat:UI of form fields to show form #364
Conversation
WalkthroughThe changes introduce a dynamic form functionality featuring custom widgets such as Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (5)
- src/components/CustomRadioWidget.tsx (1 hunks)
- src/components/DynamicForm.tsx (3 hunks)
- src/components/MultiSelectCheckboxes.tsx (1 hunks)
- src/pages/addLearner.tsx (1 hunks)
- src/utils/schema.js (1 hunks)
Additional comments not posted (3)
src/utils/schema.js (3)
1-78: Verify the usage oferrorMessagein the schema.The
errorMessageproperty in thefirstNamefield is not standard JSON Schema and may require additional handling in the form rendering logic.
80-96: LGTM! Verify custom widget integration.The UI schema correctly uses custom widgets to enhance form rendering.
Ensure that the custom widgets (
CustomCheckboxWidget,CustomRadioWidget,MultiSelectCheckboxes) are properly implemented and integrated with the form renderer.
98-215: LGTM! Verify alignment with backend API.The
formReadResponseobject is comprehensive and includes all necessary details for form rendering and validation.Ensure that the endpoint (
/user/v1/add-facilitator) and field definitions align with the backend API requirements.
| interface CustomRadioWidgetProps { | ||
| value: string; | ||
| id: string; | ||
| required: boolean; | ||
| disabled: boolean; | ||
| readonly: boolean; | ||
| label: string; | ||
| options: any; | ||
| onChange: any; | ||
| } |
There was a problem hiding this comment.
Ensure correct typing for options and onChange.
The options and onChange props are typed as any. It is better to provide specific types to ensure type safety and better code readability.
interface CustomRadioWidgetProps {
value: string;
id: string;
required: boolean;
disabled: boolean;
readonly: boolean;
label: string;
options: {
enumOptions: { value: string; label: string }[];
};
onChange: (value: string) => void;
}| const CustomRadioWidget: React.FC<WidgetProps> = ({ | ||
| id, | ||
| value, | ||
| required, | ||
| disabled, | ||
| readonly, | ||
| label, | ||
| options, | ||
| onChange, | ||
| }) => { |
There was a problem hiding this comment.
Use destructuring for options.enumOptions.
Destructuring options directly in the function parameters can improve readability.
const CustomRadioWidget: React.FC<WidgetProps & CustomRadioWidgetProps> = ({
id,
value,
required,
disabled,
readonly,
label,
options: { enumOptions },
onChange,
}) => {| const handleChange = (event: any) => { | ||
| onChange(event.target.value); | ||
| }; |
There was a problem hiding this comment.
Handle onChange more robustly.
Ensure that onChange is always called with a string value.
const handleChange = (event: React.ChangeEvent<HTMLInputElement>) => {
onChange(event.target.value);
};| return ( | ||
| <div> | ||
| <FormLabel component="legend">{label}</FormLabel> | ||
| <RadioGroup id={id} value={value} onChange={handleChange} row> | ||
| {options?.enumOptions?.map((option: any) => ( | ||
| <FormControlLabel | ||
| key={option.value} | ||
| value={option.value} | ||
| control={<Radio />} | ||
| label={option.label} | ||
| disabled={disabled || readonly} | ||
| /> | ||
| ))} | ||
| </RadioGroup> | ||
| </div> | ||
| ); |
There was a problem hiding this comment.
Add id attribute to the FormLabel.
To improve accessibility, link the FormLabel to the RadioGroup using the htmlFor attribute.
return (
<div>
<FormLabel component="legend" htmlFor={id}>{label}</FormLabel>
<RadioGroup id={id} value={value} onChange={handleChange} row>
{enumOptions?.map((option) => (
<FormControlLabel
key={option.value}
value={option.value}
control={<Radio />}
label={option.label}
disabled={disabled || readonly}
/>
))}
</RadioGroup>
</div>
);| import DynamicForm from '@/components/DynamicForm'; | ||
| import React from 'react'; | ||
| import { schema, uiSchema } from '@/utils/schema'; |
There was a problem hiding this comment.
Optimize imports.
Avoid using relative paths for imports when aliases are available.
import DynamicForm from '@/components/DynamicForm';
import { schema, uiSchema } from '@/utils/schema';| import React, { useRef } from 'react'; | ||
| import { IChangeEvent } from '@rjsf/core'; | ||
| import ISubmitEvent from '@rjsf/core'; | ||
| import validator from '@rjsf/validator-ajv8'; | ||
| import { Theme as MaterialUITheme } from '@rjsf/mui'; | ||
| import { withTheme } from '@rjsf/core'; | ||
| import { RJSFSchema } from '@rjsf/utils'; | ||
| import MultiSelectCheckboxes from './MultiSelectCheckboxes'; | ||
| import CustomCheckboxWidget from './CustomCheckboxWidget'; | ||
| import CustomRadioWidget from './CustomRadioWidget'; | ||
| import CustomErrorList from './CustomErrorList'; | ||
| import { RJSFSchema, WidgetProps } from '@rjsf/utils'; |
There was a problem hiding this comment.
Optimize imports.
Avoid using relative paths for imports when aliases are available.
import { IChangeEvent, ISubmitEvent, withTheme } from '@rjsf/core';
import validator from '@rjsf/validator-ajv8';
import { Theme as MaterialUITheme } from '@rjsf/mui';
import { RJSFSchema, WidgetProps } from '@rjsf/utils';
import MultiSelectCheckboxes from '@/components/MultiSelectCheckboxes';
import CustomCheckboxWidget from '@/components/CustomCheckboxWidget';
import CustomRadioWidget from '@/components/CustomRadioWidget';
import CustomErrorList from '@/components/CustomErrorList';| widgets: { | ||
| [key: string]: React.FC<WidgetProps<any, RJSFSchema, any>>; | ||
| }; |
There was a problem hiding this comment.
Ensure correct typing for widgets.
The widgets property in DynamicFormProps should have a more specific type.
widgets: {
MultiSelectCheckboxes: typeof MultiSelectCheckboxes;
CustomCheckboxWidget: typeof CustomCheckboxWidget;
CustomRadioWidget: typeof CustomRadioWidget;
};| const widgets = { | ||
| MultiSelectCheckboxes: MultiSelectCheckboxes, | ||
| CustomCheckboxWidget: CustomCheckboxWidget, | ||
| CustomRadioWidget: CustomRadioWidget, | ||
| }; |
There was a problem hiding this comment.
Use shorthand property names.
Shorthand property names can improve readability.
const widgets = {
MultiSelectCheckboxes,
CustomCheckboxWidget,
CustomRadioWidget,
};| const handleError = (errors: any) => { | ||
| if (errors.length > 0) { | ||
| // Adjust the selector based on the actual structure of the form element names | ||
| const property = errors[0].property.replace(/^root\./, ''); | ||
| const errorField = document.querySelector( | ||
| `[name$="${property}"]` | ||
| ) as HTMLElement; | ||
|
|
||
| if (errorField) { | ||
| errorField.focus(); | ||
| } else { | ||
| // If the name-based selector fails, try to select by ID as a fallback | ||
| const fallbackField = document.getElementById(property) as HTMLElement; | ||
| if (fallbackField) { | ||
| fallbackField.focus(); | ||
| } | ||
| } | ||
| } | ||
| onError(errors); | ||
| }; |
There was a problem hiding this comment.
Improve error handling logic.
The handleError function can be simplified and made more robust.
const handleError = (errors: any) => {
if (errors.length > 0) {
const property = errors[0].property.replace(/^root\./, '');
const errorField = document.querySelector(`[name$="${property}"]`) as HTMLElement;
if (errorField) {
errorField.focus();
} else {
const fallbackField = document.getElementById(property) as HTMLElement;
if (fallbackField) {
fallbackField.focus();
}
}
}
onError(errors);
};|
|
||
| return ( | ||
| <FormWithMaterialUI | ||
| schema={schema} | ||
| uiSchema={uiSchema} | ||
| formData={formData} | ||
| onChange={onChange} | ||
| onSubmit={onSubmit} | ||
| onError={onError} | ||
| validator={validator} | ||
| liveValidate | ||
| showErrorList={false} | ||
| widgets={widgets} | ||
| noHtml5Validate | ||
| onError={handleError} | ||
| // ErrorList={CustomErrorList} |
There was a problem hiding this comment.
Improve form rendering.
Consider using a more responsive layout for the form.
return (
<FormWithMaterialUI
schema={schema}
uiSchema={uiSchema}
formData={formData}
onChange={onChange}
onSubmit={onSubmit}
validator={validator}
liveValidate
showErrorList={false}
widgets={widgets}
noHtml5Validate
onError={handleError}
// ErrorList={CustomErrorList}
/>
);
Description
Please describe the changes in your pull request in few words here.
Changes
List the changes done to fix a bug or introducing a new feature.
How to test
Describe the steps required to test the changes proposed in the pull request.
Please consider using the closing keyword if the pull request is proposed to
fix an issue already created in the repository
(https://help.github.com/articles/closing-issues-using-keywords/)
Summary by CodeRabbit
New Features
CustomRadioWidgetcomponent for custom radio button functionality.MultiSelectCheckboxescomponent for multi-select checkbox functionality.addLearnercomponent to render a dynamic form for learner registration.Bug Fixes
Enhancements
DynamicFormcomponent to include custom widgets and enhanced error handling.