From 5e49521fc24954cb0cdb30e93a28c1983bd62603 Mon Sep 17 00:00:00 2001 From: Oliver Eyton-Williams Date: Mon, 23 Aug 2021 20:34:53 +0200 Subject: [PATCH] fix(client): some editor edge cases (#43256) * fix: handle erm on line zero * refactor: fix eslint errors * fix: prevent crash on zero height editable region --- .../templates/Challenges/classic/editor.tsx | 50 ++++++++++--------- 1 file changed, 26 insertions(+), 24 deletions(-) diff --git a/client/src/templates/Challenges/classic/editor.tsx b/client/src/templates/Challenges/classic/editor.tsx index 30cb4c6528..9b03b5627f 100644 --- a/client/src/templates/Challenges/classic/editor.tsx +++ b/client/src/templates/Challenges/classic/editor.tsx @@ -335,7 +335,7 @@ const Editor = (props: EditorProps): JSX.Element => { null, () => {} ); - /* eslint-disable */ + /* eslint-enable */ editor.addAction({ id: 'execute-challenge', label: 'Run tests', @@ -365,11 +365,11 @@ const Editor = (props: EditorProps): JSX.Element => { keybindings: [monaco.KeyMod.CtrlCmd | monaco.KeyCode.KEY_E], run: () => { const currentAccessibility = storedAccessibilityMode(); - + store.set('accessibilityMode', !currentAccessibility); editor.updateOptions({ - accessibilitySupport: storedAccessibilityMode() ? 'on' : 'auto', + accessibilitySupport: storedAccessibilityMode() ? 'on' : 'auto' }); } }); @@ -733,7 +733,6 @@ const Editor = (props: EditorProps): JSX.Element => { // TODO: TESTS! // Make 100% sure this is inclusive. - // TODO: pass around monacoRef.current instead of using the global one? const getLinesBetweenRanges = ( firstRange: RangeType, secondRange: RangeType @@ -768,14 +767,17 @@ const Editor = (props: EditorProps): JSX.Element => { // editor const secondRange = model.getDecorationRange(endEditDecId); if (firstRange && secondRange) { - const { startLineNumber, endLineNumber } = getLinesBetweenRanges( - firstRange, - secondRange - ); + const editableRegion = getLinesBetweenRanges(firstRange, secondRange); + const startLineNumber = editableRegion.startLineNumber; + + let endLineNumber = editableRegion.endLineNumber; + + // TODO: this prevents the editor from crashing, but it's still possible + // for the editable region to become empty and for the editable + // decorations to get out of sync with the jaw locations. + endLineNumber = Math.max(endLineNumber, startLineNumber + 1); + endLineNumber = Math.min(endLineNumber, model.getLineCount()); - // getValueInRange includes column x if - // startColumnNumber <= x < endColumnNumber - // so we add 1 here const endColumn = model.getLineLength(endLineNumber) + 1; return new monaco.Range(startLineNumber, 1, endLineNumber, endColumn); } @@ -796,15 +798,11 @@ const Editor = (props: EditorProps): JSX.Element => { const { model } = data; const monaco = monacoRef.current; if (!model || !monaco) return; - const forbiddenRanges: [number, number][] = [ + const forbiddenRegions: [number, number][] = [ [0, editableRegion[0]], [editableRegion[1], model.getLineCount()] ]; - const ranges = forbiddenRanges.map(positions => { - return positionsToRange(model, monaco, positions); - }); - const editableRange = positionsToRange(model, monaco, [ editableRegion[0] + 1, editableRegion[1] - 1 @@ -818,34 +816,40 @@ const Editor = (props: EditorProps): JSX.Element => { // if the forbidden range includes the top of the editor // we simply don't add those decorations - if (forbiddenRanges[0][1] > 0) { + if (forbiddenRegions[0][1] > 0) { + const forbiddenRange = positionsToRange( + model, + monaco, + forbiddenRegions[0] + ); // the first range should expand at the top // TODO: Unsure what this should be - returns an array, so I added [0] @ojeytonwilliams data.startEditDecId = highlightLines( monaco.editor.TrackedRangeStickiness.GrowsOnlyWhenTypingBefore, model, - ranges[0] + forbiddenRange )[0]; highlightText( monaco.editor.TrackedRangeStickiness.GrowsOnlyWhenTypingBefore, model, - ranges[0] + forbiddenRange ); } + const forbiddenRange = positionsToRange(model, monaco, forbiddenRegions[1]); // TODO: handle the case the region covers the bottom of the editor // the second range should expand at the bottom data.endEditDecId = highlightLines( monaco.editor.TrackedRangeStickiness.GrowsOnlyWhenTypingAfter, model, - ranges[1] + forbiddenRange )[0]; highlightText( monaco.editor.TrackedRangeStickiness.GrowsOnlyWhenTypingAfter, model, - ranges[1] + forbiddenRange ); // The deleted line is always considered to be the one that has moved up. @@ -875,9 +879,7 @@ const Editor = (props: EditorProps): JSX.Element => { // edit, then a warned one. Could it track that they need to make 3 // undos? const newLineRanges = getNewLineRanges(e).map(range => { - if (monaco) { - return toStartOfLine(monaco.Range.lift(range)); - } + return toStartOfLine(monaco.Range.lift(range)); }); const deletedLine = getDeletedLine(e);