Skip to content

Commit

Permalink
fix(axis): misaligned axis with rotated histogram bar charts (opensea…
Browse files Browse the repository at this point in the history
  • Loading branch information
nickofthyme authored Sep 5, 2020
1 parent 867abbc commit a52a692
Show file tree
Hide file tree
Showing 19 changed files with 52 additions and 4 deletions.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
10 changes: 10 additions & 0 deletions packages/osd-charts/integration/tests/bar_stories.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,16 @@
import { common } from '../page_objects';

describe('Bar series stories', () => {
describe('[test] axis positions with histogram bar series', () => {
[0, 90, -90, 180].forEach((rotation) => {
it(`Should render correct axis - rotation ${rotation === -90 ? 'negative 90' : rotation}`, async () => {
await common.expectChartAtUrlToMatchScreenshot(
`http://localhost:9001/?path=/story/interactions--brush-selection-tool-on-histogram-time-charts&knob-debug=&knob-chartRotation=${rotation}`,
);
});
});
});

describe('[test] switch ordinal/linear x axis', () => {
it('using ordinal x axis', async () => {
await common.expectChartAtUrlToMatchScreenshot(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -355,9 +355,8 @@ describe('Axis computational utils', () => {

describe('getAvailableTicks', () => {
test('should compute to end of domain when histogram mode not enabled', () => {
const enableHistogramMode = false;
const scale = getScaleForAxisSpec(verticalAxisSpec, xDomain, [yDomain], 0, 0, 100, 0);
const axisPositions = getAvailableTicks(verticalAxisSpec, scale!, 0, enableHistogramMode, (v) => `${v}`);
const axisPositions = getAvailableTicks(verticalAxisSpec, scale!, 0, false, (v) => `${v}`, 0);
const expectedAxisPositions = [
{ label: '0', position: 100, value: 0 },
{ label: '0.1', position: 90, value: 0.1 },
Expand All @@ -374,6 +373,26 @@ describe('Axis computational utils', () => {
expect(axisPositions).toEqual(expectedAxisPositions);
});

test('should compute positions with rotational offset', () => {
const rotationalOffset = 2;
const scale = getScaleForAxisSpec(verticalAxisSpec, xDomain, [yDomain], 0, 0, 100, 0);
const axisPositions = getAvailableTicks(verticalAxisSpec, scale!, 0, false, (v) => `${v}`, rotationalOffset);
const expectedAxisPositions = [
{ label: '0', position: 100 + rotationalOffset, value: 0 },
{ label: '0.1', position: 90 + rotationalOffset, value: 0.1 },
{ label: '0.2', position: 80 + rotationalOffset, value: 0.2 },
{ label: '0.3', position: 70 + rotationalOffset, value: 0.3 },
{ label: '0.4', position: 60 + rotationalOffset, value: 0.4 },
{ label: '0.5', position: 50 + rotationalOffset, value: 0.5 },
{ label: '0.6', position: 40 + rotationalOffset, value: 0.6 },
{ label: '0.7', position: 30 + rotationalOffset, value: 0.7 },
{ label: '0.8', position: 20 + rotationalOffset, value: 0.8 },
{ label: '0.9', position: 10 + rotationalOffset, value: 0.9 },
{ label: '1', position: 0 + rotationalOffset, value: 1 },
];
expect(axisPositions).toEqual(expectedAxisPositions);
});

test('should extend ticks to domain + minInterval in histogram mode for linear scale', () => {
const enableHistogramMode = true;
const xBandDomain: XDomain = {
Expand All @@ -390,6 +409,7 @@ describe('Axis computational utils', () => {
1,
enableHistogramMode,
(v) => `${v}`,
0,
);
const histogramTickLabels = histogramAxisPositions.map(({ label }: AxisTick) => label);
expect(histogramTickLabels).toEqual(['0', '10', '20', '30', '40', '50', '60', '70', '80', '90', '100', '110']);
Expand All @@ -411,6 +431,7 @@ describe('Axis computational utils', () => {
1,
enableHistogramMode,
(v) => `${v}`,
0,
);
const histogramTickValues = histogramAxisPositions.map(({ value }: AxisTick) => value);

Expand Down Expand Up @@ -449,6 +470,7 @@ describe('Axis computational utils', () => {
1,
enableHistogramMode,
(v) => `${v}`,
0,
);
const histogramTickValues = histogramAxisPositions.map(({ value }: AxisTick) => value);
const expectedTickValues = [1560438420000, 1560438510000];
Expand Down
13 changes: 12 additions & 1 deletion packages/osd-charts/src/chart_types/xy_chart/utils/axis_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -511,6 +511,7 @@ export function getAvailableTicks(
totalBarsInCluster: number,
enableHistogramMode: boolean,
fallBackTickFormatter: TickFormatter,
rotationOffset: number,
tickFormatOptions?: TickFormatterOptions,
): AxisTick[] {
const ticks = scale.ticks();
Expand All @@ -534,7 +535,8 @@ export function getAvailableTicks(

const band = scale.bandwidth / (1 - scale.barsPadding);
const halfPadding = (band - scale.bandwidth) / 2;
const offset = enableHistogramMode ? -halfPadding : (scale.bandwidth * shift) / 2;
const offset =
(enableHistogramMode ? -halfPadding : (scale.bandwidth * shift) / 2) + (scale.isSingleValue() ? 0 : rotationOffset);
const tickFormatter = axisSpec.tickFormat ?? fallBackTickFormatter;

if (isSingleValueScale && hasAdditionalTicks) {
Expand Down Expand Up @@ -739,12 +741,20 @@ export function getAxisTicksPositions(
};
const { axisTitle, tickLine, tickLabel, gridLine } = axesStyles.get(id) ?? sharedAxesStyle;
const isVertical = isVerticalAxis(axisSpec.position);
// TODO: Find the true cause of the this offset error
const rotationOffset =
enableHistogramMode &&
((isVertical && [-90].includes(chartRotation)) || (!isVertical && [180].includes(chartRotation)))
? scale.step
: 0;

const allTicks = getAvailableTicks(
axisSpec,
scale,
totalGroupsCount,
enableHistogramMode,
isVertical ? fallBackTickFormatter : defaultTickFormatter,
rotationOffset,
tickFormatOptions,
);

Expand Down Expand Up @@ -791,6 +801,7 @@ export function getAxisTicksPositions(
axisVisibleTicks.set(id, visibleTicks);
axisTicks.set(id, allTicks);
});

return {
axisPositions,
axisTicks,
Expand Down
1 change: 1 addition & 0 deletions packages/osd-charts/src/mocks/scale/scale.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export class MockScale {
scaleOrThrow: jest.fn().mockImplementation((x) => x),
scale: jest.fn().mockImplementation((x) => x),
type: ScaleType.Linear,
step: 0,
bandwidth: 0,
bandwidthPadding: 0,
minInterval: 0,
Expand Down
4 changes: 4 additions & 0 deletions packages/osd-charts/src/scales/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ import { ScaleType } from './constants';
export interface Scale {
domain: any[];
range: number[];
/**
* Returns the distance between the starts of adjacent bands.
*/
step: number;
ticks: () => any[];
scaleOrThrow(value?: PrimitiveValue): number;
scale: (value?: PrimitiveValue) => number | null;
Expand Down
2 changes: 1 addition & 1 deletion packages/osd-charts/src/scales/scale_continuous.ts
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ export class ScaleContinuous implements Scale {
this.bandwidth = bandwidth * (1 - safeBarPadding);
this.bandwidthPadding = bandwidth * safeBarPadding;
this.d3Scale.range(range);
this.step = 0;
this.step = this.bandwidth + this.barsPadding + this.bandwidthPadding;
this.type = type;
this.range = range;
this.minInterval = minInterval;
Expand Down

0 comments on commit a52a692

Please sign in to comment.