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.
xh · Oct 7, 2022 · b02b76d · b02b76d
1 parent 837cb04
commit b02b76d
Show file tree

Hide file tree

Showing 4 changed files with 54 additions and 28 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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.