Use a memoized handler for question type changes

Author: Bharath-K-Shetty

src/components/interactive-builder/modals/question/question-form/question/question.component.tsx

@@ -5,7 +5,7 @@ import RenderTypeComponent from '../rendering-types/rendering-type.component';
 import QuestionTypeComponent from '../question-types/question-type.component';
 import RequiredLabel from '../common/required-label/required-label.component';
 import { useFormField } from '../../form-field-context';
-import { cleanFormFieldForType } from '../../question.modal';
+import { cleanFormFieldForType } from '../../question-utils';
 import type { FormField, RenderType } from '@openmrs/esm-form-engine-lib';
 import { questionTypes, renderTypeOptions, renderingTypes } from '@constants';
 import styles from './question.scss';
@@ -32,6 +32,15 @@ const Question: React.FC<QuestionProps> = ({ checkIfQuestionIdExists }) => {
     return checkIfQuestionIdExists(formField.id);
   }, [formField.id, checkIfQuestionIdExists]);
 
+  const handleQuestionTypeChange = useCallback(
+    (event: React.ChangeEvent<HTMLSelectElement>) => {
+      const newType = event.target.value;
+      setFormField((prevFormField) => {
+        return cleanFormFieldForType({ ...prevFormField, type: newType }, newType);
+      });
+    },
+    [setFormField],
+  );
   return (
     <>
       <TextInput
@@ -63,11 +72,7 @@ const Question: React.FC<QuestionProps> = ({ checkIfQuestionIdExists }) => {
 
       <Select
         value={formField?.type ?? 'control'}
-        onChange={(event) => {
-          const newType = event.target.value;
-          const cleaned = cleanFormFieldForType({ ...formField, type: newType }, newType);
-          setFormField(cleaned);
-        }}
+        onChange={handleQuestionTypeChange}
         id="questionType"
         invalidText={t('typeRequired', 'Type is required')}
         labelText={t('questionType', 'Question type')}

src/components/interactive-builder/modals/question/question-form/question/question.test.tsx

@@ -1,4 +1,4 @@
-import { cleanFormFieldForType } from '../../question.modal';
+import { cleanFormFieldForType } from '../../question-utils';
 import type { FormField } from '@openmrs/esm-form-engine-lib';
 
 describe('cleanFormFieldForType', () => {
@@ -14,32 +14,29 @@ describe('cleanFormFieldForType', () => {
           { concept: '111', label: 'Yes' },
           { concept: '222', label: 'No' },
         ],
-        irrelevantOption: 'remove me',
       },
       required: true,
-      extraProp: 'should be removed',
-    } as any;
+    };
 
-    // Change type from 'obs' to 'control'
+    // Change type from 'obs' to 'control' and update rendering to 'markdown'
     const newType = 'control';
-    const cleaned = cleanFormFieldForType(formField, newType);
+    const cleaned = cleanFormFieldForType(
+      { ...formField, type: newType, questionOptions: { rendering: 'markdown' } },
+      newType,
+    );
 
     // Assert that the type is updated.
     expect(cleaned.type).toBe('control');
 
-    // Assert that required properties remain.
+    // Assert that id remains.
     expect(cleaned.id).toBe('testQuestion');
-    expect(cleaned.label).toBe('Test Question');
 
-    // Irrelevant top-level properties should be removed.
-    expect(cleaned.required).toBeUndefined();
-    expect(cleaned).not.toHaveProperty('extraProp');
+    // In our design, label is required for all question types, so it should remain.
+    expect(cleaned.label).toBe('Test Question');
 
-    // For nested questionOptions, only allowed keys remain for 'control'.
-    // Allowed keys for 'control' in questionOptions are: rendering, minLength, and maxLength.
-    expect(cleaned.questionOptions).toHaveProperty('rendering', 'radio');
+    // Irrelevant nested properties should be removed.
+    expect(cleaned.questionOptions).toHaveProperty('rendering', 'markdown');
     expect(cleaned.questionOptions).not.toHaveProperty('concept');
     expect(cleaned.questionOptions).not.toHaveProperty('answers');
-    expect(cleaned.questionOptions).not.toHaveProperty('irrelevantOption');
   });
 });

src/components/interactive-builder/modals/question/question-utils.ts

@@ -0,0 +1,35 @@
+import type { FormField } from '@openmrs/esm-form-engine-lib';
+import { allowedPropertiesMapping, allowedQuestionOptionsMapping } from '@constants';
+
+/**
+ * Cleans the given questionOptions object by retaining only allowed keys for the new type.
+ */
+function cleanQuestionOptionsForType(options: any, newType: string): any {
+  const allowedOpts = allowedQuestionOptionsMapping[newType] || [];
+  const cleanedOpts = Object.fromEntries(Object.entries(options).filter(([optKey]) => allowedOpts.includes(optKey)));
+  cleanedOpts.rendering = options.rendering;
+  return cleanedOpts;
+}
+
+/**
+ * Cleans the given form field by retaining only allowed top‑level properties for the new type.
+ * Also cleans nested questionOptions using the nested mapping.
+ */
+export function cleanFormFieldForType(field: FormField, newType: string): FormField {
+  const allowedKeys = allowedPropertiesMapping[newType] || [];
+  const cleaned: Partial<FormField> = {};
+
+  allowedKeys.forEach((key) => {
+    if (key in field) {
+      (cleaned as any)[key] = field[key as keyof FormField];
+    }
+  });
+
+  // If questionOptions is allowed and exists, clean it using the nested mapping.
+  if (cleaned.questionOptions && typeof cleaned.questionOptions === 'object') {
+    cleaned.questionOptions = cleanQuestionOptionsForType(cleaned.questionOptions, newType);
+  }
+
+  cleaned.type = newType;
+  return cleaned as FormField;
+}

src/components/interactive-builder/modals/question/question.modal.tsx

@@ -29,86 +29,6 @@ interface QuestionModalProps {
   resetIndices: () => void;
 }
 
-/**
- * Common properties that are required for all question types.
- */
-const requiredProperties: Array<keyof FormField> = ['id', 'label', 'type', 'questionOptions'];
-
-/**
- * Type-specific properties.
- */
-const typeSpecificProperties: Record<string, Array<keyof FormField>> = {
-  control: [],
-  encounterDatetime: ['datePickerFormat'],
-  encounterLocation: [],
-  encounterProvider: [],
-  encounterRole: [],
-  obs: ['required'],
-  obsGroup: ['questions'],
-  patientIdentifier: [],
-  testOrder: [],
-  programState: [],
-};
-
-/**
- * Merge required properties with type-specific ones.
- */
-const allowedPropertiesMapping: Record<string, string[]> = Object.fromEntries(
-  Object.entries(typeSpecificProperties).map(([type, props]) => {
-    const mergedProps = new Set<string>([...requiredProperties, ...props]);
-    return [type, Array.from(mergedProps)];
-  }),
-);
-
-/**
- * Mapping of allowed keys for the nested questionOptions object per question type.
- */
-const allowedQuestionOptionsMapping: Record<string, string[]> = {
-  control: ['rendering', 'minLength', 'maxLength'],
-  encounterDatetime: ['rendering'],
-  encounterLocation: ['rendering'],
-  encounterProvider: ['rendering'],
-  encounterRole: ['rendering', 'isSearchable'],
-  obs: ['rendering', 'concept', 'answers'],
-  obsGroup: ['rendering', 'concept'],
-  patientIdentifier: ['rendering', 'identifierType', 'minLength', 'maxLength'],
-  testOrder: ['rendering'],
-  programState: ['rendering', 'programUuid', 'workflowUuid', 'answers'],
-};
-
-/**
- * Cleans the given questionOptions object by retaining only allowed keys for the new type.
- */
-function cleanQuestionOptionsForType(options: any, newType: string): any {
-  const allowedOpts = allowedQuestionOptionsMapping[newType] || [];
-  const cleanedOpts = Object.fromEntries(Object.entries(options).filter(([optKey]) => allowedOpts.includes(optKey)));
-  cleanedOpts.rendering = options.rendering;
-  return cleanedOpts;
-}
-
-/**
- * Cleans the given form field by retaining only allowed top‑level properties for the new type.
- * Also cleans nested questionOptions using the nested mapping.
- */
-export function cleanFormFieldForType(field: FormField, newType: string): FormField {
-  const allowedKeys = allowedPropertiesMapping[newType] || [];
-  const cleaned: Partial<FormField> = {};
-
-  allowedKeys.forEach((key) => {
-    if (key in field) {
-      (cleaned as any)[key] = field[key as keyof FormField];
-    }
-  });
-
-  // If questionOptions is allowed and exists, clean it using the nested mapping.
-  if (cleaned.questionOptions && typeof cleaned.questionOptions === 'object') {
-    cleaned.questionOptions = cleanQuestionOptionsForType(cleaned.questionOptions, newType);
-  }
-
-  cleaned.type = newType;
-  return cleaned as FormField;
-}
-
 const getAllQuestionIds = (questions?: FormField[]): string[] => {
   if (!questions) return [];
   return flattenDeep(questions.map((question) => [question.id, getAllQuestionIds(question.questions)]));
@@ -125,6 +45,9 @@ const QuestionModalContent: React.FC<QuestionModalProps> = ({
 }) => {
   const { t } = useTranslation();
   const { formField, setFormField } = useFormField();
+  /**
+   * NOTE - this does not support nested obsGroup questions
+   */
   const checkIfQuestionIdExists = useCallback(
     (idToTest: string): boolean => {
       // Get all IDs from the schema

src/constants.ts

@@ -1,4 +1,4 @@
-import type { RenderType } from '@openmrs/esm-form-engine-lib';
+import type { FormField, RenderType } from '@openmrs/esm-form-engine-lib';
 
 export const questionTypes = [
   'control',
@@ -54,3 +54,50 @@ export const renderTypeOptions: Record<Exclude<QuestionType, 'obs'>, Array<Rende
   patientIdentifier: ['text'],
   programState: ['select'],
 };
+
+/**
+  Required properties for all question types.
+ */
+export const requiredProperties: Array<keyof FormField> = ['id', 'label', 'type', 'questionOptions'];
+
+/**
+   Type-specific properties for each question type.
+ */
+export const typeSpecificProperties: Record<string, Array<keyof FormField>> = {
+  control: [],
+  encounterDatetime: ['datePickerFormat'],
+  encounterLocation: [],
+  encounterProvider: [],
+  encounterRole: [],
+  obs: ['required'],
+  obsGroup: ['questions'],
+  patientIdentifier: [],
+  testOrder: [],
+  programState: [],
+};
+
+/**
+  Merge required properties with type-specific ones.
+ */
+export const allowedPropertiesMapping: Record<string, string[]> = Object.fromEntries(
+  Object.entries(typeSpecificProperties).map(([type, props]) => {
+    const mergedProps = new Set<string>([...requiredProperties, ...props]);
+    return [type, Array.from(mergedProps)];
+  }),
+);
+
+/**
+   Mapping of allowed keys for the nested questionOptions object per question type.
+ */
+export const allowedQuestionOptionsMapping: Record<string, string[]> = {
+  control: ['rendering', 'minLength', 'maxLength'],
+  encounterDatetime: ['rendering'],
+  encounterLocation: ['rendering'],
+  encounterProvider: ['rendering'],
+  encounterRole: ['rendering', 'isSearchable'],
+  obs: ['rendering', 'concept', 'answers'],
+  obsGroup: ['rendering', 'concept'],
+  patientIdentifier: ['rendering', 'identifierType', 'minLength', 'maxLength'],
+  testOrder: ['rendering'],
+  programState: ['rendering', 'programUuid', 'workflowUuid', 'answers'],
+};

translations/en.json

@@ -228,4 +228,4 @@
   "welcomeExplainer": "Add pages, sections and questions to your form. The Preview tab automatically updates as you build your form. For a detailed explanation of what constitutes an OpenMRS form schema, please read through the ",
   "welcomeHeading": "Interactive schema builder",
   "yes": "Yes"
-}
+}