Skip to content

Commit

Permalink
Review comments
Browse files Browse the repository at this point in the history
Signed-off-by: Neha Gokhale <[email protected]>
  • Loading branch information
nmgokhale committed Feb 16, 2024
1 parent 37f0c0c commit 3fd0159
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 28 deletions.
4 changes: 2 additions & 2 deletions canvas_modules/common-canvas/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand All @@ -72,14 +73,18 @@ 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) {
this.props.controller.updatePropertyValue(this.props.propertyId, newValue, skipValidate);
}
}

getCodemirrorState() {
return this.editor?.viewState?.state;
}

editorDidMount(editor) {
this.editor = editor;
}
Expand All @@ -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 };
Expand All @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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();
Expand All @@ -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() {
Expand All @@ -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)) {
Expand All @@ -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))
});
}

Expand Down Expand Up @@ -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
});
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
.elyra-CodeMirror {
.cm-editor {
height: inherit;
// overflow: auto;
width: 100%;
background: $field-02;
color: $text-01;
Expand All @@ -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 {
Expand Down
1 change: 0 additions & 1 deletion canvas_modules/harness/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down

0 comments on commit 3fd0159

Please sign in to comment.