fix(AlertReportModal): Text Area Change (#17176)

* Text Area Change * added unique key to component * tests passing * changed css * data flow and naming convention
apache · Oct 29, 2021 · 5948a9f · 5948a9f
1 parent d0bad96
commit 5948a9f
Show file tree

Hide file tree

Showing 12 changed files with 24 additions and 39 deletions.
diff --git a/superset-frontend/package-lock.json b/superset-frontend/package-lock.json
diff --git a/superset-frontend/package.json b/superset-frontend/package.json
@@ -139,7 +139,7 @@
     "query-string": "^6.13.7",
     "re-resizable": "^6.6.1",
     "react": "^16.13.1",
-    "react-ace": "^5.10.0",
+    "react-ace": "^9.4.4",
     "react-checkbox-tree": "^1.5.1",
     "react-color": "^2.13.8",
     "react-datetime": "^3.0.4",

diff --git a/superset-frontend/src/SqlLab/components/TemplateParamsEditor/TemplateParamsEditor.test.tsx b/superset-frontend/src/SqlLab/components/TemplateParamsEditor/TemplateParamsEditor.test.tsx
@@ -51,7 +51,7 @@ describe('TemplateParamsEditor', () => {
     const spy = jest.spyOn(brace, 'acequire');
     spy.mockReturnValue({ setCompleters: () => 'foo' });
     await waitFor(() => {
-      expect(baseElement.querySelector('#brace-editor')).toBeInTheDocument();
+      expect(baseElement.querySelector('#ace-editor')).toBeInTheDocument();
     });
   });
 });
diff --git a/superset-frontend/src/SqlLab/main.less b/superset-frontend/src/SqlLab/main.less
@@ -207,7 +207,7 @@ div.Workspace {
     flex-direction: column;
   }
 
-  #brace-editor {
+  #ace-editor {
     height: calc(100% - 51px);
     flex-grow: 1;
   }

diff --git a/superset-frontend/src/components/AsyncAceEditor/AsyncAceEditor.test.tsx b/superset-frontend/src/components/AsyncAceEditor/AsyncAceEditor.test.tsx
@@ -30,7 +30,7 @@ import AsyncAceEditor, {
   AsyncAceEditorOptions,
 } from 'src/components/AsyncAceEditor';
 
-const selector = '[id="brace-editor"]';
+const selector = '[id="ace-editor"]';
 
 test('renders SQLEditor', async () => {
   const { container } = render(<SQLEditor />);

diff --git a/superset-frontend/src/components/AsyncAceEditor/index.tsx b/superset-frontend/src/components/AsyncAceEditor/index.tsx
@@ -23,7 +23,7 @@ import {
   Position,
   TextMode as OrigTextMode,
 } from 'brace';
-import AceEditor, { AceEditorProps } from 'react-ace';
+import AceEditor, { IAceEditorProps } from 'react-ace';
 import AsyncEsmComponent, {
   PlaceholderProps,
 } from 'src/components/AsyncEsmComponent';
@@ -72,7 +72,7 @@ const aceModuleLoaders = {
 
 export type AceModule = keyof typeof aceModuleLoaders;
 
-export type AsyncAceEditorProps = AceEditorProps & {
+export type AsyncAceEditorProps = IAceEditorProps & {
   keywords?: AceCompleterKeyword[];
 };
 
@@ -83,7 +83,7 @@ export type AsyncAceEditorOptions = {
   defaultTheme?: AceEditorTheme;
   defaultTabSize?: number;
   placeholder?: React.ComponentType<
-    PlaceholderProps & Partial<AceEditorProps>
+    PlaceholderProps & Partial<IAceEditorProps>
   > | null;
 };
 
@@ -120,7 +120,6 @@ export default function AsyncAceEditor(
           theme = inferredTheme,
           tabSize = defaultTabSize,
           defaultValue = '',
-          value = '',
           ...props
         },
         ref,
@@ -153,7 +152,6 @@ export default function AsyncAceEditor(
             theme={theme}
             tabSize={tabSize}
             defaultValue={defaultValue}
-            value={value || ''}
             {...props}
           />
         );

diff --git a/superset-frontend/src/dashboard/components/CssEditor/CssEditor.test.tsx b/superset-frontend/src/dashboard/components/CssEditor/CssEditor.test.tsx
@@ -19,12 +19,12 @@
 import React from 'react';
 import { render, screen, waitFor } from 'spec/helpers/testing-library';
 import { CssEditor as AceCssEditor } from 'src/components/AsyncAceEditor';
-import { AceEditorProps } from 'react-ace';
+import { IAceEditorProps } from 'react-ace';
 import userEvent from '@testing-library/user-event';
 import CssEditor from '.';
 
 jest.mock('src/components/AsyncAceEditor', () => ({
-  CssEditor: ({ value, onChange }: AceEditorProps) => (
+  CssEditor: ({ value, onChange }: IAceEditorProps) => (
     <textarea
       defaultValue={value}
       onChange={value => onChange?.(value.target.value)}

diff --git a/superset-frontend/src/dashboard/stylesheets/components/markdown.less b/superset-frontend/src/dashboard/stylesheets/components/markdown.less
@@ -42,7 +42,7 @@
     cursor: move;
   }
 
-  #brace-editor {
+  #ace-editor {
     border: none;
   }
 }

diff --git a/superset-frontend/src/explore/components/controls/TextAreaControl.jsx b/superset-frontend/src/explore/components/controls/TextAreaControl.jsx
@@ -19,10 +19,8 @@
 import React from 'react';
 import PropTypes from 'prop-types';
 import { TextArea } from 'src/common/components';
-import { debounce } from 'lodash';
 import { t } from '@superset-ui/core';
 
-import { FAST_DEBOUNCE } from 'src/constants';
 import Button from 'src/components/Button';
 import { TextAreaEditor } from 'src/components/AsyncAceEditor';
 import ModalTrigger from 'src/components/ModalTrigger';
@@ -32,7 +30,7 @@ import ControlHeader from 'src/explore/components/ControlHeader';
 const propTypes = {
   name: PropTypes.string,
   onChange: PropTypes.func,
-  value: PropTypes.string,
+  initialValue: PropTypes.string,
   height: PropTypes.number,
   minLines: PropTypes.number,
   maxLines: PropTypes.number,
@@ -51,7 +49,7 @@ const propTypes = {
 
 const defaultProps = {
   onChange: () => {},
-  value: '',
+  initialValue: '',
   height: 250,
   minLines: 3,
   maxLines: 10,
@@ -60,29 +58,12 @@ const defaultProps = {
 };
 
 export default class TextAreaControl extends React.Component {
-  constructor() {
-    super();
-    this.state = {
-      value: '',
-    };
-    this.onAceChangeDebounce = debounce(value => {
-      this.onAceChange(value);
-    }, FAST_DEBOUNCE);
-  }
-
   onControlChange(event) {
     const { value } = event.target;
-    this.setState({ value });
-    this.props.onChange(value);
-  }
-
-  onAceChange(value) {
-    this.setState({ value });
     this.props.onChange(value);
   }
 
   renderEditor(inModal = false) {
-    const value = this.state.value || this.props.value;
     const minLines = inModal ? 40 : this.props.minLines || 12;
     if (this.props.language) {
       const style = { border: '1px solid #CCC' };
@@ -95,20 +76,22 @@ export default class TextAreaControl extends React.Component {
           style={style}
           minLines={minLines}
           maxLines={inModal ? 1000 : this.props.maxLines}
-          onChange={this.onAceChangeDebounce}
+          onChange={this.props.onChange}
           width="100%"
           height={`${minLines}em`}
           editorProps={{ $blockScrolling: true }}
-          value={value}
+          defaultValue={this.props.initialValue}
           readOnly={this.props.readOnly}
+          key={this.props.name}
+          {...this.props}
         />
       );
     }
     return (
       <TextArea
         placeholder={t('textarea')}
         onChange={this.onControlChange.bind(this)}
-        value={value}
+        defaultValue={this.props.initialValue}
         disabled={this.props.readOnly}
         style={{ height: this.props.height }}
       />

diff --git a/superset-frontend/src/views/CRUD/alert/AlertList.tsx b/superset-frontend/src/views/CRUD/alert/AlertList.tsx
@@ -134,6 +134,8 @@ function AlertList({
     setAlertModalOpen(true);
   }
 
+  const generateKey = () => `${new Date().getTime()}`;
+
   const canEdit = hasPerm('can_write');
   const canDelete = hasPerm('can_write');
   const canCreate = hasPerm('can_write');
@@ -449,6 +451,7 @@ function AlertList({
         }}
         show={alertModalOpen}
         isReport={isReportEnabled}
+        key={currentAlert?.id || generateKey()}
       />
       {currentAlertDeleting && (
         <DeleteModal

diff --git a/superset-frontend/src/views/CRUD/alert/AlertReportModal.test.jsx b/superset-frontend/src/views/CRUD/alert/AlertReportModal.test.jsx
@@ -215,7 +215,7 @@ describe('AlertReportModal', () => {
     const editWrapper = await mountAndWait(props);
     const input = editWrapper.find(TextAreaControl);
     expect(input).toExist();
-    expect(input.props().value).toEqual('SELECT NaN');
+    expect(input.props().initialValue).toEqual('SELECT NaN');
   });
 
   it('renders five select element when in report mode', () => {

diff --git a/superset-frontend/src/views/CRUD/alert/AlertReportModal.tsx b/superset-frontend/src/views/CRUD/alert/AlertReportModal.tsx
@@ -1138,7 +1138,8 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
                   maxLines={15}
                   onChange={onSQLChange}
                   readOnly={false}
-                  value={currentAlert ? currentAlert.sql : ''}
+                  initialValue={resource?.sql}
+                  key={currentAlert?.id}
                 />
               </StyledInputContainer>
               <div className="inline-container wrap">