From b02b76d80d596370d0b87dd85907cad0f3d6feff Mon Sep 17 00:00:00 2001 From: Jakub Kupsik <101586582+jskupsik@users.noreply.github.com> Date: Fri, 7 Oct 2022 15:55:58 -0700 Subject: [PATCH] Filter chooser model now accepts shorthand number inputs (#3155) * Fixed FilterChooser rendering '[object Object]' using suffixed/formatted numbers * Generalized the `parseValue` from NumberInput.js and reused it as the default numeric parser for FieldChooserFieldSpec. --- CHANGELOG.md | 1 + cmp/filter/FilterChooserFieldSpec.js | 20 ++++++++++++++-- desktop/cmp/input/NumberInput.js | 27 ++-------------------- format/FormatNumber.js | 34 +++++++++++++++++++++++++++- 4 files changed, 54 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7189433ea0..6477d16be2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,7 @@ ### 🐞 Bug Fixes +* FilterChooserModel now accepts numeric shorthand for numeric fields. (ie, '1,337k' => 1337000.0) * Boolean filterChooser fields with enableValues=false will no longer suggest true for 'false'. * Change to `CompoundTaskObserver` to prioritize using specific messages from subtasks over the overall task message. diff --git a/cmp/filter/FilterChooserFieldSpec.js b/cmp/filter/FilterChooserFieldSpec.js index ac75218653..0d1d6331c1 100644 --- a/cmp/filter/FilterChooserFieldSpec.js +++ b/cmp/filter/FilterChooserFieldSpec.js @@ -6,9 +6,13 @@ */ import {BaseFilterFieldSpec} from '@xh/hoist/data/filter/BaseFilterFieldSpec'; import {FieldType, parseFieldValue} from '@xh/hoist/data'; -import {fmtDate} from '@xh/hoist/format'; +import {fmtDate, parseNumber} from '@xh/hoist/format'; import {stripTags, throwIf} from '@xh/hoist/utils/js'; import {isFunction, isNil} from 'lodash'; +import {isValidElement} from 'react'; +import {renderToStaticMarkup} from 'react-dom/server'; + +const {INT, NUMBER} = FieldType; /** * Filter field specification class for the typeahead `FilterChooser` component. Manages additional @@ -47,7 +51,7 @@ export class FilterChooserFieldSpec extends BaseFilterFieldSpec { super(rest); this.valueRenderer = valueRenderer; - this.valueParser = valueParser; + this.valueParser = this.parseValueParser(valueParser); this.example = this.parseExample(example); if (!this.hasExplicitValues && @@ -77,6 +81,10 @@ export class FilterChooserFieldSpec extends BaseFilterFieldSpec { let ret; if (isFunction(this.valueRenderer)) { ret = this.valueRenderer(value, op); + if (isValidElement(ret)) { + // Prevents [object Object] rendering + ret = renderToStaticMarkup(ret); + } } else if (this.isDateBasedFieldType) { ret = fmtDate(value); } else { @@ -109,6 +117,14 @@ export class FilterChooserFieldSpec extends BaseFilterFieldSpec { return 'value'; } + parseValueParser(valueParser) { + // Default numeric parser + if (!valueParser && (this.fieldType === INT || this.fieldType === NUMBER)) { + return (input, op) => parseNumber(input); + } + return valueParser; + } + loadValuesFromSource() { const {field, source} = this, values = new Set(); diff --git a/desktop/cmp/input/NumberInput.js b/desktop/cmp/input/NumberInput.js index 00adad95ba..7c51bd9281 100644 --- a/desktop/cmp/input/NumberInput.js +++ b/desktop/cmp/input/NumberInput.js @@ -8,7 +8,7 @@ import composeRefs from '@seznam/compose-react-refs'; import {HoistInputModel, HoistInputPropTypes, useHoistInputModel} from '@xh/hoist/cmp/input'; import {hoistCmp} from '@xh/hoist/core'; import '@xh/hoist/desktop/register'; -import {fmtNumber} from '@xh/hoist/format'; +import {fmtNumber, parseNumber} from '@xh/hoist/format'; import {numericInput} from '@xh/hoist/kit/blueprint'; import {wait} from '@xh/hoist/promise'; import {debounced, throwIf, withDefault} from '@xh/hoist/utils/js'; @@ -127,8 +127,6 @@ NumberInput.hasLayoutSupport = true; //----------------------- class NumberInputModel extends HoistInputModel { - static shorthandValidator = /((\.\d+)|(\d+(\.\d+)?))([kmb])\b/i; - constructor() { super(); throwIf(Math.log10(this.scaleFactor) % 1 !== 0, 'scaleFactor must be a factor of 10'); @@ -213,28 +211,7 @@ class NumberInputModel extends HoistInputModel { } parseValue(value) { - if (isNil(value) || value === '') return null; - - value = value.toString(); - value = value.replace(/,/g, ''); - - if (NumberInputModel.shorthandValidator.test(value)) { - const num = +value.substring(0, value.length - 1), - lastChar = value.charAt(value.length - 1).toLowerCase(); - - switch (lastChar) { - case 'k': - return num * 1000; - case 'm': - return num * 1000000; - case 'b': - return num * 1000000000; - default: - return NaN; - } - } - - return parseFloat(value); + return parseNumber(value); } noteFocused() { diff --git a/format/FormatNumber.js b/format/FormatNumber.js index 8deacff8fa..8dcab1ff52 100644 --- a/format/FormatNumber.js +++ b/format/FormatNumber.js @@ -5,7 +5,7 @@ * Copyright © 2022 Extremely Heavy Industries Inc. */ import {span} from '@xh/hoist/cmp/layout'; -import {defaults, isFinite, isFunction, isPlainObject, isString} from 'lodash'; +import {defaults, isFinite, isFunction, isNil, isPlainObject, isString} from 'lodash'; import numbro from 'numbro'; import {fmtSpan} from './FormatMisc'; import {createRenderer, saveOriginal} from './FormatUtils'; @@ -369,6 +369,38 @@ export const numberRenderer = createRenderer(fmtNumber), priceRenderer = createRenderer(fmtPrice), percentRenderer = createRenderer(fmtPercent); + +const shorthandValidator = /((\.\d+)|(\d+(\.\d+)?))([kmb])\b/i; + +/** + * @param {string} value - A string that represents a shorthand numerical value + * @returns {number} - The number represented by the shorthand string, or NaN + */ +export function parseNumber(value) { + if (isNil(value) || value === '') return null; + + value = value.toString(); + value = value.replace(/,/g, ''); + + if (shorthandValidator.test(value)) { + const num = +value.substring(0, value.length - 1), + lastChar = value.charAt(value.length - 1).toLowerCase(); + + switch (lastChar) { + case 'k': + return num * 1000; + case 'm': + return num * 1000000; + case 'b': + return num * 1000000000; + default: + return NaN; + } + } + + return parseFloat(value); +} + /** * @callback fmtNumber~tooltipFn - renderer for a custom tooltip. * @param {number} originalValue - number to be formatted.