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(histogram): fix annotation and tooltip overflow with single value #343

Merged
merged 2 commits into from
Aug 26, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
162 changes: 76 additions & 86 deletions .playground/playgroud.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,17 @@ import {
Chart,
getAxisId,
getSpecId,
niceTimeFormatter,
Position,
ScaleType,
Settings,
BarSeries,
LineAnnotation,
RectAnnotation,
getAnnotationId,
AnnotationDomainTypes,
HistogramBarSeries,
niceTimeFormatByDay,
timeFormatter,
} from '../src';
import { KIBANA_METRICS } from '../src/utils/data_samples/test_dataset_kibana';
import { CursorEvent } from '../src/specs/settings';
import { CursorUpdateListener } from '../src/chart_types/xy_chart/store/chart_state';
import { Icon } from '../src/components/icons/icon';

export class Playground extends React.Component {
ref1 = React.createRef<Chart>();
Expand All @@ -32,86 +30,78 @@ export class Playground extends React.Component {

render() {
return (
<>
{renderChart(
'1',
this.ref1,
KIBANA_METRICS.metrics.kibana_os_load[0].data.slice(0, 15),
this.onCursorUpdate,
true,
)}
{renderChart(
'2',
this.ref2,
KIBANA_METRICS.metrics.kibana_os_load[1].data.slice(0, 15),
this.onCursorUpdate,
true,
)}
{renderChart('3', this.ref3, KIBANA_METRICS.metrics.kibana_os_load[1].data.slice(15, 30), this.onCursorUpdate)}
</>
<div className="chart">
<Chart size={{ height: 200 }}>
<Settings
tooltip={{ type: 'vertical' }}
debug={false}
legendPosition={Position.Right}
showLegend={true}
xDomain={{
min: 1566079200000,
max: 1566079200000,
minInterval: 86400000,
}}
/>
<Axis id={getAxisId('count')} title="count" position={Position.Left} tickFormat={(d) => d.toFixed(2)} />
<Axis
id={getAxisId('timestamp')}
title="timestamp"
position={Position.Bottom}
tickFormat={timeFormatter(niceTimeFormatByDay(1))}
/>
<RectAnnotation
annotationId={getAnnotationId('annotation1')}
dataValues={[
{
coordinates: {
x0: 1566103254092,
},
details: `06:40`,
},
]}
style={{
stroke: 'rgba(0, 0, 0, 0)',
strokeWidth: 1,
opacity: 0.5,
fill: 'rgba(200, 0, 0, 0.5)',
}}
zIndex={-2}
/>
<RectAnnotation
annotationId={getAnnotationId('annotation2')}
dataValues={[
{
coordinates: {
x1: 1566088404270,
},
details: `02:33`,
},
]}
style={{
stroke: 'rgba(0, 0, 0, 0)',
strokeWidth: 1,
opacity: 0.5,
fill: 'rgba(0, 0, 200, 0.5)',
}}
zIndex={-2}
/>
<HistogramBarSeries
id={getSpecId('dataset B')}
xScaleType={ScaleType.Time}
yScaleType={ScaleType.Linear}
data={[[1566079200000, 10]]}
xAccessor={0}
yAccessors={[1]}
timeZone={'local'}
barSeriesStyle={{
rect: {
opacity: 0.7,
},
}}
/>
</Chart>
</div>
);
}
}

function renderChart(
key: string,
ref: React.RefObject<Chart>,
data: any,
onCursorUpdate?: CursorUpdateListener,
timeSeries: boolean = false,
) {
return (
<div key={key} className="chart">
<Chart ref={ref}>
<Settings
tooltip={{ type: 'vertical' }}
debug={false}
legendPosition={Position.Right}
showLegend={true}
onCursorUpdate={onCursorUpdate}
rotation={0}
/>
<Axis
id={getAxisId('timestamp')}
title="timestamp"
position={Position.Bottom}
tickFormat={niceTimeFormatter([1555819200000, 1555905600000])}
/>
<Axis id={getAxisId('count')} title="count" position={Position.Left} tickFormat={(d) => d.toFixed(2)} />
<LineAnnotation
annotationId={getAnnotationId('annotation1')}
domainType={AnnotationDomainTypes.XDomain}
dataValues={[
{
dataValue: KIBANA_METRICS.metrics.kibana_os_load[1].data[5][0],
details: 'tooltip 1',
},
{
dataValue: KIBANA_METRICS.metrics.kibana_os_load[1].data[9][0],
details: 'tooltip 2',
},
]}
hideLinesTooltips={true}
marker={<Icon type="alert" />}
/>
<BarSeries
id={getSpecId('dataset A with long title')}
xScaleType={timeSeries ? ScaleType.Time : ScaleType.Linear}
yScaleType={ScaleType.Linear}
data={data}
xAccessor={0}
yAccessors={[1]}
/>
<BarSeries
id={getSpecId('dataset B')}
xScaleType={ScaleType.Time}
yScaleType={ScaleType.Linear}
data={KIBANA_METRICS.metrics.kibana_os_load[1].data.slice(0, 15)}
xAccessor={0}
yAccessors={[1]}
stackAccessors={[0]}
/>
</Chart>
</div>
);
}
10 changes: 5 additions & 5 deletions src/chart_types/xy_chart/annotations/annotation_utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1384,14 +1384,14 @@ describe('annotation utils', () => {
const dimensions = computeRectAnnotationDimensions(annotationRectangle, yScales, xScale, true, 0);

const [dims1, dims2, dims3, dims4] = dimensions;
expect(dims1.rect.x).toBe(0);
expect(dims1.rect.x).toBe(10);
expect(dims1.rect.y).toBe(0);
expect(dims1.rect.width).toBe(10);
expect(dims1.rect.width).toBeCloseTo(100);
expect(dims1.rect.height).toBe(100);

expect(dims2.rect.x).toBe(10);
expect(dims2.rect.x).toBe(0);
expect(dims2.rect.y).toBe(0);
expect(dims2.rect.width).toBeCloseTo(100);
expect(dims2.rect.width).toBe(10);
expect(dims2.rect.height).toBe(100);

expect(dims3.rect.x).toBe(0);
Expand Down Expand Up @@ -1425,8 +1425,8 @@ describe('annotation utils', () => {
const dimensions = computeRectAnnotationDimensions(annotationRectangle, yScales, xScale, false, 0);

const expectedDimensions = [
{ rect: { x: 0, y: 0, width: 10, height: 100 } },
{ rect: { x: 10, y: 0, width: 90, height: 100 } },
{ rect: { x: 0, y: 0, width: 10, height: 100 } },
{ rect: { x: 0, y: 0, width: 100, height: 10 } },
{ rect: { x: 0, y: 10, width: 100, height: 90 } },
];
Expand Down
9 changes: 4 additions & 5 deletions src/chart_types/xy_chart/annotations/annotation_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,6 @@ export function computeRectAnnotationDimensions(
const yDomain = yScale.domain;
const lastX = xDomain[xDomain.length - 1];
const xMinInterval = xScale.minInterval;

const rectsProps: AnnotationRectProps[] = [];

dataValues.forEach((dataValue: RectAnnotationDatum) => {
Expand All @@ -410,15 +409,15 @@ export function computeRectAnnotationDimensions(
return;
}

if (x0 == null) {
if (x1 == null) {
// if x1 is defined, we want the rect to draw to the end of the scale
// if we're in histogram mode, extend domain end by min interval
x0 = enableHistogramMode ? lastX + xMinInterval : lastX;
x1 = enableHistogramMode && !xScale.isSingleValue() ? lastX + xMinInterval : lastX;
}

if (x1 == null) {
if (x0 == null) {
// if x0 is defined, we want the rect to draw to the start of the scale
x1 = xDomain[0];
x0 = xDomain[0];
}

if (y0 == null) {
Expand Down
10 changes: 9 additions & 1 deletion src/chart_types/xy_chart/utils/scales.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,15 @@ export function computeXScale(options: XScaleOptions): Scale {
domain: adjustedDomain,
range: [start, end],
},
{ bandwidth: bandwidth / totalBarsInCluster, minInterval, timeZone, totalBarsInCluster, barsPadding, ticks },
{
bandwidth: bandwidth / totalBarsInCluster,
minInterval,
timeZone,
totalBarsInCluster,
barsPadding,
ticks,
isSingleValueHistogram,
},
);

return scale;
Expand Down
39 changes: 26 additions & 13 deletions src/utils/scales/scale_continuous.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,18 @@ interface ScaleOptions {
* @default 10
*/
ticks: number;
/** true if the scale was adjusted to fit one single value histogram */
isSingleValueHistogram: boolean;
}

const defaultScaleOptions: ScaleOptions = {
bandwidth: 0,
minInterval: 0,
timeZone: 'utc',
totalBarsInCluster: 1,
barsPadding: 0,
ticks: 10,
isSingleValueHistogram: false,
};
export class ScaleContinuous implements Scale {
readonly bandwidth: number;
readonly totalBarsInCluster: number;
Expand All @@ -118,22 +128,21 @@ export class ScaleContinuous implements Scale {
readonly tickValues: number[];
readonly timeZone: string;
readonly barsPadding: number;
readonly isSingleValueHistogram: boolean;
private readonly d3Scale: any;

constructor(scaleData: ScaleData, options?: Partial<ScaleOptions>) {
const { type, domain, range } = scaleData;
const scaleOptions: ScaleOptions = mergePartial(
{
bandwidth: 0,
minInterval: 0,
timeZone: 'utc',
totalBarsInCluster: 1,
barsPadding: 0,
ticks: 10,
},
options,
);
const { bandwidth, minInterval, timeZone, totalBarsInCluster, barsPadding, ticks } = scaleOptions;
const {
bandwidth,
minInterval,
timeZone,
totalBarsInCluster,
barsPadding,
ticks,
isSingleValueHistogram,
} = mergePartial(defaultScaleOptions, options);
nickofthyme marked this conversation as resolved.
Show resolved Hide resolved

this.d3Scale = SCALES[type]();
if (type === ScaleType.Log) {
this.domain = limitLogScaleDomain(domain);
Expand All @@ -154,6 +163,7 @@ export class ScaleContinuous implements Scale {
this.isInverted = this.domain[0] > this.domain[1];
this.timeZone = timeZone;
this.totalBarsInCluster = totalBarsInCluster;
this.isSingleValueHistogram = isSingleValueHistogram;
if (type === ScaleType.Time) {
const startDomain = DateTime.fromMillis(this.domain[0], { zone: this.timeZone });
const endDomain = DateTime.fromMillis(this.domain[1], { zone: this.timeZone });
Expand Down Expand Up @@ -227,6 +237,9 @@ export class ScaleContinuous implements Scale {
return prevValue;
}
isSingleValue() {
if (this.isSingleValueHistogram) {
return true;
}
if (this.domain.length < 2) {
return true;
}
Expand Down