diff --git a/packages/@ourworldindata/grapher/src/axis/Axis.test.ts b/packages/@ourworldindata/grapher/src/axis/Axis.test.ts index a24e55fbf5..2d135aa335 100755 --- a/packages/@ourworldindata/grapher/src/axis/Axis.test.ts +++ b/packages/@ourworldindata/grapher/src/axis/Axis.test.ts @@ -7,7 +7,7 @@ import { SynthesizeGDPTable, } from "@ourworldindata/core-table" import { AxisConfig } from "./AxisConfig" -import { AxisAlign } from "@ourworldindata/utils" +import { AxisAlign, last } from "@ourworldindata/utils" it("can create an axis", () => { const axisConfig = new AxisConfig({ @@ -88,7 +88,21 @@ it("respects nice parameter", () => { axis.range = [0, 300] const tickValues = axis.getTickValues() expect(tickValues[0].value).toEqual(0) - expect(tickValues[tickValues.length - 1].value).toEqual(100) + expect(last(tickValues)?.value).toEqual(100) +}) + +it("fine-tunes d3's nice implementation", () => { + const config: AxisConfigInterface = { + min: 0.0001, + max: 90.0001, + maxTicks: 10, + nice: true, + } + const axis = new AxisConfig(config).toVerticalAxis() + axis.range = [0, 300] + const tickValues = axis.getTickValues() + expect(tickValues[0].value).toEqual(0) + expect(last(tickValues)?.value).toEqual(90) }) it("creates compact labels", () => { diff --git a/packages/@ourworldindata/grapher/src/axis/Axis.ts b/packages/@ourworldindata/grapher/src/axis/Axis.ts index e3bbb71c11..7b18dc4427 100644 --- a/packages/@ourworldindata/grapher/src/axis/Axis.ts +++ b/packages/@ourworldindata/grapher/src/axis/Axis.ts @@ -21,6 +21,7 @@ import { ValueRange, cloneDeep, OwidVariableRoundingMode, + last, } from "@ourworldindata/utils" import { AxisConfig, AxisManager } from "./AxisConfig" import { MarkdownTextWrap } from "@ourworldindata/components" @@ -217,11 +218,48 @@ abstract class AbstractAxis { }) } + private static makeScaleNice( + scale: ScaleLinear, + totalTicksTarget: number + ): { scale: ScaleLinear; ticks?: number[] } { + let ticks = scale.ticks(totalTicksTarget) + + // use d3's nice function when there is only one tick + if (ticks.length < 2) return { scale: scale.nice(totalTicksTarget) } + + const tickStep = ticks[1] - ticks[0] + const firstTick = ticks[0] + const lastTick = last(ticks)! + + // if the the max or min value exceeds the last grid line by more than 25%, + // expand the domain to include an additional grid line + const [minValue, maxValue] = scale.domain() + if (maxValue > lastTick + 0.25 * tickStep) { + scale.domain([scale.domain()[0], lastTick + tickStep]) + ticks = [...ticks, lastTick + tickStep] + } + if (minValue < firstTick - 0.25 * tickStep) { + scale.domain([firstTick - tickStep, scale.domain()[1]]) + ticks = [firstTick - tickStep, ...ticks] + } + + return { scale, ticks } + } + + private niceTicks?: number[] @computed private get d3_scale(): Scale { - const d3Scale = - this.scaleType === ScaleType.log ? scaleLog : scaleLinear + const isLogScale = this.scaleType === ScaleType.log + const d3Scale = isLogScale ? scaleLog : scaleLinear let scale = d3Scale().domain(this.domain).range(this.range) - scale = this.nice ? scale.nice(this.totalTicksTarget) : scale + + if (this.nice) { + const { scale: niceScale, ticks: niceTicks } = + AbstractAxis.makeScaleNice(scale, this.totalTicksTarget) + scale = niceScale + this.niceTicks = niceTicks + } else { + this.niceTicks = undefined + } if (this.config.domainValues) { // compute bandwidth and adjust the scale @@ -362,9 +400,12 @@ abstract class AbstractAxis { } } } else { + const d3_ticks = + this.niceTicks ?? d3_scale.ticks(this.totalTicksTarget) + // Only use priority 2 here because we want the start / end ticks // to be priority 1 - ticks = d3_scale.ticks(this.totalTicksTarget).map((tickValue) => ({ + ticks = d3_ticks.map((tickValue) => ({ value: tickValue, priority: 2, })) diff --git a/packages/@ourworldindata/grapher/src/facetChart/FacetChart.tsx b/packages/@ourworldindata/grapher/src/facetChart/FacetChart.tsx index e66b7faa5b..4ed977e511 100644 --- a/packages/@ourworldindata/grapher/src/facetChart/FacetChart.tsx +++ b/packages/@ourworldindata/grapher/src/facetChart/FacetChart.tsx @@ -280,8 +280,7 @@ export class FacetChart // We infer that the user cares about the trend if the axis is not uniform // and the metrics on all facets are the same - const careAboutTrend = - !this.uniformYAxis && this.facetStrategy === FacetStrategy.entity + const careAboutTrend = !this.uniformYAxis if (careAboutTrend) { // Force disable nice axes if we care about the trend, // because nice axes misrepresent trends. diff --git a/packages/@ourworldindata/grapher/src/lineCharts/LineChart.tsx b/packages/@ourworldindata/grapher/src/lineCharts/LineChart.tsx index 8a87922921..d89d77d7e3 100644 --- a/packages/@ourworldindata/grapher/src/lineCharts/LineChart.tsx +++ b/packages/@ourworldindata/grapher/src/lineCharts/LineChart.tsx @@ -1461,9 +1461,9 @@ export class LineChart } @computed private get yAxisConfig(): AxisConfig { - // TODO: enable nice axis ticks for linear scales return new AxisConfig( { + nice: this.manager.yAxisConfig?.scaleType !== ScaleType.log, // if we only have a single y value (probably 0), we want the // horizontal axis to be at the bottom of the chart. // see https://github.com/owid/owid-grapher/pull/975#issuecomment-890798547 diff --git a/packages/@ourworldindata/grapher/src/stackedCharts/AbstractStackedChart.tsx b/packages/@ourworldindata/grapher/src/stackedCharts/AbstractStackedChart.tsx index 5b40a3086b..812da2c381 100644 --- a/packages/@ourworldindata/grapher/src/stackedCharts/AbstractStackedChart.tsx +++ b/packages/@ourworldindata/grapher/src/stackedCharts/AbstractStackedChart.tsx @@ -253,8 +253,13 @@ export class AbstractStackedChart } @computed private get yAxisConfig(): AxisConfig { - // TODO: enable nice axis ticks for linear scales - return new AxisConfig(this.manager.yAxisConfig, this) + return new AxisConfig( + { + nice: true, + ...this.manager.yAxisConfig, + }, + this + ) } // implemented in subclasses