From 3fd0159e01f5ff6c3d1a09dcf25fcbb5e7cce238 Mon Sep 17 00:00:00 2001 From: Neha Gokhale Date: Fri, 16 Feb 2024 09:43:59 -0800 Subject: [PATCH] Review comments Signed-off-by: Neha Gokhale --- canvas_modules/common-canvas/package.json | 4 +-- .../expression-builder/expression-builder.jsx | 33 +++++++++++-------- .../controls/expression/expression.jsx | 19 ++++++----- .../controls/expression/expression.scss | 3 -- canvas_modules/harness/package.json | 1 - 5 files changed, 32 insertions(+), 28 deletions(-) diff --git a/canvas_modules/common-canvas/package.json b/canvas_modules/common-canvas/package.json index 760d18d6c3..a251b911f3 100644 --- a/canvas_modules/common-canvas/package.json +++ b/canvas_modules/common-canvas/package.json @@ -63,7 +63,7 @@ "@rollup/plugin-babel": "5.3.0", "@rollup/plugin-commonjs": "21.0.1", "@rollup/plugin-json": "4.1.0", - "@rollup/plugin-node-resolve": "^13.0.6", + "@rollup/plugin-node-resolve": "13.0.6", "@rollup/plugin-url": "6.1.0", "autoprefixer": "9.8.8", "babel-jest": "26.3.0", @@ -98,7 +98,7 @@ "react-redux": "7.2.1", "react-test-renderer": "16.13.1", "reactable": "1.1.0", - "rollup": "^2.60.1", + "rollup": "2.60.1", "rollup-plugin-auto-external": "2.0.0", "rollup-plugin-scss": "3.0.0", "rollup-plugin-terser": "7.0.2", diff --git a/canvas_modules/common-canvas/src/common-properties/controls/expression/expression-builder/expression-builder.jsx b/canvas_modules/common-canvas/src/common-properties/controls/expression/expression-builder/expression-builder.jsx index dc8097d197..2bf9c0327c 100644 --- a/canvas_modules/common-canvas/src/common-properties/controls/expression/expression-builder/expression-builder.jsx +++ b/canvas_modules/common-canvas/src/common-properties/controls/expression/expression-builder/expression-builder.jsx @@ -31,38 +31,39 @@ export default class ExpressionBuilder extends React.Component { this.onChange = this.onChange.bind(this); this.onBlur = this.onBlur.bind(this); this.onSelectionChange = this.onSelectionChange.bind(this); + this.getCodemirrorState = this.getCodemirrorState.bind(this); } onChange(newValue) { const value = (typeof newValue === "string") ? newValue : newValue.toString(); - const somethingSelected = this.editor.viewState.state.selection.ranges.some((r) => !r.empty); - let cursor = this.editor.viewState.state.selection.main.head; + const somethingSelected = this.getCodemirrorState()?.selection.ranges.some((r) => !r.empty); + let cursor = this.getCodemirrorState()?.selection.main.head; if (cursor === 0 && !somethingSelected) { // TODO: Doesn't work when I explicitly set the cursor to 0 // When nothing selected, set cursor at the end of the line - this.editor.dispatch({ selection: { anchor: this.editor.viewState.state.doc.length } }); - cursor = this.editor.viewState.state.selection.main.head; + this.editor.dispatch({ selection: { anchor: this.getCodemirrorState()?.doc.length } }); + cursor = this.getCodemirrorState()?.selection.main.head; this.editor.focus(); } let selectionOffset = 1; if (somethingSelected) { selectionOffset = 0; - this.editor.dispatch(this.editor.viewState.state.replaceSelection(value), { scrollIntoView: true }); + this.editor.dispatch(this.getCodemirrorState()?.replaceSelection(value), { scrollIntoView: true }); this.editor.focus(); } else { let buffer = " "; // if adding to a parenth/bracket/brace expression, no need for space - const currentLineNumber = this.editor.viewState.state.doc.lineAt(this.editor.viewState.state.selection.main.head).number; - const charBefore = this.editor.viewState.state.doc.line(currentLineNumber).text[cursor - 1]; + const currentLineNumber = this.getCodemirrorState()?.doc.lineAt(this.getCodemirrorState()?.selection.main.head).number; + const charBefore = this.getCodemirrorState()?.doc.line(currentLineNumber).text[cursor - 1]; // edge case of cursor being at line 0, char 0 is still handled here if (["(", "[", "{"].indexOf(charBefore) !== -1) { buffer = ""; } - this.editor.dispatch(this.editor.viewState.state.replaceSelection(buffer + value), { scrollIntoView: true }); + this.editor.dispatch(this.getCodemirrorState()?.replaceSelection(buffer + value), { scrollIntoView: true }); this.editor.focus(); } this._setSelection(value, cursor, selectionOffset); // This is needed to generate a render so that the selection will appear. - const exprValue = this.editor.viewState.state.doc.toString(); + const exprValue = this.getCodemirrorState()?.doc.toString(); this.props.controller.updatePropertyValue(this.props.propertyId, exprValue, true); } @@ -72,7 +73,7 @@ export default class ExpressionBuilder extends React.Component { onBlur(editor, evt) { const currentValue = this.props.controller.getPropertyValue(this.props.propertyId); - const newValue = editor.viewState.state.doc.toString(); + const newValue = editor?.viewState?.state?.doc.toString(); const skipValidate = this.expressionSelectionPanel && evt && this.expressionSelectionPanel.contains(evt.relatedTarget); // update property value when value is updated OR value is to be validated if (!isEqual(currentValue, newValue) || !skipValidate) { @@ -80,6 +81,10 @@ export default class ExpressionBuilder extends React.Component { } } + getCodemirrorState() { + return this.editor?.viewState?.state; + } + editorDidMount(editor) { this.editor = editor; } @@ -97,9 +102,9 @@ export default class ExpressionBuilder extends React.Component { } // if the newValue doesn't have a param holder // set it to the first param holder found in the expression - const lineCount = this.editor.viewState.state.doc.lines; + const lineCount = this.getCodemirrorState()?.doc.lines; for (let index = 0; index < lineCount; index++) { - const line = this.editor.viewState.state.doc.line(index + 1).text; + const line = this.getCodemirrorState()?.doc.line(index + 1).text; const paramOffset = line.indexOf("?"); if (paramOffset !== -1) { const selection = { anchor: paramOffset + 1, head: paramOffset }; @@ -109,8 +114,8 @@ export default class ExpressionBuilder extends React.Component { } } // if no parameter holders found then set it to end of insert string - const insertSelection = { anchor: this.editor.viewState.state.selection.main.anchor, - head: this.editor.viewState.state.selection.main.head }; + const insertSelection = { anchor: this.getCodemirrorState()?.selection.main.anchor, + head: this.getCodemirrorState()?.selection.main.head }; this.onSelectionChange([insertSelection]); return; } diff --git a/canvas_modules/common-canvas/src/common-properties/controls/expression/expression.jsx b/canvas_modules/common-canvas/src/common-properties/controls/expression/expression.jsx index e85aac72f7..ceea94373a 100644 --- a/canvas_modules/common-canvas/src/common-properties/controls/expression/expression.jsx +++ b/canvas_modules/common-canvas/src/common-properties/controls/expression/expression.jsx @@ -54,8 +54,6 @@ const pxPerLine = 26; const defaultCharPerLine = 30; const maxLineHeight = 15 * pxPerLine; // 20 lines const minLineHeight = 4 * pxPerLine; // 4 lines - -const editable = new Compartment; // eslint-disable-line class ExpressionControl extends React.Component { constructor(props) { super(props); @@ -64,7 +62,7 @@ class ExpressionControl extends React.Component { validationInProgress: false, expressionEditorHeight: 0 }; - + this.editable = new Compartment; // eslint-disable-line new-parens this.editorRef = React.createRef(); this.origHint = []; this.expressionInfo = this.props.controller.getExpressionInfo(); @@ -80,6 +78,7 @@ class ExpressionControl extends React.Component { this.events = this.events.bind(this); this.handleUpdate = this.handleUpdate.bind(this); this.setCodeMirrorEditable = this.setCodeMirrorEditable.bind(this); + this.getCodemirrorState = this.getCodemirrorState.bind(this); } componentDidMount() { @@ -89,8 +88,8 @@ class ExpressionControl extends React.Component { // this is needed to ensure expression builder selection works. componentDidUpdate(prevProps) { // When code is edited in expression builder, reflect changes in expression flyout - if (!isEqual(this.editor.viewState.state.doc.toString(), this.props.value)) { - this.editor.dispatch({ changes: { from: 0, to: this.editor.viewState.state.doc.length, insert: this.props.value } }); + if (!isEqual(this.getCodemirrorState()?.doc.toString(), this.props.value)) { + this.editor.dispatch({ changes: { from: 0, to: this.getCodemirrorState()?.doc.length, insert: this.props.value } }); } // Toggle editable mode in Codemirror editor if (!isEqual(prevProps.state, this.props.state)) { @@ -109,10 +108,14 @@ class ExpressionControl extends React.Component { } } + getCodemirrorState() { + return this.editor?.viewState?.state; + } + // Set codemirror editor non-editable when disabled setCodeMirrorEditable(value) { this.editor.dispatch({ - effects: editable.reconfigure(EditorView.editable.of(value)) + effects: this.editable.reconfigure(EditorView.editable.of(value)) }); } @@ -205,7 +208,7 @@ class ExpressionControl extends React.Component { language, placeholder(this.props.control.additionalText), this.handleUpdate(), - editable.of(EditorView.editable.of(!(this.props.state === STATES.DISABLED))) + this.editable.of(EditorView.editable.of(!(this.props.state === STATES.DISABLED))) ], parent: this.editorRef.current }); @@ -270,7 +273,7 @@ class ExpressionControl extends React.Component { // this will ensure the expression builder can save values onBlur this.props.onBlur(editor, evt); } else { - const newValue = editor.viewState.state.doc.toString(); + const newValue = editor?.viewState?.state?.doc.toString(); // don't validate when opening the expression builder const skipValidate = evt && evt.relatedTarget && evt.relatedTarget.classList.contains("properties-expression-button"); this.props.controller.updatePropertyValue(this.props.propertyId, newValue, skipValidate); diff --git a/canvas_modules/common-canvas/src/common-properties/controls/expression/expression.scss b/canvas_modules/common-canvas/src/common-properties/controls/expression/expression.scss index db902fb9ff..bf57011fae 100644 --- a/canvas_modules/common-canvas/src/common-properties/controls/expression/expression.scss +++ b/canvas_modules/common-canvas/src/common-properties/controls/expression/expression.scss @@ -20,7 +20,6 @@ .elyra-CodeMirror { .cm-editor { height: inherit; - // overflow: auto; width: 100%; background: $field-02; color: $text-01; @@ -31,13 +30,11 @@ } .cm-content { - // padding-top: $spacing-05; .cm-line { padding-left: $spacing-03; @include carbon--type-style("code-02"); } .cm-placeholder { - // opacity: 0.5; @include carbon--type-style("code-02"); } .cm-cursor { diff --git a/canvas_modules/harness/package.json b/canvas_modules/harness/package.json index b26445c665..87cbf88007 100644 --- a/canvas_modules/harness/package.json +++ b/canvas_modules/harness/package.json @@ -47,7 +47,6 @@ "carbon-components": "10.56.0", "carbon-components-react": "7.56.0", "carbon-icons": "7.0.7", - "codemirror": "6.0.1", "css-loader": "4.3.0", "cypress": "12.17.0", "eslint": "7.9.0",