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

feat(heatmap): move onBrushEnd from config to Settings #1369

Merged
merged 9 commits into from
Sep 15, 2021
15 changes: 8 additions & 7 deletions packages/charts/api/charts.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,10 @@ export const BrushAxis: Readonly<{
export type BrushAxis = $Values<typeof BrushAxis>;

// @public (undocumented)
export type BrushEndListener = (brushArea: XYBrushArea) => void;
export type BrushEndListener = (brushAreaEvent: BrushEvent) => void;

// @public (undocumented)
export type BrushEvent = XYBrushEvent | HeatmapBrushEvent;

// Warning: (ae-forgotten-export) The symbol "SpecRequiredProps" needs to be exported by the entry point index.d.ts
// Warning: (ae-forgotten-export) The symbol "SpecOptionalProps" needs to be exported by the entry point index.d.ts
Expand Down Expand Up @@ -1008,8 +1011,6 @@ export interface HeatmapConfig {
// (undocumented)
maxRowHeight: Pixels;
// (undocumented)
onBrushEnd?: (brushArea: HeatmapBrushEvent) => void;
// (undocumented)
timeZone: string;
// (undocumented)
width: Pixels;
Expand Down Expand Up @@ -2399,7 +2400,7 @@ export interface WordModel {
export type XScaleType = typeof ScaleType.Ordinal | ScaleContinuousType;

// @public (undocumented)
export interface XYBrushArea {
export interface XYBrushEvent {
// (undocumented)
x?: [number, number];
// (undocumented)
Expand Down Expand Up @@ -2437,9 +2438,9 @@ export type YDomainRange = YDomainBase & DomainRange & LogScaleOptions;

// Warnings were encountered during analysis:
//
// src/chart_types/heatmap/layout/types/config_types.ts:20:13 - (ae-forgotten-export) The symbol "SizeRatio" needs to be exported by the entry point index.d.ts
// src/chart_types/heatmap/layout/types/config_types.ts:50:5 - (ae-forgotten-export) The symbol "TextAlign" needs to be exported by the entry point index.d.ts
// src/chart_types/heatmap/layout/types/config_types.ts:51:5 - (ae-forgotten-export) The symbol "TextBaseline" needs to be exported by the entry point index.d.ts
// src/chart_types/heatmap/layout/types/config_types.ts:19:13 - (ae-forgotten-export) The symbol "SizeRatio" needs to be exported by the entry point index.d.ts
// src/chart_types/heatmap/layout/types/config_types.ts:47:5 - (ae-forgotten-export) The symbol "TextAlign" needs to be exported by the entry point index.d.ts
// src/chart_types/heatmap/layout/types/config_types.ts:48:5 - (ae-forgotten-export) The symbol "TextBaseline" needs to be exported by the entry point index.d.ts
// src/chart_types/partition_chart/layout/types/config_types.ts:139:5 - (ae-forgotten-export) The symbol "TimeMs" needs to be exported by the entry point index.d.ts
// src/chart_types/partition_chart/layout/types/config_types.ts:140:5 - (ae-forgotten-export) The symbol "AnimKeyframe" needs to be exported by the entry point index.d.ts

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ export const config: Config = {
maxColumnWidth: 30,
fontFamily: 'Sans-Serif',

onBrushEnd: undefined,

brushArea: {
visible: true,
fill: 'black', // black === transparent
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
import { Pixels, SizeRatio } from '../../../../common/geometry';
import { Font, FontFamily, TextAlign, TextBaseline } from '../../../../common/text_utils';
import { Color } from '../../../../utils/common';
import { Cell } from './viewmodel_types';

/**
* @public
Expand All @@ -24,8 +23,6 @@ export interface Config {
fontFamily: FontFamily;
timeZone: string;

onBrushEnd?: (brushArea: HeatmapBrushEvent) => void;

/**
* Config of the mask over the area outside of the selected cells
*/
Expand Down Expand Up @@ -94,10 +91,3 @@ export interface Config {
};
maxLegendHeight?: number;
}

/** @public */
export type HeatmapBrushEvent = {
cells: Cell[];
x: (string | number)[];
y: (string | number)[];
};
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,13 @@ import { ChartType } from '../../..';
import { Pixels } from '../../../../common/geometry';
import { Box } from '../../../../common/text_utils';
import { Fill, Line, Rect, Stroke } from '../../../../geoms/types';
import { HeatmapBrushEvent } from '../../../../specs/settings';
import { Color } from '../../../../utils/common';
import { Point } from '../../../../utils/point';
import { PrimitiveValue } from '../../../partition_chart/layout/utils/group_by_rollup';
import { config } from '../config/config';
import { HeatmapCellDatum } from '../viewmodel/viewmodel';
import { Config, HeatmapBrushEvent } from './config_types';
import { Config } from './config_types';

/** @internal */
export interface Value {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ describe('Categorical heatmap brush', () => {
onBrushEndMock = jest.fn();
MockStore.addSpecs(
[
MockGlobalSpec.settingsNoMargins(),
MockGlobalSpec.settingsNoMargins({
onBrushEnd: onBrushEndMock,
}),
MockSeriesSpec.heatmap({
xScaleType: ScaleType.Ordinal,
data: [
Expand Down Expand Up @@ -55,7 +57,6 @@ describe('Categorical heatmap brush', () => {
visible: false,
},
margin: { top: 0, bottom: 0, left: 0, right: 0 },
onBrushEnd: onBrushEndMock,
},
}),
],
Expand Down Expand Up @@ -86,7 +87,9 @@ describe('Temporal heatmap brush', () => {
onBrushEndMock = jest.fn();
MockStore.addSpecs(
[
MockGlobalSpec.settingsNoMargins(),
MockGlobalSpec.settingsNoMargins({
onBrushEnd: onBrushEndMock,
}),
MockSeriesSpec.heatmap({
xScaleType: ScaleType.Time,
data: [
Expand Down Expand Up @@ -116,7 +119,6 @@ describe('Temporal heatmap brush', () => {
visible: false,
},
margin: { top: 0, bottom: 0, left: 0, right: 0 },
onBrushEnd: onBrushEndMock,
},
}),
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ describe('Heatmap picked cells', () => {
onBrushEndMock = jest.fn();
MockStore.addSpecs(
[
MockGlobalSpec.settingsNoMargins(),
MockGlobalSpec.settingsNoMargins({ onBrushEnd: onBrushEndMock }),
MockSeriesSpec.heatmap({
xScaleType: ScaleType.Ordinal,
data: [
Expand Down Expand Up @@ -55,7 +55,6 @@ describe('Heatmap picked cells', () => {
visible: true,
},
margin: { top: 0, bottom: 0, left: 0, right: 0 },
onBrushEnd: onBrushEndMock,
},
}),
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,21 @@
*/

import { createCustomCachedSelector } from '../../../../state/create_selector';
import { getSettingsSpecSelector } from '../../../../state/selectors/get_settings_specs';
import { getHeatmapConfigSelector } from './get_heatmap_config';

/**
* The brush is available only if a onBrushEnd listener is configured
* @internal
*/
export const isBrushAvailableSelector = createCustomCachedSelector([getHeatmapConfigSelector], (config): boolean => {
return Boolean(config.onBrushEnd) && config.brushTool.visible;
});
export const isBrushAvailableSelector = createCustomCachedSelector(
[getHeatmapConfigSelector, getSettingsSpecSelector],
(config, { onBrushEnd }): boolean => {
return Boolean(onBrushEnd) && config.brushTool.visible;
},
);

/** @internal */
export const isBrushEndProvided = createCustomCachedSelector([getHeatmapConfigSelector], (config): boolean => {
return Boolean(config.onBrushEnd);
export const isBrushEndProvided = createCustomCachedSelector([getSettingsSpecSelector], ({ onBrushEnd }): boolean => {
return Boolean(onBrushEnd);
});
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ import { ChartType } from '../../..';
import { GlobalChartState } from '../../../../state/chart_state';
import { createCustomCachedSelector } from '../../../../state/create_selector';
import { getLastDragSelector } from '../../../../state/selectors/get_last_drag';
import { getSettingsSpecSelector } from '../../../../state/selectors/get_settings_specs';
import { DragCheckProps, hasDragged } from '../../../../utils/events';
import { getHeatmapConfigSelector } from './get_heatmap_config';
import { getPickedCells } from './get_picked_cells';
import { getSpecOrNull } from './heatmap_spec';
import { isBrushEndProvided } from './is_brush_available';
Expand All @@ -35,7 +35,7 @@ export function createOnBrushEndCaller(): (state: GlobalChartState) => void {
return;
}
selector = createCustomCachedSelector(
[getLastDragSelector, getSpecOrNull, getHeatmapConfigSelector, getPickedCells],
[getLastDragSelector, getSpecOrNull, getSettingsSpecSelector, getPickedCells],
(lastDrag, spec, { onBrushEnd }, pickedCells): void => {
const nextProps: DragCheckProps = {
lastDrag,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { Rect } from '../../../geoms/types';
import { MockAnnotationSpec, MockGlobalSpec, MockSeriesSpec } from '../../../mocks/specs/specs';
import { MockStore } from '../../../mocks/store';
import { ScaleType } from '../../../scales/constants';
import { SettingsSpec, XYBrushArea } from '../../../specs';
import { BrushEvent, SettingsSpec } from '../../../specs';
import { SpecType, TooltipType, BrushAxis } from '../../../specs/constants';
import { onExternalPointerEvent } from '../../../state/actions/events';
import { onPointerMove, onMouseDown, onMouseUp } from '../../../state/actions/mouse';
Expand Down Expand Up @@ -800,7 +800,7 @@ describe('Chart state pointer interactions', () => {
});
describe('brush', () => {
test('can respond to a brush end event', () => {
const brushEndListener = jest.fn<void, [XYBrushArea]>((): void => undefined);
const brushEndListener = jest.fn<void, [BrushEvent]>((): void => undefined);
const onBrushCaller = createOnBrushEndCaller();
store.subscribe(() => {
onBrushCaller(store.getState());
Expand Down Expand Up @@ -894,7 +894,7 @@ describe('Chart state pointer interactions', () => {
}
});
test('can respond to a brush end event on rotated chart', () => {
const brushEndListener = jest.fn<void, [XYBrushArea]>((): void => undefined);
const brushEndListener = jest.fn<void, [BrushEvent]>((): void => undefined);
const onBrushCaller = createOnBrushEndCaller();
store.subscribe(() => {
onBrushCaller(store.getState());
Expand Down Expand Up @@ -966,7 +966,7 @@ describe('Chart state pointer interactions', () => {
}
});
test('can respond to a Y brush', () => {
const brushEndListener = jest.fn<void, [XYBrushArea]>((): void => undefined);
const brushEndListener = jest.fn<void, [BrushEvent]>((): void => undefined);
const onBrushCaller = createOnBrushEndCaller();
store.subscribe(() => {
onBrushCaller(store.getState());
Expand Down Expand Up @@ -1042,7 +1042,7 @@ describe('Chart state pointer interactions', () => {
}
});
test('can respond to rectangular brush', () => {
const brushEndListener = jest.fn<void, [XYBrushArea]>((): void => undefined);
const brushEndListener = jest.fn<void, [BrushEvent]>((): void => undefined);
const onBrushCaller = createOnBrushEndCaller();
store.subscribe(() => {
onBrushCaller(store.getState());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { Selector } from 'reselect';

import { ChartType } from '../../..';
import { Scale } from '../../../../scales';
import { GroupBrushExtent, XYBrushArea } from '../../../../specs';
import { GroupBrushExtent, XYBrushEvent } from '../../../../specs';
import { BrushAxis } from '../../../../specs/constants';
import { DragState, GlobalChartState } from '../../../../state/chart_state';
import { createCustomCachedSelector } from '../../../../state/create_selector';
Expand Down Expand Up @@ -74,11 +74,11 @@ export function createOnBrushEndCaller(): (state: GlobalChartState) => void {
onBrushEnd,
};
if (lastDrag !== null && hasDragged(prevProps, nextProps) && onBrushEnd) {
const brushArea: XYBrushArea = {};
const brushAreaEvent: XYBrushEvent = {};
const { yScales, xScale } = computedScales;

if (brushAxis === BrushAxis.X || brushAxis === BrushAxis.Both) {
brushArea.x = getXBrushExtent(
brushAreaEvent.x = getXBrushExtent(
chartDimensions,
lastDrag,
rotation,
Expand All @@ -91,7 +91,7 @@ export function createOnBrushEndCaller(): (state: GlobalChartState) => void {
);
}
if (brushAxis === BrushAxis.Y || brushAxis === BrushAxis.Both) {
brushArea.y = getYBrushExtents(
brushAreaEvent.y = getYBrushExtents(
chartDimensions,
lastDrag,
rotation,
Expand All @@ -100,8 +100,8 @@ export function createOnBrushEndCaller(): (state: GlobalChartState) => void {
minBrushDelta,
);
}
if (brushArea.x !== undefined || brushArea.y !== undefined) {
onBrushEnd(brushArea);
if (brushAreaEvent.x !== undefined || brushAreaEvent.y !== undefined) {
onBrushEnd(brushAreaEvent);
}
}
prevProps = nextProps;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ exports[`Chart should render the legend name test 1`] = `
</Highlighter>
</Connect(Highlighter)>
<Connect(BrushTool)>
<BrushTool initialized={true} projectionContainer={{...}} mainProjectionArea={{...}} isBrushAvailable={false} isBrushing={false} brushArea={{...}} zIndex={0} dispatch={[Function: dispatch]} />
<BrushTool initialized={true} projectionContainer={{...}} mainProjectionArea={{...}} isBrushAvailable={false} isBrushing={false} brushEvent={{...}} zIndex={0} dispatch={[Function: dispatch]} />
</Connect(BrushTool)>
</div>
</ChartContainer>
Expand Down
12 changes: 6 additions & 6 deletions packages/charts/src/components/brush/brush.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ interface StateProps {
projectionContainer: Dimensions;
isBrushing: boolean | undefined;
isBrushAvailable: boolean | undefined;
brushArea: Dimensions | null;
brushEvent: Dimensions | null;
zIndex: number;
}

Expand Down Expand Up @@ -101,12 +101,12 @@ class BrushToolComponent extends React.Component<Props> {
}

private drawCanvas = () => {
const { brushArea, mainProjectionArea, fillColor } = this.props;
const { brushEvent, mainProjectionArea, fillColor } = this.props;
const { ctx } = this;
if (!ctx || !brushArea) {
if (!ctx || !brushEvent) {
return;
}
const { top, left, width, height } = brushArea;
const { top, left, width, height } = brushEvent;
withContext(ctx, () => {
ctx.scale(this.devicePixelRatio, this.devicePixelRatio);
withClip(
Expand Down Expand Up @@ -155,7 +155,7 @@ const mapStateToProps = (state: GlobalChartState): StateProps => {
},
isBrushing: false,
isBrushAvailable: false,
brushArea: null,
brushEvent: null,
zIndex: 0,
};
}
Expand All @@ -165,7 +165,7 @@ const mapStateToProps = (state: GlobalChartState): StateProps => {
mainProjectionArea: getInternalMainProjectionAreaSelector(state),
isBrushAvailable: getInternalIsBrushingAvailableSelector(state),
isBrushing: getInternalIsBrushingSelector(state),
brushArea: getInternalBrushAreaSelector(state),
brushEvent: getInternalBrushAreaSelector(state),
zIndex: state.zIndex,
};
};
Expand Down
2 changes: 1 addition & 1 deletion packages/charts/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ export * from './chart_types/partition_chart/layout/utils/group_by_rollup';

// heatmap
export { Cell } from './chart_types/heatmap/layout/types/viewmodel_types';
export { Config as HeatmapConfig, HeatmapBrushEvent } from './chart_types/heatmap/layout/types/config_types';
export { Config as HeatmapConfig } from './chart_types/heatmap/layout/types/config_types';
export { ColorBand, HeatmapBandsColorScale } from './chart_types/heatmap/specs/heatmap';

// utilities
Expand Down
16 changes: 14 additions & 2 deletions packages/charts/src/specs/settings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ export interface GroupBrushExtent {
}

/** @public */
export interface XYBrushArea {
export interface XYBrushEvent {
x?: [number, number];
y?: Array<GroupBrushExtent>;
}
Expand Down Expand Up @@ -140,8 +140,20 @@ export type ElementClickListener = (
export type ElementOverListener = (
elements: Array<XYChartElementEvent | PartitionElementEvent | HeatmapElementEvent | WordCloudElementEvent>,
) => void;

/** @public */
export type BrushEvent = XYBrushEvent | HeatmapBrushEvent;

/** @public */
export type BrushEndListener = (brushArea: XYBrushArea) => void;
export type BrushEndListener = (brushAreaEvent: BrushEvent) => void;

/** @public */
export type HeatmapBrushEvent = {
cells: Cell[];
x: (string | number)[];
y: (string | number)[];
};

/** @public */
export type LegendItemListener = (series: SeriesIdentifier[]) => void;
/**
Expand Down
4 changes: 2 additions & 2 deletions packages/charts/src/utils/events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*/

import { Scale } from '../scales';
import { BrushEndListener, isPointerOverEvent, PointerEvent, PointerOverEvent, HeatmapSpec } from '../specs';
import { BrushEndListener, isPointerOverEvent, PointerEvent, PointerOverEvent } from '../specs';
import { DragState } from '../state/chart_state';

/** @internal */
Expand All @@ -20,7 +20,7 @@ export function isValidPointerOverEvent(

/** @internal */
export interface DragCheckProps {
onBrushEnd: BrushEndListener | HeatmapSpec['config']['onBrushEnd'] | undefined;
onBrushEnd: BrushEndListener | undefined;
lastDrag: DragState | null;
}

Expand Down
Loading