LevisNgigi commented on code in PR #38524:
URL: https://github.com/apache/superset/pull/38524#discussion_r2939611297
##########
superset-frontend/packages/superset-ui-core/src/components/EditableTitle/index.tsx:
##########
@@ -89,11 +89,20 @@ export function EditableTitle({
onEditingChange,
...rest
}: EditableTitleProps) {
- const [isEditing, setIsEditing] = useState(editing);
+ const isControlled = editing !== undefined;
+ const [isEditingInternal, setIsEditingInternal] = useState(editing ?? false);
Review Comment:
Thanks for the suggestion! In React’s controlled/uncontrolled pattern, a
component is considered controlled based on the presence of the value prop
(editing), not the callback. onEditingChange is only used to notify the parent
of updates and doesn’t necessarily mean the parent controls the state.
Using onEditingChange !== undefined || editing could also misclassify the
component when editing is false. For that reason, checking only the presence of
editing is the more reliable pattern:
const isControlled = editing !== undefined;
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
