fix: Fix OneClick links not filtering plots (#1217)

- Update `ChartPanel` to get the filter value from `filterList` - Hide operator selection for filter source and chart links - Don't allow linking multiple columns from the same table to a chart target - Fix issue with link in progress not being reset after creating a new link Fixes #1198
deephaven · Apr 17, 2023 · 9b20f9e · 9b20f9e
1 parent 8c7207c
commit 9b20f9e
Show file tree

Hide file tree

Showing 8 changed files with 113 additions and 65 deletions.
diff --git a/packages/chart/src/ChartModel.ts b/packages/chart/src/ChartModel.ts
@@ -3,14 +3,7 @@
 
 import { Formatter } from '@deephaven/jsapi-utils';
 import { Layout, PlotData } from 'plotly.js';
-
-export type FilterColumnMap = Map<
-  string,
-  {
-    name: string;
-    type: string;
-  }
->;
+import { FilterColumnMap, FilterMap } from './ChartUtils';
 
 export type ChartEvent = CustomEvent;
 /**
@@ -71,7 +64,7 @@ class ChartModel {
   }
 
   // eslint-disable-next-line @typescript-eslint/no-empty-function
-  setFilter(filter: Map<string, string>): void {}
+  setFilter(filter: FilterMap): void {}
 
   /**
    * Close this model, clean up any underlying subscriptions

diff --git a/packages/chart/src/ChartUtils.ts b/packages/chart/src/ChartUtils.ts
@@ -36,6 +36,16 @@ import {
 import { assertNotNull, Range } from '@deephaven/utils';
 import ChartTheme from './ChartTheme';
 
+export type FilterColumnMap = Map<
+  string,
+  {
+    name: string;
+    type: string;
+  }
+>;
+
+export type FilterMap = Map<string, unknown>;
+
 export interface ChartModelSettings {
   hiddenSeries?: string[];
   type?: keyof SeriesPlotStyle;

diff --git a/packages/chart/src/FigureChartModel.ts b/packages/chart/src/FigureChartModel.ts
@@ -17,8 +17,13 @@ import type {
   DateTimeColumnFormatter,
   Formatter,
 } from '@deephaven/jsapi-utils';
-import ChartModel, { ChartEvent, FilterColumnMap } from './ChartModel';
-import ChartUtils, { AxisTypeMap, ChartModelSettings } from './ChartUtils';
+import ChartModel, { ChartEvent } from './ChartModel';
+import ChartUtils, {
+  AxisTypeMap,
+  ChartModelSettings,
+  FilterColumnMap,
+  FilterMap,
+} from './ChartUtils';
 import ChartTheme from './ChartTheme';
 
 const log = Log.module('FigureChartModel');
@@ -103,7 +108,7 @@ class FigureChartModel extends ChartModel {
 
   filterColumnMap: FilterColumnMap;
 
-  lastFilter: Map<string, string>;
+  lastFilter: FilterMap;
 
   isConnected: boolean; // Assume figure is connected to start
 
@@ -703,7 +708,7 @@ class FigureChartModel extends ChartModel {
    * Sets the filter on the model. Will only set the values that have changed.
    * @param filterMap Map of filter column names to values
    */
-  setFilter(filterMap: Map<string, string>): void {
+  setFilter(filterMap: FilterMap): void {
     if (this.oneClicks.length === 0) {
       log.warn('Trying to set a filter, but no one click!');
       return;

diff --git a/packages/dashboard-core-plugins/src/linker/Linker.tsx b/packages/dashboard-core-plugins/src/linker/Linker.tsx
@@ -353,6 +353,21 @@ export class Linker extends Component<LinkerProps, LinkerState> {
           this.deleteLinks(linksToDelete);
           break;
         }
+        case 'chartLink': {
+          const existingLinkEnd = isReversed === true ? start : end;
+          const existingLinkStart = isReversed === true ? end : start;
+          log.debug('creating chartlink', { existingLinkEnd, start, end });
+          // Don't allow linking more than one column per source to each chart column
+          const linksToDelete = links.filter(
+            ({ end: panelLinkEnd, start: panelLinkStart }) =>
+              panelLinkStart?.panelId === existingLinkStart.panelId &&
+              panelLinkEnd?.panelId === existingLinkEnd.panelId &&
+              panelLinkEnd?.columnName === existingLinkEnd.columnName &&
+              panelLinkEnd?.columnType === existingLinkEnd.columnType
+          );
+          this.deleteLinks(linksToDelete);
+          break;
+        }
         case 'tableLink':
           // No-op
           break;
@@ -642,15 +657,17 @@ export class Linker extends Component<LinkerProps, LinkerState> {
     }
   }
 
-  updateLinkInProgressType(
-    linkInProgress: Link,
-    type: LinkType = 'invalid'
-  ): void {
-    this.setState({
-      linkInProgress: {
-        ...linkInProgress,
-        type,
-      },
+  updateLinkInProgressType(type: LinkType = 'invalid'): void {
+    this.setState(({ linkInProgress }) => {
+      if (linkInProgress !== undefined) {
+        return {
+          linkInProgress: {
+            ...linkInProgress,
+            type,
+          },
+        };
+      }
+      return null;
     });
   }
 
@@ -664,7 +681,7 @@ export class Linker extends Component<LinkerProps, LinkerState> {
     if (tableColumn == null) {
       if (linkInProgress?.start != null) {
         // Link started, end point is not a valid target
-        this.updateLinkInProgressType(linkInProgress);
+        this.updateLinkInProgressType();
       }
       return false;
     }
@@ -673,7 +690,7 @@ export class Linker extends Component<LinkerProps, LinkerState> {
     if (!isLinkableColumn(tableColumn)) {
       log.debug2('Column is not filterable', tableColumn.description);
       if (linkInProgress?.start != null) {
-        this.updateLinkInProgressType(linkInProgress, 'invalid');
+        this.updateLinkInProgressType('invalid');
       }
       return false;
     }
@@ -701,7 +718,7 @@ export class Linker extends Component<LinkerProps, LinkerState> {
         ? LinkerUtils.getLinkType(end, start, isolatedLinkerPanelId)
         : LinkerUtils.getLinkType(start, end, isolatedLinkerPanelId);
 
-    this.updateLinkInProgressType(linkInProgress, type);
+    this.updateLinkInProgressType(type);
 
     return type !== 'invalid';
   }

diff --git a/packages/dashboard-core-plugins/src/linker/LinkerLink.tsx b/packages/dashboard-core-plugins/src/linker/LinkerLink.tsx
@@ -14,6 +14,7 @@ import {
 import Log from '@deephaven/log';
 import { TableUtils } from '@deephaven/jsapi-utils';
 import './LinkerLink.scss';
+import { LinkType } from './LinkerUtils';
 
 const log = Log.module('LinkerLink');
 
@@ -37,6 +38,7 @@ export type LinkerLinkProps = {
   y2: number;
   id: string;
   className: string;
+  type: LinkType;
   operator: FilterTypeValue;
   isSelected: boolean;
   startColumnType: string | null;
@@ -202,6 +204,7 @@ export class LinkerLink extends Component<LinkerLinkProps, LinkerLinkState> {
       y2,
       id,
       startColumnType,
+      type,
     } = this.props;
     const { isHovering } = this.state;
 
@@ -297,6 +300,8 @@ export class LinkerLink extends Component<LinkerLinkProps, LinkerLinkState> {
       }
     }
 
+    const showOperator = type !== 'chartLink' && type !== 'filterSource';
+
     return (
       <>
         <svg
@@ -323,32 +328,34 @@ export class LinkerLink extends Component<LinkerLinkProps, LinkerLinkState> {
         </svg>
         {startColumnType != null && isSelected && (
           <>
-            <Button
-              kind="primary"
-              className="btn-fab btn-operator"
-              style={{
-                top: midY + (slopeAtMid >= 0 ? topOffsetY : bottomOffsetY),
-                left: midX + (slopeAtMid >= 0 ? topOffsetX : bottomOffsetX),
-              }}
-              onClick={() => {
-                // no-op: click is handled in `DropdownMenu'
-              }}
-              icon={
-                <div className="fa-md fa-layers">
-                  <b>{symbol}</b>
-                  <FontAwesomeIcon
-                    icon={vsTriangleDown}
-                    transform="right-8 down-9 shrink-5"
-                  />
-                </div>
-              }
-              tooltip="Change comparison operator"
-            >
-              <DropdownMenu
-                actions={this.getDropdownActions}
-                popperOptions={{ placement: 'bottom-start' }}
-              />
-            </Button>
+            {showOperator && (
+              <Button
+                kind="primary"
+                className="btn-fab btn-operator"
+                style={{
+                  top: midY + (slopeAtMid >= 0 ? topOffsetY : bottomOffsetY),
+                  left: midX + (slopeAtMid >= 0 ? topOffsetX : bottomOffsetX),
+                }}
+                onClick={() => {
+                  // no-op: click is handled in `DropdownMenu'
+                }}
+                icon={
+                  <div className="fa-md fa-layers">
+                    <b>{symbol}</b>
+                    <FontAwesomeIcon
+                      icon={vsTriangleDown}
+                      transform="right-8 down-9 shrink-5"
+                    />
+                  </div>
+                }
+                tooltip="Change comparison operator"
+              >
+                <DropdownMenu
+                  actions={this.getDropdownActions}
+                  popperOptions={{ placement: 'bottom-start' }}
+                />
+              </Button>
+            )}
             <Button
               kind="primary"
               className="btn-fab btn-delete"

diff --git a/packages/dashboard-core-plugins/src/linker/LinkerOverlayContent.tsx b/packages/dashboard-core-plugins/src/linker/LinkerOverlayContent.tsx
@@ -17,6 +17,7 @@ import {
   Link,
   LinkerCoordinate,
   LinkPoint,
+  LinkType,
 } from './LinkerUtils';
 import LinkerLink from './LinkerLink';
 import './LinkerOverlayContent.scss';
@@ -30,6 +31,7 @@ export type VisibleLink = {
   y2: number;
   id: string;
   className: string;
+  type: LinkType;
   operator: FilterTypeValue;
   startColumnType: string | null;
 };
@@ -290,6 +292,7 @@ export class LinkerOverlayContent extends Component<
             className,
             operator,
             startColumnType,
+            type,
           };
         } catch (error) {
           log.warn('Unable to get point for link', link, error);
@@ -305,7 +308,17 @@ export class LinkerOverlayContent extends Component<
         })}
       >
         {visibleLinks.map(
-          ({ x1, y1, x2, y2, id, className, operator, startColumnType }) => (
+          ({
+            x1,
+            y1,
+            x2,
+            y2,
+            id,
+            className,
+            operator,
+            startColumnType,
+            type,
+          }) => (
             <LinkerLink
               className={className}
               id={id}
@@ -319,6 +332,7 @@ export class LinkerOverlayContent extends Component<
               isSelected={selectedIds.has(id)}
               operator={operator}
               startColumnType={startColumnType}
+              type={type}
               onOperatorChanged={this.handleOperatorChanged}
             />
           )

diff --git a/packages/dashboard-core-plugins/src/linker/LinkerUtils.ts b/packages/dashboard-core-plugins/src/linker/LinkerUtils.ts
@@ -5,7 +5,7 @@ import { TypeValue as FilterTypeValue } from '@deephaven/filters';
 import Log from '@deephaven/log';
 import { ChartPanel, IrisGridPanel, DropdownFilterPanel } from '../panels';
 
-export type LinkType = 'invalid' | 'filterSource' | 'tableLink';
+export type LinkType = 'invalid' | 'filterSource' | 'tableLink' | 'chartLink';
 
 export type LinkPoint = {
   panelId: string | string[];
@@ -154,6 +154,7 @@ class LinkerUtils {
     // If all checks pass, link type is determined by the target panel component
     switch (end.panelComponent) {
       case LayoutUtils.getComponentName(ChartPanel):
+        return 'chartLink';
       case LayoutUtils.getComponentName(IrisGridPanel):
         return 'tableLink';
       case LayoutUtils.getComponentName(DropdownFilterPanel):

diff --git a/packages/dashboard-core-plugins/src/panels/ChartPanel.tsx b/packages/dashboard-core-plugins/src/panels/ChartPanel.tsx
@@ -9,6 +9,7 @@ import {
   ChartModel,
   ChartModelSettings,
   ChartUtils,
+  FilterMap,
   isFigureChartModel,
 } from '@deephaven/chart';
 import {
@@ -63,7 +64,7 @@ import ChartColumnSelectorOverlay, {
   SelectorColumn,
 } from './ChartColumnSelectorOverlay';
 import './ChartPanel.scss';
-import { Link } from '../linker/LinkerUtils';
+import { Link, LinkFilterMap } from '../linker/LinkerUtils';
 import { PanelState as IrisGridPanelState } from './IrisGridPanel';
 import { isChartPanelTableMetadata } from './ChartPanelUtils';
 import { ColumnSelectionValidator } from '../linker/ColumnSelectionValidator';
@@ -73,8 +74,6 @@ const UPDATE_MODEL_DEBOUNCE = 150;
 
 export type InputFilterMap = Map<string, InputFilter>;
 
-export type FilterMap = Map<string, string>;
-
 export type LinkedColumnMap = Map<string, { name: string; type: string }>;
 
 export type ChartPanelFigureMetadata = {
@@ -109,7 +108,7 @@ export interface ChartPanelTableSettings {
   partitionColumn?: string;
 }
 export interface GLChartPanelState {
-  filterValueMap: [string, string][];
+  filterValueMap: [string, unknown][];
   settings: Partial<ChartModelSettings>;
   tableSettings: ChartPanelTableSettings;
   irisGridState?: {
@@ -159,10 +158,10 @@ interface ChartPanelState {
 
   // Map of all non-empty filters applied to the chart.
   // Initialize the filter map to the previously stored values; input filters will be applied after load.
-  filterMap: Map<string, string>;
+  filterMap: FilterMap;
   // Map of filter values set from links, stored in panelState.
   // Combined with inputFilters to get applied filters (filterMap).
-  filterValueMap: Map<string, string>;
+  filterValueMap: FilterMap;
   model?: ChartModel;
   columnMap: ColumnMap;
 
@@ -786,20 +785,22 @@ export class ChartPanel extends Component<ChartPanelProps, ChartPanelState> {
    * Set chart filters based on the filter map
    * @param filterMapParam Filter map
    */
-  setFilterMap(
-    filterMapParam: Map<string, { columnType: string; value: string }>
-  ): void {
+  setFilterMap(filterMapParam: LinkFilterMap): void {
     log.debug('setFilterMap', filterMapParam);
     this.setState(state => {
       const { columnMap, filterMap } = state;
-      let updatedFilterMap: null | Map<string, string> = null;
+      let updatedFilterMap: null | FilterMap = null;
       const filterValueMap = new Map(state.filterValueMap);
-
-      filterMapParam.forEach(({ columnType, value }, columnName) => {
+      filterMapParam.forEach(({ columnType, filterList }, columnName) => {
         const column = columnMap.get(columnName);
         if (column == null || column.type !== columnType) {
           return;
         }
+        if (filterList.length < 1) {
+          log.debug('Ignoring empty filterList for column', columnName);
+          return;
+        }
+        const { value } = filterList[0];
         filterValueMap.set(columnName, value);
         if (filterMap.get(columnName) !== value) {
           if (updatedFilterMap === null) {