diff --git a/adminSiteClient/EditorCustomizeTab.tsx b/adminSiteClient/EditorCustomizeTab.tsx index cdbc1cb260b..ae3cdf5bedc 100644 --- a/adminSiteClient/EditorCustomizeTab.tsx +++ b/adminSiteClient/EditorCustomizeTab.tsx @@ -577,9 +577,9 @@ export class EditorCustomizeTab< } resetButton={{ onClick: () => - (yAxisConfig.min = undefined), + (yAxisConfig.min = Infinity), disabled: - yAxisConfig.min === undefined, + yAxisConfig.min === Infinity, }} allowDecimal allowNegative @@ -592,9 +592,9 @@ export class EditorCustomizeTab< } resetButton={{ onClick: () => - (yAxisConfig.max = undefined), + (yAxisConfig.max = -Infinity), disabled: - yAxisConfig.max === undefined, + yAxisConfig.max === -Infinity, }} allowDecimal allowNegative diff --git a/db/migration/1729763649580-SetYAxisMinDefaultToZero.ts b/db/migration/1729763649580-SetYAxisMinDefaultToZero.ts new file mode 100644 index 00000000000..f9e39a91942 --- /dev/null +++ b/db/migration/1729763649580-SetYAxisMinDefaultToZero.ts @@ -0,0 +1,95 @@ +import { defaultGrapherConfig } from "@ourworldindata/grapher" +import { mergeGrapherConfigs } from "@ourworldindata/utils" +import { MigrationInterface, QueryRunner } from "typeorm" + +export class SetYAxisMinDefaultToZero1729763649580 + implements MigrationInterface +{ + public async up(queryRunner: QueryRunner): Promise { + // charts with inheritance disabled: + // set yAxis.min explicitly to "auto" for line charts + // that used to rely on "auto" being the default. + await queryRunner.query(` + -- sql + UPDATE chart_configs cc + JOIN charts c ON cc.id = c.configId + SET + -- using JSON_MERGE_PATCH instead of JSON_SET in case yAxis doesn't exist + cc.patch = JSON_MERGE_PATCH(cc.patch, '{"yAxis":{"min":"auto"}}') + WHERE + cc.full ->> '$.type' = 'LineChart' + AND c.isInheritanceEnabled IS FALSE + AND cc.patch ->> '$.yAxis.min' IS NULL + `) + + // charts with inheritance enabled: + // set yAxis.min explicitly to "auto" for line charts + // that used to rely on "auto" being the default. + // this is the case for charts where neither the patch config + // of the chart itself nor the indicator config have yAxis.min set + await queryRunner.query(` + -- sql + UPDATE chart_configs cc + JOIN charts c ON cc.id = c.configId + -- keep charts without parent indicator + LEFT JOIN charts_x_parents cxp ON cxp.chartId = c.id + LEFT JOIN variables v ON v.id = cxp.variableId + LEFT JOIN chart_configs cc_admin ON cc_admin.id = v.grapherConfigIdAdmin + SET + -- using JSON_MERGE_PATCH instead of JSON_SET in case yAxis doesn't exist + cc.patch = JSON_MERGE_PATCH(cc.patch, '{"yAxis":{"min":"auto"}}') + WHERE + cc.full ->> '$.type' = 'LineChart' + AND c.isInheritanceEnabled IS TRUE + AND cc.patch ->> '$.yAxis.min' IS NULL + AND ( + cc_admin.full IS NULL + OR cc_admin.full ->> '$.yAxis.min' IS NULL + ) + `) + + // recompute all full configs (we need to update all charts + // since the defaultGrapherConfig has changed) + const charts = await queryRunner.query(` + -- sql + SELECT + cc.id AS configId, + cc.patch AS patchConfig, + cc_etl.patch AS etlConfig, + cc_admin.patch AS adminConfig + FROM charts c + JOIN chart_configs cc ON cc.id = c.configId + LEFT JOIN charts_x_parents cxp ON cxp.chartId = c.id + LEFT JOIN variables v ON v.id = cxp.variableId + LEFT JOIN chart_configs cc_etl ON cc_etl.id = v.grapherConfigIdETL + LEFT JOIN chart_configs cc_admin ON cc_admin.id = v.grapherConfigIdAdmin + `) + for (const chart of charts) { + const patchConfig = JSON.parse(chart.patchConfig) + const etlConfig = chart.etlConfig ? JSON.parse(chart.etlConfig) : {} + const adminConfig = chart.adminConfig + ? JSON.parse(chart.adminConfig) + : {} + + const fullConfig = mergeGrapherConfigs( + defaultGrapherConfig, + etlConfig, + adminConfig, + patchConfig + ) + + await queryRunner.query( + ` + -- sql + UPDATE chart_configs cc + SET cc.full = ? + WHERE cc.id = ? + `, + [JSON.stringify(fullConfig), chart.configId] + ) + } + } + + // eslint-disable-next-line @typescript-eslint/no-empty-function + public async down(): Promise {} +} diff --git a/explorer/GrapherGrammar.ts b/explorer/GrapherGrammar.ts index ecc953d3779..1397ee4c328 100644 --- a/explorer/GrapherGrammar.ts +++ b/explorer/GrapherGrammar.ts @@ -15,7 +15,6 @@ import { EnumCellDef, Grammar, IntegerCellDef, - NumericCellDef, SlugDeclarationCellDef, SlugsDeclarationCellDef, StringCellDef, @@ -23,6 +22,7 @@ import { IndicatorIdsOrEtlPathsCellDef, IndicatorIdOrEtlPathCellDef, GrapherCellDef, + NumericOrAutoCellDef, } from "../gridLang/GridLangConstants.js" const toTerminalOptions = (keywords: string[]): CellDef[] => { @@ -176,7 +176,7 @@ export const GrapherGrammar: Grammar = { toGrapherObject: (value) => ({ yAxis: { canChangeScaleType: value } }), }, yAxisMin: { - ...NumericCellDef, + ...NumericOrAutoCellDef, keyword: "yAxisMin", description: "Set the minimum value for the yAxis", toGrapherObject: (value) => ({ yAxis: { min: value } }), diff --git a/gridLang/GridLangConstants.ts b/gridLang/GridLangConstants.ts index 9259535b7cc..75ba1e15a2e 100644 --- a/gridLang/GridLangConstants.ts +++ b/gridLang/GridLangConstants.ts @@ -102,6 +102,19 @@ export const NumericCellDef: CellDef = { parse: (value: any) => parseFloat(value), } +export const NumericOrAutoCellDef: CellDef = { + keyword: "", + cssClass: "NumericCellDef", + description: "", + regex: /^(-?\d+\.?\d*|auto)$/, + requirementsDescription: `Must be a number or "auto"`, + valuePlaceholder: "98.6", + parse: (value: any) => { + if (value === "auto") return "auto" + return parseFloat(value) + }, +} + export const IntegerCellDef: CellDef = { keyword: "", cssClass: "IntegerCellDef", diff --git a/packages/@ourworldindata/grapher/src/axis/AxisConfig.test.ts b/packages/@ourworldindata/grapher/src/axis/AxisConfig.test.ts index cd8f5f25baa..9d4bd130dda 100755 --- a/packages/@ourworldindata/grapher/src/axis/AxisConfig.test.ts +++ b/packages/@ourworldindata/grapher/src/axis/AxisConfig.test.ts @@ -3,15 +3,15 @@ import { AxisConfig } from "../axis/AxisConfig" import { ScaleType } from "@ourworldindata/types" -it("serializes max to 'auto' if undefined", () => { - const axis = new AxisConfig({ min: 1 }) +it("serializes max to 'auto' if -Infinity", () => { + const axis = new AxisConfig({ min: 1, max: -Infinity }) const obj = axis.toObject() expect(obj.min).toBe(1) expect(obj.max).toBe("auto") }) -it("serializes min to 'auto' if undefined", () => { - const axis = new AxisConfig({ max: 0 }) +it("serializes min to 'auto' if Infinity", () => { + const axis = new AxisConfig({ max: 0, min: Infinity }) const obj = axis.toObject() expect(obj.max).toBe(0) expect(obj.min).toBe("auto") diff --git a/packages/@ourworldindata/grapher/src/axis/AxisConfig.ts b/packages/@ourworldindata/grapher/src/axis/AxisConfig.ts index 2226665d24c..676da96add0 100644 --- a/packages/@ourworldindata/grapher/src/axis/AxisConfig.ts +++ b/packages/@ourworldindata/grapher/src/axis/AxisConfig.ts @@ -45,10 +45,17 @@ class AxisConfigDefaults implements AxisConfigInterface { @observable.ref domainValues?: number[] = undefined } -function parseAutoOrNumberFromJSON( +function parseMinFromJSON( value: AxisMinMaxValueStr.auto | number | undefined ): number | undefined { - if (value === AxisMinMaxValueStr.auto) return undefined + if (value === AxisMinMaxValueStr.auto) return Infinity + return value +} + +function parseMaxFromJSON( + value: AxisMinMaxValueStr.auto | number | undefined +): number | undefined { + if (value === AxisMinMaxValueStr.auto) return -Infinity return value } @@ -67,8 +74,8 @@ export class AxisConfig // todo: test/refactor updateFromObject(props?: AxisConfigInterface): void { if (props) extend(this, props) - if (props?.min) this.min = parseAutoOrNumberFromJSON(props?.min) - if (props?.max) this.max = parseAutoOrNumberFromJSON(props?.max) + if (props?.min) this.min = parseMinFromJSON(props?.min) + if (props?.max) this.max = parseMaxFromJSON(props?.max) } toObject(): AxisConfigInterface { @@ -96,8 +103,8 @@ export class AxisConfig deleteRuntimeAndUnchangedProps(obj, new AxisConfigDefaults()) - if (obj.min === undefined) obj.min = AxisMinMaxValueStr.auto - if (obj.max === undefined) obj.max = AxisMinMaxValueStr.auto + if (obj.min === Infinity) obj.min = AxisMinMaxValueStr.auto + if (obj.max === -Infinity) obj.max = AxisMinMaxValueStr.auto return obj } diff --git a/packages/@ourworldindata/grapher/src/lineCharts/LineChart.tsx b/packages/@ourworldindata/grapher/src/lineCharts/LineChart.tsx index a7713038b1f..47a3620b927 100644 --- a/packages/@ourworldindata/grapher/src/lineCharts/LineChart.tsx +++ b/packages/@ourworldindata/grapher/src/lineCharts/LineChart.tsx @@ -1344,6 +1344,8 @@ export class LineChart // horizontal axis to be at the bottom of the chart. // see https://github.com/owid/owid-grapher/pull/975#issuecomment-890798547 singleValueAxisPointAlign: AxisAlign.start, + // default to 0 if not set + min: 0, ...this.manager.yAxisConfig, }, this diff --git a/packages/@ourworldindata/grapher/src/schema/defaultGrapherConfig.ts b/packages/@ourworldindata/grapher/src/schema/defaultGrapherConfig.ts index 6497d08f13c..dd0f68334e2 100644 --- a/packages/@ourworldindata/grapher/src/schema/defaultGrapherConfig.ts +++ b/packages/@ourworldindata/grapher/src/schema/defaultGrapherConfig.ts @@ -26,7 +26,6 @@ export const defaultGrapherConfig = { maxTime: "latest", yAxis: { removePointsOutsideDomain: false, - min: "auto", scaleType: "linear", max: "auto", canChangeScaleType: false, @@ -66,7 +65,6 @@ export const defaultGrapherConfig = { }, xAxis: { removePointsOutsideDomain: false, - min: "auto", scaleType: "linear", max: "auto", canChangeScaleType: false, diff --git a/packages/@ourworldindata/grapher/src/schema/grapher-schema.005.yaml b/packages/@ourworldindata/grapher/src/schema/grapher-schema.005.yaml index 79ca82b4122..4465a68f6c0 100644 --- a/packages/@ourworldindata/grapher/src/schema/grapher-schema.005.yaml +++ b/packages/@ourworldindata/grapher/src/schema/grapher-schema.005.yaml @@ -514,8 +514,9 @@ $defs: type: string description: Axis label min: - description: Minimum domain value of the axis. Inferred from data if set to "auto". - default: auto + description: | + Minimum domain value of the axis. Inferred from data if set to "auto". + Usually defaults to "auto", but defaults to 0 for line charts on the y-axis. oneOf: - type: number - type: string