From 7759f8fc78bebaef518ef003aa7e1159e2ca76cd Mon Sep 17 00:00:00 2001 From: AyushAgrawal-A2 Date: Wed, 18 Dec 2024 03:28:01 +0530 Subject: [PATCH 1/9] formula dashed rectangle highlights --- .../inlineEditor/inlineEditorFormula.ts | 15 ++++-- .../UI/cellHighlights/CellHighlights.ts | 4 +- .../src/app/gridGL/helpers/selection.ts | 6 ++- quadratic-client/src/app/gridGL/types/size.ts | 10 +--- .../src/app/helpers/formulaNotation.ts | 51 +++++++++++-------- .../hooks/useEditorCellHighlights.ts | 14 +++-- quadratic-core/src/formulas/cell_ref.rs | 12 ++--- quadratic-rust-client/src/parse_formula.rs | 13 +++-- 8 files changed, 68 insertions(+), 57 deletions(-) diff --git a/quadratic-client/src/app/gridGL/HTMLGrid/inlineEditor/inlineEditorFormula.ts b/quadratic-client/src/app/gridGL/HTMLGrid/inlineEditor/inlineEditorFormula.ts index 749b5b2792..426252fe43 100644 --- a/quadratic-client/src/app/gridGL/HTMLGrid/inlineEditor/inlineEditorFormula.ts +++ b/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(); diff --git a/quadratic-client/src/app/gridGL/UI/cellHighlights/CellHighlights.ts b/quadratic-client/src/app/gridGL/UI/cellHighlights/CellHighlights.ts index 4d13d87404..99bb94b6d6 100644 --- a/quadratic-client/src/app/gridGL/UI/cellHighlights/CellHighlights.ts +++ b/quadratic-client/src/app/gridGL/UI/cellHighlights/CellHighlights.ts @@ -9,8 +9,8 @@ import { 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 { CellPosition, ParseFormulaReturnType } from '@/app/helpers/formulaNotation'; +import type { JsCellsAccessed, JsCoordinate, Span } from '@/app/quadratic-core-types'; import { colors } from '@/app/theme/colors'; import { Container, Graphics } from 'pixi.js'; diff --git a/quadratic-client/src/app/gridGL/helpers/selection.ts b/quadratic-client/src/app/gridGL/helpers/selection.ts index 38dd48ca41..6528d9a72d 100644 --- a/quadratic-client/src/app/gridGL/helpers/selection.ts +++ b/quadratic-client/src/app/gridGL/helpers/selection.ts @@ -5,8 +5,10 @@ import { Rectangle } from 'pixi.js'; // returns rectangle representing the range in col/row coordinates export function getRangeRectangleFromCellRefRange({ range }: CellRefRange): Rectangle { const { col, row } = range.start; - const startCol = Number(col.coord); - const startRow = Number(row.coord); + let startCol = Number(col.coord); + if (startCol === -1) startCol = 1; + let startRow = Number(row.coord); + if (startRow === -1) startRow = 1; const end = range.end; let endCol = Number(end.col.coord); diff --git a/quadratic-client/src/app/gridGL/types/size.ts b/quadratic-client/src/app/gridGL/types/size.ts index 368a4fe730..423ed69f9e 100644 --- a/quadratic-client/src/app/gridGL/types/size.ts +++ b/quadratic-client/src/app/gridGL/types/size.ts @@ -1,4 +1,5 @@ -import { JsCoordinate } from '@/app/quadratic-core-types'; +import type { JsCoordinate } from '@/app/quadratic-core-types'; +import type { Rectangle } from 'pixi.js'; export interface SheetPosTS { x: number; @@ -11,13 +12,6 @@ export interface Size { height: number; } -export interface Rectangle { - x: number; - y: number; - width: number; - height: number; -} - export function coordinateEqual(a: JsCoordinate, b: JsCoordinate): boolean { return a.x === b.x && a.y === b.y; } diff --git a/quadratic-client/src/app/helpers/formulaNotation.ts b/quadratic-client/src/app/helpers/formulaNotation.ts index 34f3acb26f..59505eb4cb 100644 --- a/quadratic-client/src/app/helpers/formulaNotation.ts +++ b/quadratic-client/src/app/helpers/formulaNotation.ts @@ -1,8 +1,8 @@ import { sheets } from '@/app/grid/controller/Sheets'; -import { StringId } from '@/app/helpers/getKey'; -import { JsCellsAccessed, JsCoordinate } from '@/app/quadratic-core-types'; -import { CellRefId } from '@/app/ui/menus/CodeEditor/hooks/useEditorCellHighlights'; -import { Rectangle } from 'pixi.js'; +import type { StringId } from '@/app/helpers/getKey'; +import type { JsCellsAccessed, JsCoordinate, Span } from '@/app/quadratic-core-types'; +import type { CellRefId } from '@/app/ui/menus/CodeEditor/hooks/useEditorCellHighlights'; +import type { Rectangle } from 'pixi.js'; export function getCoordinatesFromStringId(stringId: StringId): [number, number] { // required for type inference @@ -16,13 +16,6 @@ export interface CellPosition { sheet?: string; } -export type Span = { start: number; end: number }; - -export type CellRefCoord = { - x: { type: 'Relative' | 'Absolute'; coord: number }; - y: { type: 'Relative' | 'Absolute'; coord: number }; -}; - export type CellRef = | { type: 'CellRange'; @@ -46,35 +39,51 @@ export type ParseFormulaReturnType = { }[]; }; -export function parseFormulaReturnToCellsAccessed(parseFormulaReturn: ParseFormulaReturnType): JsCellsAccessed[] { +export function parseFormulaReturnToCellsAccessed( + parseFormulaReturn: ParseFormulaReturnType, + codeCellPos: JsCoordinate, + codeCellSheetId: string +): JsCellsAccessed[] { const isAbsolute = (type: 'Relative' | 'Absolute') => type === 'Absolute'; - const returnArray: JsCellsAccessed[] = []; + const jsCellsAccessed: JsCellsAccessed[] = []; for (const cellRef of parseFormulaReturn.cell_refs) { const start = cellRef.cell_ref.type === 'CellRange' ? cellRef.cell_ref.start : cellRef.cell_ref.pos; const end = cellRef.cell_ref.type === 'CellRange' ? cellRef.cell_ref.end : cellRef.cell_ref.pos; - const cellsAccessed = { - sheetId: cellRef.sheet ?? '', + const cellsAccessed: JsCellsAccessed = { + sheetId: cellRef.sheet ?? codeCellSheetId, ranges: [ { range: { start: { - col: { coord: BigInt(start.x.coord), is_absolute: isAbsolute(start.x.type) }, - row: { coord: BigInt(start.y.coord), is_absolute: isAbsolute(start.y.type) }, + col: { + coord: BigInt(isAbsolute(start.x.type) ? start.x.coord : start.x.coord + codeCellPos.x), + is_absolute: isAbsolute(start.x.type), + }, + row: { + coord: BigInt(isAbsolute(start.y.type) ? start.y.coord : start.y.coord + codeCellPos.y), + is_absolute: isAbsolute(start.y.type), + }, }, end: { - col: { coord: BigInt(end.x.coord), is_absolute: isAbsolute(end.x.type) }, - row: { coord: BigInt(end.y.coord), is_absolute: isAbsolute(end.y.type) }, + col: { + coord: BigInt(isAbsolute(end.x.type) ? end.x.coord : end.x.coord + codeCellPos.x), + is_absolute: isAbsolute(end.x.type), + }, + row: { + coord: BigInt(isAbsolute(end.y.type) ? end.y.coord : end.y.coord + codeCellPos.y), + is_absolute: isAbsolute(end.y.type), + }, }, }, }, ], }; - returnArray.push(cellsAccessed); + jsCellsAccessed.push(cellsAccessed); } - return returnArray; + return jsCellsAccessed; } export function getCellFromFormulaNotation(sheetId: string, cellRefId: CellRefId, editorCursorPosition: JsCoordinate) { diff --git a/quadratic-client/src/app/ui/menus/CodeEditor/hooks/useEditorCellHighlights.ts b/quadratic-client/src/app/ui/menus/CodeEditor/hooks/useEditorCellHighlights.ts index c08670f92b..9ab4eb5bb4 100644 --- a/quadratic-client/src/app/ui/menus/CodeEditor/hooks/useEditorCellHighlights.ts +++ b/quadratic-client/src/app/ui/menus/CodeEditor/hooks/useEditorCellHighlights.ts @@ -5,9 +5,9 @@ import { } from '@/app/atoms/codeEditorAtom'; import { pixiApp } from '@/app/gridGL/pixiApp/PixiApp'; import { codeCellIsAConnection } from '@/app/helpers/codeCellLanguage'; -import { parseFormulaReturnToCellsAccessed, ParseFormulaReturnType, Span } from '@/app/helpers/formulaNotation'; +import { parseFormulaReturnToCellsAccessed, ParseFormulaReturnType } from '@/app/helpers/formulaNotation'; import { getKey, StringId } from '@/app/helpers/getKey'; -import { JsCoordinate } from '@/app/quadratic-core-types'; +import { JsCoordinate, Span } from '@/app/quadratic-core-types'; import { parseFormula } from '@/app/quadratic-rust-client/quadratic_rust_client'; import { colors } from '@/app/theme/colors'; import { Monaco } from '@monaco-editor/react'; @@ -17,8 +17,7 @@ import { useRecoilValue } from 'recoil'; export function extractCellsFromParseFormula( parsedFormula: ParseFormulaReturnType, - cell: JsCoordinate, - sheet: string + cell: JsCoordinate ): { cellId: CellRefId; span: Span; index: number }[] { return parsedFormula.cell_refs.map(({ cell_ref, span }, index) => { if (cell_ref.type === 'CellRange') { @@ -100,10 +99,9 @@ export const useEditorCellHighlights = ( } else if (codeCell.language === 'Formula') { const parsed = JSON.parse(parseFormula(modelValue, codeCell.pos.x, codeCell.pos.y)) as ParseFormulaReturnType; if (parsed) { - // pixiApp.cellHighlights.fromFormula(parsed, codeCell.pos, codeCell.sheetId); - let cellsAccessed = parseFormulaReturnToCellsAccessed(parsed); - pixiApp.cellHighlights.fromCellsAccessed(unsavedChanges ? null : cellsAccessed); - const extractedCells = extractCellsFromParseFormula(parsed, codeCell.pos, codeCell.sheetId); + const cellsAccessed = parseFormulaReturnToCellsAccessed(parsed, codeCell.pos, codeCell.sheetId); + pixiApp.cellHighlights.fromCellsAccessed(cellsAccessed); + const extractedCells = extractCellsFromParseFormula(parsed, codeCell.pos); extractedCells.forEach((value, index) => { const { cellId, span } = value; const startPosition = model.getPositionAt(span.start); diff --git a/quadratic-core/src/formulas/cell_ref.rs b/quadratic-core/src/formulas/cell_ref.rs index 51a485766e..b0adf19b3b 100644 --- a/quadratic-core/src/formulas/cell_ref.rs +++ b/quadratic-core/src/formulas/cell_ref.rs @@ -203,13 +203,13 @@ impl CellRef { } // replace unbounded values with the given value - pub fn replace_unbounded(&mut self, value: i64) { + pub fn replace_unbounded(&mut self, value: i64, pos: Pos) { // TODO(ddimaria): the -1 is a hack, replace after testing - if self.x.get_value() == UNBOUNDED || self.x.get_value() == UNBOUNDED - 1 { + if self.x.get_value(pos.x) == UNBOUNDED { self.x.replace_value(value); } // TODO(ddimaria): the -1 is a hack, replace after testing - if self.y.get_value() == UNBOUNDED || self.y.get_value() == UNBOUNDED - 1 { + if self.y.get_value(pos.y) == UNBOUNDED { self.y.replace_value(value); } } @@ -289,15 +289,15 @@ impl CellRefCoord { format!("{row}{}", self.prefix()) } - pub fn get_value(self) -> i64 { + pub fn get_value(self, base: i64) -> i64 { match self { - CellRefCoord::Relative(delta) => delta, + CellRefCoord::Relative(delta) => delta.saturating_add(base), CellRefCoord::Absolute(coord) => coord, } } pub fn replace_value(&mut self, value: i64) { *self = match self { - CellRefCoord::Relative(_) => Self::Relative(value), + CellRefCoord::Relative(_) => Self::Absolute(value), CellRefCoord::Absolute(_) => Self::Absolute(value), }; } diff --git a/quadratic-rust-client/src/parse_formula.rs b/quadratic-rust-client/src/parse_formula.rs index e2f6a7402e..8feb3c7d73 100644 --- a/quadratic-rust-client/src/parse_formula.rs +++ b/quadratic-rust-client/src/parse_formula.rs @@ -69,20 +69,23 @@ impl From> for JsCellRefSpan { pub fn parse_formula(formula_string: &str, x: f64, y: f64) -> String { let x = x as i64; let y = y as i64; - let pos = Pos { x, y }; + let code_cell_pos = Pos { x, y }; - let parse_error = formulas::parse_formula(formula_string, pos).err(); + let parse_error = formulas::parse_formula(formula_string, code_cell_pos).err(); let result = JsFormulaParseResult { parse_error_msg: parse_error.as_ref().map(|e| e.msg.to_string()), parse_error_span: parse_error.and_then(|e| e.span), - cell_refs: formulas::find_cell_references(formula_string, pos) + cell_refs: formulas::find_cell_references(formula_string, code_cell_pos) .into_iter() .map(|mut spanned| { if let RangeRef::CellRange { mut start, mut end } = spanned.inner { - start.replace_unbounded(0); - end.replace_unbounded(-1); + start.replace_unbounded(1, code_cell_pos); + end.replace_unbounded(-1, code_cell_pos); spanned.inner = RangeRef::CellRange { start, end }; + } else if let RangeRef::Cell { mut pos } = spanned.inner { + pos.replace_unbounded(-1, code_cell_pos); + spanned.inner = RangeRef::Cell { pos }; } spanned.into() }) From 6db56825991ed5c541f35ebb92337dda0a440b13 Mon Sep 17 00:00:00 2001 From: AyushAgrawal-A2 Date: Wed, 18 Dec 2024 04:52:45 +0530 Subject: [PATCH 2/9] formula marching highlights and selected cell fill --- .../inlineEditor/inlineEditorFormula.ts | 2 +- .../inlineEditor/inlineEditorKeyboard.ts | 2 +- .../UI/cellHighlights/CellHighlights.ts | 178 +++--------------- .../UI/cellHighlights/cellHighlightsDraw.ts | 153 ++++++--------- .../src/app/helpers/formulaNotation.ts | 60 +----- .../ui/menus/CodeEditor/CodeEditorBody.tsx | 2 - .../hooks/useEditorCellHighlights.ts | 21 ++- .../hooks/useEditorOnSelectionChange.ts | 45 ----- .../renderWebWorker/renderWebWorker.ts | 2 +- 9 files changed, 107 insertions(+), 358 deletions(-) delete mode 100644 quadratic-client/src/app/ui/menus/CodeEditor/hooks/useEditorOnSelectionChange.ts diff --git a/quadratic-client/src/app/gridGL/HTMLGrid/inlineEditor/inlineEditorFormula.ts b/quadratic-client/src/app/gridGL/HTMLGrid/inlineEditor/inlineEditorFormula.ts index 426252fe43..fc54303c6e 100644 --- a/quadratic-client/src/app/gridGL/HTMLGrid/inlineEditor/inlineEditorFormula.ts +++ b/quadratic-client/src/app/gridGL/HTMLGrid/inlineEditor/inlineEditorFormula.ts @@ -65,7 +65,7 @@ class InlineEditorFormula { const editorCursorPosition = inlineEditorMonaco.getPosition(); if (editorCursorPosition && range.containsPosition(editorCursorPosition)) { - pixiApp.cellHighlights.setHighlightedCell(index); + pixiApp.cellHighlights.setSelectedCell(index); } }); diff --git a/quadratic-client/src/app/gridGL/HTMLGrid/inlineEditor/inlineEditorKeyboard.ts b/quadratic-client/src/app/gridGL/HTMLGrid/inlineEditor/inlineEditorKeyboard.ts index c392181226..734b4b702c 100644 --- a/quadratic-client/src/app/gridGL/HTMLGrid/inlineEditor/inlineEditorKeyboard.ts +++ b/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'); diff --git a/quadratic-client/src/app/gridGL/UI/cellHighlights/CellHighlights.ts b/quadratic-client/src/app/gridGL/UI/cellHighlights/CellHighlights.ts index 99bb94b6d6..e74a3afda4 100644 --- a/quadratic-client/src/app/gridGL/UI/cellHighlights/CellHighlights.ts +++ b/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 type { CellPosition, ParseFormulaReturnType } from '@/app/helpers/formulaNotation'; -import type { JsCellsAccessed, JsCoordinate, Span } 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,57 +51,37 @@ 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) { @@ -135,18 +89,16 @@ export class CellHighlights extends Container { } 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,16 +121,6 @@ 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; @@ -186,75 +128,17 @@ export class CellHighlights extends Container { 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(); } } diff --git a/quadratic-client/src/app/gridGL/UI/cellHighlights/cellHighlightsDraw.ts b/quadratic-client/src/app/gridGL/UI/cellHighlights/cellHighlightsDraw.ts index 5635ffcb74..6d8a665f59 100644 --- a/quadratic-client/src/app/gridGL/UI/cellHighlights/cellHighlightsDraw.ts +++ b/quadratic-client/src/app/gridGL/UI/cellHighlights/cellHighlightsDraw.ts @@ -4,57 +4,77 @@ import { getRangeScreenRectangleFromCellRefRange } from '@/app/gridGL/helpers/se import { pixiApp } from '@/app/gridGL/pixiApp/PixiApp'; import { CURSOR_THICKNESS, FILL_ALPHA } from '@/app/gridGL/UI/Cursor'; import { CellRefRange } from '@/app/quadratic-core-types'; -import { Graphics, Rectangle } from 'pixi.js'; +import { Graphics } from 'pixi.js'; + +export function drawDashedRectangle(options: { g: Graphics; color: number; isSelected: boolean; range: CellRefRange }) { + const { g, color, isSelected, range } = options; + + const selectionRect = getRangeScreenRectangleFromCellRefRange(range); + const bounds = pixiApp.viewport.getVisibleBounds(); + if (!intersects.rectangleRectangle(selectionRect, bounds)) { + return; + } + + g.lineStyle({ + width: CURSOR_THICKNESS, + color, + alignment: 0.5, + texture: generatedTextures.dashedHorizontal, + }); + g.moveTo(selectionRect.left, selectionRect.top); + g.lineTo(Math.min(selectionRect.right, bounds.right), selectionRect.top); + if (selectionRect.bottom <= bounds.bottom) { + g.moveTo(Math.min(selectionRect.right, bounds.right), selectionRect.bottom); + g.lineTo(selectionRect.left, selectionRect.bottom); + } + + g.lineStyle({ + width: CURSOR_THICKNESS, + color, + alignment: 0.5, + texture: generatedTextures.dashedVertical, + }); + g.moveTo(selectionRect.left, Math.min(selectionRect.bottom, bounds.bottom)); + g.lineTo(selectionRect.left, selectionRect.top); + if (selectionRect.right <= bounds.right) { + g.moveTo(selectionRect.right, Math.min(selectionRect.bottom, bounds.bottom)); + g.lineTo(selectionRect.right, selectionRect.top); + } -export function drawDashedRectangle(options: { - g: Graphics; - color: number; - isSelected: boolean; - startCell: Rectangle; - endCell?: Rectangle; -}) { - const { g, color, isSelected, startCell, endCell } = options; - const minX = Math.min(startCell.x, endCell?.x ?? Infinity); - const minY = Math.min(startCell.y, endCell?.y ?? Infinity); - const maxX = Math.max(startCell.width + startCell.x, endCell ? endCell.x + endCell.width : -Infinity); - const maxY = Math.max(startCell.y + startCell.height, endCell ? endCell.y + endCell.height : -Infinity); - - const path = [ - [maxX, minY], - [maxX, maxY], - [minX, maxY], - [minX, minY], - ]; - - // have to fill a rect because setting multiple line styles makes it unable to be filled if (isSelected) { g.lineStyle({ alignment: 0, }); - g.moveTo(minX, minY); + g.moveTo(selectionRect.left, selectionRect.top); g.beginFill(color, FILL_ALPHA); - g.drawRect(minX, minY, maxX - minX, maxY - minY); + g.drawRect( + selectionRect.left, + selectionRect.top, + Math.min(selectionRect.right, bounds.right) - selectionRect.left, + Math.min(selectionRect.bottom, bounds.bottom) - selectionRect.top + ); g.endFill(); } +} - g.moveTo(minX, minY); - for (let i = 0; i < path.length; i++) { - const texture = i % 2 === 0 ? generatedTextures.dashedHorizontal : generatedTextures.dashedVertical; - g.lineStyle({ - width: CURSOR_THICKNESS, - color, - alignment: 0, - texture, - }); - g.lineTo(path[i][0], path[i][1]); +export function drawDashedRectangleMarching(options: { + g: Graphics; + color: number; + march: number; + range: CellRefRange; +}) { + const { g, color, march, range } = options; + + const selectionRect = getRangeScreenRectangleFromCellRefRange(range); + const bounds = pixiApp.viewport.getVisibleBounds(); + if (!intersects.rectangleRectangle(selectionRect, bounds)) { + return; } -} -export function drawDashedRectangleMarching(g: Graphics, color: number, startCell: Rectangle, march: number) { - const minX = startCell.x; - const minY = startCell.y; - const maxX = startCell.width + startCell.x; - const maxY = startCell.y + startCell.height; + const minX = selectionRect.left; + const minY = selectionRect.top; + const maxX = selectionRect.right; + const maxY = selectionRect.bottom; g.clear(); @@ -113,56 +133,3 @@ export function drawDashedRectangleMarching(g: Graphics, color: number, startCel g.lineTo(minX + DASHED_THICKNESS, clamp(y, minY, maxY)); } } - -export function drawDashedRectangleForCellsAccessed(options: { - g: Graphics; - color: number; - isSelected: boolean; - range: CellRefRange; -}) { - const { g, color, isSelected, range } = options; - const bounds = pixiApp.viewport.getVisibleBounds(); - const selectionRect = getRangeScreenRectangleFromCellRefRange(range); - if (intersects.rectangleRectangle(selectionRect, bounds)) { - g.lineStyle({ - width: CURSOR_THICKNESS, - color, - alignment: 0.5, - texture: generatedTextures.dashedHorizontal, - }); - g.moveTo(selectionRect.left, selectionRect.top); - g.lineTo(Math.min(selectionRect.right, bounds.right), selectionRect.top); - if (selectionRect.bottom <= bounds.bottom) { - g.moveTo(Math.min(selectionRect.right, bounds.right), selectionRect.bottom); - g.lineTo(selectionRect.left, selectionRect.bottom); - } - - g.lineStyle({ - width: CURSOR_THICKNESS, - color, - alignment: 0.5, - texture: generatedTextures.dashedVertical, - }); - g.moveTo(selectionRect.left, Math.min(selectionRect.bottom, bounds.bottom)); - g.lineTo(selectionRect.left, selectionRect.top); - if (selectionRect.right <= bounds.right) { - g.moveTo(selectionRect.right, Math.min(selectionRect.bottom, bounds.bottom)); - g.lineTo(selectionRect.right, selectionRect.top); - } - - if (isSelected) { - g.lineStyle({ - alignment: 0, - }); - g.moveTo(selectionRect.left, selectionRect.top); - g.beginFill(color, FILL_ALPHA); - g.drawRect( - selectionRect.left, - selectionRect.top, - Math.min(selectionRect.right, bounds.right) - selectionRect.left, - Math.min(selectionRect.bottom, bounds.bottom) - selectionRect.top - ); - g.endFill(); - } - } -} diff --git a/quadratic-client/src/app/helpers/formulaNotation.ts b/quadratic-client/src/app/helpers/formulaNotation.ts index 59505eb4cb..8aa28f7cee 100644 --- a/quadratic-client/src/app/helpers/formulaNotation.ts +++ b/quadratic-client/src/app/helpers/formulaNotation.ts @@ -1,22 +1,12 @@ -import { sheets } from '@/app/grid/controller/Sheets'; -import type { StringId } from '@/app/helpers/getKey'; import type { JsCellsAccessed, JsCoordinate, Span } from '@/app/quadratic-core-types'; -import type { CellRefId } from '@/app/ui/menus/CodeEditor/hooks/useEditorCellHighlights'; -import type { Rectangle } from 'pixi.js'; -export function getCoordinatesFromStringId(stringId: StringId): [number, number] { - // required for type inference - const [x, y] = stringId.split(',').map((val) => parseInt(val)); - return [x, y]; -} - -export interface CellPosition { +interface CellPosition { x: { type: 'Relative' | 'Absolute'; coord: number }; y: { type: 'Relative' | 'Absolute'; coord: number }; sheet?: string; } -export type CellRef = +type CellRef = | { type: 'CellRange'; start: CellPosition; @@ -85,49 +75,3 @@ export function parseFormulaReturnToCellsAccessed( return jsCellsAccessed; } - -export function getCellFromFormulaNotation(sheetId: string, cellRefId: CellRefId, editorCursorPosition: JsCoordinate) { - const isSimpleCell = !cellRefId.includes(':'); - - if (isSimpleCell) { - const [x, y] = getCoordinatesFromStringId(cellRefId); - return getCellWithLimit(sheetId, editorCursorPosition, y, x); - } - const [startCell, endCell] = cellRefId.split(':') as [StringId, StringId]; - const [startCellX, startCellY] = getCoordinatesFromStringId(startCell); - const [endCellX, endCellY] = getCoordinatesFromStringId(endCell); - - return { - startCell: getCellWithLimit(sheetId, editorCursorPosition, startCellY, startCellX), - endCell: getCellWithLimit(sheetId, editorCursorPosition, endCellY, endCellX), - }; -} - -function getCellWithLimit( - sheetId: string, - editorCursorPosition: JsCoordinate, - row: number, - column: number, - offset = 20000 -): Rectangle { - // getCell is slow with more than 9 digits, so limit if column or row is > editorCursorPosition + an offset - // If it's a single cell to be highlighted, it won't be visible anyway, and if it's a range - // It will highlight beyond the what's visible in the viewport - return sheets.sheet.getCellOffsets( - Math.min(column, editorCursorPosition.x + offset), - Math.min(row, editorCursorPosition.y + offset) - ); -} - -export function isCellRangeTypeGuard(obj: any): obj is { startCell: Rectangle; endCell: Rectangle } { - return ( - typeof obj === 'object' && - obj !== null && - 'startCell' in obj && - typeof obj.startCell === 'object' && - obj.startCell !== null && - 'endCell' in obj && - typeof obj.endCell === 'object' && - obj.endCell !== null - ); -} diff --git a/quadratic-client/src/app/ui/menus/CodeEditor/CodeEditorBody.tsx b/quadratic-client/src/app/ui/menus/CodeEditor/CodeEditorBody.tsx index 523b1b51cc..b3635e5753 100644 --- a/quadratic-client/src/app/ui/menus/CodeEditor/CodeEditorBody.tsx +++ b/quadratic-client/src/app/ui/menus/CodeEditor/CodeEditorBody.tsx @@ -17,7 +17,6 @@ import { CodeEditorPlaceholder } from '@/app/ui/menus/CodeEditor/CodeEditorPlace import { FormulaLanguageConfig, FormulaTokenizerConfig } from '@/app/ui/menus/CodeEditor/FormulaLanguageModel'; import { useCloseCodeEditor } from '@/app/ui/menus/CodeEditor/hooks/useCloseCodeEditor'; import { useEditorCellHighlights } from '@/app/ui/menus/CodeEditor/hooks/useEditorCellHighlights'; -import { useEditorOnSelectionChange } from '@/app/ui/menus/CodeEditor/hooks/useEditorOnSelectionChange'; import { useEditorReturn } from '@/app/ui/menus/CodeEditor/hooks/useEditorReturn'; import { insertCellRef } from '@/app/ui/menus/CodeEditor/insertCellRef'; import { @@ -71,7 +70,6 @@ export const CodeEditorBody = (props: CodeEditorBodyProps) => { const [isValidRef, setIsValidRef] = useState(false); const [monacoInst, setMonacoInst] = useState(null); useEditorCellHighlights(isValidRef, editorInst, monacoInst); - useEditorOnSelectionChange(isValidRef, editorInst, monacoInst); useEditorReturn(isValidRef, editorInst, monacoInst); const { closeEditor } = useCloseCodeEditor({ diff --git a/quadratic-client/src/app/ui/menus/CodeEditor/hooks/useEditorCellHighlights.ts b/quadratic-client/src/app/ui/menus/CodeEditor/hooks/useEditorCellHighlights.ts index 9ab4eb5bb4..e404534124 100644 --- a/quadratic-client/src/app/ui/menus/CodeEditor/hooks/useEditorCellHighlights.ts +++ b/quadratic-client/src/app/ui/menus/CodeEditor/hooks/useEditorCellHighlights.ts @@ -6,12 +6,12 @@ import { import { pixiApp } from '@/app/gridGL/pixiApp/PixiApp'; import { codeCellIsAConnection } from '@/app/helpers/codeCellLanguage'; import { parseFormulaReturnToCellsAccessed, ParseFormulaReturnType } from '@/app/helpers/formulaNotation'; -import { getKey, StringId } from '@/app/helpers/getKey'; -import { JsCoordinate, Span } from '@/app/quadratic-core-types'; +import { getKey, type StringId } from '@/app/helpers/getKey'; +import type { JsCoordinate, Span } from '@/app/quadratic-core-types'; import { parseFormula } from '@/app/quadratic-rust-client/quadratic_rust_client'; import { colors } from '@/app/theme/colors'; -import { Monaco } from '@monaco-editor/react'; -import * as monaco from 'monaco-editor'; +import type { Monaco } from '@monaco-editor/react'; +import type * as monaco from 'monaco-editor'; import { useEffect, useRef } from 'react'; import { useRecoilValue } from 'recoil'; @@ -41,8 +41,7 @@ export function extractCellsFromParseFormula( }); } -export type CellRefId = StringId | `${StringId}:${StringId}`; -export type CellMatch = Map; +type CellRefId = StringId | `${StringId}:${StringId}`; export const createFormulaStyleHighlights = () => { const id = 'useEditorCellHighlights'; @@ -83,7 +82,7 @@ export const useEditorCellHighlights = ( const model = editorInst.getModel(); if (!model) return; - const onChangeModel = () => { + const onChange = () => { if (decorations) decorations.current?.clear(); const cellColorReferences = new Map(); @@ -129,7 +128,7 @@ export const useEditorCellHighlights = ( const editorCursorPosition = editorInst.getPosition(); if (editorCursorPosition && range.containsPosition(editorCursorPosition)) { - pixiApp.cellHighlights.setHighlightedCell(index); + pixiApp.cellHighlights.setSelectedCell(index); } }); @@ -139,8 +138,10 @@ export const useEditorCellHighlights = ( } }; - onChangeModel(); - editorInst.onDidChangeModelContent(() => onChangeModel()); + onChange(); + + editorInst.onDidChangeModelContent(() => onChange()); + editorInst.onDidChangeCursorPosition(() => onChange()); }, [ cellsAccessed, codeCell.language, diff --git a/quadratic-client/src/app/ui/menus/CodeEditor/hooks/useEditorOnSelectionChange.ts b/quadratic-client/src/app/ui/menus/CodeEditor/hooks/useEditorOnSelectionChange.ts deleted file mode 100644 index 03073b7536..0000000000 --- a/quadratic-client/src/app/ui/menus/CodeEditor/hooks/useEditorOnSelectionChange.ts +++ /dev/null @@ -1,45 +0,0 @@ -import { codeEditorCodeCellAtom } from '@/app/atoms/codeEditorAtom'; -import { pixiApp } from '@/app/gridGL/pixiApp/PixiApp'; -import { Monaco } from '@monaco-editor/react'; -import * as monaco from 'monaco-editor'; -import { useEffect } from 'react'; -import { useRecoilValue } from 'recoil'; - -export const useEditorOnSelectionChange = ( - isValidRef: boolean, - editorInst: monaco.editor.IStandaloneCodeEditor | null, - monacoInst: Monaco | null -) => { - const { language } = useRecoilValue(codeEditorCodeCellAtom); - useEffect(() => { - if (language !== 'Formula') return; - - if (!isValidRef || !editorInst || !monacoInst) return; - - const model = editorInst.getModel(); - if (!model) return; - - editorInst.onDidChangeCursorPosition((e) => { - pixiApp.cellHighlights.getHighlightedCells().find((value) => { - const span = value.span; - const startPosition = model.getPositionAt(span.start); - const endPosition = model.getPositionAt(span.end); - const range = new monacoInst.Range( - startPosition.lineNumber, - startPosition.column, - endPosition.lineNumber, - endPosition.column - ); - - if (range.containsPosition(e.position)) { - pixiApp.cellHighlights.setHighlightedCell(value.index); - return true; - } - - pixiApp.cellHighlights.setHighlightedCell(-1); - - return false; - }); - }); - }, [isValidRef, editorInst, monacoInst, language]); -}; diff --git a/quadratic-client/src/app/web-workers/renderWebWorker/renderWebWorker.ts b/quadratic-client/src/app/web-workers/renderWebWorker/renderWebWorker.ts index 08ae040c79..4195768537 100644 --- a/quadratic-client/src/app/web-workers/renderWebWorker/renderWebWorker.ts +++ b/quadratic-client/src/app/web-workers/renderWebWorker/renderWebWorker.ts @@ -25,7 +25,7 @@ class RenderWebWorker { this.worker.onerror = (e) => console.warn(`[render.worker] error: ${e.message}`, e); } - async init(coreMessagePort: MessagePort) { + init(coreMessagePort: MessagePort) { if (!this.worker) { throw new Error('Expected worker to be initialized in renderWebWorker.init'); } From c073088f8e08b128e618adb98fa79ea34636a827 Mon Sep 17 00:00:00 2001 From: David Figatner Date: Wed, 18 Dec 2024 04:41:34 -0800 Subject: [PATCH 3/9] fix A1 printing for rows --- .../a1/a1_selection/a1_selection_select.rs | 2 +- quadratic-core/src/a1/a1_selection/mod.rs | 6 ++-- quadratic-core/src/a1/ref_range_bounds/mod.rs | 36 ++++++++++++++++--- .../ref_range_bounds_create.rs | 6 ++-- 4 files changed, 39 insertions(+), 11 deletions(-) diff --git a/quadratic-core/src/a1/a1_selection/a1_selection_select.rs b/quadratic-core/src/a1/a1_selection/a1_selection_select.rs index 50cf3ca83f..4bc6cf8a7f 100644 --- a/quadratic-core/src/a1/a1_selection/a1_selection_select.rs +++ b/quadratic-core/src/a1/a1_selection/a1_selection_select.rs @@ -475,7 +475,7 @@ mod tests { fn test_select_row() { let mut selection = A1Selection::test_a1("A1"); selection.select_row(2, false, false, false, 1); - assert_eq!(selection.test_to_string(), "2"); + assert_eq!(selection.test_to_string(), "A2:2"); } #[test] diff --git a/quadratic-core/src/a1/a1_selection/mod.rs b/quadratic-core/src/a1/a1_selection/mod.rs index 181dbcdbb3..98bf4d91ab 100644 --- a/quadratic-core/src/a1/a1_selection/mod.rs +++ b/quadratic-core/src/a1/a1_selection/mod.rs @@ -553,7 +553,7 @@ mod tests { let selection = A1Selection::from_row_ranges(&[1..=5, 10..=12, 15..=15], SheetId::test()); assert_eq!( selection.to_string(Some(SheetId::test()), &HashMap::new()), - "1:5,10:12,15", + "1:5,10:12,A15:15", ); } @@ -598,7 +598,7 @@ mod tests { }; assert_eq!( selection.to_string(Some(SheetId::test()), &HashMap::new()), - "A:E,J:L,O,1:5,10:12,15", + "A:E,J:L,O,1:5,10:12,A15:15", ); } @@ -619,7 +619,7 @@ mod tests { fn test_extra_comma() { let sheet_id = SheetId::test(); let selection = A1Selection::from_str("1,", &sheet_id, &HashMap::new()).unwrap(); - assert_eq!(selection.to_string(Some(sheet_id), &HashMap::new()), "1"); + assert_eq!(selection.to_string(Some(sheet_id), &HashMap::new()), "A1:1"); } #[test] diff --git a/quadratic-core/src/a1/ref_range_bounds/mod.rs b/quadratic-core/src/a1/ref_range_bounds/mod.rs index a03e128984..e84f83b391 100644 --- a/quadratic-core/src/a1/ref_range_bounds/mod.rs +++ b/quadratic-core/src/a1/ref_range_bounds/mod.rs @@ -41,12 +41,25 @@ impl fmt::Display for RefRangeBounds { self.end.col.fmt_as_column(f)?; } } else if self.is_row_range() { - if self.start.row() == self.end.row() { - self.start.row.fmt_as_row(f)?; - } else { + // handle special case of An: (show as An: instead of n:) + if self.end.col.is_unbounded() + && self.end.row.is_unbounded() + && self.start.col.coord == 1 + { + write!(f, "A")?; self.start.row.fmt_as_row(f)?; write!(f, ":")?; - self.end.row.fmt_as_row(f)?; + } else { + if self.start.row() == self.end.row() { + write!(f, "A")?; + self.start.row.fmt_as_row(f)?; + write!(f, ":")?; + self.start.row.fmt_as_row(f)?; + } else { + self.start.row.fmt_as_row(f)?; + write!(f, ":")?; + self.end.row.fmt_as_row(f)?; + } } } else { write!(f, "{}", self.start)?; @@ -167,4 +180,19 @@ mod tests { assert_eq!(range.end.col.coord, 2); assert!(range.end.row.is_unbounded()); } + + #[test] + fn test_display_row_range() { + let range = RefRangeBounds::new_infinite_row(1); + assert_eq!(range.to_string(), "A1:1"); + + let range = RefRangeBounds::new_infinite_rows(1, 2); + assert_eq!(range.to_string(), "1:2"); + } + + #[test] + fn test_display_infinite_a_n() { + let range = RefRangeBounds::test_a1("15:"); + assert_eq!(range.to_string(), "A15:"); + } } diff --git a/quadratic-core/src/a1/ref_range_bounds/ref_range_bounds_create.rs b/quadratic-core/src/a1/ref_range_bounds/ref_range_bounds_create.rs index 8f2710645f..750dd1ac1e 100644 --- a/quadratic-core/src/a1/ref_range_bounds/ref_range_bounds_create.rs +++ b/quadratic-core/src/a1/ref_range_bounds/ref_range_bounds_create.rs @@ -131,7 +131,7 @@ mod tests { #[test] fn test_new_relative_all_from() { let range = RefRangeBounds::new_relative_all_from(Pos { x: 1, y: 2 }); - assert_eq!(range.to_string(), "2:"); + assert_eq!(range.to_string(), "A2:"); } #[test] @@ -155,7 +155,7 @@ mod tests { #[test] fn test_new_relative_row() { let range = RefRangeBounds::new_relative_row(5); - assert_eq!(range.to_string(), "5"); + assert_eq!(range.to_string(), "A5:5"); } #[test] @@ -175,7 +175,7 @@ mod tests { // Test same row case let range = RefRangeBounds::new_relative_row_range(3, 3); - assert_eq!(range.to_string(), "3"); + assert_eq!(range.to_string(), "A3:3"); } #[test] From e5a85cab99f70bb2e617cdd4d9599239d476ee93 Mon Sep 17 00:00:00 2001 From: David Figatner Date: Wed, 18 Dec 2024 06:48:17 -0800 Subject: [PATCH 4/9] added a momentum checker for wheel events --- .../gridGL/pixiApp/MomentumScrollDetector.ts | 54 +++++++++++++++++++ .../src/app/gridGL/pixiApp/PixiApp.ts | 3 ++ .../src/app/gridGL/pixiApp/Update.ts | 1 - .../app/gridGL/pixiApp/viewport/Viewport.ts | 8 ++- 4 files changed, 63 insertions(+), 3 deletions(-) create mode 100644 quadratic-client/src/app/gridGL/pixiApp/MomentumScrollDetector.ts diff --git a/quadratic-client/src/app/gridGL/pixiApp/MomentumScrollDetector.ts b/quadratic-client/src/app/gridGL/pixiApp/MomentumScrollDetector.ts new file mode 100644 index 0000000000..ec13203fe2 --- /dev/null +++ b/quadratic-client/src/app/gridGL/pixiApp/MomentumScrollDetector.ts @@ -0,0 +1,54 @@ +const SAMPLE_SIZE = 5; +const DELTA_THRESHOLD = 1; +const TIMING_TOLERANCE_MS = 50; + +interface SavedWheelEvent { + time: number; + delta: number; + deltaMode: number; +} + +export class MomentumScrollDetector { + private wheelEvents: SavedWheelEvent[] = []; + + constructor() { + window.addEventListener('wheel', this.handleWheel, { passive: true }); + } + + destroy() { + window.removeEventListener('wheel', this.handleWheel); + } + + handleWheel = (e: WheelEvent) => { + const now = Date.now(); + this.addEvent({ + time: now, + delta: Math.abs(e.deltaY), + deltaMode: e.deltaMode, + }); + }; + + addEvent(event: SavedWheelEvent) { + this.wheelEvents.push(event); + while (this.wheelEvents.length > SAMPLE_SIZE) { + this.wheelEvents.shift(); + } + } + + hasMomentumScroll() { + if (this.wheelEvents.length < SAMPLE_SIZE) return false; + + const hasSmoothing = this.wheelEvents.every((event, i, events) => { + if (i === 0) return true; + return event.delta <= events[i - 1].delta * DELTA_THRESHOLD; + }); + + const hasConsistentTiming = this.wheelEvents.every((event, i, events) => { + if (i === 0) return true; + const timeDelta = event.time - events[i - 1].time; + return timeDelta < TIMING_TOLERANCE_MS; + }); + + return hasSmoothing && hasConsistentTiming; + } +} diff --git a/quadratic-client/src/app/gridGL/pixiApp/PixiApp.ts b/quadratic-client/src/app/gridGL/pixiApp/PixiApp.ts index df1952bb21..06c009f991 100644 --- a/quadratic-client/src/app/gridGL/pixiApp/PixiApp.ts +++ b/quadratic-client/src/app/gridGL/pixiApp/PixiApp.ts @@ -22,6 +22,7 @@ import { CellsSheets } from '@/app/gridGL/cells/CellsSheets'; import { CellsImages } from '@/app/gridGL/cells/cellsImages/CellsImages'; import { Pointer } from '@/app/gridGL/interaction/pointer/Pointer'; import { ensureVisible } from '@/app/gridGL/interaction/viewportHelper'; +import { MomentumScrollDetector } from '@/app/gridGL/pixiApp/MomentumScrollDetector'; import { pixiAppSettings } from '@/app/gridGL/pixiApp/PixiAppSettings'; import { Update } from '@/app/gridGL/pixiApp/Update'; import { urlParams } from '@/app/gridGL/pixiApp/urlParams/urlParams'; @@ -68,6 +69,7 @@ export class PixiApp { validations: UIValidations; renderer!: Renderer; + momentumDetector: MomentumScrollDetector; stage = new Container(); loading = true; destroyed = false; @@ -90,6 +92,7 @@ export class PixiApp { this.validations = new UIValidations(); this.viewport = new Viewport(); this.background = new Background(); + this.momentumDetector = new MomentumScrollDetector(); } init() { diff --git a/quadratic-client/src/app/gridGL/pixiApp/Update.ts b/quadratic-client/src/app/gridGL/pixiApp/Update.ts index d193c74942..a95048619b 100644 --- a/quadratic-client/src/app/gridGL/pixiApp/Update.ts +++ b/quadratic-client/src/app/gridGL/pixiApp/Update.ts @@ -49,7 +49,6 @@ export class Update { this.raf = requestAnimationFrame(this.update); return; } - pixiApp.viewport.updateViewport(); let rendererDirty = diff --git a/quadratic-client/src/app/gridGL/pixiApp/viewport/Viewport.ts b/quadratic-client/src/app/gridGL/pixiApp/viewport/Viewport.ts index 9f323fe9bf..42e2d70ca2 100644 --- a/quadratic-client/src/app/gridGL/pixiApp/viewport/Viewport.ts +++ b/quadratic-client/src/app/gridGL/pixiApp/viewport/Viewport.ts @@ -213,8 +213,12 @@ export class Viewport extends PixiViewport { if (!this.snapState) { const headings = pixiApp.headings.headingSize; if (this.x > headings.width || this.y > headings.height) { - this.snapTimeout = Date.now(); - this.snapState = 'waiting'; + if (pixiApp.momentumDetector.hasMomentumScroll()) { + this.startSnap(); + } else { + this.snapTimeout = Date.now(); + this.snapState = 'waiting'; + } } } else if (this.snapState === 'waiting' && this.snapTimeout) { if (Date.now() - this.snapTimeout > WAIT_TO_SNAP_TIME) { From beeb7fd5309a46d706a234494d37526682d4e4ab Mon Sep 17 00:00:00 2001 From: David Figatner Date: Wed, 18 Dec 2024 06:49:05 -0800 Subject: [PATCH 5/9] fix comment --- .../src/app/gridGL/pixiApp/MomentumScrollDetector.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/quadratic-client/src/app/gridGL/pixiApp/MomentumScrollDetector.ts b/quadratic-client/src/app/gridGL/pixiApp/MomentumScrollDetector.ts index ec13203fe2..75fb7fc9a2 100644 --- a/quadratic-client/src/app/gridGL/pixiApp/MomentumScrollDetector.ts +++ b/quadratic-client/src/app/gridGL/pixiApp/MomentumScrollDetector.ts @@ -1,3 +1,7 @@ +//! This is a momentum scroll detector that uses a simple algorithm to detect if +//! the user is scrolling with momentum. It is not perfect and may not work in +//! all cases, but it is a good start. + const SAMPLE_SIZE = 5; const DELTA_THRESHOLD = 1; const TIMING_TOLERANCE_MS = 50; From 928aae10f82b9213fea31053984c9f972c8945eb Mon Sep 17 00:00:00 2001 From: AyushAgrawal-A2 Date: Wed, 18 Dec 2024 21:31:50 +0530 Subject: [PATCH 6/9] fix code run order for insert/delete col/row --- quadratic-core/src/grid/sheet/col_row/column.rs | 6 ++++-- quadratic-core/src/grid/sheet/col_row/row.rs | 4 ++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/quadratic-core/src/grid/sheet/col_row/column.rs b/quadratic-core/src/grid/sheet/col_row/column.rs index caa3ed85df..74c232ad16 100644 --- a/quadratic-core/src/grid/sheet/col_row/column.rs +++ b/quadratic-core/src/grid/sheet/col_row/column.rs @@ -173,6 +173,7 @@ impl Sheet { columns_to_update.push(*col); } } + columns_to_update.sort_by(|a, b| a.cmp(b)); for col in columns_to_update { if let Some(mut column_data) = self.columns.remove(&col) { column_data.x -= 1; @@ -187,6 +188,7 @@ impl Sheet { code_runs_to_move.push(*pos); } } + code_runs_to_move.sort_by(|a, b| a.x.cmp(&b.x)); for old_pos in code_runs_to_move { let new_pos = Pos { x: old_pos.x - 1, @@ -244,7 +246,7 @@ impl Sheet { columns_to_update.push(*col); } } - columns_to_update.reverse(); + columns_to_update.sort_by(|a, b| b.cmp(a)); for col in columns_to_update { if let Some(mut column_data) = self.columns.remove(&col) { column_data.x += 1; @@ -259,7 +261,7 @@ impl Sheet { code_runs_to_move.push(*pos); } } - code_runs_to_move.reverse(); + code_runs_to_move.sort_by(|a, b| b.x.cmp(&a.x)); for old_pos in code_runs_to_move { let new_pos = Pos { x: old_pos.x + 1, diff --git a/quadratic-core/src/grid/sheet/col_row/row.rs b/quadratic-core/src/grid/sheet/col_row/row.rs index 82600f903e..7b58357b0b 100644 --- a/quadratic-core/src/grid/sheet/col_row/row.rs +++ b/quadratic-core/src/grid/sheet/col_row/row.rs @@ -201,7 +201,7 @@ impl Sheet { code_runs_to_move.push(*pos); } } - code_runs_to_move.sort_unstable(); + code_runs_to_move.sort_by(|a, b| a.y.cmp(&b.y)); for old_pos in code_runs_to_move { let new_pos = Pos { x: old_pos.x, @@ -293,7 +293,7 @@ impl Sheet { code_runs_to_move.push(*pos); } } - code_runs_to_move.reverse(); + code_runs_to_move.sort_by(|a, b| b.y.cmp(&a.y)); for old_pos in code_runs_to_move { let new_pos = Pos { x: old_pos.x, From 7f9d7750161c32a08d2d9a13711a988ae175b098 Mon Sep 17 00:00:00 2001 From: AyushAgrawal-A2 Date: Wed, 18 Dec 2024 21:41:07 +0530 Subject: [PATCH 7/9] clippy --- quadratic-core/src/a1/ref_range_bounds/mod.rs | 18 ++++++++---------- .../src/grid/sheet/col_row/column.rs | 2 +- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/quadratic-core/src/a1/ref_range_bounds/mod.rs b/quadratic-core/src/a1/ref_range_bounds/mod.rs index e84f83b391..7106d14c75 100644 --- a/quadratic-core/src/a1/ref_range_bounds/mod.rs +++ b/quadratic-core/src/a1/ref_range_bounds/mod.rs @@ -49,17 +49,15 @@ impl fmt::Display for RefRangeBounds { write!(f, "A")?; self.start.row.fmt_as_row(f)?; write!(f, ":")?; + } else if self.start.row() == self.end.row() { + write!(f, "A")?; + self.start.row.fmt_as_row(f)?; + write!(f, ":")?; + self.start.row.fmt_as_row(f)?; } else { - if self.start.row() == self.end.row() { - write!(f, "A")?; - self.start.row.fmt_as_row(f)?; - write!(f, ":")?; - self.start.row.fmt_as_row(f)?; - } else { - self.start.row.fmt_as_row(f)?; - write!(f, ":")?; - self.end.row.fmt_as_row(f)?; - } + self.start.row.fmt_as_row(f)?; + write!(f, ":")?; + self.end.row.fmt_as_row(f)?; } } else { write!(f, "{}", self.start)?; diff --git a/quadratic-core/src/grid/sheet/col_row/column.rs b/quadratic-core/src/grid/sheet/col_row/column.rs index 74c232ad16..d6dfc0e0ea 100644 --- a/quadratic-core/src/grid/sheet/col_row/column.rs +++ b/quadratic-core/src/grid/sheet/col_row/column.rs @@ -173,7 +173,7 @@ impl Sheet { columns_to_update.push(*col); } } - columns_to_update.sort_by(|a, b| a.cmp(b)); + columns_to_update.sort(); for col in columns_to_update { if let Some(mut column_data) = self.columns.remove(&col) { column_data.x -= 1; From b060fac4b9315e0b5874f313e7a20efbdb1d8c89 Mon Sep 17 00:00:00 2001 From: David Figatner Date: Wed, 18 Dec 2024 08:32:37 -0800 Subject: [PATCH 8/9] add rust code to determine if getCells should be forced to be 2d --- .../execution/auto_resize_row_heights.rs | 1 + .../execution/run_code/get_cells.rs | 81 ++++++++++++++++--- .../execution/run_code/run_javascript.rs | 2 + .../execution/run_code/run_python.rs | 2 + 4 files changed, 73 insertions(+), 13 deletions(-) diff --git a/quadratic-core/src/controller/execution/auto_resize_row_heights.rs b/quadratic-core/src/controller/execution/auto_resize_row_heights.rs index 94485b97bf..5dbd65e0c5 100644 --- a/quadratic-core/src/controller/execution/auto_resize_row_heights.rs +++ b/quadratic-core/src/controller/execution/auto_resize_row_heights.rs @@ -510,6 +510,7 @@ mod tests { y: 1, w: 1, h: 1, + two_dimensional: false, }) ); // pending cal diff --git a/quadratic-core/src/controller/execution/run_code/get_cells.rs b/quadratic-core/src/controller/execution/run_code/get_cells.rs index 710075f7bf..b5defd33c5 100644 --- a/quadratic-core/src/controller/execution/run_code/get_cells.rs +++ b/quadratic-core/src/controller/execution/run_code/get_cells.rs @@ -1,7 +1,9 @@ use ts_rs::TS; use uuid::Uuid; -use crate::{controller::GridController, error_core::CoreError, RunError, RunErrorMsg}; +use crate::{ + controller::GridController, error_core::CoreError, CellRefRange, RunError, RunErrorMsg, +}; use serde::{Deserialize, Serialize}; #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, TS)] @@ -11,6 +13,7 @@ pub struct CellA1Response { pub y: i64, pub w: i64, pub h: i64, + pub two_dimensional: bool, } #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, TS)] @@ -129,6 +132,19 @@ impl GridController { }); let response = if let Some(rect) = rects.first() { + // cases for forced two-dimensional get_cells: + // 1. rect.width > 1 + // 2. any range where end.col is unbounded + let two_dimensional = if rect.width() > 1 { + true + } else if let Some(range) = selection.ranges.first() { + match range { + CellRefRange::Sheet { range } => range.end.col.is_unbounded(), + } + } else { + false + }; + let cells = sheet.get_cells_response(*rect); CellA1Response { cells, @@ -136,6 +152,7 @@ impl GridController { y: rect.min.y, w: rect.width() as i64, h: rect.height() as i64, + two_dimensional, } } else { CellA1Response { @@ -144,6 +161,7 @@ impl GridController { y: 1, w: 0, h: 0, + two_dimensional: false, } }; @@ -154,13 +172,12 @@ impl GridController { } #[cfg(test)] +#[serial_test::parallel] mod test { use super::*; use crate::{grid::CodeCellLanguage, Pos, Rect, SheetPos}; - use serial_test::parallel; #[test] - #[parallel] fn test_calculation_get_cells_bad_transaction_id() { let mut gc = GridController::test(); @@ -170,7 +187,6 @@ mod test { } #[test] - #[parallel] fn test_calculation_get_cells_no_transaction() { let mut gc = GridController::test(); @@ -180,7 +196,6 @@ mod test { } #[test] - #[parallel] fn test_calculation_get_cells_transaction_but_no_current_sheet_pos() { let mut gc = GridController::test(); let sheet_id = gc.sheet_ids()[0]; @@ -203,7 +218,6 @@ mod test { } #[test] - #[parallel] fn test_calculation_get_cells_sheet_name_not_found() { let mut gc = GridController::test(); let sheet_id = gc.sheet_ids()[0]; @@ -238,7 +252,6 @@ mod test { // This was previously disallowed. It is now allowed to unlock appending results. // Leaving in some commented out code in case we want to revert this behavior. #[test] - #[parallel] fn test_calculation_get_cells_self_reference() { let mut gc = GridController::test(); let sheet_id = gc.sheet_ids()[0]; @@ -284,7 +297,6 @@ mod test { } #[test] - #[parallel] fn test_calculation_get_cells() { let mut gc = GridController::test(); let sheet_id = gc.sheet_ids()[0]; @@ -325,13 +337,13 @@ mod test { x: 1, y: 1, w: 1, - h: 1 + h: 1, + two_dimensional: false, }) ); } #[test] - #[parallel] fn calculation_get_cells_with_no_y1() { let mut gc = GridController::test(); let sheet_id = gc.sheet_ids()[0]; @@ -426,13 +438,13 @@ mod test { x: 1, y: 1, w: 1, - h: 5 + h: 5, + two_dimensional: false, }) ); } #[test] - #[parallel] fn calculation_get_cells_a1() { let mut gc = GridController::test(); let sheet_id = gc.sheet_ids()[0]; @@ -468,7 +480,50 @@ mod test { x: 1, y: 1, w: 1, - h: 1 + h: 1, + two_dimensional: false, + }) + ); + } + + #[test] + fn calculation_get_cells_a1_two_dimensional() { + let mut gc = GridController::test(); + let sheet_id = gc.sheet_ids()[0]; + + gc.set_cell_value( + SheetPos { + x: 2, + y: 1, + sheet_id, + }, + "test".to_string(), + None, + ); + + gc.set_code_cell( + SheetPos::new(sheet_id, 1, 1), + CodeCellLanguage::Python, + "".to_string(), + None, + ); + let transaction_id = gc.last_transaction().unwrap().id; + let result = + gc.calculation_get_cells_a1(transaction_id.to_string(), "B:".to_string(), None); + assert_eq!( + result, + Ok(CellA1Response { + cells: vec![JsGetCellResponse { + x: 2, + y: 1, + value: "test".into(), + type_name: "text".into() + }], + x: 2, + y: 1, + w: 1, + h: 1, + two_dimensional: true, }) ); } diff --git a/quadratic-core/src/controller/execution/run_code/run_javascript.rs b/quadratic-core/src/controller/execution/run_code/run_javascript.rs index d60507d85e..28b712b805 100644 --- a/quadratic-core/src/controller/execution/run_code/run_javascript.rs +++ b/quadratic-core/src/controller/execution/run_code/run_javascript.rs @@ -154,6 +154,7 @@ mod tests { y: 1, w: 1, h: 1, + two_dimensional: false, }) ); @@ -235,6 +236,7 @@ mod tests { y: 1, w: 1, h: 1, + two_dimensional: false, }) ); assert!(gc diff --git a/quadratic-core/src/controller/execution/run_code/run_python.rs b/quadratic-core/src/controller/execution/run_code/run_python.rs index 3e568a5ce7..c62688888f 100644 --- a/quadratic-core/src/controller/execution/run_code/run_python.rs +++ b/quadratic-core/src/controller/execution/run_code/run_python.rs @@ -163,6 +163,7 @@ mod tests { y: 1, w: 1, h: 1, + two_dimensional: false, }) ); @@ -248,6 +249,7 @@ mod tests { y: 1, w: 1, h: 1, + two_dimensional: false, }) ); assert!(gc From 8318196bf3e8b856eedd73afdbf079139a816c25 Mon Sep 17 00:00:00 2001 From: David Figatner Date: Wed, 18 Dec 2024 08:46:59 -0800 Subject: [PATCH 9/9] changed bad test --- .../src/grid/sheet/borders/borders_query.rs | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/quadratic-core/src/grid/sheet/borders/borders_query.rs b/quadratic-core/src/grid/sheet/borders/borders_query.rs index b6cda0207f..4b8985fe87 100644 --- a/quadratic-core/src/grid/sheet/borders/borders_query.rs +++ b/quadratic-core/src/grid/sheet/borders/borders_query.rs @@ -87,20 +87,6 @@ impl Borders { } } - // Then check if current borders exist where update has none - if border_update.left.is_none() && !self.left.is_all_default() { - return false; - } - if border_update.right.is_none() && !self.right.is_all_default() { - return false; - } - if border_update.top.is_none() && !self.top.is_all_default() { - return false; - } - if border_update.bottom.is_none() && !self.bottom.is_all_default() { - return false; - } - true } @@ -167,6 +153,6 @@ mod tests { assert!(!borders.is_toggle_borders(&border_update)); borders.set_style_cell(pos![A1], BorderStyleCell::all()); - assert!(!borders.is_toggle_borders(&border_update)); + assert!(borders.is_toggle_borders(&border_update)); } }