(core) Makes filter counts take other column filters into account

Summary:
Makes filter counts take other column filters into account.

 - Changes the summaries rows to reflect hidden rows:
    - hidden rows are added to the `Other Values` summary
    - show the unique number of other values as `Other Values (12)`
 - Also, adds a sort button to the column filter menu

Test Plan: Adds browser test.

Reviewers: paulfitz, jarek

Reviewed By: jarek

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

app/client/components/BaseView.js

@@ -647,7 +647,7 @@ BaseView.prototype.getLastDataRowIndex = function() {
  * Creates and opens ColumnFilterMenu for a given field, and returns its PopupControl.
  */
 BaseView.prototype.createFilterMenu = function(openCtl, field, onClose) {
-  return createFilterMenu(openCtl, this._sectionFilter, field, this._filteredRowSource, this.tableModel.tableData, onClose);
+  return createFilterMenu(openCtl, this._sectionFilter, field, this._mainRowSource, this.tableModel.tableData, onClose);
 };
 
 /**

app/client/models/ColumnFilter.ts

@@ -1,8 +1,8 @@
+import {ColumnFilterFunc, makeFilterFunc} from "app/common/ColumnFilterFunc";
 import {CellValue} from 'app/common/DocActions';
+import {FilterSpec, FilterState, makeFilterState} from "app/common/FilterState";
 import {nativeCompare} from 'app/common/gutil';
 import {Computed, Disposable, Observable} from 'grainjs';
-import {ColumnFilterFunc, makeFilterFunc} from "app/common/ColumnFilterFunc";
-import {FilterSpec, FilterState, makeFilterState} from "app/common/FilterState";
 
 /**
  * ColumnFilter implements a custom filter on a column, i.e. a filter that's diverged from what's

app/client/models/ColumnFilterMenuModel.ts

@@ -1,8 +1,8 @@
+import { ColumnFilter } from "app/client/models/ColumnFilter";
+import { localeCompare, nativeCompare } from "app/common/gutil";
+import { CellValue } from "app/plugin/GristData";
 import { Computed, Disposable, Observable } from "grainjs";
 import escapeRegExp = require("lodash/escapeRegExp");
-import { CellValue } from "app/plugin/GristData";
-import { localeCompare } from "app/common/gutil";
-import { ColumnFilter } from "./ColumnFilter";
 
 const MAXIMUM_SHOWN_FILTER_ITEMS = 500;
 
@@ -11,22 +11,37 @@ export interface IFilterCount {
   count: number;
 }
 
+type ICompare<T> = (a: T, b: T) => number
 
 export class ColumnFilterMenuModel extends Disposable {
 
   public readonly searchValue = Observable.create(this, '');
 
+  public readonly isSortedByCount = Observable.create(this, false);
+
   // computes a set of all keys that matches the search text.
   public readonly filterSet = Computed.create(this, this.searchValue, (_use, searchValue) => {
     const searchRegex = new RegExp(escapeRegExp(searchValue), 'i');
-    return new Set(this._valueCount.filter(([_, {label}]) => searchRegex.test(label)).map(([key]) => key));
+    return new Set(
+      this._valueCount
+        .filter(([_, {count}]) => count)
+        .filter(([_, {label}]) => searchRegex.test(label))
+        .map(([key]) => key)
+    );
   });
 
   // computes the sorted array of all values (ie: pair of key and IFilterCount) that matches the search text.
-  public readonly filteredValues = Computed.create(this, this.filterSet, (_use, filter) => {
+  public readonly filteredValues = Computed.create(
-    return this._valueCount.filter(([key]) => filter.has(key))
+    this, this.filterSet, this.isSortedByCount,
-      .sort((a, b) => localeCompare(a[1].label, b[1].label));
+    (_use, filter, isSortedByCount) => {
-  });
+      const comparator: ICompare<[CellValue, IFilterCount]> = isSortedByCount ?
+        (a, b) => nativeCompare(b[1].count, a[1].count) :
+        (a, b) => localeCompare(a[1].label, b[1].label);
+      return this._valueCount
+        .filter(([key]) => filter.has(key))
+        .sort(comparator);
+    }
+  );
 
   // computes the array of all values that does NOT matches the search text
   public readonly otherValues = Computed.create(this, this.filterSet, (_use, filter) => {
@@ -34,13 +49,15 @@ export class ColumnFilterMenuModel extends Disposable {
   });
 
   // computes the array of keys that matches the search text
-  public readonly filteredKeys = Computed.create(this, this.filteredValues, (_use, values) => (
+  public readonly filteredKeys = Computed.create(this, this.filterSet, (_use, filter) => {
-    values.map(([key]) => key)
+    return this._valueCount
-  ));
+      .filter(([key]) => filter.has(key))
+      .map(([key]) => key);
+  });
 
-  public readonly valuesBeyondLimit = Computed.create(this, this.filteredValues, (_use, values) => (
+  public readonly valuesBeyondLimit = Computed.create(this, this.filteredValues, (_use, filteredValues) => {
-    values.slice(this.limitShown)
+    return filteredValues.slice(this.limitShown);
-  ));
+  });
 
   constructor(public columnFilter: ColumnFilter, private _valueCount: Array<[CellValue, IFilterCount]>,
               public limitShown: number = MAXIMUM_SHOWN_FILTER_ITEMS) {

app/client/models/SectionFilter.ts

@@ -1,17 +1,21 @@
 import {KoArray} from 'app/client/lib/koArray';
+import {ColumnFilter} from 'app/client/models/ColumnFilter';
 import {ViewFieldRec} from 'app/client/models/DocModel';
 import {RowId} from 'app/client/models/rowset';
 import {TableData} from 'app/client/models/TableData';
-import {Computed, Disposable, MutableObsArray, obsArray, Observable} from 'grainjs';
+import {buildColFilter, ColumnFilterFunc} from 'app/common/ColumnFilterFunc';
-import {ColumnFilter} from './ColumnFilter';
+import {buildRowFilter, RowFilterFunc, RowValueFunc } from 'app/common/RowFilterFunc';
-import {buildRowFilter, RowFilterFunc, RowValueFunc } from "app/common/RowFilterFunc";
+import {Computed, Disposable, MutableObsArray, obsArray, Observable, UseCB} from 'grainjs';
-import {buildColFilter} from "app/common/ColumnFilterFunc";
+
+export {ColumnFilterFunc} from 'app/common/ColumnFilterFunc';
 
 interface OpenColumnFilter {
   fieldRef: number;
   colFilter: ColumnFilter;
 }
 
+type ColFilterCB = (field: ViewFieldRec, colFilter: ColumnFilterFunc|null) => ColumnFilterFunc|null;
+
 /**
  * SectionFilter represents a collection of column filters in place for a view section. It is created
  * out of `viewFields` and `tableData`, and provides a Computed `sectionFilterFunc` that users can
@@ -28,31 +32,22 @@ export class SectionFilter extends Disposable {
   private _openFilterOverride: Observable<OpenColumnFilter|null> = Observable.create(this, null);
   private _tempRows: MutableObsArray<RowId> = obsArray();
 
-  constructor(viewFields: ko.Computed<KoArray<ViewFieldRec>>, tableData: TableData) {
+  constructor(public viewFields: ko.Computed<KoArray<ViewFieldRec>>, private _tableData: TableData) {
     super();
 
     const columnFilterFunc = Computed.create(this, this._openFilterOverride, (use, openFilter) => {
-      const fields = use(use(viewFields).getObservable());
+      const openFilterFilterFunc = openFilter && use(openFilter.colFilter.filterFunc);
-      const funcs: Array<RowFilterFunc<RowId> | null> = fields.map(f => {
+      function getFilterFunc(field: ViewFieldRec, colFilter: ColumnFilterFunc|null) {
-        const filterFunc = (openFilter && openFilter.fieldRef === f.getRowId()) ?
+        if (openFilter?.fieldRef === field.getRowId()) {
-          use(openFilter.colFilter.filterFunc) :
+          return openFilterFilterFunc;
-          buildColFilter(use(f.activeFilter), use(f.column).type());
+        }
-
+        return colFilter;
-        const getter = tableData.getRowPropFunc(use(f.colId));
+      }
-
+      return this._buildPlainFilterFunc(getFilterFunc, use);
-        if (!filterFunc || !getter) { return null; }
-
-        return buildRowFilter(getter as RowValueFunc<RowId>, filterFunc);
-      })
-      .filter(f => f !== null); // Filter out columns that don't have a filter
-
-      return (rowId: RowId) => funcs.every(f => Boolean(f && f(rowId)));
     });
 
     this.sectionFilterFunc = Computed.create(this, columnFilterFunc, this._tempRows,
-      (_use, filterFunc, tempRows) => {
+      (_use, filterFunc, tempRows) => this._addRowsToFilter(filterFunc, tempRows));
-      return (rowId: RowId) => tempRows.includes(rowId) || (typeof rowId !== 'number') || filterFunc(rowId);
-    });
 
     // Prune temporary rowIds that are no longer being filtered out.
     this.autoDispose(columnFilterFunc.addListener(f => {
@@ -84,4 +79,39 @@ export class SectionFilter extends Disposable {
   public resetTemporaryRows() {
     this._tempRows.set([]);
   }
+
+  /**
+   * Builds a filter function that combines the filter function of all the fields. You can use
+   * `getFilterFunc(field, colFilter)` to customize the filter func for each field. It calls
+   * `getFilterFunc` right away. Also, all the rows that were added with `addTemporaryRow()` bypass
+   * the filter.
+   */
+  public buildFilterFunc(getFilterFunc: ColFilterCB, use: UseCB) {
+    return this._addRowsToFilter(this._buildPlainFilterFunc(getFilterFunc, use), this._tempRows.get());
+  }
+
+  private _addRowsToFilter(filterFunc: RowFilterFunc<RowId>, rows: RowId[]) {
+    return (rowId: RowId) => rows.includes(rowId) || (typeof rowId !== 'number') || filterFunc(rowId);
+  }
+
+  /**
+   * Internal that helps build a filter function that combines the filter function of all
+   * fields. You can use `getFilterFunc(field, colFilter)` to customize the filter func for each
+   * field. It calls `getFilterFunc` right away
+   */
+  private _buildPlainFilterFunc(getFilterFunc: ColFilterCB, use: UseCB): RowFilterFunc<RowId> {
+    const fields = use(use(this.viewFields).getObservable());
+    const funcs: Array<RowFilterFunc<RowId> | null> = fields.map(f => {
+      const colFilter = buildColFilter(use(f.activeFilter), use(use(f.column).type));
+      const filterFunc = getFilterFunc(f, colFilter);
+
+      const getter = this._tableData.getRowPropFunc(f.colId.peek());
+
+      if (!filterFunc || !getter) { return null; }
+
+      return buildRowFilter(getter as RowValueFunc<RowId>, filterFunc);
+    }).filter(f => f !== null); // Filter out columns that don't have a filter
+
+    return (rowId: RowId) => funcs.every(f => Boolean(f && f(rowId)));
+  }
 }

app/client/ui/ColumnFilterMenu.ts

@@ -7,20 +7,25 @@
 import {allInclusive, ColumnFilter} from 'app/client/models/ColumnFilter';
 import {ColumnFilterMenuModel, IFilterCount} from 'app/client/models/ColumnFilterMenuModel';
 import {ViewFieldRec, ViewSectionRec} from 'app/client/models/DocModel';
-import {FilteredRowSource} from 'app/client/models/rowset';
+import {RowId, RowSource} from 'app/client/models/rowset';
-import {SectionFilter} from 'app/client/models/SectionFilter';
+import {ColumnFilterFunc, SectionFilter} from 'app/client/models/SectionFilter';
 import {TableData} from 'app/client/models/TableData';
 import {basicButton, primaryButton} from 'app/client/ui2018/buttons';
-import {cssCheckboxSquare, cssLabel, cssLabelText} from 'app/client/ui2018/checkbox';
+import {cssCheckboxSquare, cssLabel, cssLabelText, Indeterminate,
+        labeledTriStateSquareCheckbox} from 'app/client/ui2018/checkbox';
 import {colors, vars} from 'app/client/ui2018/cssVars';
 import {icon} from 'app/client/ui2018/icons';
 import {menuCssClass, menuDivider} from 'app/client/ui2018/menus';
 import {CellValue} from 'app/common/DocActions';
-import {Computed, Disposable, dom, DomElementMethod, IDisposableOwner, input, makeTestId, styled} from 'grainjs';
+import {isEquivalentFilter} from "app/common/FilterState";
+import {Computed, dom, DomElementMethod, IDisposableOwner, input, makeTestId, 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 {IOpenController, IPopupOptions, setPopupToCreateDom} from 'popweasel';
-import {isEquivalentFilter} from "app/common/FilterState";
 import {decodeObject} from 'app/plugin/objtypes';
 import {isList} from 'app/common/gristTypes';
 
@@ -49,7 +54,7 @@ export function columnFilterMenu(owner: IDisposableOwner, opts: IFilterMenuOptio
     }
   });
 
-  const {searchValue: searchValueObs, filteredValues, filteredKeys} = model;
+  const {searchValue: searchValueObs, filteredValues, filteredKeys, isSortedByCount} = model;
 
   const isAboveLimitObs = Computed.create(owner, (use) => use(model.valuesBeyondLimit).length > 0);
   const isSearchingObs = Computed.create(owner, (use) => Boolean(use(searchValueObs)));
@@ -124,7 +129,12 @@ export function columnFilterMenu(owner: IDisposableOwner, opts: IFilterMenuOptio
             testId('bulk-action'),
           )
         ];
-      })
+      }),
+      cssSortIcon(
+        'Sort',
+        cssSortIcon.cls('-active', isSortedByCount),
+        dom.on('click', () => isSortedByCount.set(!isSortedByCount.get())),
+      )
     ),
     cssItemList(
       testId('list'),
@@ -146,19 +156,22 @@ export function columnFilterMenu(owner: IDisposableOwner, opts: IFilterMenuOptio
       dom.domComputed((use) => {
         const isAboveLimit = use(isAboveLimitObs);
         const searchValue = use(isSearchingObs);
+        const otherValues = use(model.otherValues);
+        const anyOtherValues = Boolean(otherValues.length);
+        const valuesBeyondLimit = use(model.valuesBeyondLimit);
         if (isAboveLimit) {
           return searchValue ? [
-            buildSummary('Other Matching', BeyondLimit, model),
+            buildSummary('Other Matching', valuesBeyondLimit, false, model),
-            buildSummary('Other Non-Matching', OtherValues, model),
+            buildSummary('Other Non-Matching', otherValues, true, model),
           ] : [
-            buildSummary('Other Values', BeyondLimit, model),
+            buildSummary('Other Values', concat(otherValues, valuesBeyondLimit), false, model),
-            buildSummary('Future Values', OtherValues, model)
+            buildSummary('Future Values', [], true, model),
           ];
         } else {
-          return searchValue ? [
+          return anyOtherValues ? [
-            buildSummary('Others', OtherValues, model)
+            buildSummary('Others', otherValues, true, model)
           ] : [
-            buildSummary('Future Values', OtherValues, model)
+            buildSummary('Future Values', [], true, model)
           ];
         }
       }),
@@ -173,95 +186,93 @@ export function columnFilterMenu(owner: IDisposableOwner, opts: IFilterMenuOptio
   return filterMenu;
 }
 
-// Describes the model for one summary checkbox.
-interface SummaryModel extends Disposable {
 
-  // Whether checkbox is checked
+/**
-  isChecked: Computed<boolean> ;
+ * Builds a tri-state checkbox that summaries the state of all the `values`. The special value
+ * `Future Values` which turns the filter into an inclusion filter or exclusion filter, can be
+ * added to the summary using `switchFilterType`. Uses `label` as label and also expects
+ * `model` as the column filter menu model.
+ *
+ * The checkbox appears checked if all values of the summary are included, unchecked if none, and in
+ * the indeterminate state if values are in mixed state.
+ *
+ * On user clicks, if checkbox is checked, it does uncheck all the values, and if the
+ * `switchFilterType` is true it also converts the filter into an inclusion filter. But if the
+ * checkbox is unchecked, or in the Indeterminate state, it does check all the values, and if the
+ * `switchFilterType` is true it also converts the filter into an exlusion filter.
+ */
+function buildSummary(label: string|Computed<string>, values: Array<[CellValue, IFilterCount]>,
+                      switchFilterType: boolean, model: ColumnFilterMenuModel) {
+  const columnFilter = model.columnFilter;
+  const checkboxState = Computed.create(
+    null, columnFilter.isInclusionFilter, columnFilter.filterFunc,
+    (_use, isInclusionFilter) => {
 
-  // Callback for when the checkbox is changed.
+      // let's gather all sub options.
-  callback: (checked: boolean) => void;
+      const subOptions = values.map((val) => ({getState: () => columnFilter.includes(val[0])}));
+      if (switchFilterType) {
+        subOptions.push({getState: () => !isInclusionFilter});
+      }
 
-  // The count.
+      // At this point if sub options is still empty let's just return false (unchecked).
-  count: Computed<string>;
+      if (!subOptions.length) { return false; }
-}
 
-// Ctor that construct a SummaryModel.
+      // let's compare the state for first sub options against all the others. If there is one
-type SummaryModelCreator = new(columnFilter: ColumnFilterMenuModel) => SummaryModel;
+      // different, then state should be `Indeterminate`, otherwise, the state will the the same as
+      // the one of the first sub option.
+      const first = subOptions[0].getState();
+      if (some(tail(subOptions), (val) => val.getState() !== first)) { return Indeterminate; }
+      return first;
 
-// Summaries all the values that are in `columnFilter.valuesBeyondLimit`, ie: it includes a count
+    }).onWrite((val) => {
-// for all the values and clicking the checkbox successively add/delete these values from the
-// `columnFilter`.
-class BeyondLimit extends Disposable implements SummaryModel {
 
-  public columnFilter = this.model.columnFilter;
+      if (switchFilterType) {
 
-  public isChecked = Computed.create(this, (use) => (
+        // Note that if `includeFutureValues` is true, we only needs to toggle the filter type
-    !use(this.model.valuesBeyondLimit).find(([key, _val]) => !this.columnFilter.includes(key))
+        // between exclusive and inclusive. Doing this will automatically excludes/includes all
-  ));
+        // other values, so no need for extra steps.
+        const state = val ?
+          {excluded: model.filteredKeys.get().filter((key) => !columnFilter.includes(key))} :
+          {included: model.filteredKeys.get().filter((key) => columnFilter.includes(key))};
+        columnFilter.setState(state);
 
-  public count = Computed.create(this, (use) => getCount(use(this.model.valuesBeyondLimit)).toLocaleString());
+      } else {
 
-  constructor(public model: ColumnFilterMenuModel) { super(); }
+        const keys = values.map(([key]) => key);
+        if (val) {
+          columnFilter.addMany(keys);
+        } else {
+          columnFilter.deleteMany(keys);
+        }
+      }
+    });
 
-  public callback(checked: boolean) {
-    const keys = this.model.valuesBeyondLimit.get().map(([key, _val]) => key);
-    if (checked) {
-      this.columnFilter.addMany(keys);
-    } else {
-      this.columnFilter.deleteMany(keys);
-    }
-  }
-}
-
-// Summaries the values that are not in columnFilter.filteredValues, it includes both the values in
-// `columnFilter.otherValues` (ie: the values that are filtered out if user is using the search) and
-// the future values. The checkbox successively turns columnFilter into an inclusion/exclusion
-// filter. The count is hidden if `columnFilter.otherValues` is empty (ie: no search is peformed =>
-// only checkbox only toggles future values)
-class OtherValues extends Disposable implements SummaryModel {
-  public columnFilter = this.model.columnFilter;
-  public isChecked = Computed.create(this, (use) => !use(this.columnFilter.isInclusionFilter));
-
-  public count = Computed.create(this, (use) => {
-    const c = getCount(use(this.model.otherValues));
-    return c ? c.toLocaleString() : '';
-  });
-
-  constructor(public model: ColumnFilterMenuModel) { super(); }
-
-  public callback(checked: boolean) {
-    const columnFilter = this.columnFilter;
-    const filteredKeys = this.model.filteredKeys;
-    const state = checked ?
-      {excluded: filteredKeys.get().filter((key) => !columnFilter.includes(key))} :
-      {included: filteredKeys.get().filter((key) => columnFilter.includes(key))};
-    return columnFilter.setState(state);
-  }
-}
-
-function buildSummary(label: string, SummaryModelCtor: SummaryModelCreator, model: ColumnFilterMenuModel) {
-  const summaryModel = new SummaryModelCtor(model);
   return cssMenuItem(
-    dom.autoDispose(summaryModel),
+    dom.autoDispose(checkboxState),
     testId('summary'),
-    cssLabel(
+    labeledTriStateSquareCheckbox(
-      cssCheckboxSquare(
+      checkboxState,
-        {type: 'checkbox'},
+      `${label} ${formatUniqueCount(values)}`.trim()
-        dom.on('change', (_ev, elem) => summaryModel.callback(elem.checked)),
-        dom.prop('checked', summaryModel.isChecked)
-      ),
-      cssItemValue(label),
     ),
-    summaryModel.count !== undefined ? cssItemCount(dom.text(summaryModel.count), testId('count')) : null,
+    cssItemCount(formatCount(values), testId('count')),
   );
 }
 
+function formatCount(values: Array<[CellValue, IFilterCount]>) {
+  const count = getCount(values);
+  return count ? count.toLocaleString() : '';
+}
+
+function formatUniqueCount(values: Array<[CellValue, IFilterCount]>) {
+  const count = values.length;
+  return count ? '(' + count.toLocaleString() + ')' : '';
+}
+
 /**
  * Returns content for the newly created columnFilterMenu; for use with setPopupToCreateDom().
  */
 export function createFilterMenu(openCtl: IOpenController, sectionFilter: SectionFilter, field: ViewFieldRec,
-                                 rowSource: FilteredRowSource, tableData: TableData, onClose: () => void = noop) {
+                                 rowSource: RowSource, tableData: TableData, onClose: () => void = noop) {
   // Go through all of our shown and hidden rows, and count them up by the values in this column.
   const keyMapFunc = tableData.getRowPropFunc(field.column().colId())!;
   const labelGetter = tableData.getRowPropFunc(field.displayColModel().colId())!;
@@ -270,17 +281,24 @@ export function createFilterMenu(openCtl: IOpenController, sectionFilter: Sectio
   const activeFilterBar = field.viewSection.peek().activeFilterBar;
   const columnType = field.column().type.peek();
 
-  const valueCounts: Map<CellValue, {label: string, count: number}> = new Map();
+  function getFilterFunc(f: ViewFieldRec, colFilter: ColumnFilterFunc|null) {
-  // TODO: as of now, this is not working for non text-or-numeric columns, ie: for Date column it is
+    return f.getRowId() === field.getRowId() ? null : colFilter;
-  // not possible to search for anything. Likely caused by the key being something completely
+  }
-  // different than the label.
+  const filterFunc = Computed.create(null, use => sectionFilter.buildFilterFunc(getFilterFunc, use));
-  addCountsToMap(valueCounts, rowSource.getAllRows() as Iterable<number>, { keyMapFunc, labelMapFunc, columnType });
+  openCtl.autoDispose(filterFunc);
-  addCountsToMap(valueCounts, rowSource.getHiddenRows() as Iterable<number>, { keyMapFunc, labelMapFunc, columnType });
 
   const columnFilter = ColumnFilter.create(openCtl, field.activeFilter.peek(), columnType);
-  const model = ColumnFilterMenuModel.create(openCtl, columnFilter, Array.from(valueCounts));
   sectionFilter.setFilterOverride(field.getRowId(), columnFilter); // Will be removed on menu disposal
 
+  const [allRows, hiddenRows] = partition(Array.from(rowSource.getAllRows()), filterFunc.get());
+  const valueCounts: Map<CellValue, {label: string, count: number}> = new Map();
+  addCountsToMap(valueCounts, allRows, {keyMapFunc, labelMapFunc, columnType});
+  addCountsToMap(valueCounts, hiddenRows, {keyMapFunc, labelMapFunc, columnType,
+                                                               areHiddenRows: true});
+
+  const model = ColumnFilterMenuModel.create(openCtl, columnFilter, Array.from(valueCounts));
+
+
   return columnFilterMenu(openCtl, {
     model,
     valueCounts,
@@ -300,6 +318,7 @@ interface ICountOptions {
   keyMapFunc?: (v: any) => any;
   labelMapFunc?: (v: any) => any;
   columnType?: string;
+  areHiddenRows?: boolean;
 }
 
 /**
@@ -309,39 +328,38 @@ interface ICountOptions {
  * The optional column type controls how complex cell values are decomposed into keys (e.g. Choice Lists have
  * the possible choices as keys).
  */
-function addCountsToMap(valueMap: Map<CellValue, IFilterCount>, rowIds: Iterable<Number>,
+function addCountsToMap(valueMap: Map<CellValue, IFilterCount>, rowIds: RowId[],
-                        { keyMapFunc = identity, labelMapFunc = identity, columnType }: ICountOptions) {
+                        { keyMapFunc = identity, labelMapFunc = identity, columnType,
+                          areHiddenRows = false }: ICountOptions) {
 
   for (const rowId of rowIds) {
     let key = keyMapFunc(rowId);
 
     // If row contains a list and the column is a Choice List, treat each choice as a separate key
     if (isList(key) && columnType === 'ChoiceList') {
-      const list = decodeObject(key);
+      const list = decodeObject(key) as unknown[];
-      addListCountsToMap(valueMap, list as unknown[]);
+      for (const item of list) {
+        addSingleCountToMap(valueMap, item, () => item, areHiddenRows);
+      }
       continue;
     }
-
     // For complex values, serialize the value to allow them to be properly stored
     if (Array.isArray(key)) { key = JSON.stringify(key); }
-    if (valueMap.get(key)) {
+    addSingleCountToMap(valueMap, key, () => labelMapFunc(rowId), areHiddenRows);
-      valueMap.get(key)!.count++;
-    } else {
-      valueMap.set(key, { label: labelMapFunc(rowId), count: 1 });
-    }
   }
 }
 
 /**
- * Adds each item in `list` to `valueMap`.
+ * Adds the `value` to `valueMap` using `labelGetter` to get the label and increments `count` unless
+ * isHiddenRow is true.
  */
-function addListCountsToMap(valueMap: Map<CellValue, IFilterCount>, list: any[]) {
+function addSingleCountToMap(valueMap: Map<CellValue, IFilterCount>, value: any, labelGetter: () => any,
-  for (const item of list) {
+                       isHiddenRow: boolean) {
-    if (valueMap.get(item)) {
+  if (!valueMap.has(value)) {
-      valueMap.get(item)!.count++;
+    valueMap.set(value, { label: labelGetter(), count: 0 });
-    } else {
+  }
-      valueMap.set(item, { label: item, count: 1 });
+  if (!isHiddenRow) {
-    }
+    valueMap.get(value)!.count++;
   }
 }
 
@@ -404,6 +422,7 @@ const cssSelectAll = styled('div', `
 const cssDotSeparator = styled('span', `
   color: ${colors.lightGreen};
   margin: 0 4px;
+  user-select: none;
 `);
 const cssMenuDivider = styled(menuDivider, `
   flex-shrink: 0;
@@ -464,3 +483,10 @@ const cssNoResults = styled(cssMenuItem, `
   color: ${colors.slate};
   justify-content: center;
 `);
+const cssSortIcon = styled(icon, `
+  --icon-color: ${colors.slate};
+  margin-left: auto;
+  &-active {
+    --icon-color: ${colors.lightGreen}
+  }
+`);