From eb9e67e36a34ee85baff108d5d8db41a653e25cf Mon Sep 17 00:00:00 2001 From: Christopher Chedeau Date: Sun, 20 Dec 2020 04:08:22 -0800 Subject: [PATCH] improvement: Support numbers with commas in them (#2636) --- src/analytics.ts | 25 ++++++++++-------- src/charts.ts | 27 +++++++++++--------- src/packages/excalidraw/CHANGELOG.MD | 1 + src/tests/__snapshots__/charts.test.tsx.snap | 20 +++++++++++++++ src/tests/charts.test.tsx | 13 ++++++++++ 5 files changed, 63 insertions(+), 23 deletions(-) create mode 100644 src/tests/__snapshots__/charts.test.tsx.snap create mode 100644 src/tests/charts.test.tsx diff --git a/src/analytics.ts b/src/analytics.ts index dc4cd0c5..25586273 100644 --- a/src/analytics.ts +++ b/src/analytics.ts @@ -11,14 +11,17 @@ export const EVENT_SHAPE = "shape"; export const EVENT_SHARE = "share"; export const EVENT_MAGIC = "magic"; -export const trackEvent = window.gtag - ? (category: string, name: string, label?: string, value?: number) => { - window.gtag("event", name, { - event_category: category, - event_label: label, - value, - }); - } - : (category: string, name: string, label?: string, value?: number) => { - console.info("Track Event", category, name, label, value); - }; +export const trackEvent = + typeof window !== "undefined" && window.gtag + ? (category: string, name: string, label?: string, value?: number) => { + window.gtag("event", name, { + event_category: category, + event_label: label, + value, + }); + } + : typeof process !== "undefined" && process?.env?.JEST_WORKER_ID + ? (category: string, name: string, label?: string, value?: number) => {} + : (category: string, name: string, label?: string, value?: number) => { + console.info("Track Event", category, name, label, value); + }; diff --git a/src/charts.ts b/src/charts.ts index b2b2a27c..5e49e936 100644 --- a/src/charts.ts +++ b/src/charts.ts @@ -19,15 +19,15 @@ export const NOT_SPREADSHEET = "NOT_SPREADSHEET"; export const VALID_SPREADSHEET = "VALID_SPREADSHEET"; type ParseSpreadsheetResult = - | { type: typeof NOT_SPREADSHEET } + | { type: typeof NOT_SPREADSHEET; reason: string } | { type: typeof VALID_SPREADSHEET; spreadsheet: Spreadsheet }; const tryParseNumber = (s: string): number | null => { - const match = /^[$€£¥₩]?([0-9]+(\.[0-9]+)?)$/.exec(s); + const match = /^[$€£¥₩]?([0-9,]+(\.[0-9]+)?)$/.exec(s); if (!match) { return null; } - return parseFloat(match[1]); + return parseFloat(match[1].replace(/,/g, "")); }; const isNumericColumn = (lines: string[][], columnIndex: number) => @@ -37,12 +37,12 @@ const tryParseCells = (cells: string[][]): ParseSpreadsheetResult => { const numCols = cells[0].length; if (numCols > 2) { - return { type: NOT_SPREADSHEET }; + return { type: NOT_SPREADSHEET, reason: "More than 2 columns" }; } if (numCols === 1) { if (!isNumericColumn(cells, 0)) { - return { type: NOT_SPREADSHEET }; + return { type: NOT_SPREADSHEET, reason: "Value is not numeric" }; } const hasHeader = tryParseNumber(cells[0][0]) === null; @@ -51,7 +51,7 @@ const tryParseCells = (cells: string[][]): ParseSpreadsheetResult => { ); if (values.length < 2) { - return { type: NOT_SPREADSHEET }; + return { type: NOT_SPREADSHEET, reason: "Less than two rows" }; } return { @@ -67,7 +67,7 @@ const tryParseCells = (cells: string[][]): ParseSpreadsheetResult => { const valueColumnIndex = isNumericColumn(cells, 0) ? 0 : 1; if (!isNumericColumn(cells, valueColumnIndex)) { - return { type: NOT_SPREADSHEET }; + return { type: NOT_SPREADSHEET, reason: "Value is not numeric" }; } const labelColumnIndex = (valueColumnIndex + 1) % 2; @@ -75,7 +75,7 @@ const tryParseCells = (cells: string[][]): ParseSpreadsheetResult => { const rows = hasHeader ? cells.slice(1) : cells; if (rows.length < 2) { - return { type: NOT_SPREADSHEET }; + return { type: NOT_SPREADSHEET, reason: "Less than 2 rows" }; } return { @@ -104,13 +104,13 @@ export const tryParseSpreadsheet = (text: string): ParseSpreadsheetResult => { // Copy/paste from excel, spreadhseets, tsv, csv. // For now we only accept 2 columns with an optional header - // Check for tab separeted values + // Check for tab separated values let lines = text .trim() .split("\n") .map((line) => line.trim().split("\t")); - // Check for comma separeted files + // Check for comma separated files if (lines.length && lines[0].length !== 2) { lines = text .trim() @@ -119,14 +119,17 @@ export const tryParseSpreadsheet = (text: string): ParseSpreadsheetResult => { } if (lines.length === 0) { - return { type: NOT_SPREADSHEET }; + return { type: NOT_SPREADSHEET, reason: "No values" }; } const numColsFirstLine = lines[0].length; const isSpreadsheet = lines.every((line) => line.length === numColsFirstLine); if (!isSpreadsheet) { - return { type: NOT_SPREADSHEET }; + return { + type: NOT_SPREADSHEET, + reason: "All rows don't have same number of columns", + }; } const result = tryParseCells(lines); diff --git a/src/packages/excalidraw/CHANGELOG.MD b/src/packages/excalidraw/CHANGELOG.MD index d09b1d14..2c846557 100644 --- a/src/packages/excalidraw/CHANGELOG.MD +++ b/src/packages/excalidraw/CHANGELOG.MD @@ -31,6 +31,7 @@ Please add the latest change on the top under the correct section. - Fix resizing the pasted charts [#2586](https://github.com/excalidraw/excalidraw/pull/2586) - Fix element visibility and zoom on cursor when canvas offset isn't 0. [#2534](https://github.com/excalidraw/excalidraw/pull/2534) - Fix Library Menu Layout [#2502](https://github.com/excalidraw/excalidraw/pull/2502) +- Support number with commas in charts [#2636](https://github.com/excalidraw/excalidraw/pull/2636) ### Improvements diff --git a/src/tests/__snapshots__/charts.test.tsx.snap b/src/tests/__snapshots__/charts.test.tsx.snap new file mode 100644 index 00000000..1bb5c2a4 --- /dev/null +++ b/src/tests/__snapshots__/charts.test.tsx.snap @@ -0,0 +1,20 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`tryParseSpreadsheet works for numbers with comma in them 1`] = ` +Object { + "spreadsheet": Object { + "labels": Array [ + "Week 1", + "Week 2", + "Week 3", + ], + "title": "Users", + "values": Array [ + 814, + 10301, + 4264, + ], + }, + "type": "VALID_SPREADSHEET", +} +`; diff --git a/src/tests/charts.test.tsx b/src/tests/charts.test.tsx new file mode 100644 index 00000000..a4bce153 --- /dev/null +++ b/src/tests/charts.test.tsx @@ -0,0 +1,13 @@ +import { tryParseSpreadsheet } from "../charts"; + +describe("tryParseSpreadsheet", () => { + it("works for numbers with comma in them", () => { + const result = tryParseSpreadsheet( + `Week Index${"\t"}Users +Week 1${"\t"}814 +Week 2${"\t"}10,301 +Week 3${"\t"}4,264`, + ); + expect(result).toMatchSnapshot(); + }); +});