From 9b20f9e8f3912959e32ae8d8d597ee584357ad70 Mon Sep 17 00:00:00 2001 From: Vlad Babich Date: Mon, 17 Apr 2023 11:20:24 -0600 Subject: [PATCH] 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 --- packages/chart/src/ChartModel.ts | 11 +--- packages/chart/src/ChartUtils.ts | 10 ++++ packages/chart/src/FigureChartModel.ts | 13 ++-- .../src/linker/Linker.tsx | 41 +++++++++---- .../src/linker/LinkerLink.tsx | 59 +++++++++++-------- .../src/linker/LinkerOverlayContent.tsx | 16 ++++- .../src/linker/LinkerUtils.ts | 3 +- .../src/panels/ChartPanel.tsx | 25 ++++---- 8 files changed, 113 insertions(+), 65 deletions(-) diff --git a/packages/chart/src/ChartModel.ts b/packages/chart/src/ChartModel.ts index 1fd279f2c1..a53b30f4a1 100644 --- 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): 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 index 00a178945a..b7f2898b5b 100644 --- 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; + export interface ChartModelSettings { hiddenSeries?: string[]; type?: keyof SeriesPlotStyle; diff --git a/packages/chart/src/FigureChartModel.ts b/packages/chart/src/FigureChartModel.ts index 8490b4a82c..484c49c86f 100644 --- 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; + 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): 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 index 32a70b8542..f963e0c2f9 100644 --- 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 { 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 { } } - 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 { 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 { 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 { ? 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 index 6524271e6c..39ccd4ab82 100644 --- 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 { y2, id, startColumnType, + type, } = this.props; const { isHovering } = this.state; @@ -297,6 +300,8 @@ export class LinkerLink extends Component { } } + const showOperator = type !== 'chartLink' && type !== 'filterSource'; + return ( <> { {startColumnType != null && isSelected && ( <> - + {showOperator && ( + + )}