Skip to content

Commit

Permalink
fix(line_annotation): use scaleAndValidate for line annotations (#236)
Browse files Browse the repository at this point in the history
  • Loading branch information
emmacunningham authored Jun 14, 2019
1 parent 01d2b60 commit 48b180a
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 13 deletions.
3 changes: 3 additions & 0 deletions src/state/annotation_marker.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ describe('annotation marker', () => {
xScale,
Position.Left,
0,
false,
);
const expectedDimensions = [
{
Expand Down Expand Up @@ -96,6 +97,7 @@ describe('annotation marker', () => {
xScale,
Position.Left,
0,
false,
);
const expectedDimensions = [
{
Expand Down Expand Up @@ -135,6 +137,7 @@ describe('annotation marker', () => {
xScale,
Position.Left,
0,
false,
);
const expectedDimensions = [
{
Expand Down
54 changes: 54 additions & 0 deletions src/state/annotation_utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,7 @@ describe('annotation utils', () => {
xScale,
Position.Left,
0,
false,
);
const expectedDimensions = [
{
Expand Down Expand Up @@ -239,6 +240,7 @@ describe('annotation utils', () => {
xScale,
Position.Right,
0,
false,
);
const expectedDimensions = [
{
Expand Down Expand Up @@ -275,6 +277,7 @@ describe('annotation utils', () => {
xScale,
Position.Left,
0,
false,
);
const expectedDimensions = [
{
Expand Down Expand Up @@ -309,6 +312,7 @@ describe('annotation utils', () => {
xScale,
Position.Left,
0,
false,
);
expect(dimensions).toEqual(null);
});
Expand Down Expand Up @@ -336,6 +340,7 @@ describe('annotation utils', () => {
xScale,
Position.Left,
0,
false,
);
const expectedDimensions = [
{
Expand Down Expand Up @@ -370,6 +375,7 @@ describe('annotation utils', () => {
xScale,
Position.Top,
0,
false,
);
const expectedDimensions = [
{
Expand Down Expand Up @@ -404,6 +410,7 @@ describe('annotation utils', () => {
xScale,
Position.Bottom,
0,
false,
);
const expectedDimensions = [
{
Expand All @@ -415,6 +422,41 @@ describe('annotation utils', () => {
expect(dimensions).toEqual(expectedDimensions);
});

test('should compute line annotation dimensions for xDomain in histogramMode with extended upper bound', () => {
const chartRotation: Rotation = 0;
const yScales: Map<GroupId, Scale> = new Map();
const xScale: Scale = continuousScale;

const annotationId = getAnnotationId('foo-line');
const lineAnnotation: LineAnnotationSpec = {
annotationType: 'line',
annotationId,
domainType: AnnotationDomainTypes.XDomain,
dataValues: [{ dataValue: 10.5, details: 'foo' }],
groupId,
style: DEFAULT_ANNOTATION_LINE_STYLE,
};

const dimensions = computeLineAnnotationDimensions(
lineAnnotation,
chartDimensions,
chartRotation,
yScales,
xScale,
Position.Bottom,
0,
true,
);
const expectedDimensions = [
{
position: [105, 0, 105, 20],
details: { detailsText: 'foo', headerText: '10.5' },
tooltipLinePosition: [105, 0, 105, 20],
},
];
expect(dimensions).toEqual(expectedDimensions);
});

test('should compute line annotation dimensions for xDomain on a xScale (chartRotation 90, ordinal scale)', () => {
const chartRotation: Rotation = 90;
const yScales: Map<GroupId, Scale> = new Map();
Expand All @@ -439,6 +481,7 @@ describe('annotation utils', () => {
xScale,
Position.Left,
0,
false,
);
const expectedDimensions = [
{
Expand Down Expand Up @@ -474,6 +517,7 @@ describe('annotation utils', () => {
xScale,
Position.Left,
0,
false,
);
const expectedDimensions = [
{
Expand Down Expand Up @@ -509,6 +553,7 @@ describe('annotation utils', () => {
xScale,
Position.Left,
0,
false,
);
const expectedDimensions = [
{
Expand Down Expand Up @@ -544,6 +589,7 @@ describe('annotation utils', () => {
xScale,
Position.Top,
0,
false,
);
const expectedDimensions = [
{
Expand Down Expand Up @@ -578,6 +624,7 @@ describe('annotation utils', () => {
xScale,
Position.Bottom,
0,
false,
);
const expectedDimensions = [
{
Expand Down Expand Up @@ -614,6 +661,7 @@ describe('annotation utils', () => {
xScale,
Position.Right,
0,
false,
);

expect(emptyXDimensions).toEqual([]);
Expand All @@ -635,6 +683,7 @@ describe('annotation utils', () => {
continuousScale,
Position.Right,
0,
false,
);

expect(invalidStringXDimensions).toEqual([]);
Expand All @@ -656,6 +705,7 @@ describe('annotation utils', () => {
continuousScale,
Position.Right,
0,
false,
);

expect(emptyOutOfBoundsXDimensions).toEqual([]);
Expand All @@ -677,6 +727,7 @@ describe('annotation utils', () => {
xScale,
Position.Right,
0,
false,
);

expect(emptyYDimensions).toEqual([]);
Expand All @@ -698,6 +749,7 @@ describe('annotation utils', () => {
xScale,
Position.Right,
0,
false,
);

expect(emptyOutOfBoundsYDimensions).toEqual([]);
Expand All @@ -719,6 +771,7 @@ describe('annotation utils', () => {
continuousScale,
Position.Right,
0,
false,
);

expect(invalidStringYDimensions).toEqual([]);
Expand All @@ -741,6 +794,7 @@ describe('annotation utils', () => {
continuousScale,
Position.Right,
0,
false,
);

expect(hiddenAnnotationDimensions).toEqual(null);
Expand Down
19 changes: 7 additions & 12 deletions src/state/annotation_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ export function computeXDomainLineAnnotationDimensions(
chartDimensions: Dimensions,
lineColor: string,
xScaleOffset: number,
enableHistogramMode: boolean,
marker?: JSX.Element,
markerDimensions?: { width: number; height: number },
): AnnotationLineProps[] {
Expand All @@ -187,6 +188,7 @@ export function computeXDomainLineAnnotationDimensions(
const markerOffsets = markerDimensions || { width: 0, height: 0 };
const lineProps: AnnotationLineProps[] = [];

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

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

const scaledXValue = xScale.scale(dataValue);
const scaledXValue = scaleAndValidateDatum(dataValue, xScale, alignWithTick);

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

if (isContinuous) {
const [domainStart, domainEnd] = xScale.domain;

if (domainStart > dataValue || domainEnd < dataValue) {
return;
}
}

const xDomainPosition = scaledXValue + offset;

let linePosition: AnnotationLinePosition = [0, 0, 0, 0];
Expand Down Expand Up @@ -280,6 +272,7 @@ export function computeLineAnnotationDimensions(
xScale: Scale,
axisPosition: Position,
xScaleOffset: number,
enableHistogramMode: boolean,
): AnnotationLineProps[] | null {
const { domainType, dataValues, marker, markerDimensions, hideLines } = annotationSpec;

Expand All @@ -304,6 +297,7 @@ export function computeLineAnnotationDimensions(
chartDimensions,
lineColor,
xScaleOffset,
enableHistogramMode,
marker,
markerDimensions,
);
Expand Down Expand Up @@ -545,6 +539,7 @@ export function computeAnnotationDimensions(
xScale,
annotationAxisPosition,
xScaleOffset - clusterOffset,
enableHistogramMode,
);

if (dimensions) {
Expand Down
2 changes: 1 addition & 1 deletion stories/bar_chart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1404,7 +1404,7 @@ storiesOf('Bar Chart', module)
<LineAnnotation
annotationId={getAnnotationId('line-annotation')}
domainType={AnnotationDomainTypes.XDomain}
dataValues={[{ dataValue: 2 }, { dataValue: 2.5 }]}
dataValues={[{ dataValue: 2 }, { dataValue: 2.5 }, { dataValue: 3.5 }]}
style={lineAnnotationStyle}
marker={<div style={{ background: 'red', width: 10, height: 10 }} />}
/>
Expand Down

0 comments on commit 48b180a

Please sign in to comment.