Skip to content

Commit 9b20f9e

Browse files
authored
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
1 parent 8c7207c commit 9b20f9e

8 files changed

Lines changed: 113 additions & 65 deletions

File tree

packages/chart/src/ChartModel.ts

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,7 @@
33

44
import { Formatter } from '@deephaven/jsapi-utils';
55
import { Layout, PlotData } from 'plotly.js';
6-
7-
export type FilterColumnMap = Map<
8-
string,
9-
{
10-
name: string;
11-
type: string;
12-
}
13-
>;
6+
import { FilterColumnMap, FilterMap } from './ChartUtils';
147

158
export type ChartEvent = CustomEvent;
169
/**
@@ -71,7 +64,7 @@ class ChartModel {
7164
}
7265

7366
// eslint-disable-next-line @typescript-eslint/no-empty-function
74-
setFilter(filter: Map<string, string>): void {}
67+
setFilter(filter: FilterMap): void {}
7568

7669
/**
7770
* Close this model, clean up any underlying subscriptions

packages/chart/src/ChartUtils.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,16 @@ import {
3636
import { assertNotNull, Range } from '@deephaven/utils';
3737
import ChartTheme from './ChartTheme';
3838

39+
export type FilterColumnMap = Map<
40+
string,
41+
{
42+
name: string;
43+
type: string;
44+
}
45+
>;
46+
47+
export type FilterMap = Map<string, unknown>;
48+
3949
export interface ChartModelSettings {
4050
hiddenSeries?: string[];
4151
type?: keyof SeriesPlotStyle;

packages/chart/src/FigureChartModel.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,13 @@ import type {
1717
DateTimeColumnFormatter,
1818
Formatter,
1919
} from '@deephaven/jsapi-utils';
20-
import ChartModel, { ChartEvent, FilterColumnMap } from './ChartModel';
21-
import ChartUtils, { AxisTypeMap, ChartModelSettings } from './ChartUtils';
20+
import ChartModel, { ChartEvent } from './ChartModel';
21+
import ChartUtils, {
22+
AxisTypeMap,
23+
ChartModelSettings,
24+
FilterColumnMap,
25+
FilterMap,
26+
} from './ChartUtils';
2227
import ChartTheme from './ChartTheme';
2328

2429
const log = Log.module('FigureChartModel');
@@ -103,7 +108,7 @@ class FigureChartModel extends ChartModel {
103108

104109
filterColumnMap: FilterColumnMap;
105110

106-
lastFilter: Map<string, string>;
111+
lastFilter: FilterMap;
107112

108113
isConnected: boolean; // Assume figure is connected to start
109114

@@ -703,7 +708,7 @@ class FigureChartModel extends ChartModel {
703708
* Sets the filter on the model. Will only set the values that have changed.
704709
* @param filterMap Map of filter column names to values
705710
*/
706-
setFilter(filterMap: Map<string, string>): void {
711+
setFilter(filterMap: FilterMap): void {
707712
if (this.oneClicks.length === 0) {
708713
log.warn('Trying to set a filter, but no one click!');
709714
return;

packages/dashboard-core-plugins/src/linker/Linker.tsx

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -353,6 +353,21 @@ export class Linker extends Component<LinkerProps, LinkerState> {
353353
this.deleteLinks(linksToDelete);
354354
break;
355355
}
356+
case 'chartLink': {
357+
const existingLinkEnd = isReversed === true ? start : end;
358+
const existingLinkStart = isReversed === true ? end : start;
359+
log.debug('creating chartlink', { existingLinkEnd, start, end });
360+
// Don't allow linking more than one column per source to each chart column
361+
const linksToDelete = links.filter(
362+
({ end: panelLinkEnd, start: panelLinkStart }) =>
363+
panelLinkStart?.panelId === existingLinkStart.panelId &&
364+
panelLinkEnd?.panelId === existingLinkEnd.panelId &&
365+
panelLinkEnd?.columnName === existingLinkEnd.columnName &&
366+
panelLinkEnd?.columnType === existingLinkEnd.columnType
367+
);
368+
this.deleteLinks(linksToDelete);
369+
break;
370+
}
356371
case 'tableLink':
357372
// No-op
358373
break;
@@ -642,15 +657,17 @@ export class Linker extends Component<LinkerProps, LinkerState> {
642657
}
643658
}
644659

645-
updateLinkInProgressType(
646-
linkInProgress: Link,
647-
type: LinkType = 'invalid'
648-
): void {
649-
this.setState({
650-
linkInProgress: {
651-
...linkInProgress,
652-
type,
653-
},
660+
updateLinkInProgressType(type: LinkType = 'invalid'): void {
661+
this.setState(({ linkInProgress }) => {
662+
if (linkInProgress !== undefined) {
663+
return {
664+
linkInProgress: {
665+
...linkInProgress,
666+
type,
667+
},
668+
};
669+
}
670+
return null;
654671
});
655672
}
656673

@@ -664,7 +681,7 @@ export class Linker extends Component<LinkerProps, LinkerState> {
664681
if (tableColumn == null) {
665682
if (linkInProgress?.start != null) {
666683
// Link started, end point is not a valid target
667-
this.updateLinkInProgressType(linkInProgress);
684+
this.updateLinkInProgressType();
668685
}
669686
return false;
670687
}
@@ -673,7 +690,7 @@ export class Linker extends Component<LinkerProps, LinkerState> {
673690
if (!isLinkableColumn(tableColumn)) {
674691
log.debug2('Column is not filterable', tableColumn.description);
675692
if (linkInProgress?.start != null) {
676-
this.updateLinkInProgressType(linkInProgress, 'invalid');
693+
this.updateLinkInProgressType('invalid');
677694
}
678695
return false;
679696
}
@@ -701,7 +718,7 @@ export class Linker extends Component<LinkerProps, LinkerState> {
701718
? LinkerUtils.getLinkType(end, start, isolatedLinkerPanelId)
702719
: LinkerUtils.getLinkType(start, end, isolatedLinkerPanelId);
703720

704-
this.updateLinkInProgressType(linkInProgress, type);
721+
this.updateLinkInProgressType(type);
705722

706723
return type !== 'invalid';
707724
}

packages/dashboard-core-plugins/src/linker/LinkerLink.tsx

Lines changed: 33 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
import Log from '@deephaven/log';
1515
import { TableUtils } from '@deephaven/jsapi-utils';
1616
import './LinkerLink.scss';
17+
import { LinkType } from './LinkerUtils';
1718

1819
const log = Log.module('LinkerLink');
1920

@@ -37,6 +38,7 @@ export type LinkerLinkProps = {
3738
y2: number;
3839
id: string;
3940
className: string;
41+
type: LinkType;
4042
operator: FilterTypeValue;
4143
isSelected: boolean;
4244
startColumnType: string | null;
@@ -202,6 +204,7 @@ export class LinkerLink extends Component<LinkerLinkProps, LinkerLinkState> {
202204
y2,
203205
id,
204206
startColumnType,
207+
type,
205208
} = this.props;
206209
const { isHovering } = this.state;
207210

@@ -297,6 +300,8 @@ export class LinkerLink extends Component<LinkerLinkProps, LinkerLinkState> {
297300
}
298301
}
299302

303+
const showOperator = type !== 'chartLink' && type !== 'filterSource';
304+
300305
return (
301306
<>
302307
<svg
@@ -323,32 +328,34 @@ export class LinkerLink extends Component<LinkerLinkProps, LinkerLinkState> {
323328
</svg>
324329
{startColumnType != null && isSelected && (
325330
<>
326-
<Button
327-
kind="primary"
328-
className="btn-fab btn-operator"
329-
style={{
330-
top: midY + (slopeAtMid >= 0 ? topOffsetY : bottomOffsetY),
331-
left: midX + (slopeAtMid >= 0 ? topOffsetX : bottomOffsetX),
332-
}}
333-
onClick={() => {
334-
// no-op: click is handled in `DropdownMenu'
335-
}}
336-
icon={
337-
<div className="fa-md fa-layers">
338-
<b>{symbol}</b>
339-
<FontAwesomeIcon
340-
icon={vsTriangleDown}
341-
transform="right-8 down-9 shrink-5"
342-
/>
343-
</div>
344-
}
345-
tooltip="Change comparison operator"
346-
>
347-
<DropdownMenu
348-
actions={this.getDropdownActions}
349-
popperOptions={{ placement: 'bottom-start' }}
350-
/>
351-
</Button>
331+
{showOperator && (
332+
<Button
333+
kind="primary"
334+
className="btn-fab btn-operator"
335+
style={{
336+
top: midY + (slopeAtMid >= 0 ? topOffsetY : bottomOffsetY),
337+
left: midX + (slopeAtMid >= 0 ? topOffsetX : bottomOffsetX),
338+
}}
339+
onClick={() => {
340+
// no-op: click is handled in `DropdownMenu'
341+
}}
342+
icon={
343+
<div className="fa-md fa-layers">
344+
<b>{symbol}</b>
345+
<FontAwesomeIcon
346+
icon={vsTriangleDown}
347+
transform="right-8 down-9 shrink-5"
348+
/>
349+
</div>
350+
}
351+
tooltip="Change comparison operator"
352+
>
353+
<DropdownMenu
354+
actions={this.getDropdownActions}
355+
popperOptions={{ placement: 'bottom-start' }}
356+
/>
357+
</Button>
358+
)}
352359
<Button
353360
kind="primary"
354361
className="btn-fab btn-delete"

packages/dashboard-core-plugins/src/linker/LinkerOverlayContent.tsx

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
Link,
1818
LinkerCoordinate,
1919
LinkPoint,
20+
LinkType,
2021
} from './LinkerUtils';
2122
import LinkerLink from './LinkerLink';
2223
import './LinkerOverlayContent.scss';
@@ -30,6 +31,7 @@ export type VisibleLink = {
3031
y2: number;
3132
id: string;
3233
className: string;
34+
type: LinkType;
3335
operator: FilterTypeValue;
3436
startColumnType: string | null;
3537
};
@@ -290,6 +292,7 @@ export class LinkerOverlayContent extends Component<
290292
className,
291293
operator,
292294
startColumnType,
295+
type,
293296
};
294297
} catch (error) {
295298
log.warn('Unable to get point for link', link, error);
@@ -305,7 +308,17 @@ export class LinkerOverlayContent extends Component<
305308
})}
306309
>
307310
{visibleLinks.map(
308-
({ x1, y1, x2, y2, id, className, operator, startColumnType }) => (
311+
({
312+
x1,
313+
y1,
314+
x2,
315+
y2,
316+
id,
317+
className,
318+
operator,
319+
startColumnType,
320+
type,
321+
}) => (
309322
<LinkerLink
310323
className={className}
311324
id={id}
@@ -319,6 +332,7 @@ export class LinkerOverlayContent extends Component<
319332
isSelected={selectedIds.has(id)}
320333
operator={operator}
321334
startColumnType={startColumnType}
335+
type={type}
322336
onOperatorChanged={this.handleOperatorChanged}
323337
/>
324338
)

packages/dashboard-core-plugins/src/linker/LinkerUtils.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { TypeValue as FilterTypeValue } from '@deephaven/filters';
55
import Log from '@deephaven/log';
66
import { ChartPanel, IrisGridPanel, DropdownFilterPanel } from '../panels';
77

8-
export type LinkType = 'invalid' | 'filterSource' | 'tableLink';
8+
export type LinkType = 'invalid' | 'filterSource' | 'tableLink' | 'chartLink';
99

1010
export type LinkPoint = {
1111
panelId: string | string[];
@@ -154,6 +154,7 @@ class LinkerUtils {
154154
// If all checks pass, link type is determined by the target panel component
155155
switch (end.panelComponent) {
156156
case LayoutUtils.getComponentName(ChartPanel):
157+
return 'chartLink';
157158
case LayoutUtils.getComponentName(IrisGridPanel):
158159
return 'tableLink';
159160
case LayoutUtils.getComponentName(DropdownFilterPanel):

packages/dashboard-core-plugins/src/panels/ChartPanel.tsx

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
ChartModel,
1010
ChartModelSettings,
1111
ChartUtils,
12+
FilterMap,
1213
isFigureChartModel,
1314
} from '@deephaven/chart';
1415
import {
@@ -63,7 +64,7 @@ import ChartColumnSelectorOverlay, {
6364
SelectorColumn,
6465
} from './ChartColumnSelectorOverlay';
6566
import './ChartPanel.scss';
66-
import { Link } from '../linker/LinkerUtils';
67+
import { Link, LinkFilterMap } from '../linker/LinkerUtils';
6768
import { PanelState as IrisGridPanelState } from './IrisGridPanel';
6869
import { isChartPanelTableMetadata } from './ChartPanelUtils';
6970
import { ColumnSelectionValidator } from '../linker/ColumnSelectionValidator';
@@ -73,8 +74,6 @@ const UPDATE_MODEL_DEBOUNCE = 150;
7374

7475
export type InputFilterMap = Map<string, InputFilter>;
7576

76-
export type FilterMap = Map<string, string>;
77-
7877
export type LinkedColumnMap = Map<string, { name: string; type: string }>;
7978

8079
export type ChartPanelFigureMetadata = {
@@ -109,7 +108,7 @@ export interface ChartPanelTableSettings {
109108
partitionColumn?: string;
110109
}
111110
export interface GLChartPanelState {
112-
filterValueMap: [string, string][];
111+
filterValueMap: [string, unknown][];
113112
settings: Partial<ChartModelSettings>;
114113
tableSettings: ChartPanelTableSettings;
115114
irisGridState?: {
@@ -159,10 +158,10 @@ interface ChartPanelState {
159158

160159
// Map of all non-empty filters applied to the chart.
161160
// Initialize the filter map to the previously stored values; input filters will be applied after load.
162-
filterMap: Map<string, string>;
161+
filterMap: FilterMap;
163162
// Map of filter values set from links, stored in panelState.
164163
// Combined with inputFilters to get applied filters (filterMap).
165-
filterValueMap: Map<string, string>;
164+
filterValueMap: FilterMap;
166165
model?: ChartModel;
167166
columnMap: ColumnMap;
168167

@@ -786,20 +785,22 @@ export class ChartPanel extends Component<ChartPanelProps, ChartPanelState> {
786785
* Set chart filters based on the filter map
787786
* @param filterMapParam Filter map
788787
*/
789-
setFilterMap(
790-
filterMapParam: Map<string, { columnType: string; value: string }>
791-
): void {
788+
setFilterMap(filterMapParam: LinkFilterMap): void {
792789
log.debug('setFilterMap', filterMapParam);
793790
this.setState(state => {
794791
const { columnMap, filterMap } = state;
795-
let updatedFilterMap: null | Map<string, string> = null;
792+
let updatedFilterMap: null | FilterMap = null;
796793
const filterValueMap = new Map(state.filterValueMap);
797-
798-
filterMapParam.forEach(({ columnType, value }, columnName) => {
794+
filterMapParam.forEach(({ columnType, filterList }, columnName) => {
799795
const column = columnMap.get(columnName);
800796
if (column == null || column.type !== columnType) {
801797
return;
802798
}
799+
if (filterList.length < 1) {
800+
log.debug('Ignoring empty filterList for column', columnName);
801+
return;
802+
}
803+
const { value } = filterList[0];
803804
filterValueMap.set(columnName, value);
804805
if (filterMap.get(columnName) !== value) {
805806
if (updatedFilterMap === null) {

0 commit comments

Comments
 (0)