Skip to content

Commit 60040b8

Browse files
committed
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.
1 parent b41e43a commit 60040b8

File tree

5 files changed

+86
-41
lines changed

5 files changed

+86
-41
lines changed

packages/decap-cms-core/src/components/Editor/EditorControlPane/EditorControl.js

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,6 @@ class EditorControl extends React.Component {
139139
removeInsertedMedia: PropTypes.func.isRequired,
140140
persistMedia: PropTypes.func.isRequired,
141141
onValidate: PropTypes.func,
142-
processControlRef: PropTypes.func,
143142
controlRef: PropTypes.func,
144143
query: PropTypes.func.isRequired,
145144
queryHits: PropTypes.object,
@@ -201,7 +200,6 @@ class EditorControl extends React.Component {
201200
removeInsertedMedia,
202201
persistMedia,
203202
onValidate,
204-
processControlRef,
205203
controlRef,
206204
query,
207205
queryHits,
@@ -329,7 +327,6 @@ class EditorControl extends React.Component {
329327
resolveWidget={resolveWidget}
330328
widget={widget}
331329
getEditorComponents={getEditorComponents}
332-
ref={processControlRef && partial(processControlRef, field)}
333330
controlRef={controlRef}
334331
editorControl={ConnectedEditorControl}
335332
query={query}

packages/decap-cms-core/src/components/Editor/EditorControlPane/EditorControlPane.js

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -99,15 +99,17 @@ export default class ControlPane extends React.Component {
9999
selectedLocale: this.props.locale,
100100
};
101101

102-
componentValidate = {};
102+
childRefs = {};
103103

104-
controlRef(field, wrappedControl) {
104+
controlRef = (field, wrappedControl) => {
105105
if (!wrappedControl) return;
106106
const name = field.get('name');
107+
this.childRefs[name] = wrappedControl;
108+
};
107109

108-
this.componentValidate[name] =
109-
wrappedControl.innerWrappedControl?.validate || wrappedControl.validate;
110-
}
110+
getControlRef = field => wrappedControl => {
111+
this.controlRef(field, wrappedControl);
112+
};
111113

112114
handleLocaleChange = val => {
113115
this.setState({ selectedLocale: val });
@@ -152,7 +154,11 @@ export default class ControlPane extends React.Component {
152154
validate = async () => {
153155
this.props.fields.forEach(field => {
154156
if (field.get('widget') === 'hidden') return;
155-
this.componentValidate[field.get('name')]();
157+
const control = this.childRefs[field.get('name')];
158+
const validateFn = control?.innerWrappedControl?.validate ?? control?.validate;
159+
if (validateFn) {
160+
validateFn();
161+
}
156162
});
157163
};
158164

@@ -227,8 +233,7 @@ export default class ControlPane extends React.Component {
227233
onChange(field, newValue, newMetadata, i18n);
228234
}}
229235
onValidate={onValidate}
230-
processControlRef={this.controlRef.bind(this)}
231-
controlRef={this.controlRef}
236+
controlRef={this.getControlRef(field)}
232237
entry={entry}
233238
collection={collection}
234239
isDisabled={isDuplicate}

packages/decap-cms-core/src/components/Editor/EditorControlPane/Widget.js

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ export default class Widget extends Component {
4444
fieldsErrors: ImmutablePropTypes.map,
4545
onChange: PropTypes.func.isRequired,
4646
onValidate: PropTypes.func,
47+
controlRef: PropTypes.func,
4748
onOpenMediaLibrary: PropTypes.func.isRequired,
4849
onClearMediaControl: PropTypes.func.isRequired,
4950
onRemoveMediaControl: PropTypes.func.isRequired,
@@ -55,7 +56,6 @@ export default class Widget extends Component {
5556
widget: PropTypes.object.isRequired,
5657
getEditorComponents: PropTypes.func.isRequired,
5758
isFetching: PropTypes.bool,
58-
controlRef: PropTypes.func,
5959
query: PropTypes.func.isRequired,
6060
clearSearch: PropTypes.func.isRequired,
6161
clearFieldErrors: PropTypes.func.isRequired,
@@ -112,6 +112,11 @@ export default class Widget extends Component {
112112
*/
113113
const { shouldComponentUpdate: scu } = this.innerWrappedControl;
114114
this.wrappedControlShouldComponentUpdate = scu && scu.bind(this.innerWrappedControl);
115+
116+
// Call the control ref if provided, passing this Widget instance
117+
if (this.props.controlRef) {
118+
this.props.controlRef(this);
119+
}
115120
};
116121

117122
getValidateValue = () => {

packages/decap-cms-widget-list/src/ListControl.js

Lines changed: 55 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ function LabelComponent({ field, isActive, hasErrors, uniqueFieldId, isFieldOpti
171171
}
172172

173173
export default class ListControl extends React.Component {
174-
validations = [];
174+
childRefs = {};
175175

176176
static propTypes = {
177177
metadata: ImmutablePropTypes.map,
@@ -365,16 +365,18 @@ export default class ListControl extends React.Component {
365365
processControlRef = ref => {
366366
if (!ref) return;
367367
const {
368-
validate,
369368
props: { validationKey: key },
370369
} = ref;
371-
this.validations.push({ key, validate });
370+
this.childRefs[key] = ref;
371+
this.props.controlRef?.(this);
372372
};
373373

374374
validate = () => {
375-
if (this.getValueType()) {
376-
this.validations.forEach(item => {
377-
item.validate();
375+
// First validate child widgets if this is a complex list
376+
const hasChildWidgets = this.getValueType() && Object.keys(this.childRefs).length > 0;
377+
if (hasChildWidgets) {
378+
Object.values(this.childRefs).forEach(widget => {
379+
widget?.validate?.();
378380
});
379381
} else {
380382
this.props.validate();
@@ -431,7 +433,17 @@ export default class ListControl extends React.Component {
431433
handleRemove = (index, event) => {
432434
event.preventDefault();
433435
const { itemsCollapsed } = this.state;
434-
const { value, metadata, onChange, field, clearFieldErrors } = this.props;
436+
const {
437+
value,
438+
metadata,
439+
onChange,
440+
field,
441+
clearFieldErrors,
442+
onValidateObject,
443+
forID,
444+
fieldsErrors,
445+
} = this.props;
446+
435447
const collectionName = field.get('name');
436448
const isSingleField = this.getValueType() === valueTypes.SINGLE;
437449

@@ -441,17 +453,41 @@ export default class ListControl extends React.Component {
441453
? { [collectionName]: metadata.removeIn(metadataRemovePath) }
442454
: metadata;
443455

456+
// Get the key of the item being removed
457+
const removedKey = this.state.keys[index];
458+
459+
// Update state while preserving keys for remaining items
460+
const newKeys = [...this.state.keys];
461+
newKeys.splice(index, 1);
444462
itemsCollapsed.splice(index, 1);
445-
// clear validations
446-
this.validations = [];
447463

448464
this.setState({
449465
itemsCollapsed: [...itemsCollapsed],
450-
keys: Array.from({ length: value.size - 1 }, () => uuid()),
466+
keys: newKeys,
451467
});
452468

453-
onChange(value.remove(index), parsedMetadata);
454-
clearFieldErrors();
469+
// Clear the ref for the removed item
470+
delete this.childRefs[removedKey];
471+
472+
const newValue = value.delete(index);
473+
474+
// Clear errors for the removed item and its children
475+
if (fieldsErrors) {
476+
Object.entries(fieldsErrors.toJS()).forEach(([fieldId, errors]) => {
477+
if (errors.some(err => err.parentIds?.includes(removedKey))) {
478+
clearFieldErrors(fieldId);
479+
}
480+
});
481+
}
482+
483+
// If list is empty, mark it as valid
484+
if (newValue.size === 0) {
485+
clearFieldErrors(forID);
486+
onValidateObject(forID, []);
487+
}
488+
489+
// Update the value last to ensure all error states are cleared
490+
onChange(newValue, parsedMetadata);
455491
};
456492

457493
handleItemCollapseToggle = (index, event) => {
@@ -527,7 +563,7 @@ export default class ListControl extends React.Component {
527563
}
528564

529565
onSortEnd = ({ oldIndex, newIndex }) => {
530-
const { value, clearFieldErrors } = this.props;
566+
const { value } = this.props;
531567
const { itemsCollapsed, keys } = this.state;
532568

533569
// Update value
@@ -541,18 +577,13 @@ export default class ListControl extends React.Component {
541577
const updatedItemsCollapsed = [...itemsCollapsed];
542578
updatedItemsCollapsed.splice(newIndex, 0, collapsed);
543579

544-
// Reset item to ensure updated state
545-
const updatedKeys = keys.map((key, keyIndex) => {
546-
if (keyIndex === oldIndex || keyIndex === newIndex) {
547-
return uuid();
548-
}
549-
return key;
550-
});
551-
this.setState({ itemsCollapsed: updatedItemsCollapsed, keys: updatedKeys });
580+
// Move keys to maintain relationships
581+
const movedKey = keys[oldIndex];
582+
const updatedKeys = [...keys];
583+
updatedKeys.splice(oldIndex, 1);
584+
updatedKeys.splice(newIndex, 0, movedKey);
552585

553-
//clear error fields and remove old validations
554-
clearFieldErrors();
555-
this.validations = this.validations.filter(item => updatedKeys.includes(item.key));
586+
this.setState({ itemsCollapsed: updatedItemsCollapsed, keys: updatedKeys });
556587
};
557588

558589
hasError = index => {

packages/decap-cms-widget-object/src/ObjectControl.js

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,14 @@ const styleStrings = {
2222
};
2323

2424
export default class ObjectControl extends React.Component {
25-
componentValidate = {};
25+
childRefs = {};
26+
27+
processControlRef = ref => {
28+
if (!ref) return;
29+
const name = ref.props.field.get('name');
30+
this.childRefs[name] = ref;
31+
this.props.controlRef?.(ref);
32+
};
2633

2734
static propTypes = {
2835
onChangeObject: PropTypes.func.isRequired,
@@ -70,7 +77,9 @@ export default class ObjectControl extends React.Component {
7077
fields = List.isList(fields) ? fields : List([fields]);
7178
fields.forEach(field => {
7279
if (field.get('widget') === 'hidden') return;
73-
this.componentValidate[field.get('name')]();
80+
const name = field.get('name');
81+
const control = this.childRefs[name];
82+
control?.validate?.();
7483
});
7584
};
7685

@@ -83,7 +92,6 @@ export default class ObjectControl extends React.Component {
8392
metadata,
8493
fieldsErrors,
8594
editorControl: EditorControl,
86-
controlRef,
8795
parentIds,
8896
isFieldDuplicate,
8997
isFieldHidden,
@@ -111,8 +119,7 @@ export default class ObjectControl extends React.Component {
111119
fieldsMetaData={metadata}
112120
fieldsErrors={fieldsErrors}
113121
onValidate={onValidateObject}
114-
processControlRef={controlRef && controlRef.bind(this)}
115-
controlRef={controlRef}
122+
controlRef={this.processControlRef}
116123
parentIds={[...parentIds, forID]}
117124
isDisabled={isDuplicate}
118125
isHidden={isHidden}

0 commit comments

Comments
 (0)