msyavuz commented on code in PR #33443:
URL: https://github.com/apache/superset/pull/33443#discussion_r2104708466
##########
superset-frontend/src/components/Datasource/DatasourceEditor.jsx:
##########
@@ -996,8 +1009,42 @@ class DatasourceEditor extends PureComponent {
);
}
+ renderSqlEditotOverlay = () => (
Review Comment:
small typo
```suggestion
renderSqlEditorOverlay = () => (
```
##########
superset-frontend/src/SqlLab/components/SqlEditor/index.tsx:
##########
@@ -890,6 +893,68 @@ const SqlEditor: FC<Props> = ({
dispatch(queryEditorSetCursorPosition(queryEditor, newPosition));
};
+ const copyQuery = (callback: (text: string) => void) => {
+ callback(currentSQL.current);
+ };
+ const renderCopyQueryButton = () => (
+ <Button type="primary">{t('COPY QUERY')}</Button>
+ );
+
+ const renderDatasetWarning = () => (
+ <Alert
Review Comment:
There is an action prop on Alert component it should be enough along with
message and description:
https://ant.design/components/alert#alert-demo-action
##########
superset-frontend/src/components/Datasource/DatasourceEditor.jsx:
##########
@@ -1097,35 +1144,63 @@ class DatasourceEditor extends PureComponent {
description={t(
'When specifying SQL, the datasource acts as a view. ' +
'Superset will use this statement as a subquery while
grouping and filtering ' +
- 'on the generated parent queries.',
+ 'on the generated parent queries.' +
+ 'If changes are made to your SQL query, ' +
+ 'columns in your dataset will be synced when saving
the dataset.',
)}
control={
- <TextAreaControl
- language="sql"
- offerEditInModal={false}
- minLines={10}
- maxLines={Infinity}
- readOnly={!this.state.isEditMode}
- resize="both"
- tooltipOptions={sqlTooltipOptions}
- />
+ this.props.isQueryRunning ? (
+ <>
+ {this.renderSqlEditotOverlay()}
+ <TextAreaControl
+ language="sql"
+ offerEditInModal={false}
+ minLines={10}
+ maxLines={Infinity}
+ readOnly={!this.state.isEditMode}
+ resize="both"
+ />
+ </>
+ ) : (
+ <TextAreaControl
+ language="sql"
+ offerEditInModal={false}
+ minLines={10}
+ maxLines={Infinity}
+ readOnly={!this.state.isEditMode}
+ resize="both"
+ />
+ )
}
additionalControl={
<div
- css={css`
- position: absolute;
- right: 0;
- top: 0;
- z-index: 2;
- `}
+ css={{
+ position: 'absolute',
+ right: 0,
+ top: 0,
+ zIndex: 2,
+ }}
Review Comment:
Is this change necessary?
##########
superset-frontend/src/components/Datasource/DatasourceEditor.jsx:
##########
@@ -1097,35 +1144,63 @@ class DatasourceEditor extends PureComponent {
description={t(
'When specifying SQL, the datasource acts as a view. ' +
'Superset will use this statement as a subquery while
grouping and filtering ' +
- 'on the generated parent queries.',
+ 'on the generated parent queries.' +
+ 'If changes are made to your SQL query, ' +
+ 'columns in your dataset will be synced when saving
the dataset.',
)}
control={
- <TextAreaControl
- language="sql"
- offerEditInModal={false}
- minLines={10}
- maxLines={Infinity}
- readOnly={!this.state.isEditMode}
- resize="both"
- tooltipOptions={sqlTooltipOptions}
- />
+ this.props.isQueryRunning ? (
+ <>
+ {this.renderSqlEditotOverlay()}
+ <TextAreaControl
+ language="sql"
+ offerEditInModal={false}
+ minLines={10}
+ maxLines={Infinity}
+ readOnly={!this.state.isEditMode}
+ resize="both"
+ />
+ </>
+ ) : (
+ <TextAreaControl
+ language="sql"
+ offerEditInModal={false}
+ minLines={10}
+ maxLines={Infinity}
+ readOnly={!this.state.isEditMode}
+ resize="both"
+ />
+ )
Review Comment:
The condition only affects `this.renderSqlEditorOverlay()` Can't this be
shortened then to something like this:
```suggestion
<>
{this.props.isQueryRunning &&
this.renderSqlEditotOverlay()}
<TextAreaControl
language="sql"
offerEditInModal={false}
minLines={10}
maxLines={Infinity}
readOnly={!this.state.isEditMode}
resize="both"
/>
</>
```
--
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: notifications-unsubscr...@superset.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org
