Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ use 'nice' axis ticks for linear scales / TAS-795 #4404

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 16 additions & 2 deletions packages/@ourworldindata/grapher/src/axis/Axis.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down Expand Up @@ -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", () => {
Expand Down
49 changes: 45 additions & 4 deletions packages/@ourworldindata/grapher/src/axis/Axis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
ValueRange,
cloneDeep,
OwidVariableRoundingMode,
last,
} from "@ourworldindata/utils"
import { AxisConfig, AxisManager } from "./AxisConfig"
import { MarkdownTextWrap } from "@ourworldindata/components"
Expand Down Expand Up @@ -217,11 +218,48 @@ abstract class AbstractAxis {
})
}

private static makeScaleNice(
scale: ScaleLinear<number, number>,
totalTicksTarget: number
): { scale: ScaleLinear<number, number>; 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
Expand Down Expand Up @@ -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,
}))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading