(core) Adds new range filter for numeric columns

Summary:
Shows the range filter next to the filter by values on filter menu. When users
set min and/or max, it takes precendence over the filter by values.

If users set:
 - `[] < [max]` behaves as `less than max`.
 - `[min] < []` behaves as `more than min`.
 - `[min] < [max]` behaves as `between min and max`
 - bounds are always inclusives.
 - when users change min or max the values of the by values filter
   gets checked/unchecked depending on whether they are included by
   the range filter.
 - when users clicks any btn/checkbox of the by values filter both min
   and max input gets cleared, and the filter convert to a filter by
   values.

Test Plan: Adds both projets and nbrowser tests.

Reviewers: jarek

Reviewed By: jarek

Differential Revision: https://phab.getgrist.com/D3435

app/client/models/ColumnFilter.ts

@@ -1,6 +1,6 @@
 import {ColumnFilterFunc, makeFilterFunc} from "app/common/ColumnFilterFunc";
 import {CellValue} from 'app/common/DocActions';
-import {FilterSpec, FilterState, makeFilterState} from "app/common/FilterState";
+import {FilterSpec, FilterState, isRangeFilter, makeFilterState} from "app/common/FilterState";
 import {nativeCompare} from 'app/common/gutil';
 import {Computed, Disposable, Observable} from 'grainjs';
 
@@ -13,6 +13,10 @@ import {Computed, Disposable, Observable} from 'grainjs';
  * been customized.
  */
 export class ColumnFilter extends Disposable {
+
+  public min = Observable.create<number|undefined>(this, undefined);
+  public max = Observable.create<number|undefined>(this, undefined);
+
   public readonly filterFunc = Observable.create<ColumnFilterFunc>(this, () => true);
 
   // Computed that returns true if filter is an inclusion filter, false otherwise.
@@ -25,9 +29,11 @@ export class ColumnFilter extends Disposable {
   private _values: Set<CellValue>;
 
   constructor(private _initialFilterJson: string, private _columnType: string = '',
-              public visibleColumnType: string = '') {
+              public visibleColumnType: string = '', private _allValues: CellValue[] = []) {
     super();
     this.setState(_initialFilterJson);
+    this.autoDispose(this.min.addListener(() => this._updateState()));
+    this.autoDispose(this.max.addListener(() => this._updateState()));
   }
 
   public get columnType() {
@@ -36,13 +42,25 @@ export class ColumnFilter extends Disposable {
 
   public setState(filterJson: string|FilterSpec) {
     const state = makeFilterState(filterJson);
+    if (isRangeFilter(state)) {
+      this.min.set(state.min);
+      this.max.set(state.max);
+      // Setting _include to false allows to make sure that the filter reverts to all values
+      // included when users delete one bound (min or max) while the other bound is already
+      // undefined (filter reverts to switching by value when both min and max are undefined).
+      this._include = false;
+      this._values = new Set();
+    } else {
+      this.min.set(undefined);
+      this.max.set(undefined);
       this._include = state.include;
       this._values = state.values;
+    }
     this._updateState();
   }
 
   public includes(val: CellValue): boolean {
-    return this._values.has(val) === this._include;
+    return this.filterFunc.get()(val);
   }
 
   public add(val: CellValue) {
@@ -50,6 +68,7 @@ export class ColumnFilter extends Disposable {
   }
 
   public addMany(values: CellValue[]) {
+    this._toValues();
     for (const val of values) {
       this._include ? this._values.add(val) : this._values.delete(val);
     }
@@ -61,6 +80,7 @@ export class ColumnFilter extends Disposable {
   }
 
   public deleteMany(values: CellValue[]) {
+    this._toValues();
     for (const val of values) {
       this._include ? this._values.delete(val) : this._values.add(val);
     }
@@ -81,8 +101,14 @@ export class ColumnFilter extends Disposable {
 
   // For saving the filter value back.
   public makeFilterJson(): string {
+    let filter: any;
+    if (this.min.get() !== undefined || this.max.get() !== undefined) {
+      filter = {min: this.min.get(), max: this.max.get()};
+    } else {
       const values = Array.from(this._values).sort(nativeCompare);
-    return JSON.stringify(this._include ? {included: values} : {excluded: values});
+      filter = {[this._include ? 'included' : 'excluded']: values};
+    }
+    return JSON.stringify(filter);
   }
 
   public hasChanged(): boolean {
@@ -94,7 +120,21 @@ export class ColumnFilter extends Disposable {
   }
 
   private _getState(): FilterState {
-    return {include: this._include, values: this._values};
+    return {include: this._include, values: this._values, min: this.min.get(), max: this.max.get()};
+  }
+
+  private _isRange() {
+    return isRangeFilter(this._getState());
+  }
+
+  private _toValues() {
+    if (this._isRange()) {
+      const func = this.filterFunc.get();
+      const state = this._include ?
+        { included: this._allValues.filter((val) => func(val)) } :
+        { excluded: this._allValues.filter((val) => !func(val)) };
+      this.setState(state);
+    }
   }
 }
 

app/client/ui/ColumnFilterMenu.ts

@@ -19,16 +19,18 @@ import {icon} from 'app/client/ui2018/icons';
 import {menuCssClass, menuDivider} from 'app/client/ui2018/menus';
 import {CellValue} from 'app/common/DocActions';
 import {isEquivalentFilter} from "app/common/FilterState";
-import {Computed, dom, DomElementArg, DomElementMethod, IDisposableOwner, input, makeTestId, styled} from 'grainjs';
+import {Computed, dom, DomElementArg, DomElementMethod, IDisposableOwner, input, makeTestId, Observable,
+  styled} from 'grainjs';
 import concat = require('lodash/concat');
 import identity = require('lodash/identity');
 import noop = require('lodash/noop');
 import partition = require('lodash/partition');
 import some = require('lodash/some');
 import tail = require('lodash/tail');
+import debounce = require('lodash/debounce');
 import {IOpenController, IPopupOptions, setPopupToCreateDom} from 'popweasel';
 import {decodeObject} from 'app/plugin/objtypes';
-import {isList, isRefListType} from 'app/common/gristTypes';
+import {isList, isNumberType, isRefListType} from 'app/common/gristTypes';
 import {choiceToken} from 'app/client/widgets/ChoiceToken';
 import {ChoiceOptions} from 'app/client/widgets/ChoiceTextBox';
 import {cssInvalidToken} from 'app/client/widgets/ChoiceListCell';
@@ -39,12 +41,13 @@ interface IFilterMenuOptions {
   doSave: (reset: boolean) => void;
   onClose: () => void;
   renderValue: (key: CellValue, value: IFilterCount) => DomElementArg;
+  valueParser?: (val: string) => any;
 }
 
 const testId = makeTestId('test-filter-menu-');
 
 export function columnFilterMenu(owner: IDisposableOwner, opts: IFilterMenuOptions): HTMLElement {
-  const { model, doSave, onClose, renderValue } = opts;
+  const { model, doSave, onClose, renderValue, valueParser } = opts;
   const { columnFilter } = model;
   // Save the initial state to allow reverting back to it on Cancel
   const initialStateJson = columnFilter.makeFilterJson();
@@ -52,12 +55,14 @@ export function columnFilterMenu(owner: IDisposableOwner, opts: IFilterMenuOptio
   // Map to keep track of displayed checkboxes
   const checkboxMap: Map<CellValue, HTMLInputElement> = new Map();
 
-  // Listen for changes to filterFunc, and update checkboxes accordingly
+  // Listen for changes to filterFunc, and update checkboxes accordingly. Debounce is needed to
-  const filterListener = columnFilter.filterFunc.addListener(func => {
+  // prevent some weirdness when users click on a checkbox while focus was on a range input (causing
+  // sometimes the checkbox to not toggle)
+  const filterListener = columnFilter.filterFunc.addListener(debounce(func => {
     for (const [value, elem] of checkboxMap) {
       elem.checked = func(value);
     }
-  });
+  }));
 
   const {searchValue: searchValueObs, filteredValues, filteredKeys, isSortedByCount} = model;
 
@@ -65,10 +70,11 @@ export function columnFilterMenu(owner: IDisposableOwner, opts: IFilterMenuOptio
   const isSearchingObs = Computed.create(owner, (use) => Boolean(use(searchValueObs)));
 
   let searchInput: HTMLInputElement;
+  let minRangeInput: HTMLInputElement;
   let reset = false;
 
-  // Gives focus to the searchInput on open
+  // Gives focus to the searchInput on open (or to the min input if the range filter is present).
-  setTimeout(() => searchInput.focus(), 0);
+  setTimeout(() => (minRangeInput || searchInput).select(), 0);
 
   const filterMenu: HTMLElement = cssMenu(
     { tabindex: '-1' }, // Allow menu to be focused
@@ -80,6 +86,18 @@ export function columnFilterMenu(owner: IDisposableOwner, opts: IFilterMenuOptio
       Enter: () => onClose(),
       Escape: () => onClose()
     }),
+
+    // Filter by range
+    dom.maybe(isNumberType(columnFilter.columnType), () => [
+      cssRangeHeader('Filter by Range'),
+      cssRangeContainer(
+        minRangeInput = rangeInput('Min ', columnFilter.min, {valueParser}, testId('min')),
+        cssRangeInputSeparator('→'),
+        rangeInput('Max ', columnFilter.max, {valueParser}, testId('max')),
+      ),
+      cssMenuDivider(),
+    ]),
+
     cssMenuHeader(
       cssSearchIcon('Search'),
       searchInput = cssSearch(
@@ -149,8 +167,9 @@ export function columnFilterMenu(owner: IDisposableOwner, opts: IFilterMenuOptio
           cssLabel(
             cssCheckboxSquare(
               {type: 'checkbox'},
-              dom.on('change', (_ev, elem) =>
+              dom.on('change', (_ev, elem) => {
-                elem.checked ? columnFilter.add(key) : columnFilter.delete(key)),
+                elem.checked ? columnFilter.add(key) : columnFilter.delete(key);
+             }),
               (elem) => { elem.checked = columnFilter.includes(key); checkboxMap.set(key, elem); },
               dom.style('position', 'relative'),
             ),
@@ -194,6 +213,37 @@ export function columnFilterMenu(owner: IDisposableOwner, opts: IFilterMenuOptio
   return filterMenu;
 }
 
+function rangeInput(placeholder: string, obs: Observable<number|undefined>,
+                    opts: {valueParser?: (val: string) => any},
+                    ...args: DomElementArg[]) {
+  const valueParser = opts.valueParser || Number;
+  const formatValue = ((val: any) => val?.toString() || '');
+  let editMode = false;
+  let el: HTMLInputElement;
+  // keep input content in sync only when no edit are going on.
+  const lis = obs.addListener(() => editMode ? null : el.value = formatValue(obs.get()));
+  // handle change
+  const onBlur = () => {
+    onInput.flush();
+    editMode = false;
+    el.value = formatValue(obs.get());
+  };
+  const onInput = debounce(() => {
+    editMode = true;
+    const val = el.value ? valueParser(el.value) : undefined;
+    if (val === undefined || !isNaN(val)) {
+      obs.set(val);
+    }
+  }, 10);
+  return el = cssRangeInput(
+    {inputmode: 'numeric', placeholder, value: formatValue(obs.get())},
+    dom.on('input', onInput),
+    dom.on('blur', onBlur),
+    dom.autoDispose(lis),
+    ...args
+  );
+}
+
 
 /**
  * Builds a tri-state checkbox that summaries the state of all the `values`. The special value
@@ -304,6 +354,7 @@ export function createFilterMenu(openCtl: IOpenController, sectionFilter: Sectio
   const visibleColumnType = fieldOrColumn.visibleColModel.peek()?.type.peek() || columnType;
   const {keyMapFunc, labelMapFunc, valueMapFunc} = getMapFuncs(columnType, tableData, filterInfo.fieldOrColumn);
   const activeFilterBar = sectionFilter.viewSection.activeFilterBar;
+  const valueParser = (fieldOrColumn as any).createValueParser?.();
 
   function getFilterFunc(col: ViewFieldRec|ColumnRec, colFilter: ColumnFilterFunc|null) {
     return col.getRowId() === fieldOrColumn.getRowId() ? null : colFilter;
@@ -311,9 +362,6 @@ export function createFilterMenu(openCtl: IOpenController, sectionFilter: Sectio
   const filterFunc = Computed.create(null, use => sectionFilter.buildFilterFunc(getFilterFunc, use));
   openCtl.autoDispose(filterFunc);
 
-  const columnFilter = ColumnFilter.create(openCtl, filterInfo.filter.peek(), columnType, visibleColumnType);
-  sectionFilter.setFilterOverride(fieldOrColumn.getRowId(), columnFilter); // Will be removed on menu disposal
-
   const [allRows, hiddenRows] = partition(Array.from(rowSource.getAllRows()), filterFunc.get());
   const valueCounts = getEmptyCountMap(fieldOrColumn);
   addCountsToMap(valueCounts, allRows, {keyMapFunc, labelMapFunc, columnType,
@@ -321,7 +369,11 @@ export function createFilterMenu(openCtl: IOpenController, sectionFilter: Sectio
   addCountsToMap(valueCounts, hiddenRows, {keyMapFunc, labelMapFunc, columnType,
                                            areHiddenRows: true, valueMapFunc});
 
-  const model = ColumnFilterMenuModel.create(openCtl, columnFilter, Array.from(valueCounts));
+  const valueCountsArr = Array.from(valueCounts);
+  const columnFilter = ColumnFilter.create(openCtl, filterInfo.filter.peek(), columnType, visibleColumnType,
+                                           valueCountsArr.map((arr) => arr[0]));
+  sectionFilter.setFilterOverride(fieldOrColumn.getRowId(), columnFilter); // Will be removed on menu disposal
+  const model = ColumnFilterMenuModel.create(openCtl, columnFilter, valueCountsArr);
 
   return columnFilterMenu(openCtl, {
     model,
@@ -339,6 +391,7 @@ export function createFilterMenu(openCtl: IOpenController, sectionFilter: Sectio
       }
     },
     renderValue: getRenderFunc(columnType, fieldOrColumn),
+    valueParser,
   });
 }
 
@@ -616,3 +669,24 @@ const cssToken = styled('div', `
   margin-left: 8px;
   margin-right: 12px;
 `);
+const cssRangeHeader = styled(cssMenuItem, `
+  padding: unset;
+  border-radius: 0 0 3px 0;
+  text-transform: uppercase;
+  font-size: var(--grist-x-small-font-size);
+  margin: 16px 16px 6px 16px;
+`);
+const cssRangeContainer = styled(cssMenuItem, `
+  display: flex;
+  justify-content: left;
+  column-gap: 10px;
+`);
+const cssRangeInputSeparator = styled('span', `
+  font-weight: 600;
+  position: relative;
+  top: 3px;
+  color: var(--grist-color-slate);
+`);
+const cssRangeInput = styled('input', `
+  width: 80px;
+`);

app/common/ColumnFilterFunc.ts

@@ -1,14 +1,34 @@
 import {CellValue} from "app/common/DocActions";
-import {FilterState, makeFilterState} from "app/common/FilterState";
+import {FilterState, isRangeFilter, makeFilterState} from "app/common/FilterState";
 import {decodeObject} from "app/plugin/objtypes";
-import {isList, isListType} from "./gristTypes";
+import {isList, isListType, isNumberType} from "./gristTypes";
 
 export type ColumnFilterFunc = (value: CellValue) => boolean;
 
 // Returns a filter function for a particular column: the function takes a cell value and returns
 // whether it's accepted according to the given FilterState.
-export function makeFilterFunc({ include, values }: FilterState,
+export function makeFilterFunc(state: FilterState,
                                columnType?: string): ColumnFilterFunc {
+
+  if (isRangeFilter(state)) {
+    const {min, max} = state;
+    if (isNumberType(columnType)) {
+      return (val) => {
+        if (typeof val !== 'number') { return false; }
+        return (
+          (max === undefined ? true : val <= max) &&
+            (min === undefined ? true : min <= val)
+        );
+      };
+    } else {
+      // Although it is not possible to set a range filter for non numeric columns, this still can
+      // happen as a result of a column type conversion. In this case, let's include all values.
+      return () => true;
+    }
+  }
+
+  const {include, values} = state;
+
   // NOTE: This logic results in complex values and their stringified JSON representations as equivalent.
   // For example, a TypeError in the formula column and the string '["E","TypeError"]' would be seen as the same.
   // TODO: This narrow corner case seems acceptable for now, but may be worth revisiting.
@@ -17,7 +37,6 @@ export function makeFilterFunc({ include, values }: FilterState,
       const list = decodeObject(val) as unknown[];
       return list.some(item => values.has(item as any) === include);
     }
-
     return (values.has(Array.isArray(val) ? JSON.stringify(val) : val) === include);
   };
 }

app/common/FilterState.ts

@@ -4,19 +4,32 @@ import { CellValue } from "app/common/DocActions";
 export interface FilterSpec {
   included?: CellValue[];
   excluded?: CellValue[];
+  min?: number;
+  max?: number;
 }
 
+
+export type FilterState = ByValueFilterState | RangeFilterState
+
 // A more efficient representation of filter state for a column than FilterSpec.
-export interface FilterState {
+interface ByValueFilterState {
   include: boolean;
   values: Set<CellValue>;
 }
 
+interface RangeFilterState {
+  min?: number;
+  max?: number;
+}
+
 // Creates a FilterState. Accepts spec as a json string or a FilterSpec.
 export function makeFilterState(spec: string | FilterSpec): FilterState {
   if (typeof (spec) === 'string') {
     return makeFilterState((spec && JSON.parse(spec)) || {});
   }
+  if (spec.min !== undefined || spec.max !== undefined) {
+    return {min: spec.min, max: spec.max};
+  }
   return {
     include: Boolean(spec.included),
     values: new Set(spec.included || spec.excluded || []),
@@ -26,8 +39,23 @@ export function makeFilterState(spec: string | FilterSpec): FilterState {
 // Returns true if state and spec are equivalent, false otherwise.
 export function isEquivalentFilter(state: FilterState, spec: FilterSpec): boolean {
   const other = makeFilterState(spec);
+  if (!isRangeFilter(state) && !isRangeFilter(other)) {
     if (state.include !== other.include) { return false; }
     if (state.values.size !== other.values.size) { return false; }
+    if (other.values) {
       for (const val of other.values) { if (!state.values.has(val)) { return false; } }
+    }
+  } else {
+    if (isRangeFilter(state) && isRangeFilter(other)) {
+      if (state.min !== other.min || state.max !== other.max) { return false; }
+    } else {
+      return false;
+    }
+  }
   return true;
 }
+
+export function isRangeFilter(state: FilterState): state is RangeFilterState {
+  const {min, max} = state as any;
+  return min !== undefined || max !== undefined;
+}

app/common/gristTypes.ts

@@ -337,6 +337,10 @@ export function isListType(type: string) {
   return type === "ChoiceList" || isRefListType(type);
 }
 
+export function isNumberType(type: string|undefined) {
+  return ['Numeric', 'Int'].includes(type || '');
+}
+
 export function isFullReferencingType(type: string) {
   return type.startsWith('Ref:') || isRefListType(type);
 }