diff --git a/app/client/widgets/NumericEditor.ts b/app/client/widgets/NumericEditor.ts new file mode 100644 index 00000000..d5b2ba61 --- /dev/null +++ b/app/client/widgets/NumericEditor.ts @@ -0,0 +1,17 @@ +import {FieldOptions} from 'app/client/widgets/NewBaseEditor'; +import {NTextEditor} from 'app/client/widgets/NTextEditor'; + +export class NumericEditor extends NTextEditor { + constructor(protected options: FieldOptions) { + if (!options.editValue && typeof options.cellValue === 'number') { + // If opening a number for editing, we render it using the basic string representation (e.g. + // no currency symbols or groupings), but it's important to use the right locale so that the + // number can be parsed back (e.g. correct decimal separator). + const locale = options.field.documentSettings.peek().locale; + const fmt = new Intl.NumberFormat(locale, {useGrouping: false, maximumFractionDigits: 20}); + const editValue = fmt.format(options.cellValue); + options = {...options, editValue}; + } + super(options); + } +} diff --git a/app/client/widgets/UserType.ts b/app/client/widgets/UserType.ts index e3e2f223..c1a1fb20 100644 --- a/app/client/widgets/UserType.ts +++ b/app/client/widgets/UserType.ts @@ -84,7 +84,7 @@ export const typeDefs: any = { widgets: { TextBox: { cons: 'NumericTextBox', - editCons: 'TextEditor', + editCons: 'NumericEditor', icon: 'FieldTextbox', options: { alignment: 'right', @@ -98,7 +98,7 @@ export const typeDefs: any = { }, Spinner: { cons: 'Spinner', - editCons: 'TextEditor', + editCons: 'NumericEditor', icon: 'FieldSpinner', options: { alignment: 'right', diff --git a/app/client/widgets/UserTypeImpl.ts b/app/client/widgets/UserTypeImpl.ts index 55b9498d..e9c5b25e 100644 --- a/app/client/widgets/UserTypeImpl.ts +++ b/app/client/widgets/UserTypeImpl.ts @@ -15,6 +15,7 @@ import {NewAbstractWidget} from 'app/client/widgets/NewAbstractWidget'; import {NewBaseEditor} from 'app/client/widgets/NewBaseEditor'; import {NTextBox} from 'app/client/widgets/NTextBox'; import {NTextEditor} from 'app/client/widgets/NTextEditor'; +import {NumericEditor} from 'app/client/widgets/NumericEditor'; import {NumericTextBox} from 'app/client/widgets/NumericTextBox'; import {Reference} from 'app/client/widgets/Reference'; import {ReferenceEditor} from 'app/client/widgets/ReferenceEditor'; @@ -32,6 +33,7 @@ export const nameToWidget = { 'TextBox': NTextBox, 'TextEditor': NTextEditor, 'NumericTextBox': NumericTextBox, + 'NumericEditor': NumericEditor, 'HyperLinkTextBox': HyperLinkTextBox, 'HyperLinkEditor': HyperLinkEditor, 'Spinner': Spinner, diff --git a/app/common/NumberParse.ts b/app/common/NumberParse.ts index bda8b1be..3650eb5e 100644 --- a/app/common/NumberParse.ts +++ b/app/common/NumberParse.ts @@ -7,6 +7,7 @@ import {DocumentSettings} from 'app/common/DocumentSettings'; import {getDistinctValues} from 'app/common/gutil'; import {getCurrency, NumberFormatOptions, NumMode, parseNumMode} from 'app/common/NumberFormat'; +import {buildNumberFormat} from 'app/common/NumberFormat'; import escapeRegExp = require('lodash/escapeRegExp'); import last = require('lodash/last'); @@ -72,7 +73,7 @@ export default class NumberParse { // with corresponding digits from 0123456789. private readonly _replaceDigits: (s: string) => string; - constructor(locale: string, currency: string) { + constructor(public readonly locale: string, public readonly currency: string) { const parts = new Map(); for (const numMode of NumMode.values) { const formatter = Intl.NumberFormat(locale, parseNumMode(numMode, currency)); @@ -316,6 +317,12 @@ export default class NumberParse { result.decimals = decimals; } + // We should only set maxDecimals if the default maxDecimals is too low. + const tmpNF = buildNumberFormat(result, {locale: this.locale, currency: this.currency}).resolvedOptions(); + if (maxDecimals > tmpNF.maximumFractionDigits) { + result.maxDecimals = maxDecimals; + } + return result; } } diff --git a/app/common/ValueGuesser.ts b/app/common/ValueGuesser.ts index 2db42032..0ba5f173 100644 --- a/app/common/ValueGuesser.ts +++ b/app/common/ValueGuesser.ts @@ -69,7 +69,8 @@ abstract class ValueGuesser { const parsed = this.parse(value); // Give up if too many strings failed to parse or if the parsed value changes when converted back to text - if (typeof parsed === "string" && ++unparsed > maxUnparsed || formatter.formatAny(parsed) !== value) { + if ((typeof parsed === "string" && ++unparsed > maxUnparsed) || + !this.isEqualFormatted(formatter.formatAny(parsed), value)) { return null; } result.push(parsed); @@ -83,6 +84,10 @@ abstract class ValueGuesser { protected allowBlank(): boolean { return true; } + + protected isEqualFormatted(formatted1: string, formatted2: string): boolean { + return formatted1 === formatted2; + } } class BoolGuesser extends ValueGuesser { @@ -126,6 +131,14 @@ class NumericGuesser extends ValueGuesser { public parse(value: string): number | string { return this._parser.cleanParse(value); } + + protected isEqualFormatted(formatted1: string, formatted2: string): boolean { + // Consider format guessing successful if it returns the typed-in numeric value exactly or + // differing only in whitespace. + formatted1 = formatted1.replace(NumberParse.removeCharsRegex, ""); + formatted2 = formatted2.replace(NumberParse.removeCharsRegex, ""); + return formatted1 === formatted2; + } } class DateGuesser extends ValueGuesser { diff --git a/test/common/NumberParse.ts b/test/common/NumberParse.ts index d8980624..73f55585 100644 --- a/test/common/NumberParse.ts +++ b/test/common/NumberParse.ts @@ -230,7 +230,7 @@ describe("NumberParse", function() { assert.deepEqual(parser.guessOptions(["$1"]), {numMode: "currency", decimals: 0}); assert.deepEqual(parser.guessOptions(["$1.2"]), {numMode: "currency", decimals: 0}); assert.deepEqual(parser.guessOptions(["$1.23"]), {numMode: "currency"}); - assert.deepEqual(parser.guessOptions(["$1.234"]), {numMode: "currency"}); + assert.deepEqual(parser.guessOptions(["$1.234"]), {numMode: "currency", maxDecimals: 3}); // Otherwise decimal places are guessed based on trailing zeroes assert.deepEqual(parser.guessOptions(["$1.0"]), {numMode: "currency", decimals: 1}); diff --git a/test/nbrowser/LanguageSettings.ts b/test/nbrowser/LanguageSettings.ts index 551dca2a..834e556a 100644 --- a/test/nbrowser/LanguageSettings.ts +++ b/test/nbrowser/LanguageSettings.ts @@ -23,8 +23,9 @@ describe("LanguageSettings", function() { for (const [locale, countryCode, language] of locales) { describe(`correctly detects browser language ${locale}`, () => { // Change the language to the one we want to test. - withLang(locale); + const skipStatus = withLang(locale); before(async function() { + if (skipStatus.skipped) { return; } const session = await gu.session().personalSite.anon.login(); await session.loadRelPath("/"); await gu.waitForDocMenuToLoad(); @@ -116,34 +117,38 @@ describe("LanguageSettings", function() { const anonym = await gu.session().personalSite.anon.login(); await anonym.loadRelPath("/"); await gu.waitForDocMenuToLoad(); + assert.isNull(await languageInCookie()); // Change language to french. await langButton().click(); await driver.find(".test-language-lang-fr").click(); await waitForLangButton("fr"); + assert.equal(await languageInCookie(), "fr"); // Login as user. await user.login(); await anonym.loadRelPath("/"); await gu.waitForDocMenuToLoad(); + // But now we should have a cookie (cookie is reused). + assert.equal(await languageInCookie(), 'fr'); // Language should still be french. await waitForHiddenButton("fr"); - // But now we should have a cookie (cookie is reused). - assert.equal(await languageInCookie(), 'fr'); await clearCookie(); }); }); describe("for logged in user with nb-NO", function() { - withLang("de"); + const skipStatus = withLang("de"); let session: gu.Session; before(async function() { + if (skipStatus.skipped) { return; } session = await gu.session().login(); await session.loadRelPath("/"); await gu.waitForDocMenuToLoad(); }); after(async function() { + if (skipStatus.skipped) { return; } await clearCookie(); const api = session.createHomeApi(); await api.updateUserLocale(null); @@ -215,12 +220,13 @@ async function languageInCookie(): Promise { return cookie2.match(/grist_user_locale=([^;]+)/)?.[1] ?? null; } -function withLang(locale: string) { +function withLang(locale: string): {skipped: boolean} { let customDriver: WebDriver; let oldLanguage: string | undefined; + const skipStatus = {skipped: false}; before(async function() { - // On Mac we can't change the language, so skip the test. - if (await gu.isMac()) { return this.skip(); } + // On Mac we can't change the language (except for English), so skip the test. + if (await gu.isMac() && locale !== 'en') { skipStatus.skipped = true; return this.skip(); } oldLanguage = process.env.LANGUAGE; // How to run chrome with a different language: // https://developer.chrome.com/docs/extensions/reference/i18n/#how-to-set-browsers-locale @@ -238,12 +244,13 @@ function withLang(locale: string) { await gu.waitForDocMenuToLoad(); }); after(async function() { - if (await gu.isMac()) { return this.skip(); } + if (skipStatus.skipped) { return; } gu.setDriver(); server.setDriver(); await customDriver.quit(); process.env.LANGUAGE = oldLanguage; }); + return skipStatus; } function langButton() { diff --git a/test/nbrowser/NumericEditor.ts b/test/nbrowser/NumericEditor.ts new file mode 100644 index 00000000..9427e0fb --- /dev/null +++ b/test/nbrowser/NumericEditor.ts @@ -0,0 +1,149 @@ +import type {UserAPI} from 'app/common/UserAPI'; +import * as gu from 'test/nbrowser/gristUtils'; +import {setupTestSuite} from 'test/nbrowser/testUtils'; +import {assert, driver, Key} from 'mocha-webdriver'; + +describe('NumericEditor', function() { + this.timeout(20000); + const cleanup = setupTestSuite(); + afterEach(() => gu.checkForErrors()); + + interface Entry { + initial: string; + workaround?: () => Promise; + expect: string; + exp0: string; + expPlain: string; + expFmt: string; + } + interface TestOpts { + localeCode: string; + locale: string; + entriesByColumn: Entry[]; + valPlain: string; + valFmt: string; + } + + describe('locale en-US', testSuite({ + localeCode: 'en-US', + locale: 'United States (English)', + entriesByColumn: [ + {initial: '17', expect: '17', exp0: '0', expPlain: '-4.4', expFmt: '-1234.56' }, + {initial: '17.500', expect: '17.500', exp0: '0.000', expPlain: '-4.400', expFmt: '-1234.560' }, + {initial: '(1.2)', expect: '(1.2)', exp0: ' 0 ', expPlain: '(4.4)', expFmt: '(1234.56)' }, + {initial: '1000.456', expect: '1000.456', exp0: '0', expPlain: '-4.4', expFmt: '-1234.56' }, + {initial: '1,000.0', expect: '1,000.0', exp0: '0.0', expPlain: '-4.4', expFmt: '-1,234.56' }, + {initial: '$5.00', expect: '$5.00', exp0: '$0.00', expPlain: '-$4.40', expFmt: '-$1,234.56'}, + {initial: '4.5%', expect: '4.5%', exp0: '0%', expPlain: '-440%', expFmt: '-123,456%' }, + ], + valPlain: '-4.4', + valFmt: '(1,234.56)', + })); + + describe('locale de-DE', testSuite({ + localeCode: 'de-DE', + locale: 'Germany (German)', + entriesByColumn: [ + {initial: '17', expect: '17', exp0: '0', expPlain: '-4,4', expFmt: '-1234,56' }, + {initial: '17,500', expect: '17,500', exp0: '0,000', expPlain: '-4,400', expFmt: '-1234,560' }, + {initial: '(1,2)', expect: '(1,2)', exp0: ' 0 ', expPlain: '(4,4)', expFmt: '(1234,56)' }, + {initial: '1000,456', expect: '1000,456', exp0: '0', expPlain: '-4,4', expFmt: '-1234,56' }, + {initial: '1.000,0', expect: '1.000,0', exp0: '0,0', expPlain: '-4,4', expFmt: '-1.234,56', }, + {initial: '5,00€', expect: '5,00 €', exp0: '0,00 €', expPlain: '-4,40 €', expFmt: '-1.234,56 €' }, + {initial: '4,5%', expect: '4,5 %', exp0: '0 %', expPlain: '-440 %', expFmt: '-123.456 %' }, + ], + valPlain: '-4,4', + valFmt: '(1.234,56)', + })); + + function testSuite(options: TestOpts) { + const {entriesByColumn} = options; + + return function() { + let docId: string; + let api: UserAPI; + let MODKEY: string; + + before(async function() { + MODKEY = await gu.modKey(); + const session = await gu.session().login(); + docId = await session.tempNewDoc(cleanup, `NumericEditor-${options.localeCode}`); + api = session.createHomeApi(); + + await api.applyUserActions(docId, [ + // Make sure there are as many columns as entriesByColumn. + ...entriesByColumn.slice(3).map(() => ['AddVisibleColumn', 'Table1', '', {}]), + ]); + + // Set locale for the document. + await gu.openDocumentSettings(); + await driver.findWait('.test-locale-autocomplete', 500).click(); + await driver.sendKeys(options.locale, Key.ENTER); + await gu.waitForServer(); + assert.equal(await driver.find('.test-locale-autocomplete input').value(), options.locale); + await gu.openPage('Table1'); + }); + + beforeEach(async function() { + // Scroll grid to the left before each test case. + await gu.sendKeys(Key.HOME); + }); + + it('should create Numeric columns with suitable format', async function() { + // Entering a value into an empty column should switch it to a suitably formatted Numeric. + for (const [i, entry] of Object.entries(entriesByColumn)) { + await gu.getCell({rowNum: 1, col: Number(i)}).click(); + await entry.workaround?.(); + await gu.enterCell(entry.initial); + assert.equal(await gu.getCell({rowNum: 1, col: Number(i)}).getText(), entry.expect); + } + + // Add a new row, which should be filled with 0's. Check what formatted 0's look like. + await gu.sendKeys(Key.chord(MODKEY, Key.ENTER)); + await gu.waitForServer(); + + // Check what 0's look like. + for (const [i, entry] of Object.entries(entriesByColumn)) { + assert.equal(await gu.getCell({rowNum: 2, col: Number(i)}).getText(), entry.exp0); + } + }); + + it('should support entering plain numbers into a formatted column', async function() { + const rowNum = 3; + const cols = entriesByColumn.map((_, i) => i); + + await gu.enterGridRows({rowNum, col: 0}, + [ entriesByColumn.map(() => options.valPlain) ]); + + assert.deepEqual(await gu.getVisibleGridCells({rowNums: [rowNum], cols}), + entriesByColumn.map((entry) => entry.expPlain)); + }); + + it('should support entering formatted numbers into a formatted column', async function() { + const rowNum = 4; + const cols = entriesByColumn.map((_, i) => i); + + await gu.enterGridRows({rowNum, col: 0}, + [ entriesByColumn.map(() => options.valFmt) ]); + + assert.deepEqual(await gu.getVisibleGridCells({rowNums: [rowNum], cols}), + entriesByColumn.map((entry) => entry.expFmt)); + }); + + it('should allow editing and saving a formatted value', async function() { + for (const [i, entry] of Object.entries(entriesByColumn)) { + await gu.getCell({rowNum: 1, col: Number(i)}).click(); + await gu.sendKeys(Key.ENTER); + await gu.waitAppFocus(false); + // Save the value the way it's opened in the editor. It's important that it doesn't + // change interpretation (there was a bug related to this, when a value with "," decimal + // separator would open with a "." decimal separator, and get saved back incorrectly). + await gu.sendKeys(Key.ENTER); + await gu.waitForServer(); + await gu.waitAppFocus(true); + assert.equal(await gu.getCell({rowNum: 1, col: Number(i)}).getText(), entry.expect); + } + }); + }; + } +});