refactor: simplify widget reference handling

Replace the confusing dual-prop pattern (controlRef/processControlRef) with a more idiomatic React approach using class variables. Having two parallel props for ref handling made the code hard to understand as it wasn't clear which prop was responsible for what. The old pattern of taking these callbacks via props and binding them to 'this' was also hard to debug and trace through the component hierarchy. Changes: - Use class variables to bind widget ref handlers - Consolidate ref handling into a single clear responsibility - Store widget refs directly in childRefs object - Clean up refs properly when removing items - Fix: clear validation state when list becomes empty This makes the code easier to understand and debug while laying the groundwork for future features like programmatic expansion and focusing of nested items.
decaporg · Jan 11, 2025 · 60040b8 · 60040b8
1 parent b41e43a
commit 60040b8
Show file tree

Hide file tree

Showing 5 changed files with 86 additions and 41 deletions.
diff --git a/packages/decap-cms-core/src/components/Editor/EditorControlPane/EditorControl.js b/packages/decap-cms-core/src/components/Editor/EditorControlPane/EditorControl.js
@@ -139,7 +139,6 @@ class EditorControl extends React.Component {
     removeInsertedMedia: PropTypes.func.isRequired,
     persistMedia: PropTypes.func.isRequired,
     onValidate: PropTypes.func,
-    processControlRef: PropTypes.func,
     controlRef: PropTypes.func,
     query: PropTypes.func.isRequired,
     queryHits: PropTypes.object,
@@ -201,7 +200,6 @@ class EditorControl extends React.Component {
       removeInsertedMedia,
       persistMedia,
       onValidate,
-      processControlRef,
       controlRef,
       query,
       queryHits,
@@ -329,7 +327,6 @@ class EditorControl extends React.Component {
               resolveWidget={resolveWidget}
               widget={widget}
               getEditorComponents={getEditorComponents}
-              ref={processControlRef && partial(processControlRef, field)}
               controlRef={controlRef}
               editorControl={ConnectedEditorControl}
               query={query}

diff --git a/packages/decap-cms-core/src/components/Editor/EditorControlPane/EditorControlPane.js b/packages/decap-cms-core/src/components/Editor/EditorControlPane/EditorControlPane.js
@@ -99,15 +99,17 @@ export default class ControlPane extends React.Component {
     selectedLocale: this.props.locale,
   };
 
-  componentValidate = {};
+  childRefs = {};
 
-  controlRef(field, wrappedControl) {
+  controlRef = (field, wrappedControl) => {
     if (!wrappedControl) return;
     const name = field.get('name');
+    this.childRefs[name] = wrappedControl;
+  };
 
-    this.componentValidate[name] =
-      wrappedControl.innerWrappedControl?.validate || wrappedControl.validate;
-  }
+  getControlRef = field => wrappedControl => {
+    this.controlRef(field, wrappedControl);
+  };
 
   handleLocaleChange = val => {
     this.setState({ selectedLocale: val });
@@ -152,7 +154,11 @@ export default class ControlPane extends React.Component {
   validate = async () => {
     this.props.fields.forEach(field => {
       if (field.get('widget') === 'hidden') return;
-      this.componentValidate[field.get('name')]();
+      const control = this.childRefs[field.get('name')];
+      const validateFn = control?.innerWrappedControl?.validate ?? control?.validate;
+      if (validateFn) {
+        validateFn();
+      }
     });
   };
 
@@ -227,8 +233,7 @@ export default class ControlPane extends React.Component {
                   onChange(field, newValue, newMetadata, i18n);
                 }}
                 onValidate={onValidate}
-                processControlRef={this.controlRef.bind(this)}
-                controlRef={this.controlRef}
+                controlRef={this.getControlRef(field)}
                 entry={entry}
                 collection={collection}
                 isDisabled={isDuplicate}

diff --git a/packages/decap-cms-core/src/components/Editor/EditorControlPane/Widget.js b/packages/decap-cms-core/src/components/Editor/EditorControlPane/Widget.js
@@ -44,6 +44,7 @@ export default class Widget extends Component {
     fieldsErrors: ImmutablePropTypes.map,
     onChange: PropTypes.func.isRequired,
     onValidate: PropTypes.func,
+    controlRef: PropTypes.func,
     onOpenMediaLibrary: PropTypes.func.isRequired,
     onClearMediaControl: PropTypes.func.isRequired,
     onRemoveMediaControl: PropTypes.func.isRequired,
@@ -55,7 +56,6 @@ export default class Widget extends Component {
     widget: PropTypes.object.isRequired,
     getEditorComponents: PropTypes.func.isRequired,
     isFetching: PropTypes.bool,
-    controlRef: PropTypes.func,
     query: PropTypes.func.isRequired,
     clearSearch: PropTypes.func.isRequired,
     clearFieldErrors: PropTypes.func.isRequired,
@@ -112,6 +112,11 @@ export default class Widget extends Component {
      */
     const { shouldComponentUpdate: scu } = this.innerWrappedControl;
     this.wrappedControlShouldComponentUpdate = scu && scu.bind(this.innerWrappedControl);
+
+    // Call the control ref if provided, passing this Widget instance
+    if (this.props.controlRef) {
+      this.props.controlRef(this);
+    }
   };
 
   getValidateValue = () => {

diff --git a/packages/decap-cms-widget-list/src/ListControl.js b/packages/decap-cms-widget-list/src/ListControl.js
@@ -171,7 +171,7 @@ function LabelComponent({ field, isActive, hasErrors, uniqueFieldId, isFieldOpti
 }
 
 export default class ListControl extends React.Component {
-  validations = [];
+  childRefs = {};
 
   static propTypes = {
     metadata: ImmutablePropTypes.map,
@@ -365,16 +365,18 @@ export default class ListControl extends React.Component {
   processControlRef = ref => {
     if (!ref) return;
     const {
-      validate,
       props: { validationKey: key },
     } = ref;
-    this.validations.push({ key, validate });
+    this.childRefs[key] = ref;
+    this.props.controlRef?.(this);
   };
 
   validate = () => {
-    if (this.getValueType()) {
-      this.validations.forEach(item => {
-        item.validate();
+    // First validate child widgets if this is a complex list
+    const hasChildWidgets = this.getValueType() && Object.keys(this.childRefs).length > 0;
+    if (hasChildWidgets) {
+      Object.values(this.childRefs).forEach(widget => {
+        widget?.validate?.();
       });
     } else {
       this.props.validate();
@@ -431,7 +433,17 @@ export default class ListControl extends React.Component {
   handleRemove = (index, event) => {
     event.preventDefault();
     const { itemsCollapsed } = this.state;
-    const { value, metadata, onChange, field, clearFieldErrors } = this.props;
+    const {
+      value,
+      metadata,
+      onChange,
+      field,
+      clearFieldErrors,
+      onValidateObject,
+      forID,
+      fieldsErrors,
+    } = this.props;
+
     const collectionName = field.get('name');
     const isSingleField = this.getValueType() === valueTypes.SINGLE;
 
@@ -441,17 +453,41 @@ export default class ListControl extends React.Component {
         ? { [collectionName]: metadata.removeIn(metadataRemovePath) }
         : metadata;
 
+    // Get the key of the item being removed
+    const removedKey = this.state.keys[index];
+
+    // Update state while preserving keys for remaining items
+    const newKeys = [...this.state.keys];
+    newKeys.splice(index, 1);
     itemsCollapsed.splice(index, 1);
-    // clear validations
-    this.validations = [];
 
     this.setState({
       itemsCollapsed: [...itemsCollapsed],
-      keys: Array.from({ length: value.size - 1 }, () => uuid()),
+      keys: newKeys,
     });
 
-    onChange(value.remove(index), parsedMetadata);
-    clearFieldErrors();
+    // Clear the ref for the removed item
+    delete this.childRefs[removedKey];
+
+    const newValue = value.delete(index);
+
+    // Clear errors for the removed item and its children
+    if (fieldsErrors) {
+      Object.entries(fieldsErrors.toJS()).forEach(([fieldId, errors]) => {
+        if (errors.some(err => err.parentIds?.includes(removedKey))) {
+          clearFieldErrors(fieldId);
+        }
+      });
+    }
+
+    // If list is empty, mark it as valid
+    if (newValue.size === 0) {
+      clearFieldErrors(forID);
+      onValidateObject(forID, []);
+    }
+
+    // Update the value last to ensure all error states are cleared
+    onChange(newValue, parsedMetadata);
   };
 
   handleItemCollapseToggle = (index, event) => {
@@ -527,7 +563,7 @@ export default class ListControl extends React.Component {
   }
 
   onSortEnd = ({ oldIndex, newIndex }) => {
-    const { value, clearFieldErrors } = this.props;
+    const { value } = this.props;
     const { itemsCollapsed, keys } = this.state;
 
     // Update value
@@ -541,18 +577,13 @@ export default class ListControl extends React.Component {
     const updatedItemsCollapsed = [...itemsCollapsed];
     updatedItemsCollapsed.splice(newIndex, 0, collapsed);
 
-    // Reset item to ensure updated state
-    const updatedKeys = keys.map((key, keyIndex) => {
-      if (keyIndex === oldIndex || keyIndex === newIndex) {
-        return uuid();
-      }
-      return key;
-    });
-    this.setState({ itemsCollapsed: updatedItemsCollapsed, keys: updatedKeys });
+    // Move keys to maintain relationships
+    const movedKey = keys[oldIndex];
+    const updatedKeys = [...keys];
+    updatedKeys.splice(oldIndex, 1);
+    updatedKeys.splice(newIndex, 0, movedKey);
 
-    //clear error fields and remove old validations
-    clearFieldErrors();
-    this.validations = this.validations.filter(item => updatedKeys.includes(item.key));
+    this.setState({ itemsCollapsed: updatedItemsCollapsed, keys: updatedKeys });
   };
 
   hasError = index => {

diff --git a/packages/decap-cms-widget-object/src/ObjectControl.js b/packages/decap-cms-widget-object/src/ObjectControl.js
@@ -22,7 +22,14 @@ const styleStrings = {
 };
 
 export default class ObjectControl extends React.Component {
-  componentValidate = {};
+  childRefs = {};
+
+  processControlRef = ref => {
+    if (!ref) return;
+    const name = ref.props.field.get('name');
+    this.childRefs[name] = ref;
+    this.props.controlRef?.(ref);
+  };
 
   static propTypes = {
     onChangeObject: PropTypes.func.isRequired,
@@ -70,7 +77,9 @@ export default class ObjectControl extends React.Component {
     fields = List.isList(fields) ? fields : List([fields]);
     fields.forEach(field => {
       if (field.get('widget') === 'hidden') return;
-      this.componentValidate[field.get('name')]();
+      const name = field.get('name');
+      const control = this.childRefs[name];
+      control?.validate?.();
     });
   };
 
@@ -83,7 +92,6 @@ export default class ObjectControl extends React.Component {
       metadata,
       fieldsErrors,
       editorControl: EditorControl,
-      controlRef,
       parentIds,
       isFieldDuplicate,
       isFieldHidden,
@@ -111,8 +119,7 @@ export default class ObjectControl extends React.Component {
         fieldsMetaData={metadata}
         fieldsErrors={fieldsErrors}
         onValidate={onValidateObject}
-        processControlRef={controlRef && controlRef.bind(this)}
-        controlRef={controlRef}
+        controlRef={this.processControlRef}
         parentIds={[...parentIds, forID]}
         isDisabled={isDuplicate}
         isHidden={isHidden}