Merge branch 'qa' of github.com:quadratichq/quadratic into copy-march…

…ing-ants

quadratic-client/src/app/gridGL/HTMLGrid/inlineEditor/inlineEditorFormula.ts

@@ -7,8 +7,8 @@ import { inlineEditorHandler } from '@/app/gridGL/HTMLGrid/inlineEditor/inlineEd
 import { inlineEditorKeyboard } from '@/app/gridGL/HTMLGrid/inlineEditor/inlineEditorKeyboard';
 import { inlineEditorMonaco } from '@/app/gridGL/HTMLGrid/inlineEditor/inlineEditorMonaco';
 import { pixiApp } from '@/app/gridGL/pixiApp/PixiApp';
-import { SheetPosTS } from '@/app/gridGL/types/size';
-import { ParseFormulaReturnType } from '@/app/helpers/formulaNotation';
+import type { SheetPosTS } from '@/app/gridGL/types/size';
+import { parseFormulaReturnToCellsAccessed, type ParseFormulaReturnType } from '@/app/helpers/formulaNotation';
 import { checkFormula, parseFormula } from '@/app/quadratic-rust-client/quadratic_rust_client';
 import { colors } from '@/app/theme/colors';
 import { extractCellsFromParseFormula } from '@/app/ui/menus/CodeEditor/hooks/useEditorCellHighlights';
@@ -26,9 +26,14 @@ class InlineEditorFormula {
   cellHighlights(location: SheetPosTS, formula: string) {
     const parsed = JSON.parse(parseFormula(formula, location.x, location.y)) as ParseFormulaReturnType;
     if (parsed) {
-      pixiApp.cellHighlights.fromFormula(parsed, { x: location.x, y: location.y }, location.sheetId);
-
-      const extractedCells = extractCellsFromParseFormula(parsed, { x: location.x, y: location.y }, location.sheetId);
+      const cellsAccessed = parseFormulaReturnToCellsAccessed(
+        parsed,
+        { x: location.x, y: location.y },
+        location.sheetId
+      );
+      pixiApp.cellHighlights.fromCellsAccessed(cellsAccessed);
+
+      const extractedCells = extractCellsFromParseFormula(parsed, { x: location.x, y: location.y });
       const newDecorations: monaco.editor.IModelDeltaDecoration[] = [];
       const cellColorReferences = new Map<string, number>();
 
@@ -60,7 +65,7 @@ class InlineEditorFormula {
         const editorCursorPosition = inlineEditorMonaco.getPosition();
 
         if (editorCursorPosition && range.containsPosition(editorCursorPosition)) {
-          pixiApp.cellHighlights.setHighlightedCell(index);
+          pixiApp.cellHighlights.setSelectedCell(index);
         }
       });
 

quadratic-client/src/app/gridGL/HTMLGrid/inlineEditor/inlineEditorKeyboard.ts

@@ -419,7 +419,7 @@ class InlineEditorKeyboard {
     if (!location) return;
 
     inlineEditorHandler.cursorIsMoving = false;
-    pixiApp.cellHighlights.clearHighlightedCell();
+    pixiApp.cellHighlights.clearSelectedCell();
     const editingSheet = sheets.getById(location.sheetId);
     if (!editingSheet) {
       throw new Error('Expected editingSheet to be defined in resetKeyboardPosition');

quadratic-client/src/app/gridGL/UI/UICopy.ts

@@ -1,7 +1,6 @@
 import { events } from '@/app/events/events';
 import { sheets } from '@/app/grid/controller/Sheets';
 import { DASHED } from '@/app/gridGL/generateTextures';
-import { intersects } from '@/app/gridGL/helpers/intersects';
 import { pixiApp } from '@/app/gridGL/pixiApp/PixiApp';
 import { drawDashedRectangleMarching } from '@/app/gridGL/UI/cellHighlights/cellHighlightsDraw';
 import { getCSSVariableTint } from '@/app/helpers/convertColor';
@@ -58,39 +57,18 @@ export class UICopy extends Graphics {
 
   private draw() {
     if (!this.ranges) return;
-    const bounds = pixiApp.viewport.getVisibleBounds();
     let render = false;
     this.ranges.forEach((cellRefRange) => {
-      if (!cellRefRange.range) return;
-      const range = cellRefRange.range;
-      let minX = Number(range.start.col.coord);
-      let minY = Number(range.start.row.coord);
-      let maxX: number;
-      if (range.end.col.coord < 0) {
-        maxX = bounds.width + DASHED;
-      } else {
-        minX = Math.min(minX, Number(range.end.col.coord));
-        maxX = Math.max(Number(range.start.col.coord), Number(range.end.col.coord));
-      }
-      let maxY: number;
-      if (range.end.row.coord < 0) {
-        maxY = bounds.height + DASHED;
-      } else {
-        minY = Math.min(minY, Number(range.end.row.coord));
-        maxY = Math.max(Number(range.start.row.coord), Number(range.end.row.coord));
-      }
-      const rect = sheets.sheet.getScreenRectangle(minX, minY, maxX - minX + 1, maxY - minY + 1);
-      rect.x += RECT_OFFSET;
-      rect.y += RECT_OFFSET;
-      rect.width -= RECT_OFFSET * 2;
-      rect.height -= RECT_OFFSET * 2;
       const color = getCSSVariableTint('primary');
-      drawDashedRectangleMarching(this, color, rect, this.march, true, ALPHA);
-      if (!render) {
-        if (intersects.rectangleRectangle(rect, bounds)) {
-          render = true;
-        }
-      }
+      render ||= drawDashedRectangleMarching({
+        g: this,
+        color,
+        march: this.march,
+        noFill: true,
+        alpha: ALPHA,
+        offset: RECT_OFFSET,
+        range: cellRefRange,
+      });
     });
     if (render) {
       pixiApp.setViewportDirty();

quadratic-client/src/app/gridGL/UI/cellHighlights/CellHighlights.ts

@@ -3,43 +3,18 @@ import { sheets } from '@/app/grid/controller/Sheets';
 import { DASHED } from '@/app/gridGL/generateTextures';
 import { inlineEditorHandler } from '@/app/gridGL/HTMLGrid/inlineEditor/inlineEditorHandler';
 import { pixiApp } from '@/app/gridGL/pixiApp/PixiApp';
-import {
-  drawDashedRectangle,
-  drawDashedRectangleForCellsAccessed,
-  drawDashedRectangleMarching,
-} from '@/app/gridGL/UI/cellHighlights/cellHighlightsDraw';
+import { drawDashedRectangle, drawDashedRectangleMarching } from '@/app/gridGL/UI/cellHighlights/cellHighlightsDraw';
 import { convertColorStringToTint } from '@/app/helpers/convertColor';
-import { CellPosition, ParseFormulaReturnType, Span } from '@/app/helpers/formulaNotation';
-import { JsCellsAccessed, JsCoordinate } from '@/app/quadratic-core-types';
+import type { JsCellsAccessed } from '@/app/quadratic-core-types';
 import { colors } from '@/app/theme/colors';
 import { Container, Graphics } from 'pixi.js';
 
-// TODO: these files need to be cleaned up and properly typed. Lots of untyped
-// data passed around within the data.
-
-export interface HighlightedCellRange {
-  column: number;
-  row: number;
-  width: number;
-  height: number;
-  span: Span;
-  sheet: string;
-  index: number;
-}
-
-export interface HighlightedCell {
-  column: number;
-  row: number;
-  sheet: string;
-}
-
 const NUM_OF_CELL_REF_COLORS = colors.cellHighlightColor.length;
 const MARCH_ANIMATE_TIME_MS = 80;
 
 export class CellHighlights extends Container {
-  private highlightedCells: HighlightedCellRange[] = [];
   private cellsAccessed: JsCellsAccessed[] = [];
-  highlightedCellIndex: number | undefined;
+  private selectedCellIndex: number | undefined;
 
   private highlights: Graphics;
   private marchingHighlight: Graphics;
@@ -65,9 +40,8 @@ export class CellHighlights extends Container {
   };
 
   clear() {
-    this.highlightedCells = [];
     this.cellsAccessed = [];
-    this.highlightedCellIndex = undefined;
+    this.selectedCellIndex = undefined;
     this.highlights.clear();
     this.marchingHighlight.clear();
     pixiApp.setViewportDirty();
@@ -77,76 +51,54 @@ export class CellHighlights extends Container {
   private draw() {
     this.highlights.clear();
 
-    const highlightedCells = [...this.highlightedCells];
-    const highlightedCellIndex = this.highlightedCellIndex;
-    highlightedCells
-      .filter((cells) => cells.sheet === sheets.current)
-      .forEach((cell, index) => {
-        const colorNumber = convertColorStringToTint(colors.cellHighlightColor[cell.index % NUM_OF_CELL_REF_COLORS]);
-        const cursorCell = sheets.sheet.getScreenRectangle(cell.column, cell.row, cell.width, cell.height);
+    if (!this.cellsAccessed.length) return;
 
-        // We do not draw the dashed rectangle if the inline Formula editor's cell
-        // cursor is moving (it's handled by updateMarchingHighlights instead).
-        if (
-          highlightedCellIndex === undefined ||
-          highlightedCellIndex !== index ||
-          !inlineEditorHandler.cursorIsMoving
-        ) {
-          drawDashedRectangle({
-            g: this.highlights,
-            color: colorNumber,
-            isSelected: highlightedCellIndex === index,
-            startCell: cursorCell,
-          });
-        }
-      });
+    const selectedCellIndex = this.selectedCellIndex;
 
     const cellsAccessed = [...this.cellsAccessed];
     cellsAccessed
       .filter(({ sheetId }) => sheetId === sheets.current)
       .flatMap(({ ranges }) => ranges)
       .forEach((range, index) => {
-        drawDashedRectangleForCellsAccessed({
-          g: this.highlights,
-          color: convertColorStringToTint(colors.cellHighlightColor[index % NUM_OF_CELL_REF_COLORS]),
-          isSelected: false,
-          range,
-        });
+        if (selectedCellIndex === undefined || selectedCellIndex !== index || !inlineEditorHandler.cursorIsMoving) {
+          drawDashedRectangle({
+            g: this.highlights,
+            color: convertColorStringToTint(colors.cellHighlightColor[index % NUM_OF_CELL_REF_COLORS]),
+            isSelected: selectedCellIndex === index,
+            range,
+          });
+        }
       });
 
-    if (highlightedCells.length || cellsAccessed.length) {
-      pixiApp.setViewportDirty();
-    }
+    pixiApp.setViewportDirty();
   }
 
   // Draws the marching highlights by using an offset dashed line to create the
   // marching effect.
   private updateMarchingHighlight() {
     if (!inlineEditorHandler.cursorIsMoving) {
-      this.highlightedCellIndex = undefined;
+      this.selectedCellIndex = undefined;
       return;
     }
     // Index may not have been set yet.
-    if (this.highlightedCellIndex === undefined) return;
+    if (this.selectedCellIndex === undefined) return;
     if (this.marchLastTime === 0) {
       this.marchLastTime = Date.now();
     } else if (Date.now() - this.marchLastTime < MARCH_ANIMATE_TIME_MS) {
       return;
     } else {
       this.marchLastTime = Date.now();
     }
-    const highlightedCell = this.highlightedCells[this.highlightedCellIndex];
-    if (!highlightedCell) return;
-    const colorNumber = convertColorStringToTint(
-      colors.cellHighlightColor[highlightedCell.index % NUM_OF_CELL_REF_COLORS]
-    );
-    const cursorCell = sheets.sheet.getScreenRectangle(
-      highlightedCell.column,
-      highlightedCell.row,
-      highlightedCell.width,
-      highlightedCell.height
-    );
-    drawDashedRectangleMarching(this.marchingHighlight, colorNumber, cursorCell, this.march);
+    const selectedCellIndex = this.selectedCellIndex;
+    const accessedCell = this.cellsAccessed[selectedCellIndex];
+    if (!accessedCell) return;
+    const colorNumber = convertColorStringToTint(colors.cellHighlightColor[selectedCellIndex % NUM_OF_CELL_REF_COLORS]);
+    drawDashedRectangleMarching({
+      g: this.marchingHighlight,
+      color: colorNumber,
+      march: this.march,
+      range: accessedCell.ranges[0],
+    });
     this.march = (this.march + 1) % Math.floor(DASHED);
     pixiApp.setViewportDirty();
   }
@@ -169,92 +121,24 @@ export class CellHighlights extends Container {
     return this.dirty || inlineEditorHandler.cursorIsMoving;
   }
 
-  private getSheet(cellSheet: string | undefined, sheetId: string): string {
-    if (!cellSheet) return sheetId;
-
-    // It may come in as either a sheet id or a sheet name.
-    if (sheets.getById(cellSheet)) {
-      return cellSheet;
-    }
-    return sheets.getSheetByName(cellSheet)?.id ?? sheetId;
-  }
-
   evalCoord(cell: { type: 'Relative' | 'Absolute'; coord: number }, origin: number) {
     const isRelative = cell.type === 'Relative';
     const getOrigin = isRelative ? origin : 0;
 
     return getOrigin + cell.coord;
   }
 
-  private fromCellRange(
-    cellRange: { type: 'CellRange'; start: CellPosition; end: CellPosition; sheet?: string },
-    origin: JsCoordinate,
-    sheet: string,
-    span: Span,
-    index: number
-  ) {
-    const startX = this.evalCoord(cellRange.start.x, origin.x);
-    const startY = this.evalCoord(cellRange.start.y, origin.y);
-    const endX = this.evalCoord(cellRange.end.x, origin.x);
-    const endY = this.evalCoord(cellRange.end.y, origin.y);
-    this.highlightedCells.push({
-      column: startX,
-      row: startY,
-      width: endX - startX + 1,
-      height: endY - startY + 1,
-      sheet: this.getSheet(cellRange.sheet ?? cellRange.start.sheet, sheet),
-      span,
-      index,
-    });
-  }
-
-  private fromCell(cell: CellPosition, origin: JsCoordinate, sheet: string, span: Span, index: number) {
-    this.highlightedCells.push({
-      column: this.evalCoord(cell.x, origin.x),
-      row: this.evalCoord(cell.y, origin.y),
-      width: 1,
-      height: 1,
-      sheet: this.getSheet(cell.sheet, sheet),
-      span,
-      index,
-    });
-  }
-
-  fromFormula(formula: ParseFormulaReturnType, cell: JsCoordinate, sheet: string) {
-    this.highlightedCells = [];
-
-    formula.cell_refs.forEach((cellRef, index) => {
-      switch (cellRef.cell_ref.type) {
-        case 'CellRange':
-          this.fromCellRange(cellRef.cell_ref, cell, sheet, cellRef.span, index);
-          break;
-
-        case 'Cell':
-          this.fromCell(cellRef.cell_ref.pos, cell, sheet, cellRef.span, index);
-          break;
-
-        default:
-          throw new Error('Unsupported cell-ref in fromFormula');
-      }
-    });
-    pixiApp.cellHighlights.dirty = true;
-  }
-
   fromCellsAccessed(cellsAccessed: JsCellsAccessed[] | null) {
     this.cellsAccessed = cellsAccessed ?? [];
     pixiApp.cellHighlights.dirty = true;
   }
 
-  setHighlightedCell(index: number) {
-    this.highlightedCellIndex = this.highlightedCells.findIndex((cell) => cell.index === index);
-  }
-
-  getHighlightedCells() {
-    return this.highlightedCells;
+  setSelectedCell(index: number) {
+    this.selectedCellIndex = index;
   }
 
-  clearHighlightedCell() {
-    this.highlightedCellIndex = undefined;
+  clearSelectedCell() {
+    this.selectedCellIndex = undefined;
     this.marchingHighlight.clear();
   }
 }