Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Fix OneClick links not filtering plots #1217

Merged
merged 4 commits into from
Apr 17, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 2 additions & 9 deletions packages/chart/src/ChartModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
/**
Expand Down Expand Up @@ -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
Expand Down
10 changes: 10 additions & 0 deletions packages/chart/src/ChartUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
13 changes: 9 additions & 4 deletions packages/chart/src/FigureChartModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -103,7 +108,7 @@ class FigureChartModel extends ChartModel {

filterColumnMap: FilterColumnMap;

lastFilter: Map<string, string>;
lastFilter: FilterMap;

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

Expand Down Expand Up @@ -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;
Expand Down
39 changes: 27 additions & 12 deletions packages/dashboard-core-plugins/src/linker/Linker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,19 @@ export class Linker extends Component<LinkerProps, LinkerState> {
this.deleteLinks(linksToDelete);
break;
}
case 'chartLink': {
const existingLinkEnd = isReversed === true ? start : end;
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 }) =>
panelLinkEnd?.panelId === existingLinkEnd.panelId &&
panelLinkEnd?.columnName === existingLinkEnd.columnName &&
panelLinkEnd?.columnType === existingLinkEnd.columnType
);
vbabich marked this conversation as resolved.
Show resolved Hide resolved
this.deleteLinks(linksToDelete);
break;
}
case 'tableLink':
// No-op
break;
Expand Down Expand Up @@ -642,15 +655,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;
});
}

Expand All @@ -664,7 +679,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;
}
Expand All @@ -673,7 +688,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;
}
Expand Down Expand Up @@ -701,7 +716,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';
}
Expand Down
59 changes: 33 additions & 26 deletions packages/dashboard-core-plugins/src/linker/LinkerLink.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand All @@ -37,6 +38,7 @@ export type LinkerLinkProps = {
y2: number;
id: string;
className: string;
type: LinkType;
operator: FilterTypeValue;
isSelected: boolean;
startColumnType: string | null;
Expand Down Expand Up @@ -202,6 +204,7 @@ export class LinkerLink extends Component<LinkerLinkProps, LinkerLinkState> {
y2,
id,
startColumnType,
type,
} = this.props;
const { isHovering } = this.state;

Expand Down Expand Up @@ -297,6 +300,8 @@ export class LinkerLink extends Component<LinkerLinkProps, LinkerLinkState> {
}
}

const showOperator = type !== 'chartLink' && type !== 'filterSource';

return (
<>
<svg
Expand All @@ -322,32 +327,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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
Link,
LinkerCoordinate,
LinkPoint,
LinkType,
} from './LinkerUtils';
import LinkerLink from './LinkerLink';
import './LinkerOverlayContent.scss';
Expand All @@ -30,6 +31,7 @@ export type VisibleLink = {
y2: number;
id: string;
className: string;
type: LinkType;
operator: FilterTypeValue;
startColumnType: string | null;
};
Expand Down Expand Up @@ -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);
Expand All @@ -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}
Expand All @@ -319,6 +332,7 @@ export class LinkerOverlayContent extends Component<
isSelected={selectedIds.has(id)}
operator={operator}
startColumnType={startColumnType}
type={type}
onOperatorChanged={this.handleOperatorChanged}
/>
)
Expand Down
3 changes: 2 additions & 1 deletion packages/dashboard-core-plugins/src/linker/LinkerUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[];
Expand Down Expand Up @@ -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):
Expand Down
25 changes: 13 additions & 12 deletions packages/dashboard-core-plugins/src/panels/ChartPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
ChartModel,
ChartModelSettings,
ChartUtils,
FilterMap,
isFigureChartModel,
} from '@deephaven/chart';
import {
Expand Down Expand Up @@ -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';
Expand All @@ -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 = {
Expand Down Expand Up @@ -109,7 +108,7 @@ export interface ChartPanelTableSettings {
partitionColumn?: string;
}
export interface GLChartPanelState {
filterValueMap: [string, string][];
filterValueMap: [string, unknown][];
settings: Partial<ChartModelSettings>;
tableSettings: ChartPanelTableSettings;
irisGridState?: {
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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.error('Invalid filterMap, filterList is empty', filterMapParam);
vbabich marked this conversation as resolved.
Show resolved Hide resolved
return;
}
const { value } = filterList[0];
filterValueMap.set(columnName, value);
if (filterMap.get(columnName) !== value) {
if (updatedFilterMap === null) {
Expand Down