Merge pull request #6030 from Hyperkid123/tweak-react-code-editor

Use react-codemirror onChange event instead of onBeforeChange.
ManageIQ · Aug 16, 2019 · 471bc69 · 471bc69
2 parents 1b4f058 + d7dec1a
commit 471bc69
Show file tree

Hide file tree

Showing 4 changed files with 31 additions and 17 deletions.
diff --git a/app/javascript/components/code-editor/index.js b/app/javascript/components/code-editor/index.js
@@ -1,4 +1,4 @@
-import React, { useState } from 'react';
+import React, { useState, useEffect } from 'react';
 import PropTypes from 'prop-types';
 import { Controlled as CodeMirror } from 'react-codemirror2';
 import {
@@ -22,14 +22,21 @@ const getMode = mode => ({
 })[mode] || mode;
 
 const CodeEditor = ({
-  onBeforeChange,
   mode,
   modes,
   hasError,
+  onChange,
   ...props
 }) => {
   const [codeMode, setCodeMode] = useState(mode);
-
+  const [value, setValue] = useState();
+  const [editor, setEditor] = useState();
+  useEffect(() => {
+    setValue(props.value);
+    if (editor) {
+      editor.refresh();
+    }
+  }, [editor]);
   return (
     <div>
       {modes.length > 0 && (
@@ -52,12 +59,17 @@ const CodeEditor = ({
           gutters: ['CodeMirror-lint-markers'],
         }}
         style={{ height: 'auto' }}
-        onBeforeChange={(editor, ...rest) => {
-          editor.refresh();
-          onBeforeChange(editor, ...rest);
+        onBeforeChange={(editor, _data, value) => {
+          setValue(value);
+        }}
+        onChange={(editor, _data, value) => {
+          onChange(editor, _data, value);
+        }}
+        editorDidMount={(editor) => {
+          setEditor(editor);
         }}
-        editorDidMount={editor => editor.refresh()}
         {...props}
+        value={value}
       />
     </div>
   );
@@ -66,7 +78,7 @@ const CodeEditor = ({
 CodeEditor.propTypes = {
   mode: PropTypes.oneOf(['json', 'yaml']),
   modes: PropTypes.arrayOf(PropTypes.string),
-  onBeforeChange: PropTypes.func.isRequired,
+  onChange: PropTypes.func.isRequired,
 };
 
 CodeEditor.defaultProps = {
@@ -92,7 +104,7 @@ export const DataDrivenFormCodeEditor = ({
           {isRequired ? <RequiredLabel label={label} /> : label }
         </ControlLabel>
         <CodeEditor
-          onBeforeChange={(_editor, _data, value) => onChange(value)}
+          onChange={(_editor, _data, value) => onChange(value)}
           value={value}
           hasError={!!error}
           {...props}

diff --git a/app/javascript/spec/code-editor/__snapshots__/code-editor.spec.js.snap b/app/javascript/spec/code-editor/__snapshots__/code-editor.spec.js.snap
@@ -6,6 +6,7 @@ exports[`CodeEditor component should render correctly 1`] = `
     className="miq-codemirror "
     editorDidMount={[Function]}
     onBeforeChange={[Function]}
+    onChange={[Function]}
     options={
       Object {
         "autoCloseBrackets": true,
@@ -67,6 +68,7 @@ exports[`CodeEditor component should render mode switches 1`] = `
     className="miq-codemirror "
     editorDidMount={[Function]}
     onBeforeChange={[Function]}
+    onChange={[Function]}
     options={
       Object {
         "autoCloseBrackets": true,

diff --git a/app/javascript/spec/code-editor/code-editor.spec.js b/app/javascript/spec/code-editor/code-editor.spec.js
@@ -9,7 +9,7 @@ describe('CodeEditor component', () => {
   let initialProps;
   beforeEach(() => {
     initialProps = {
-      onBeforeChange: jest.fn(),
+      onChange: jest.fn(),
     };
   });
 
@@ -24,10 +24,10 @@ describe('CodeEditor component', () => {
   });
 
   it('should mount and assign correct props to data driven variant', () => {
-    const onBeforeChange = jest.fn();
-    const wrapper = mount(<DataDrivenFormCodeEditor {...initialProps} FieldProvider={FieldProviderComponent} onBeforeChange={onBeforeChange} />);
-    wrapper.find(CodeEditor).props().onBeforeChange('foo');
-    expect(onBeforeChange).toHaveBeenCalledTimes(1);
-    expect(onBeforeChange).toHaveBeenCalledWith('foo');
+    const onChange = jest.fn();
+    const wrapper = mount(<DataDrivenFormCodeEditor {...initialProps} FieldProvider={FieldProviderComponent} onChange={onChange} />);
+    wrapper.find(CodeEditor).props().onChange('foo');
+    expect(onChange).toHaveBeenCalledTimes(1);
+    expect(onChange).toHaveBeenCalledWith('foo');
   });
 });
diff --git a/app/javascript/spec/orchestration-template/orchestration-template-form.spec.js b/app/javascript/spec/orchestration-template/orchestration-template-form.spec.js
@@ -41,7 +41,7 @@ describe('OrcherstrationTemplate form', () => {
        * Code component is not standard input element
        * Two first parameters are codemirror element and data
        */
-      wrapper.find(CodeEditor).props().onBeforeChange(null, null, 'Some random content');
+      wrapper.find(CodeEditor).props().onChange(null, null, 'Some random content');
       wrapper.update();
       wrapper.find('button.btn.btn-primary').simulate('click');
       expect(fetchMock.lastCall()).toBeTruthy();
@@ -123,7 +123,7 @@ describe('OrcherstrationTemplate form', () => {
          * Code component is not standard input element
          * Two first parameters are codemirror element and data
          */
-        wrapper.find(CodeEditor).props().onBeforeChange(null, null, 'updated content');
+        wrapper.find(CodeEditor).props().onChange(null, null, 'updated content');
         wrapper.update();
         wrapper.find('button.btn.btn-primary').simulate('click');
         /**