From 8b674853d492cb9a2d773eddafdafbab8fdd7e9d Mon Sep 17 00:00:00 2001 From: Emma Cunningham Date: Wed, 3 Jul 2019 08:15:53 -0700 Subject: [PATCH 1/6] fix: fix tooltip position for single value xScale --- src/lib/utils/scales/scale_band.ts | 3 ++ src/lib/utils/scales/scale_continuous.ts | 9 ++++++ src/lib/utils/scales/scales.ts | 1 + src/state/chart_state.ts | 3 ++ src/state/crosshair_utils.ts | 18 +++++++++++ src/state/test/interactions.test.ts | 24 +++++++++++++++ stories/bar_chart.tsx | 38 ++++++++++++++++++++++-- 7 files changed, 94 insertions(+), 2 deletions(-) diff --git a/src/lib/utils/scales/scale_band.ts b/src/lib/utils/scales/scale_band.ts index 28035f9edb..bb2fef0b15 100644 --- a/src/lib/utils/scales/scale_band.ts +++ b/src/lib/utils/scales/scale_band.ts @@ -62,6 +62,9 @@ export class ScaleBand implements Scale { invertWithStep(value: any) { return this.invertedScale(value); } + isSingleValue() { + return this.domain.length < 2; + } } export function isOrdinalScale(scale: Scale): scale is ScaleBand { diff --git a/src/lib/utils/scales/scale_continuous.ts b/src/lib/utils/scales/scale_continuous.ts index e335cf7cea..dde9f6a561 100644 --- a/src/lib/utils/scales/scale_continuous.ts +++ b/src/lib/utils/scales/scale_continuous.ts @@ -201,6 +201,15 @@ export class ScaleContinuous implements Scale { } return prevValue; } + isSingleValue() { + if (this.domain.length < 2) { + return true; + } + + const min = this.domain[0]; + const max = this.domain[this.domain.length - 1]; + return max === min; + } } export function isContinuousScale(scale: Scale): scale is ScaleContinuous { diff --git a/src/lib/utils/scales/scales.ts b/src/lib/utils/scales/scales.ts index 8c8ef0d92f..cc37a4ad36 100644 --- a/src/lib/utils/scales/scales.ts +++ b/src/lib/utils/scales/scales.ts @@ -5,6 +5,7 @@ export interface Scale { scale: (value: any) => number; invert: (value: number) => any; invertWithStep: (value: number, data: any[]) => any; + isSingleValue: () => boolean; bandwidth: number; minInterval: number; type: ScaleType; diff --git a/src/state/chart_state.ts b/src/state/chart_state.ts index 55d2d73e86..9546fe5b11 100644 --- a/src/state/chart_state.ts +++ b/src/state/chart_state.ts @@ -301,11 +301,14 @@ export class ChartStore { const updatedCursorLine = getCursorLinePosition(this.chartRotation, this.chartDimensions, this.cursorPosition); Object.assign(this.cursorLinePosition, updatedCursorLine); + const isSingleValueXScale = this.xScale.isSingleValue(); + this.tooltipPosition.transform = getTooltipPosition( this.chartDimensions, this.chartRotation, this.cursorBandPosition, this.cursorPosition, + isSingleValueXScale, ); // get the elements on at this cursor position diff --git a/src/state/crosshair_utils.ts b/src/state/crosshair_utils.ts index ec1c564429..e05a6b5e1e 100644 --- a/src/state/crosshair_utils.ts +++ b/src/state/crosshair_utils.ts @@ -122,6 +122,7 @@ export function getTooltipPosition( chartRotation: Rotation, cursorBandPosition: Dimensions, cursorPosition: { x: number; y: number }, + isSingleValueXScale: boolean, ): string { const isHorizontalRotated = isHorizontalRotation(chartRotation); const hPosition = getHorizontalTooltipPosition( @@ -129,12 +130,14 @@ export function getTooltipPosition( cursorBandPosition, chartDimensions, isHorizontalRotated, + isSingleValueXScale, ); const vPosition = getVerticalTooltipPosition( cursorPosition.y, cursorBandPosition, chartDimensions, isHorizontalRotated, + isSingleValueXScale, ); const xTranslation = `translateX(${hPosition.position}px) translateX(-${hPosition.offset}%)`; const yTranslation = `translateY(${vPosition.position}px) translateY(-${vPosition.offset}%)`; @@ -146,9 +149,17 @@ export function getHorizontalTooltipPosition( cursorBandPosition: Dimensions, chartDimensions: Dimensions, isHorizontalRotated: boolean, + isSingleValueXScale: boolean, padding: number = 20, ): { offset: number; position: number } { if (isHorizontalRotated) { + if (isSingleValueXScale) { + return { + offset: 0, + position: cursorBandPosition.left, + }; + } + if (cursorXPosition <= chartDimensions.width / 2) { return { offset: 0, @@ -180,6 +191,7 @@ export function getVerticalTooltipPosition( cursorBandPosition: Dimensions, chartDimensions: Dimensions, isHorizontalRotated: boolean, + isSingleValueXScale: boolean, padding: number = 20, ): { offset: number; @@ -198,6 +210,12 @@ export function getVerticalTooltipPosition( }; } } else { + if (isSingleValueXScale) { + return { + offset: 0, + position: cursorBandPosition.top, + }; + } if (cursorYPosition <= chartDimensions.height / 2) { return { offset: 0, diff --git a/src/state/test/interactions.test.ts b/src/state/test/interactions.test.ts index ec7d06c434..f21e7da8b8 100644 --- a/src/state/test/interactions.test.ts +++ b/src/state/test/interactions.test.ts @@ -8,6 +8,7 @@ import { ScaleContinuous } from '../../lib/utils/scales/scale_continuous'; import { ScaleType } from '../../lib/utils/scales/scales'; import { ChartStore } from '../chart_state'; import { computeSeriesDomains } from '../utils'; +import { ScaleBand } from '../../lib/utils/scales/scale_band'; const SPEC_ID = getSpecId('spec_1'); const GROUP_ID = getGroupId('group_1'); @@ -349,4 +350,27 @@ function mouseOverTestSuite(scaleType: ScaleType) { expect(onOverListener.mock.calls[0][0]).toEqual([indexedGeom2Blue.value]); expect(onOutListener).toBeCalledTimes(0); }); + + test('can position tooltip within chart when xScale is a single value scale [horizontal rotation]', () => { + const singleValueScale = + store.xScale!.type === ScaleType.Ordinal + ? new ScaleBand(['a'], [0, 0]) + : new ScaleContinuous(ScaleType.Linear, [1, 1], [0, 0]); + store.xScale = singleValueScale; + store.setCursorPosition(chartLeft + 99, chartTop + 99); + const expectedTransform = `translateX(${chartLeft}px) translateX(-0%) translateY(109px) translateY(-100%)`; + expect(store.tooltipPosition.transform).toBe(expectedTransform); + }); + + test('can position tooltip within chart when xScale is a single value scale [vertical rotation]', () => { + const singleValueScale = + store.xScale!.type === ScaleType.Ordinal + ? new ScaleBand(['a'], [0, 0]) + : new ScaleContinuous(ScaleType.Linear, [1, 1], [0, 0]); + store.chartRotation = 90; + store.xScale = singleValueScale; + store.setCursorPosition(chartLeft + 99, chartTop + 99); + const expectedTransform = `translateX(109px) translateX(-100%) translateY(${chartTop}px) translateY(-0%)`; + expect(store.tooltipPosition.transform).toBe(expectedTransform); + }); } diff --git a/stories/bar_chart.tsx b/stories/bar_chart.tsx index c7f5700431..07129dd313 100644 --- a/stories/bar_chart.tsx +++ b/stories/bar_chart.tsx @@ -856,8 +856,41 @@ storiesOf('Bar Chart', module) ); }) .add('single data chart', () => { + const hasCustomDomain = boolean('has custom domain', false); + const xDomain = hasCustomDomain + ? { + min: 0, + } + : undefined; + + const chartRotation = select( + 'chartRotation', + { + '0 deg': 0, + '90 deg': 90, + '-90 deg': -90, + '180 deg': 180, + }, + 0, + ); + + const theme = mergeWithDefaultTheme( + { + scales: { + barsPadding: number('bars padding', 0.25, { + range: true, + min: 0, + max: 1, + step: 0.1, + }), + }, + }, + LIGHT_THEME, + ); + return ( + ); From 023e27ea20b94e596111b0712261c4683e415fd7 Mon Sep 17 00:00:00 2001 From: Emma Cunningham Date: Wed, 3 Jul 2019 08:22:32 -0700 Subject: [PATCH 2/6] refactor: group single value scale tests --- src/state/test/interactions.test.ts | 41 ++++++++++++++--------------- 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/src/state/test/interactions.test.ts b/src/state/test/interactions.test.ts index f21e7da8b8..f910ba696d 100644 --- a/src/state/test/interactions.test.ts +++ b/src/state/test/interactions.test.ts @@ -351,26 +351,25 @@ function mouseOverTestSuite(scaleType: ScaleType) { expect(onOutListener).toBeCalledTimes(0); }); - test('can position tooltip within chart when xScale is a single value scale [horizontal rotation]', () => { - const singleValueScale = - store.xScale!.type === ScaleType.Ordinal - ? new ScaleBand(['a'], [0, 0]) - : new ScaleContinuous(ScaleType.Linear, [1, 1], [0, 0]); - store.xScale = singleValueScale; - store.setCursorPosition(chartLeft + 99, chartTop + 99); - const expectedTransform = `translateX(${chartLeft}px) translateX(-0%) translateY(109px) translateY(-100%)`; - expect(store.tooltipPosition.transform).toBe(expectedTransform); - }); - - test('can position tooltip within chart when xScale is a single value scale [vertical rotation]', () => { - const singleValueScale = - store.xScale!.type === ScaleType.Ordinal - ? new ScaleBand(['a'], [0, 0]) - : new ScaleContinuous(ScaleType.Linear, [1, 1], [0, 0]); - store.chartRotation = 90; - store.xScale = singleValueScale; - store.setCursorPosition(chartLeft + 99, chartTop + 99); - const expectedTransform = `translateX(109px) translateX(-100%) translateY(${chartTop}px) translateY(-0%)`; - expect(store.tooltipPosition.transform).toBe(expectedTransform); + describe('can position tooltip within chart when xScale is a single value scale', () => { + beforeEach(() => { + const singleValueScale = + store.xScale!.type === ScaleType.Ordinal + ? new ScaleBand(['a'], [0, 0]) + : new ScaleContinuous(ScaleType.Linear, [1, 1], [0, 0]); + store.xScale = singleValueScale; + }); + test('horizontal chart rotation', () => { + store.setCursorPosition(chartLeft + 99, chartTop + 99); + const expectedTransform = `translateX(${chartLeft}px) translateX(-0%) translateY(109px) translateY(-100%)`; + expect(store.tooltipPosition.transform).toBe(expectedTransform); + }); + + test('vertical chart rotation', () => { + store.chartRotation = 90; + store.setCursorPosition(chartLeft + 99, chartTop + 99); + const expectedTransform = `translateX(109px) translateX(-100%) translateY(${chartTop}px) translateY(-0%)`; + expect(store.tooltipPosition.transform).toBe(expectedTransform); + }); }); } From 1783db1b3c17e25448f8c7b73e6c2a851430d9c8 Mon Sep 17 00:00:00 2001 From: Emma Cunningham Date: Fri, 5 Jul 2019 06:30:55 -0700 Subject: [PATCH 3/6] test: add tests for isSingleValue --- src/lib/utils/scales/scale_band.test.ts | 10 ++++++++++ src/lib/utils/scales/scale_continuous.test.ts | 15 +++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/src/lib/utils/scales/scale_band.test.ts b/src/lib/utils/scales/scale_band.test.ts index 261ec51bc5..afeb7d90ed 100644 --- a/src/lib/utils/scales/scale_band.test.ts +++ b/src/lib/utils/scales/scale_band.test.ts @@ -97,4 +97,14 @@ describe('Scale Band', () => { expect(scale.invert(99.99999)).toBe('d'); expect(scale.invert(100)).toBe('d'); }); + describe('isSingleValue', () => { + it('should return true for single value scale', () => { + const scale = new ScaleBand(['a'], [0, 100]); + expect(scale.isSingleValue()).toBe(true); + }); + it('should return false for multi value scale', () => { + const scale = new ScaleBand(['a', 'b'], [0, 100]); + expect(scale.isSingleValue()).toBe(false); + }); + }); }); diff --git a/src/lib/utils/scales/scale_continuous.test.ts b/src/lib/utils/scales/scale_continuous.test.ts index 6375c21ef6..11e19f4aa8 100644 --- a/src/lib/utils/scales/scale_continuous.test.ts +++ b/src/lib/utils/scales/scale_continuous.test.ts @@ -140,6 +140,21 @@ describe('Scale Continuous', () => { expect(scaleLinear.invertWithStep(90, data)).toBe(90); }); + describe('isSingleValue', () => { + test('should return true for domain with fewer than 2 values', () => { + const scale = new ScaleContinuous(ScaleType.Linear, [], [0, 100]); + expect(scale.isSingleValue()).toBe(true); + }); + test('should return true for domain with equal min and max values', () => { + const scale = new ScaleContinuous(ScaleType.Linear, [1, 1], [0, 100]); + expect(scale.isSingleValue()).toBe(true); + }); + test('should return false for domain with differing min and max values', () => { + const scale = new ScaleContinuous(ScaleType.Linear, [1, 2], [0, 100]); + expect(scale.isSingleValue()).toBe(false); + }); + }); + describe('time ticks', () => { const timezonesToTest = ['Asia/Tokyo', 'Europe/Berlin', 'UTC', 'America/New_York', 'America/Los_Angeles']; From c73e8de5934f0c1a171758d45adbff232979d146 Mon Sep 17 00:00:00 2001 From: Emma Cunningham Date: Fri, 5 Jul 2019 06:51:40 -0700 Subject: [PATCH 4/6] docs: add stories for linear and ordinal --- stories/bar_chart.tsx | 57 ++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 54 insertions(+), 3 deletions(-) diff --git a/stories/bar_chart.tsx b/stories/bar_chart.tsx index 07129dd313..cb644e62e7 100644 --- a/stories/bar_chart.tsx +++ b/stories/bar_chart.tsx @@ -855,7 +855,7 @@ storiesOf('Bar Chart', module) ); }) - .add('single data chart', () => { + .add('single data chart [linear]', () => { const hasCustomDomain = boolean('has custom domain', false); const xDomain = hasCustomDomain ? { @@ -901,12 +901,63 @@ storiesOf('Bar Chart', module) + + ); + }) + .add('single data chart [ordinal]', () => { + const hasCustomDomain = boolean('has custom domain', false); + const xDomain = hasCustomDomain ? ['a', 'b'] : undefined; + + const chartRotation = select( + 'chartRotation', + { + '0 deg': 0, + '90 deg': 90, + '-90 deg': -90, + '180 deg': 180, + }, + 0, + ); + + const theme = mergeWithDefaultTheme( + { + scales: { + barsPadding: number('bars padding', 0.25, { + range: true, + min: 0, + max: 1, + step: 0.1, + }), + }, + }, + LIGHT_THEME, + ); + + return ( + + + + Number(d).toFixed(2)} + /> + + ); From c2c13bd9e1e2aa97daa11f6eade317e4d80325ce Mon Sep 17 00:00:00 2001 From: Emma Cunningham Date: Tue, 9 Jul 2019 07:43:16 -0700 Subject: [PATCH 5/6] refactor: use partial --- stories/bar_chart.tsx | 42 ++++++++++++++++++------------------------ 1 file changed, 18 insertions(+), 24 deletions(-) diff --git a/stories/bar_chart.tsx b/stories/bar_chart.tsx index cb644e62e7..6c0891a4f3 100644 --- a/stories/bar_chart.tsx +++ b/stories/bar_chart.tsx @@ -874,19 +874,16 @@ storiesOf('Bar Chart', module) 0, ); - const theme = mergeWithDefaultTheme( - { - scales: { - barsPadding: number('bars padding', 0.25, { - range: true, - min: 0, - max: 1, - step: 0.1, - }), - }, + const theme = { + scales: { + barsPadding: number('bars padding', 0.25, { + range: true, + min: 0, + max: 1, + step: 0.1, + }), }, - LIGHT_THEME, - ); + }; return ( @@ -926,19 +923,16 @@ storiesOf('Bar Chart', module) 0, ); - const theme = mergeWithDefaultTheme( - { - scales: { - barsPadding: number('bars padding', 0.25, { - range: true, - min: 0, - max: 1, - step: 0.1, - }), - }, + const theme = { + scales: { + barsPadding: number('bars padding', 0.25, { + range: true, + min: 0, + max: 1, + step: 0.1, + }), }, - LIGHT_THEME, - ); + }; return ( From 121a8fb8cf96bb633f3fdb0d041044462de5f475 Mon Sep 17 00:00:00 2001 From: Emma Cunningham Date: Tue, 9 Jul 2019 13:50:51 -0700 Subject: [PATCH 6/6] feat: set tooltip to follow when single value scale --- src/lib/utils/interactions.ts | 3 +++ src/state/chart_state.test.ts | 18 ++++++++++++++++++ src/state/chart_state.ts | 7 +++++++ 3 files changed, 28 insertions(+) diff --git a/src/lib/utils/interactions.ts b/src/lib/utils/interactions.ts index 481505c11e..b862622829 100644 --- a/src/lib/utils/interactions.ts +++ b/src/lib/utils/interactions.ts @@ -72,6 +72,9 @@ export function isCrosshairTooltipType(type: TooltipType) { export function isFollowTooltipType(type: TooltipType) { return type === TooltipType.Follow; } +export function isNoneTooltipType(type: TooltipType) { + return type === TooltipType.None; +} export function areIndexedGeometryArraysEquals(arr1: IndexedGeometry[], arr2: IndexedGeometry[]) { if (arr1.length !== arr2.length) { diff --git a/src/state/chart_state.test.ts b/src/state/chart_state.test.ts index 3ce4e5e96b..1d9ef63b01 100644 --- a/src/state/chart_state.test.ts +++ b/src/state/chart_state.test.ts @@ -866,4 +866,22 @@ describe('Chart Store', () => { expect(store.isCrosshairCursorVisible.get()).toBe(false); }); }); + test('should set tooltip type to follow when single value x scale', () => { + const singleValueSpec: BarSeriesSpec = { + id: SPEC_ID, + groupId: GROUP_ID, + seriesType: 'bar', + yScaleToDataExtent: false, + data: [{ x: 1, y: 1, g: 0 }], + xAccessor: 'x', + yAccessors: ['y'], + xScaleType: ScaleType.Linear, + yScaleType: ScaleType.Linear, + hideInLegend: false, + }; + + store.addSeriesSpec(singleValueSpec); + store.computeChart(); + expect(store.tooltipType.get()).toBe(TooltipType.Follow); + }); }); diff --git a/src/state/chart_state.ts b/src/state/chart_state.ts index 9546fe5b11..5dde2c88b3 100644 --- a/src/state/chart_state.ts +++ b/src/state/chart_state.ts @@ -56,6 +56,7 @@ import { getValidYPosition, isCrosshairTooltipType, isFollowTooltipType, + isNoneTooltipType, TooltipType, TooltipValue, TooltipValueFormatter, @@ -876,6 +877,12 @@ export class ChartStore { // console.log({ seriesGeometries }); this.geometries = seriesGeometries.geometries; this.xScale = seriesGeometries.scales.xScale; + + const isSingleValueXScale = this.xScale.isSingleValue(); + if (isSingleValueXScale && !isNoneTooltipType(this.tooltipType.get())) { + this.tooltipType.set(TooltipType.Follow); + } + this.yScales = seriesGeometries.scales.yScales; this.geometriesIndex = seriesGeometries.geometriesIndex; this.geometriesIndexKeys = [...this.geometriesIndex.keys()].sort(compareByValueAsc);