Change when to insert cells from click or arrowkeys

quadratichq · Jul 26, 2024 · 3e5c6ae · 3e5c6ae
1 parent fd50fa5
commit 3e5c6ae
Show file tree

Hide file tree

Showing 4 changed files with 17 additions and 13 deletions.
diff --git a/quadratic-client/src/app/gridGL/HTMLGrid/inlineEditor/inlineEditorFormula.ts b/quadratic-client/src/app/gridGL/HTMLGrid/inlineEditor/inlineEditorFormula.ts
@@ -145,13 +145,13 @@ class InlineEditorFormula {
     }
   };
 
-  // This is a bit of a hack to ensure that a formula like `SUM(A1,` is not
-  // returned as valid for the `handleCellPointerDown` call. I think Excel
-  // actually knows that it's waiting for a cell reference. Our parser is not as
-  // smart and we do not have this information.
-  private formulaIsReadyToClose() {
+  // Returns whether a cell references is likely to be a valid token at the
+  // cursor position. This is useful for determining whether the user intends to
+  // insert a cell reference when they select a different cell (either by click
+  // or by keyboard input) or whether they want to switch to a different cell.
+  wantsCellRef() {
     const lastCharacter = inlineEditorMonaco.getNonWhitespaceCharBeforeCursor();
-    return ![',', '+', '-', '*', '/', '%', '=', '<', '>', '&', '.', '(', '{'].includes(lastCharacter);
+    return ['', ',', '+', '-', '*', '/', '%', '=', '<', '>', '&', '.', '(', '{'].includes(lastCharacter);
   }
 
   // Returns whether we are editing a formula only if it is valid (used for
@@ -164,7 +164,6 @@ class InlineEditorFormula {
 
     const location = inlineEditorHandler.location;
     if (!location) return false;
-    if (!this.formulaIsReadyToClose()) return false;
     const formula = (testFormula ?? inlineEditorMonaco.get()).slice(1);
     if (!checkFormula(formula, location.x, location.y)) {
       if (skipCloseParenthesisCheck) {

diff --git a/quadratic-client/src/app/gridGL/HTMLGrid/inlineEditor/inlineEditorHandler.ts b/quadratic-client/src/app/gridGL/HTMLGrid/inlineEditor/inlineEditorHandler.ts
@@ -469,7 +469,7 @@ class InlineEditorHandler {
   // (except when editing formula).
   handleCellPointerDown() {
     if (this.open) {
-      if (!this.formula || inlineEditorFormula.isFormulaValid()) {
+      if (!this.formula || !inlineEditorFormula.wantsCellRef()) {
         this.close(0, 0, false);
       } else {
         if (!this.cursorIsMoving) {

diff --git a/quadratic-client/src/app/gridGL/HTMLGrid/inlineEditor/inlineEditorKeyboard.ts b/quadratic-client/src/app/gridGL/HTMLGrid/inlineEditor/inlineEditorKeyboard.ts
@@ -61,14 +61,14 @@ class InlineEditorKeyboard {
           if (column === target) {
             // if we're not moving and the formula is valid, close the editor
             e.stopPropagation();
-            if (inlineEditorFormula.isFormulaValid()) {
-              inlineEditorHandler.close(isRight ? 1 : -1, 0, false);
-            } else {
+            if (inlineEditorFormula.wantsCellRef()) {
               if (isRight) {
                 inlineEditorHandler.cursorIsMoving = true;
                 inlineEditorFormula.addInsertingCells(column);
                 await keyboardPosition(e);
               }
+            } else {
+              inlineEditorHandler.close(isRight ? 1 : -1, 0, false);
             }
           }
         }
@@ -106,8 +106,12 @@ class InlineEditorKeyboard {
         if (inlineEditorHandler.cursorIsMoving) {
           await keyboardPosition(e);
         } else {
-          // if we're not moving and the formula is valid, close the editor
-          if (inlineEditorFormula.isFormulaValid()) {
+          // If we're not moving and the formula doesn't want a cell reference,
+          // close the editor. We can't just use "is the formula syntactically
+          // valid" because many formulas are syntactically valid even though
+          // it's obvious the user wants to insert a cell reference. For
+          // example, `SUM(,)` with the cursor to the left of the comma.
+          if (!inlineEditorFormula.wantsCellRef()) {
             inlineEditorHandler.close(0, e.code === 'ArrowDown' ? 1 : -1, false);
             return;
           }

diff --git a/quadratic-client/src/app/gridGL/HTMLGrid/inlineEditor/inlineEditorMonaco.ts b/quadratic-client/src/app/gridGL/HTMLGrid/inlineEditor/inlineEditorMonaco.ts
@@ -307,6 +307,7 @@ class InlineEditorMonaco {
     this.editor.onDidChangeCursorPosition(inlineEditorHandler.updateMonacoCursorPosition);
     this.editor.onKeyDown((e) => inlineEditorKeyboard.keyDown(e.browserEvent));
     this.editor.onDidChangeCursorPosition(inlineEditorHandler.keepCursorVisible);
+    this.editor.onMouseDown(() => inlineEditorKeyboard.resetKeyboardPosition());
   }
 
   // Sends a keyboard event to the editor (used when returning