Skip to content

Commit 48b180a

Browse files
emmacunninghamemmacunningham
authored
fix(line_annotation): use scaleAndValidate for line annotations (#236)
1 parent 01d2b60 commit 48b180a

File tree

4 files changed

+65
-13
lines changed

4 files changed

+65
-13
lines changed

‎src/state/annotation_marker.test.tsx‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ describe('annotation marker', () => {
5757
xScale,
5858
Position.Left,
5959
0,
60+
false,
6061
);
6162
const expectedDimensions = [
6263
{
@@ -96,6 +97,7 @@ describe('annotation marker', () => {
9697
xScale,
9798
Position.Left,
9899
0,
100+
false,
99101
);
100102
const expectedDimensions = [
101103
{
@@ -135,6 +137,7 @@ describe('annotation marker', () => {
135137
xScale,
136138
Position.Left,
137139
0,
140+
false,
138141
);
139142
const expectedDimensions = [
140143
{

‎src/state/annotation_utils.test.ts‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,7 @@ describe('annotation utils', () => {
203203
xScale,
204204
Position.Left,
205205
0,
206+
false,
206207
);
207208
const expectedDimensions = [
208209
{
@@ -239,6 +240,7 @@ describe('annotation utils', () => {
239240
xScale,
240241
Position.Right,
241242
0,
243+
false,
242244
);
243245
const expectedDimensions = [
244246
{
@@ -275,6 +277,7 @@ describe('annotation utils', () => {
275277
xScale,
276278
Position.Left,
277279
0,
280+
false,
278281
);
279282
const expectedDimensions = [
280283
{
@@ -309,6 +312,7 @@ describe('annotation utils', () => {
309312
xScale,
310313
Position.Left,
311314
0,
315+
false,
312316
);
313317
expect(dimensions).toEqual(null);
314318
});
@@ -336,6 +340,7 @@ describe('annotation utils', () => {
336340
xScale,
337341
Position.Left,
338342
0,
343+
false,
339344
);
340345
const expectedDimensions = [
341346
{
@@ -370,6 +375,7 @@ describe('annotation utils', () => {
370375
xScale,
371376
Position.Top,
372377
0,
378+
false,
373379
);
374380
const expectedDimensions = [
375381
{
@@ -404,6 +410,7 @@ describe('annotation utils', () => {
404410
xScale,
405411
Position.Bottom,
406412
0,
413+
false,
407414
);
408415
const expectedDimensions = [
409416
{
@@ -415,6 +422,41 @@ describe('annotation utils', () => {
415422
expect(dimensions).toEqual(expectedDimensions);
416423
});
417424

425+
test('should compute line annotation dimensions for xDomain in histogramMode with extended upper bound', () => {
426+
const chartRotation: Rotation = 0;
427+
const yScales: Map<GroupId, Scale> = new Map();
428+
const xScale: Scale = continuousScale;
429+
430+
const annotationId = getAnnotationId('foo-line');
431+
const lineAnnotation: LineAnnotationSpec = {
432+
annotationType: 'line',
433+
annotationId,
434+
domainType: AnnotationDomainTypes.XDomain,
435+
dataValues: [{ dataValue: 10.5, details: 'foo' }],
436+
groupId,
437+
style: DEFAULT_ANNOTATION_LINE_STYLE,
438+
};
439+
440+
const dimensions = computeLineAnnotationDimensions(
441+
lineAnnotation,
442+
chartDimensions,
443+
chartRotation,
444+
yScales,
445+
xScale,
446+
Position.Bottom,
447+
0,
448+
true,
449+
);
450+
const expectedDimensions = [
451+
{
452+
position: [105, 0, 105, 20],
453+
details: { detailsText: 'foo', headerText: '10.5' },
454+
tooltipLinePosition: [105, 0, 105, 20],
455+
},
456+
];
457+
expect(dimensions).toEqual(expectedDimensions);
458+
});
459+
418460
test('should compute line annotation dimensions for xDomain on a xScale (chartRotation 90, ordinal scale)', () => {
419461
const chartRotation: Rotation = 90;
420462
const yScales: Map<GroupId, Scale> = new Map();
@@ -439,6 +481,7 @@ describe('annotation utils', () => {
439481
xScale,
440482
Position.Left,
441483
0,
484+
false,
442485
);
443486
const expectedDimensions = [
444487
{
@@ -474,6 +517,7 @@ describe('annotation utils', () => {
474517
xScale,
475518
Position.Left,
476519
0,
520+
false,
477521
);
478522
const expectedDimensions = [
479523
{
@@ -509,6 +553,7 @@ describe('annotation utils', () => {
509553
xScale,
510554
Position.Left,
511555
0,
556+
false,
512557
);
513558
const expectedDimensions = [
514559
{
@@ -544,6 +589,7 @@ describe('annotation utils', () => {
544589
xScale,
545590
Position.Top,
546591
0,
592+
false,
547593
);
548594
const expectedDimensions = [
549595
{
@@ -578,6 +624,7 @@ describe('annotation utils', () => {
578624
xScale,
579625
Position.Bottom,
580626
0,
627+
false,
581628
);
582629
const expectedDimensions = [
583630
{
@@ -614,6 +661,7 @@ describe('annotation utils', () => {
614661
xScale,
615662
Position.Right,
616663
0,
664+
false,
617665
);
618666

619667
expect(emptyXDimensions).toEqual([]);
@@ -635,6 +683,7 @@ describe('annotation utils', () => {
635683
continuousScale,
636684
Position.Right,
637685
0,
686+
false,
638687
);
639688

640689
expect(invalidStringXDimensions).toEqual([]);
@@ -656,6 +705,7 @@ describe('annotation utils', () => {
656705
continuousScale,
657706
Position.Right,
658707
0,
708+
false,
659709
);
660710

661711
expect(emptyOutOfBoundsXDimensions).toEqual([]);
@@ -677,6 +727,7 @@ describe('annotation utils', () => {
677727
xScale,
678728
Position.Right,
679729
0,
730+
false,
680731
);
681732

682733
expect(emptyYDimensions).toEqual([]);
@@ -698,6 +749,7 @@ describe('annotation utils', () => {
698749
xScale,
699750
Position.Right,
700751
0,
752+
false,
701753
);
702754

703755
expect(emptyOutOfBoundsYDimensions).toEqual([]);
@@ -719,6 +771,7 @@ describe('annotation utils', () => {
719771
continuousScale,
720772
Position.Right,
721773
0,
774+
false,
722775
);
723776

724777
expect(invalidStringYDimensions).toEqual([]);
@@ -741,6 +794,7 @@ describe('annotation utils', () => {
741794
continuousScale,
742795
Position.Right,
743796
0,
797+
false,
744798
);
745799

746800
expect(hiddenAnnotationDimensions).toEqual(null);

‎src/state/annotation_utils.ts‎

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,7 @@ export function computeXDomainLineAnnotationDimensions(
179179
chartDimensions: Dimensions,
180180
lineColor: string,
181181
xScaleOffset: number,
182+
enableHistogramMode: boolean,
182183
marker?: JSX.Element,
183184
markerDimensions?: { width: number; height: number },
184185
): AnnotationLineProps[] {
@@ -187,6 +188,7 @@ export function computeXDomainLineAnnotationDimensions(
187188
const markerOffsets = markerDimensions || { width: 0, height: 0 };
188189
const lineProps: AnnotationLineProps[] = [];
189190

191+
const alignWithTick = xScale.bandwidth > 0 && !enableHistogramMode;
190192
dataValues.forEach((datum: LineAnnotationDatum) => {
191193
const { dataValue } = datum;
192194
const details = {
@@ -195,23 +197,13 @@ export function computeXDomainLineAnnotationDimensions(
195197
};
196198

197199
const offset = xScale.bandwidth / 2 - xScaleOffset;
198-
const isContinuous = xScale.type !== ScaleType.Ordinal;
199200

200-
const scaledXValue = xScale.scale(dataValue);
201+
const scaledXValue = scaleAndValidateDatum(dataValue, xScale, alignWithTick);
201202

202-
// d3.scale will return 0 for '', rendering the line incorrectly at 0
203-
if (isNaN(scaledXValue) || (isContinuous && dataValue === '')) {
203+
if (scaledXValue == null) {
204204
return;
205205
}
206206

207-
if (isContinuous) {
208-
const [domainStart, domainEnd] = xScale.domain;
209-
210-
if (domainStart > dataValue || domainEnd < dataValue) {
211-
return;
212-
}
213-
}
214-
215207
const xDomainPosition = scaledXValue + offset;
216208

217209
let linePosition: AnnotationLinePosition = [0, 0, 0, 0];
@@ -280,6 +272,7 @@ export function computeLineAnnotationDimensions(
280272
xScale: Scale,
281273
axisPosition: Position,
282274
xScaleOffset: number,
275+
enableHistogramMode: boolean,
283276
): AnnotationLineProps[] | null {
284277
const { domainType, dataValues, marker, markerDimensions, hideLines } = annotationSpec;
285278

@@ -304,6 +297,7 @@ export function computeLineAnnotationDimensions(
304297
chartDimensions,
305298
lineColor,
306299
xScaleOffset,
300+
enableHistogramMode,
307301
marker,
308302
markerDimensions,
309303
);
@@ -545,6 +539,7 @@ export function computeAnnotationDimensions(
545539
xScale,
546540
annotationAxisPosition,
547541
xScaleOffset - clusterOffset,
542+
enableHistogramMode,
548543
);
549544

550545
if (dimensions) {

‎stories/bar_chart.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1404,7 +1404,7 @@ storiesOf('Bar Chart', module)
14041404
<LineAnnotation
14051405
annotationId={getAnnotationId('line-annotation')}
14061406
domainType={AnnotationDomainTypes.XDomain}
1407-
dataValues={[{ dataValue: 2 }, { dataValue: 2.5 }]}
1407+
dataValues={[{ dataValue: 2 }, { dataValue: 2.5 }, { dataValue: 3.5 }]}
14081408
style={lineAnnotationStyle}
14091409
marker={<div style={{ background: 'red', width: 10, height: 10 }} />}
14101410
/>

0 commit comments

Comments
 (0)