Copilot commented on code in PR #38117:
URL: https://github.com/apache/superset/pull/38117#discussion_r2834652780
##########
superset-frontend/src/explore/components/controls/TextAreaControl.tsx:
##########
@@ -186,7 +202,7 @@ class TextAreaControl extends
Component<TextAreaControlProps> {
defaultValue={this.props.initialValue}
readOnly={this.props.readOnly}
key={this.props.name}
- {...this.props}
+ {...editorProps}
onChange={this.handleChange.bind(this)}
/>
Review Comment:
There are duplicate prop assignments here. The props `mode`, `style`,
`minLines`, `maxLines`, `editorProps`, `onLoad`, `defaultValue`, `readOnly`,
`key`, and `onChange` are explicitly passed to TextAreaEditor, but some of
these may also be present in the `editorProps` spread (lines 181-192).
This could cause issues if any of these props exist in `this.props`:
- `mode` is already being passed explicitly from `this.props.language` (line
196)
- `minLines`, `maxLines` are passed explicitly (lines 198-199) but could be
in editorProps
- `defaultValue` is passed from `this.props.initialValue` (line 202) but
`initialValue` was not excluded from editorProps
- `readOnly`, `name` are passed explicitly (lines 203-204) but were not
excluded from editorProps
- `onChange` is passed last (line 206) but was not excluded from editorProps
You should exclude these additional props from the destructuring:
`language`, `initialValue`, `readOnly`, `name`, `onChange`, `minLines`,
`maxLines`, `style`, `mode`, `onLoad`, `editorProps`, and `key` to prevent
duplicate prop assignment and ensure the explicitly passed values take
precedence.
##########
superset-frontend/src/explore/components/controls/TextAreaControl.tsx:
##########
@@ -174,6 +174,22 @@ class TextAreaControl extends
Component<TextAreaControlProps> {
});
});
};
+ // Exclude props that shouldn't be passed to TextAreaEditor
+ // - theme: TextAreaEditor expects theme as a string, not the theme
object from withTheme HOC
+ // - height: ReactAce expects string, we pass number (height is
controlled via minLines/maxLines)
+ // - other control-specific props that ReactAce doesn't need
+ const {
+ theme,
+ height,
+ offerEditInModal,
+ aboveEditorSection,
+ resize,
+ textAreaStyles,
+ tooltipOptions,
+ hotkeys,
+ debounceDelay,
Review Comment:
The destructuring should also exclude ControlHeader-specific props that are
passed to the ControlHeader component but shouldn't be passed to
TextAreaEditor. These include: `label`, `description`, `validationErrors`,
`renderTrigger`, `rightNode`, `leftNode`, `onClick`, `hovered`,
`tooltipOnClick`, `warning`, and `danger`.
Additionally, `value` should be excluded as it could conflict with
ReactAce's controlled component behavior (the component uses `defaultValue`
instead on line 202).
```suggestion
debounceDelay,
// ControlHeader-specific props that should not be forwarded to
TextAreaEditor
label,
description,
validationErrors,
renderTrigger,
rightNode,
leftNode,
onClick,
hovered,
tooltipOnClick,
warning,
danger,
// value could conflict with ReactAce's controlled component behavior
value,
```
--
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]
