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: Custom point renderer #2245

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { Circle, Fill, Stroke } from '../../../../geoms/types';
import { Rotation } from '../../../../utils/common';
import { Dimensions } from '../../../../utils/dimensions';
import { PointGeometry } from '../../../../utils/geometry';
import { GeometryStateStyle } from '../../../../utils/themes/theme';
import { GeometryStateStyle, PointShape } from '../../../../utils/themes/theme';

/**
* Renders points from single series
Expand All @@ -33,6 +33,9 @@ export function renderPoints(ctx: CanvasRenderingContext2D, points: PointGeometr
...style.stroke,
color: overrideOpacity(style.stroke.color, (fillOpacity) => fillOpacity * opacity),
};
if (typeof style.shape === 'function') {
return style.shape(ctx, coordinates);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @qbob1 thanks again for contributing to elastic charts!

Looking over these changes I wonder what is the primary goal or feature that you are looking for from these changes?

Are you looking for a way to define unique shape for the points aside from the limited PointShapes? In that case we could add an option to provide a Path2D, to support any abstract path/symbol.

Or are you looking for support of value labels on line/area charts (i.e. #797)?

The reason I ask is because we are not big fans of exposing the ctx directly to the user to do with as they please. With that said I'm happy to hear your thoughts and get the changes you are looking for added to the library.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Totally understandable!
Fortunately, I was able to find a work around for #797 and posted a comment to help anyone else that may need to display the labels.
I'd say a Path2D would be excellent and a much cleaner solution.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding that comment to #797.

Perfect, if you'd like to refactor this PR or create a fresh PR to add the option to assign a Path2D to PointStyle.shape I'll take another look. Otherwise it'll probably be a few weeks before we get around to adding it ourselves.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't mind taking a crack at it. Any preference on implementation?

}
renderShape(ctx, style.shape, coordinates, fill, stroke);
});
}
Expand Down Expand Up @@ -61,7 +64,7 @@ export function renderPointGroup(
color: overrideOpacity(style.stroke.color, (fillOpacity) => fillOpacity * opacity),
};
const coordinates: Circle = { x: x + transform.x, y, radius };
const renderer = () => renderShape(ctx, style.shape, coordinates, fill, stroke);
const renderer = () => renderShape(ctx, style.shape as PointShape, coordinates, fill, stroke);
const clippings = { area: getPanelClipping(panel, rotation), shouldClip };
withPanelTransform(ctx, panel, rotation, renderingArea, renderer, clippings);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { getColorFromVariant, Rotation } from '../../../../utils/common';
import { Dimensions } from '../../../../utils/dimensions';
import { isPointGeometry, IndexedGeometry, PointGeometry } from '../../../../utils/geometry';
import { LIGHT_THEME } from '../../../../utils/themes/light_theme';
import { HighlighterStyle } from '../../../../utils/themes/theme';
import { HighlighterStyle, PointShape } from '../../../../utils/themes/theme';
import { computeChartDimensionsSelector } from '../../state/selectors/compute_chart_dimensions';
import { computeChartTransformSelector } from '../../state/selectors/compute_chart_transform';
import { getHighlightedGeomsSelector } from '../../state/selectors/get_tooltip_values_highlighted_geoms';
Expand All @@ -46,7 +46,7 @@ function getTransformForPanel(panel: Dimensions, rotation: Rotation, { left, top

function renderPath(geom: PointGeometry, radius: number) {
// keep the highlighter radius to a minimum
const [shapeFn, rotate] = ShapeRendererFn[geom.style.shape];
const [shapeFn, rotate] = ShapeRendererFn[geom.style.shape as PointShape];
return {
d: shapeFn(radius),
rotate,
Expand Down Expand Up @@ -75,8 +75,10 @@ class HighlighterComponent extends React.Component<HighlighterProps> {
const x = geom.x + geom.transform.x;
const y = geom.y + geom.transform.y;
const geomTransform = getTransformForPanel(panel, chartRotation, chartDimensions);

if (isPointGeometry(geom)) {
if (typeof geom.style.shape === 'function') {
return;
}
// using the stroke because the fill is always white on points
const fillColor = getColorFromVariant(RGBATupleToString(geom.style.stroke.color), style.point.fill);
const strokeColor = getColorFromVariant(RGBATupleToString(geom.style.stroke.color), style.point.stroke);
Expand Down
3 changes: 2 additions & 1 deletion packages/charts/src/utils/geometry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { XYChartSeriesIdentifier } from '../chart_types/xy_chart/utils/series';
import { LabelOverflowConstraint } from '../chart_types/xy_chart/utils/specs';
import { Color } from '../common/colors';
import { Fill, Stroke } from '../geoms/types';
import { Coordinate } from '../common/geometry';

/**
* The accessor type
Expand Down Expand Up @@ -70,7 +71,7 @@ export interface PointGeometry {
export interface PointGeometryStyle {
fill: Fill;
stroke: Stroke;
shape: PointShape;
shape: PointShape | ((ctx: CanvasRenderingContext2D, coordinates: { x: number; y: number }) => void);
}

/** @internal */
Expand Down
72 changes: 72 additions & 0 deletions storybook/stories/line/16_custom_point_shapes.story.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import { boolean } from '@storybook/addon-knobs';
import React from 'react';

import {
Axis,
Chart,
LineSeries,
niceTimeFormatByDay,
Position,
ScaleType,
Settings,
timeFormatter,
} from '@elastic/charts';
import { KIBANA_METRICS } from '@elastic/charts/src/utils/data_samples/test_dataset_kibana';

import { ChartsStory } from '../../types';
import { useBaseTheme } from '../../use_base_theme';
import { getColorPicker } from '../utils/components/get_color_picker';

const dateFormatter = timeFormatter(niceTimeFormatByDay(1));
const data = KIBANA_METRICS.metrics.kibana_os_load.v1.data.slice(10, 150);

export const Example: ChartsStory = (_, { title, description }) => {
const showColorPicker = boolean('Show color picker', false);

return (
<Chart title={title} description={description}>
<Settings
showLegend
showLegendExtra
legendPosition={Position.Right}
baseTheme={useBaseTheme()}
legendColorPicker={showColorPicker ? getColorPicker('leftCenter') : undefined}
/>
<Axis id="bottom" position={Position.Bottom} showOverlappingTicks tickFormat={dateFormatter} />
<Axis
id="left"
title={KIBANA_METRICS.metrics.kibana_os_load.v1.metric.title}
position={Position.Left}
tickFormat={(d) => `${Number(d).toFixed(0)}%`}
/>
<LineSeries
id="test"
xScaleType={ScaleType.Time}
yScaleType={ScaleType.Linear}
xAccessor={0}
yAccessors={[1]}
color="lightgray" // never overridden
pointStyleAccessor={(datum) => {
return {
shape: (ctx, coordinates) => {
ctx.beginPath();
ctx.ellipse(coordinates.x, coordinates.y, 5, 5, Math.PI / 4, 0, 2 * Math.PI);
ctx.fill();
ctx.fillText(Math.floor(datum.y1), coordinates.x - 5, coordinates.y - 10);
},
visible: true;
};
}}
data={data.map(([x, y], i) => [x, y + 60, i])}
/>
</Chart>
);
};
1 change: 1 addition & 0 deletions storybook/stories/line/line.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,4 @@ export { Example as testPathOrdering } from './10_test_path_ordering.story';
export { Example as lineWithMarkAccessor } from './13_line_mark_accessor.story';
export { Example as pointShapes } from './14_point_shapes.story';
export { Example as testNegativePoints } from './15_test_negative_points.story';
export { Example as customPointShapes } from './16_custom_point_shapes.story';