diff --git a/quadratic-client/src/grid/controller/Grid.ts b/quadratic-client/src/grid/controller/Grid.ts index 427fde5acf..172e2a318f 100644 --- a/quadratic-client/src/grid/controller/Grid.ts +++ b/quadratic-client/src/grid/controller/Grid.ts @@ -712,8 +712,12 @@ export class Grid { //#region Summarize //----------------- - summarizeSelection() { - return this.gridController.summarizeSelection(sheets.sheet.id, rectangleToRect(sheets.sheet.cursor.getRectangle())); + summarizeSelection(decimal_places: number = 9) { + return this.gridController.summarizeSelection( + sheets.sheet.id, + rectangleToRect(sheets.sheet.cursor.getRectangle()), + BigInt(decimal_places) + ); } //#endregion diff --git a/quadratic-client/src/gridGL/helpers/selectCells.ts b/quadratic-client/src/gridGL/helpers/selectCells.ts index abf891cd72..088aef3edd 100644 --- a/quadratic-client/src/gridGL/helpers/selectCells.ts +++ b/quadratic-client/src/gridGL/helpers/selectCells.ts @@ -4,6 +4,7 @@ import { sheets } from '../../grid/controller/Sheets'; export function selectAllCells(): void { const sheet = sheets.sheet; const bounds = grid.getGridBounds(sheet.id, true); + if (bounds) { sheet.cursor.changePosition({ multiCursor: { diff --git a/quadratic-client/src/ui/menus/BottomBar/SelectionSummary.tsx b/quadratic-client/src/ui/menus/BottomBar/SelectionSummary.tsx index 2d05ce3ef4..c8069a5b23 100644 --- a/quadratic-client/src/ui/menus/BottomBar/SelectionSummary.tsx +++ b/quadratic-client/src/ui/menus/BottomBar/SelectionSummary.tsx @@ -8,6 +8,7 @@ import BottomBarItem from './BottomBarItem'; export const SelectionSummary = () => { const cursor = sheets.sheet.cursor; + const decimal_places = 9; const isBigEnoughForActiveSelectionStats = useMediaQuery('(min-width:1000px)'); const [count, setCount] = useState(''); @@ -16,12 +17,12 @@ export const SelectionSummary = () => { const [copied, setCopied] = useState(false); const runCalculationOnActiveSelection = () => { - let result = grid.summarizeSelection(); + let result = grid.summarizeSelection(decimal_places); if (result) { setCount(result.count.toString()); setSum(result.sum !== undefined ? result.sum.toString() : undefined); - setAvg(result.average !== undefined ? result.average.toFixed(9).toString() : undefined); + setAvg(result.average !== undefined ? result.average.toString() : undefined); } else { setCount(undefined); setSum(undefined); diff --git a/quadratic-core/src/grid/sheet.rs b/quadratic-core/src/grid/sheet.rs index ddb99c9432..f16476ab7e 100644 --- a/quadratic-core/src/grid/sheet.rs +++ b/quadratic-core/src/grid/sheet.rs @@ -460,10 +460,13 @@ impl Sheet { match value { CellValue::Number(n) => { let (_, exponent) = n.as_bigint_and_exponent(); + let max_decimals = 9; + let decimals = exponent.min(max_decimals) as i16; + if is_percentage { - Some(exponent as i16 - 2) + Some(decimals - 2) } else { - Some(exponent as i16) + Some(decimals) } } _ => None, @@ -570,22 +573,35 @@ mod test { (grid_controller, sheet_id, selected) } + // assert decimal places after a set_cell_value + fn assert_decimal_places_for_number( + sheet: &mut Sheet, + x: i64, + y: i64, + value: &str, + is_percentage: bool, + expected: Option, + ) { + let pos = Pos { x, y }; + sheet.set_cell_value(pos, CellValue::Number(BigDecimal::from_str(value).unwrap())); + assert_eq!(sheet.decimal_places(pos, is_percentage), expected); + } + #[test] fn test_current_decimal_places_value() { let mut sheet = Sheet::new(SheetId::new(), String::from(""), String::from("")); - // get decimal places after a set_cell_value - sheet.set_cell_value( - Pos { x: 1, y: 2 }, - CellValue::Number(BigDecimal::from_str("12.23").unwrap()), - ); - assert_eq!(sheet.decimal_places(Pos { x: 1, y: 2 }, false), Some(2)); + // validate simple decimal places + assert_decimal_places_for_number(&mut sheet, 1, 2, "12.23", false, Some(2)); - sheet.set_cell_value( - Pos { x: 2, y: 2 }, - CellValue::Number(BigDecimal::from_str("0.23").unwrap()), - ); - assert_eq!(sheet.decimal_places(Pos { x: 2, y: 2 }, true), Some(0)); + // validate percentage + assert_decimal_places_for_number(&mut sheet, 2, 2, "0.23", true, Some(0)); + + // validate rounding + assert_decimal_places_for_number(&mut sheet, 3, 2, "9.1234567891", false, Some(9)); + + // validate percentage rounding + assert_decimal_places_for_number(&mut sheet, 3, 2, "9.1234567891", true, Some(7)); } #[test] @@ -610,25 +626,6 @@ mod test { assert_eq!(sheet.decimal_places(Pos { x: 1, y: 2 }, false), None); } - #[test] - fn test_current_decimal_places_percent() { - let mut sheet = Sheet::new(SheetId::new(), String::from(""), String::from("")); - - sheet.set_cell_value( - crate::Pos { x: 1, y: 2 }, - CellValue::Number(BigDecimal::from_str("0.24").unwrap()), - ); - - assert_eq!(sheet.decimal_places(Pos { x: 1, y: 2 }, true), Some(0)); - - sheet.set_cell_value( - crate::Pos { x: 1, y: 2 }, - CellValue::Number(BigDecimal::from_str("0.245").unwrap()), - ); - - assert_eq!(sheet.decimal_places(Pos { x: 1, y: 2 }, true), Some(1)); - } - #[test] fn test_cell_numeric_format_kind() { let mut sheet = Sheet::new(SheetId::new(), String::from(""), String::from("")); diff --git a/quadratic-core/src/util.rs b/quadratic-core/src/util.rs index 867634ccc2..f094b40159 100644 --- a/quadratic-core/src/util.rs +++ b/quadratic-core/src/util.rs @@ -275,6 +275,11 @@ pub fn date_string() -> String { now.format("%Y-%m-%d %H:%M:%S").to_string() } +pub fn round(number: f64, precision: i64) -> f64 { + let y = 10i32.pow(precision as u32) as f64; + (number * y).round() / y +} + #[cfg(test)] pub(crate) fn assert_f64_approx_eq(expected: f64, actual: &str) { const EPSILON: f64 = 0.0001; @@ -433,4 +438,13 @@ mod tests { fn test_date_string() { assert_eq!(date_string().len(), 19); } + + #[test] + fn test_round() { + assert_eq!(round(1.23456789, 0), 1.0); + assert_eq!(round(1.23456789, 1), 1.2); + assert_eq!(round(1.23456789, 2), 1.23); + assert_eq!(round(1.23456789, 3), 1.235); + assert_eq!(round(1.23456789, 4), 1.2346); + } } diff --git a/quadratic-core/src/wasm_bindings/controller/summarize.rs b/quadratic-core/src/wasm_bindings/controller/summarize.rs index 5f1e4051c7..5375b18105 100644 --- a/quadratic-core/src/wasm_bindings/controller/summarize.rs +++ b/quadratic-core/src/wasm_bindings/controller/summarize.rs @@ -1,10 +1,11 @@ +use crate::util::round; use crate::Pos; use crate::{grid::SheetId, wasm_bindings::GridController, CellValue, Rect}; use bigdecimal::{BigDecimal, ToPrimitive, Zero}; use std::str::FromStr; use wasm_bindgen::prelude::wasm_bindgen; -const MAXIMUM_SUMMARIZE_SELECTION_SIZE: u32 = 50000; +const MAX_SUMMARIZE_SELECTION_SIZE: u32 = 50000; #[wasm_bindgen] pub struct SummarizeSelectionResult { @@ -20,9 +21,10 @@ impl GridController { &mut self, sheet_id: String, rect: &Rect, + max_decimals: i64, ) -> Option { // don't allow too large of a selection - if (rect.width() * rect.height()) > MAXIMUM_SUMMARIZE_SELECTION_SIZE { + if rect.len() > MAX_SUMMARIZE_SELECTION_SIZE { return None; } @@ -60,8 +62,8 @@ impl GridController { Some(SummarizeSelectionResult { count, - sum: sum.to_f64(), - average: average.to_f64(), + sum: sum.to_f64().map(|num| round(num, max_decimals)), + average: average.to_f64().map(|num| round(num, max_decimals)), }) } } @@ -72,35 +74,62 @@ mod tests { use crate::wasm_bindings::GridController; use crate::{Pos, Rect}; + // TODO(ddimaria): move to a shared util + fn set_value(gc: &mut GridController, x: i64, y: i64, value: &str) { + let sheet_id = gc.sheet_ids()[0]; + gc.set_cell_value(sheet_id, Pos { x, y }, String::from(value), None); + } + #[test] fn test_summarize() { let mut gc = GridController::default(); let sheet_id = gc.sheet_ids()[0]; - gc.set_cell_value(sheet_id, Pos { x: 1, y: 1 }, String::from("12.12"), None); - gc.set_cell_value(sheet_id, Pos { x: 1, y: 2 }, String::from("12313"), None); - gc.set_cell_value(sheet_id, Pos { x: 1, y: 3 }, String::from("0"), None); + set_value(&mut gc, 1, 1, "12.12"); + set_value(&mut gc, 1, 2, "12313"); + set_value(&mut gc, 1, 3, "0"); + // span of 10 cells, 3 have numeric values let rect = Rect::new_span(Pos { x: 1, y: 1 }, Pos { x: 1, y: 10 }); let result = gc - .js_summarize_selection(sheet_id.to_string(), &rect) + .js_summarize_selection(sheet_id.to_string(), &rect, 9) .unwrap(); - assert_eq!(result.count, 3); assert_eq!(result.sum, Some(12325.12)); - assert_eq!(result.average, Some(4108.373333333333)); + assert_eq!(result.average, Some(4108.373333333)); + // returns zeros for an empty selection let rect = Rect::new_span(Pos { x: 100, y: 100 }, Pos { x: 1000, y: 105 }); let result = gc - .js_summarize_selection(sheet_id.to_string(), &rect) + .js_summarize_selection(sheet_id.to_string(), &rect, 9) .unwrap(); - assert_eq!(result.count, 0); assert_eq!(result.sum, Some(0.0)); assert_eq!(result.average, Some(0.0)); + // returns none if selection is too large (MAX_SUMMARIZE_SELECTION_SIZE) let rect = Rect::new_span(Pos { x: 100, y: 100 }, Pos { x: 10000, y: 10000 }); - let result = gc.js_summarize_selection(sheet_id.to_string(), &rect); + let result = gc.js_summarize_selection(sheet_id.to_string(), &rect, 9); assert!(result.is_none()); + + // rounding + set_value(&mut gc, 1, 1, "9.1234567891"); + let rect = Rect::new_span(Pos { x: 1, y: 1 }, Pos { x: 1, y: 10 }); + let result = gc + .js_summarize_selection(sheet_id.to_string(), &rect, 9) + .unwrap(); + assert_eq!(result.count, 3); + assert_eq!(result.sum, Some(12322.123456789)); + assert_eq!(result.average, Some(4107.374485596)); + + // trailing zeros + set_value(&mut gc, -1, -1, "0.00100000000000"); + let rect = Rect::new_span(Pos { x: -1, y: -1 }, Pos { x: -1, y: -10 }); + let result = gc + .js_summarize_selection(sheet_id.to_string(), &rect, 9) + .unwrap(); + assert_eq!(result.count, 1); + assert_eq!(result.sum, Some(0.001)); + assert_eq!(result.average, Some(0.001)); } }