Skip to content

Commit

Permalink
Merge pull request #41 from koorosh/barchart-scaling
Browse files Browse the repository at this point in the history
fix barCharts rendering for zero values
  • Loading branch information
koorosh authored Nov 25, 2020
2 parents 93ac0c0 + 2b275de commit 72ff0ad
Show file tree
Hide file tree
Showing 7 changed files with 57 additions and 7 deletions.
7 changes: 4 additions & 3 deletions src/barCharts/barChartFactory.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { Tooltip } from "src/index";
import classNames from "classnames/bind";
import styles from "./barCharts.module.scss";
import { NumericStatLegend } from "./numericStatLegend";
import { normalizeClosedDomain } from "./utils";

const cx = classNames.bind(styles);

Expand Down Expand Up @@ -41,14 +42,14 @@ export function barChartFactory<T>(
rows,
stdDevAccessor ? getTotalWithStdDev : getTotal,
);

const domain = normalizeClosedDomain([0, extent[1]]);
const scale = scaleLinear()
.domain([0, extent[1]])
.domain(domain)
.range([0, 100]);

return (d: T) => {
if (rows.length === 0) {
scale.domain([0, getTotal(d)]);
scale.domain(normalizeClosedDomain([0, getTotal(d)]));
}

let sum = 0;
Expand Down
14 changes: 14 additions & 0 deletions src/barCharts/barCharts.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
rowsBarChart,
} from "./barCharts";
import statementsPagePropsFixture from "src/statementsPage/components/statementsPage.fixture";
import Long from "long";

const { statements } = statementsPagePropsFixture;

Expand Down Expand Up @@ -67,6 +68,19 @@ storiesOf("BarCharts/within column (150px)", module)
const chartFactory = retryBarChart(statements);
return chartFactory(statements[0]);
})
.add("empty retryBarChart", () => {
const withoutRetries = statements.map(s => ({
...s,
stats: {
...s.stats,
count: Long.fromNumber(0),
first_attempt_count: Long.fromNumber(0),
max_retries: Long.fromNumber(0),
},
}));
const chartFactory = retryBarChart(withoutRetries);
return chartFactory(withoutRetries[0]);
})
.add("rowsBarChart", () => {
const chartFactory = rowsBarChart(statements);
return chartFactory(statements[0]);
Expand Down
5 changes: 3 additions & 2 deletions src/barCharts/latencyBreakdown.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { Duration } from "src/util/format";
import { Tooltip } from "src/index";
import classNames from "classnames/bind";
import styles from "./barCharts.module.scss";
import { clamp } from "./utils";
import { clamp, normalizeClosedDomain } from "./utils";

type StatementStatistics = protos.cockroach.server.serverpb.StatementsResponse.ICollectedStatementStatistics;
const cx = classNames.bind(styles);
Expand All @@ -33,9 +33,10 @@ export function latencyBreakdown(s: StatementStatistics) {
);

const format = (v: number) => Duration(v * 1e9);
const domain = normalizeClosedDomain([0, max]);

const scale = scaleLinear()
.domain([0, max])
.domain(domain)
.range([0, 100]);

return {
Expand Down
5 changes: 3 additions & 2 deletions src/barCharts/rowsBrealdown.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
import { scaleLinear } from "d3-scale";
import { stdDevLong } from "src/util/appStats";
import { formatTwoPlaces } from "./utils";
import { formatTwoPlaces, normalizeClosedDomain } from "./utils";
import * as protos from "@cockroachlabs/crdb-protobuf-client";

type StatementStatistics = protos.cockroach.server.serverpb.StatementsResponse.ICollectedStatementStatistics;

export function rowsBreakdown(s: StatementStatistics) {
const mean = s.stats.num_rows.mean;
const sd = stdDevLong(s.stats.num_rows, s.stats.count);
const domain = normalizeClosedDomain([0, mean + sd]);

const scale = scaleLinear()
.domain([0, mean + sd])
.domain(domain)
.range([0, 100]);

return {
Expand Down
14 changes: 14 additions & 0 deletions src/barCharts/utils.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { assert } from "chai";
import { normalizeClosedDomain } from "./utils";

describe("barCharts utils", () => {
describe("normalizeClosedDomain", () => {
it("returns input args if domain values are not equal", () => {
assert.deepStrictEqual(normalizeClosedDomain([10, 15]), [10, 15]);
});

it("returns increased end range by 1 if input start and end values are equal", () => {
assert.deepStrictEqual(normalizeClosedDomain([10, 10]), [10, 11]);
});
});
});
17 changes: 17 additions & 0 deletions src/barCharts/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,20 @@ export function approximify(value: number) {

return "" + Math.round(value);
}

/**
* normalizeClosedDomain increases collapsed domain when start and end range are equal.
* @description
* This function preserves behavior introduced by following issue in d3-scale library (starting from 2.2 version)
* https://github.com/d3/d3-scale/issues/117
* It is expected for scaling withing closed domain range, the start value is returned.
* @example
* scaleLinear().domain([0, 0])(0) // --> 0.5
* scaleLinear().domain(normalizeClosedDomain([0, 0]))(0) // --> 0
*/
export function normalizeClosedDomain([d0, d1]: Tuple<number>): Tuple<number> {
if (d0 === d1) {
return [d0, d1 + 1];
}
return [d0, d1];
}
2 changes: 2 additions & 0 deletions src/declaration.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,5 @@ type ConstructorType = new (...args: any) => any;
type FirstConstructorParameter<
P extends ConstructorType
> = ConstructorParameters<P>[0];

type Tuple<T> = [T, T];

0 comments on commit 72ff0ad

Please sign in to comment.