From 7c0efa3ce09150e9022b878c7aada40dd8daed48 Mon Sep 17 00:00:00 2001 From: sophiamersmann Date: Fri, 20 Sep 2024 11:13:23 +0200 Subject: [PATCH 1/2] =?UTF-8?q?=E2=9C=A8=20(admin)=20allow=20to=20overwrit?= =?UTF-8?q?e=20parent=20values=20with=20automatic=20defaults?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- adminSiteClient/EditorCustomizeTab.tsx | 71 +++---- adminSiteClient/EditorTextTab.tsx | 37 ++-- adminSiteClient/Forms.tsx | 193 ++++++++++-------- ...333-AddExplicitAutoValuesToChartConfigs.ts | 68 ++++++ devTools/schemaProcessor/columns.json | 24 +-- .../grapher/src/axis/AxisConfig.test.ts | 14 ++ .../grapher/src/axis/AxisConfig.ts | 15 +- .../grapher/src/core/Grapher.jsdom.test.ts | 3 +- .../grapher/src/core/Grapher.tsx | 12 ++ .../src/schema/defaultGrapherConfig.ts | 6 + .../src/schema/grapher-schema.005.yaml | 36 +++- .../types/src/grapherTypes/GrapherTypes.ts | 12 +- packages/@ourworldindata/types/src/index.ts | 1 + 13 files changed, 322 insertions(+), 170 deletions(-) create mode 100644 db/migration/1726819761333-AddExplicitAutoValuesToChartConfigs.ts diff --git a/adminSiteClient/EditorCustomizeTab.tsx b/adminSiteClient/EditorCustomizeTab.tsx index 454255fb1ea..eb27ebe3203 100644 --- a/adminSiteClient/EditorCustomizeTab.tsx +++ b/adminSiteClient/EditorCustomizeTab.tsx @@ -39,6 +39,9 @@ import Select from "react-select" import { AbstractChartEditor } from "./AbstractChartEditor.js" import { ErrorMessages } from "./ChartEditorTypes.js" +const debounceOnLeadingEdge = (fn: (...args: any[]) => void) => + debounce(fn, 0, { leading: true, trailing: false }) + @observer export class ColorSchemeSelector extends React.Component<{ grapher: Grapher @@ -307,10 +310,6 @@ class TimelineSection< return this.props.editor.grapher } - @computed get activeParentConfig() { - return this.props.editor.activeParentConfig - } - @computed get minTime() { return this.grapher.minTime } @@ -333,14 +332,25 @@ class TimelineSection< this.grapher.maxTime = value ?? TimeBoundValue.positiveInfinity } + @action.bound onBlurMinTime() { + if (this.minTime === undefined) { + this.grapher.minTime = TimeBoundValue.negativeInfinity + } + } + + @action.bound onBlurMaxTime() { + if (this.maxTime === undefined) { + this.grapher.maxTime = TimeBoundValue.positiveInfinity + } + } + @action.bound onTimelineMinTime(value: number | undefined) { this.grapher.timelineMinTime = value } @action.bound onBlurTimelineMinTime() { if (this.grapher.timelineMinTime === undefined) { - this.grapher.timelineMinTime = - this.activeParentConfig?.timelineMinTime + this.grapher.timelineMinTime = TimeBoundValue.negativeInfinity } } @@ -350,8 +360,7 @@ class TimelineSection< @action.bound onBlurTimelineMaxTime() { if (this.grapher.timelineMaxTime === undefined) { - this.grapher.timelineMaxTime = - this.activeParentConfig?.timelineMaxTime + this.grapher.timelineMaxTime = TimeBoundValue.positiveInfinity } } @@ -374,7 +383,9 @@ class TimelineSection< )} @@ -385,7 +396,9 @@ class TimelineSection< : "Selected year" } value={this.maxTime} - onValue={debounce(this.onMaxTime)} + // invoke on the leading edge to avoid interference with onBlur + onValue={debounceOnLeadingEdge(this.onMaxTime)} + onBlur={this.onBlurMaxTime} allowNegative /> @@ -395,10 +408,9 @@ class TimelineSection< label="Timeline min" value={this.timelineMinTime} // invoke on the leading edge to avoid interference with onBlur - onValue={debounce(this.onTimelineMinTime, 0, { - leading: true, - trailing: false, - })} + onValue={debounceOnLeadingEdge( + this.onTimelineMinTime + )} onBlur={this.onBlurTimelineMinTime} allowNegative /> @@ -406,10 +418,9 @@ class TimelineSection< label="Timeline max" value={this.timelineMaxTime} // invoke on the leading edge to avoid interference with onBlur - onValue={debounce(this.onTimelineMaxTime, 0, { - leading: true, - trailing: false, - })} + onValue={debounceOnLeadingEdge( + this.onTimelineMaxTime + )} onBlur={this.onBlurTimelineMaxTime} allowNegative /> @@ -527,12 +538,6 @@ export class EditorCustomizeTab< onValue={(value) => (yAxisConfig.min = value) } - onBlur={() => { - if (yAxisConfig.min === undefined) { - yAxisConfig.min = - activeParentConfig?.yAxis?.min - } - }} allowDecimal allowNegative /> @@ -542,12 +547,6 @@ export class EditorCustomizeTab< onValue={(value) => (yAxisConfig.max = value) } - onBlur={() => { - if (yAxisConfig.max === undefined) { - yAxisConfig.max = - activeParentConfig?.yAxis?.max - } - }} allowDecimal allowNegative /> @@ -612,12 +611,6 @@ export class EditorCustomizeTab< onValue={(value) => (xAxisConfig.min = value) } - onBlur={() => { - if (xAxisConfig.min === undefined) { - xAxisConfig.min = - activeParentConfig?.yAxis?.min - } - }} allowDecimal allowNegative /> @@ -627,12 +620,6 @@ export class EditorCustomizeTab< onValue={(value) => (xAxisConfig.max = value) } - onBlur={() => { - if (xAxisConfig.max === undefined) { - xAxisConfig.max = - activeParentConfig?.yAxis?.max - } - }} allowDecimal allowNegative /> diff --git a/adminSiteClient/EditorTextTab.tsx b/adminSiteClient/EditorTextTab.tsx index 7112e320ba6..9736d891ff0 100644 --- a/adminSiteClient/EditorTextTab.tsx +++ b/adminSiteClient/EditorTextTab.tsx @@ -93,11 +93,9 @@ export class EditorTextTab<
grapher.displayTitle} - writeFn={({ grapher }, newVal) => - (grapher.title = newVal) - } - readAutoFn={({ editor }) => + readFn={(grapher) => grapher.displayTitle} + writeFn={(grapher, newVal) => (grapher.title = newVal)} + auto={ editor.couldPropertyBeInherited("title") ? editor.activeParentConfig!.title : undefined @@ -106,7 +104,7 @@ export class EditorTextTab< editor.isPropertyInherited("title") || grapher.title === undefined } - store={{ grapher, editor }} + store={grapher} softCharacterLimit={100} /> {features.showEntityAnnotationInTitleToggle && ( @@ -156,11 +154,11 @@ export class EditorTextTab< /> grapher.currentSubtitle} - writeFn={({ grapher }, newVal) => + readFn={(grapher) => grapher.currentSubtitle} + writeFn={(grapher, newVal) => (grapher.subtitle = newVal) } - readAutoFn={({ editor }) => + auto={ editor.couldPropertyBeInherited("subtitle") ? editor.activeParentConfig!.subtitle : undefined @@ -169,7 +167,7 @@ export class EditorTextTab< editor.isPropertyInherited("subtitle") || grapher.subtitle === undefined } - store={{ grapher, editor }} + store={grapher} placeholder="Briefly describe the context of the data. It's best to avoid duplicating any information which can be easily inferred from other visual elements of the chart." textarea softCharacterLimit={280} @@ -192,11 +190,11 @@ export class EditorTextTab<
grapher.sourcesLine} - writeFn={({ grapher }, newVal) => + readFn={(grapher) => grapher.sourcesLine} + writeFn={(grapher, newVal) => (grapher.sourceDesc = newVal) } - readAutoFn={({ editor }) => + auto={ editor.couldPropertyBeInherited("sourceDesc") ? editor.activeParentConfig!.sourceDesc : undefined @@ -205,7 +203,7 @@ export class EditorTextTab< editor.isPropertyInherited("sourceDesc") || grapher.sourceDesc === undefined } - store={{ grapher, editor }} + store={grapher} helpText="Short comma-separated list of source names" softCharacterLimit={60} /> @@ -233,9 +231,16 @@ export class EditorTextTab< )} - grapher.note ?? ""} + writeFn={(grapher, newVal) => (grapher.note = newVal)} + auto={ + editor.couldPropertyBeInherited("note") + ? editor.activeParentConfig?.note + : undefined + } + isAuto={editor.isPropertyInherited("note")} store={grapher} helpText="Any important clarification needed to avoid miscommunication" softCharacterLimit={140} diff --git a/adminSiteClient/Forms.tsx b/adminSiteClient/Forms.tsx index c2e575045fe..3bf285e8236 100644 --- a/adminSiteClient/Forms.tsx +++ b/adminSiteClient/Forms.tsx @@ -46,6 +46,8 @@ interface TextFieldProps extends React.HTMLAttributes { softCharacterLimit?: number errorMessage?: string buttonContent?: React.ReactNode + buttonTooltipContent?: React.ReactNode + buttonDisabled?: boolean } export class TextField extends React.Component { @@ -79,6 +81,34 @@ export class TextField extends React.Component { } } + renderButton() { + const { props } = this + if (!props.buttonContent) return null + + const button = ( +
+ +
+ ) + + if (props.buttonTooltipContent) { + return ( + + {button} + + ) + } + + return button + } + render() { const { props } = this const passthroughProps = pick(props, [ @@ -120,19 +150,7 @@ export class TextField extends React.Component { onKeyDown={this.onKeyDown} {...passthroughProps} /> - {props.buttonContent && ( -
- -
- )} + {this.renderButton()} {props.helpText && ( @@ -165,6 +183,34 @@ export class TextAreaField extends React.Component { this.props.onValue?.(trimmedValue) } + renderButton() { + const { props } = this + if (!props.buttonContent) return null + + const button = ( +
+ +
+ ) + + if (props.buttonTooltipContent) { + return ( + + {button} + + ) + } + + return button + } + render() { const { props } = this const passthroughProps = pick(props, [ @@ -201,19 +247,7 @@ export class TextAreaField extends React.Component { rows={5} {...passthroughProps} /> - {props.buttonContent && ( -
- -
- )} + {this.renderButton()} {props.helpText && ( @@ -251,6 +285,7 @@ interface NumberFieldProps { helpText?: string buttonContent?: React.ReactNode onButtonClick?: () => void + buttonDisabled?: boolean } interface NumberFieldState { @@ -673,50 +708,30 @@ export class AutoTextField extends React.Component { const props = this.props const { textarea } = props - if (textarea) - return ( - - {props.isAuto ? ( - - ) : ( - - )} - - } - onButtonClick={() => props.onToggleAuto(!props.isAuto)} - /> - ) - else - return ( - - {props.isAuto ? ( - - ) : ( - - )} - - } - onButtonClick={() => props.onToggleAuto(!props.isAuto)} - /> - ) + const Field = textarea ? TextAreaField : TextField + return ( + + {props.isAuto ? ( + + ) : ( + + )} + + } + buttonTooltipContent={ +
+ {props.isAuto + ? "Automatic default. Edit to override" + : "Automatic default overwritten. Click to reset"} +
+ } + onButtonClick={() => props.onToggleAuto(!props.isAuto)} + buttonDisabled={props.isAuto} + /> + ) } } @@ -925,9 +940,9 @@ export class BindAutoStringExt< > extends React.Component< { readFn: (x: T) => string - readAutoFn?: (x: T) => string | undefined writeFn: (x: T, value: string | undefined) => void store: T + auto?: string } & Omit< AutoTextFieldProps, "onValue" | "onToggleAuto" | "value" | "isBlur" @@ -947,16 +962,14 @@ export class BindAutoStringExt< @action.bound onToggleAuto(value: boolean) { this.props.writeFn( this.props.store, - value - ? this.props.readAutoFn?.(this.props.store) - : this.props.readFn(this.props.store) + value ? this.props.auto : this.props.readFn(this.props.store) ) } render() { - const { readFn, readAutoFn, store, ...rest } = this.props + const { readFn, auto, store, ...rest } = this.props const currentReadValue = this.props.isAuto - ? readAutoFn?.(store) ?? readFn(store) + ? auto ?? readFn(store) : readFn(store) return ( { allowNegative {...props} value={props.isAuto ? undefined : props.value} - placeholder={props.isAuto ? props.value.toString() : undefined} + placeholder={props.isAuto ? props.value?.toString() : undefined} buttonContent={ -
+ {props.isAuto + ? "Automatic default. Edit to override" + : "Automatic default overwritten. Click to reset"} +
} + maxWidth={180} > - {props.isAuto ? ( - - ) : ( - - )} - +
+ {props.isAuto ? ( + + ) : ( + + )} +
+ } onButtonClick={() => props.onToggleAuto(!props.isAuto)} + buttonDisabled={props.isAuto} /> ) } diff --git a/db/migration/1726819761333-AddExplicitAutoValuesToChartConfigs.ts b/db/migration/1726819761333-AddExplicitAutoValuesToChartConfigs.ts new file mode 100644 index 00000000000..a0ab5e9e316 --- /dev/null +++ b/db/migration/1726819761333-AddExplicitAutoValuesToChartConfigs.ts @@ -0,0 +1,68 @@ +import { MigrationInterface, QueryRunner } from "typeorm" + +export class AddExplicitAutoValuesToChartConfigs1726819761333 + implements MigrationInterface +{ + public async up(queryRunner: QueryRunner): Promise { + const addExplicitAutoValueToFullChartConfigs = async ( + jsonPath: string, + defaultValue: string + ): Promise => { + await queryRunner.query( + ` + -- sql + UPDATE chart_configs cc + -- the join is necessary to limit the update to charts only + JOIN charts c ON c.configId = cc.id + SET cc.full = JSON_SET(cc.full, ?, ?) + WHERE cc.full ->> ? IS NULL + `, + [jsonPath, defaultValue, jsonPath] + ) + } + + await addExplicitAutoValueToFullChartConfigs( + "$.timelineMinTime", + "earliest" + ) + await addExplicitAutoValueToFullChartConfigs( + "$.timelineMaxTime", + "latest" + ) + await addExplicitAutoValueToFullChartConfigs("$.yAxis.min", "auto") + await addExplicitAutoValueToFullChartConfigs("$.yAxis.max", "auto") + await addExplicitAutoValueToFullChartConfigs("$.xAxis.min", "auto") + await addExplicitAutoValueToFullChartConfigs("$.xAxis.max", "auto") + } + + public async down(queryRunner: QueryRunner): Promise { + const removeExplicitAutoValueFromFullChartConfigs = async ( + jsonPath: string, + defaultValue: string + ): Promise => { + await queryRunner.query( + ` + -- sql + UPDATE chart_configs cc + JOIN charts c ON c.configId = cc.id + SET cc.full = JSON_REMOVE(cc.full, ?) + WHERE JSON_EXTRACT(cc.full, ?) = ? + `, + [jsonPath, jsonPath, defaultValue] + ) + } + + await removeExplicitAutoValueFromFullChartConfigs( + "$.timelineMinTime", + "earliest" + ) + await removeExplicitAutoValueFromFullChartConfigs( + "$.timelineMaxTime", + "latest" + ) + await removeExplicitAutoValueFromFullChartConfigs("$.yAxis.min", "auto") + await removeExplicitAutoValueFromFullChartConfigs("$.yAxis.max", "auto") + await removeExplicitAutoValueFromFullChartConfigs("$.xAxis.min", "auto") + await removeExplicitAutoValueFromFullChartConfigs("$.xAxis.max", "auto") + } +} diff --git a/devTools/schemaProcessor/columns.json b/devTools/schemaProcessor/columns.json index 48e71bf6005..75a5b3579a4 100644 --- a/devTools/schemaProcessor/columns.json +++ b/devTools/schemaProcessor/columns.json @@ -207,9 +207,9 @@ "editor": "textfield" }, { - "type": "number", + "type": ["string", "number"], "pointer": "/yAxis/min", - "editor": "numeric" + "editor": "textfield" }, { "type": "string", @@ -219,9 +219,9 @@ "enumOptions": ["linear", "log"] }, { - "type": "number", + "type": ["string", "number"], "pointer": "/yAxis/max", - "editor": "numeric" + "editor": "textfield" }, { "type": "boolean", @@ -267,9 +267,9 @@ "editor": "checkbox" }, { - "type": "integer", + "type": ["string", "number"], "pointer": "/timelineMinTime", - "editor": "numeric" + "editor": "textfield" }, { "type": "string", @@ -686,9 +686,9 @@ "editor": "textfield" }, { - "type": "number", + "type": ["string", "number"], "pointer": "/xAxis/min", - "editor": "numeric" + "editor": "textfield" }, { "type": "string", @@ -698,9 +698,9 @@ "enumOptions": ["linear", "log"] }, { - "type": "number", + "type": ["string", "number"], "pointer": "/xAxis/max", - "editor": "numeric" + "editor": "textfield" }, { "type": "boolean", @@ -715,9 +715,9 @@ "enumOptions": ["independent", "shared"] }, { - "type": "integer", + "type": ["string", "number"], "pointer": "/timelineMaxTime", - "editor": "numeric" + "editor": "textfield" }, { "type": "boolean", diff --git a/packages/@ourworldindata/grapher/src/axis/AxisConfig.test.ts b/packages/@ourworldindata/grapher/src/axis/AxisConfig.test.ts index 442422db7cf..cd8f5f25baa 100755 --- a/packages/@ourworldindata/grapher/src/axis/AxisConfig.test.ts +++ b/packages/@ourworldindata/grapher/src/axis/AxisConfig.test.ts @@ -3,6 +3,20 @@ import { AxisConfig } from "../axis/AxisConfig" import { ScaleType } from "@ourworldindata/types" +it("serializes max to 'auto' if undefined", () => { + const axis = new AxisConfig({ min: 1 }) + 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 }) + const obj = axis.toObject() + expect(obj.max).toBe(0) + expect(obj.min).toBe("auto") +}) + it("can create an axis, clone and modify the clone without affecting the original", () => { const axis = new AxisConfig({ scaleType: ScaleType.linear }) expect(axis.scaleType).toEqual(ScaleType.linear) diff --git a/packages/@ourworldindata/grapher/src/axis/AxisConfig.ts b/packages/@ourworldindata/grapher/src/axis/AxisConfig.ts index 81e8ede43ce..2226665d24c 100644 --- a/packages/@ourworldindata/grapher/src/axis/AxisConfig.ts +++ b/packages/@ourworldindata/grapher/src/axis/AxisConfig.ts @@ -11,6 +11,7 @@ import { import { observable, computed } from "mobx" import { HorizontalAxis, VerticalAxis } from "./Axis" import { + AxisMinMaxValueStr, AxisConfigInterface, FacetAxisDomain, ScaleType, @@ -44,6 +45,13 @@ class AxisConfigDefaults implements AxisConfigInterface { @observable.ref domainValues?: number[] = undefined } +function parseAutoOrNumberFromJSON( + value: AxisMinMaxValueStr.auto | number | undefined +): number | undefined { + if (value === AxisMinMaxValueStr.auto) return undefined + return value +} + export class AxisConfig extends AxisConfigDefaults implements AxisConfigInterface, Persistable @@ -59,6 +67,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) } toObject(): AxisConfigInterface { @@ -82,10 +92,13 @@ export class AxisConfig ticks: this.ticks, singleValueAxisPointAlign: this.singleValueAxisPointAlign, domainValues: this.domainValues, - }) + }) as AxisConfigInterface deleteRuntimeAndUnchangedProps(obj, new AxisConfigDefaults()) + if (obj.min === undefined) obj.min = AxisMinMaxValueStr.auto + if (obj.max === undefined) obj.max = AxisMinMaxValueStr.auto + return obj } diff --git a/packages/@ourworldindata/grapher/src/core/Grapher.jsdom.test.ts b/packages/@ourworldindata/grapher/src/core/Grapher.jsdom.test.ts index fd09fecd33e..c8ad4c18e69 100755 --- a/packages/@ourworldindata/grapher/src/core/Grapher.jsdom.test.ts +++ b/packages/@ourworldindata/grapher/src/core/Grapher.jsdom.test.ts @@ -62,7 +62,8 @@ const TestGrapherConfig = (): { it("regression fix: container options are not serialized", () => { const grapher = new Grapher({ xAxis: { min: 1 } }) const obj = grapher.toObject().xAxis! - expect(obj.max).toBe(undefined) + expect(obj.min).toBe(1) + expect(obj.scaleType).toBe(undefined) expect((obj as any).containerOptions).toBe(undefined) // Regression test: should never be a containerOptions }) diff --git a/packages/@ourworldindata/grapher/src/core/Grapher.tsx b/packages/@ourworldindata/grapher/src/core/Grapher.tsx index 3a133d616f0..d0f68bad2b5 100644 --- a/packages/@ourworldindata/grapher/src/core/Grapher.tsx +++ b/packages/@ourworldindata/grapher/src/core/Grapher.tsx @@ -545,6 +545,11 @@ export class Grapher if (obj.minTime) obj.minTime = minTimeToJSON(this.minTime) as any if (obj.maxTime) obj.maxTime = maxTimeToJSON(this.maxTime) as any + if (obj.timelineMinTime) + obj.timelineMinTime = minTimeToJSON(this.timelineMinTime) as any + if (obj.timelineMaxTime) + obj.timelineMaxTime = maxTimeToJSON(this.timelineMaxTime) as any + // todo: remove dimensions concept // if (this.legacyConfigAsAuthored?.dimensions) // obj.dimensions = this.legacyConfigAsAuthored.dimensions @@ -575,6 +580,13 @@ export class Grapher this.minTime = minTimeBoundFromJSONOrNegativeInfinity(obj.minTime) this.maxTime = maxTimeBoundFromJSONOrPositiveInfinity(obj.maxTime) + this.timelineMinTime = minTimeBoundFromJSONOrNegativeInfinity( + obj.timelineMinTime + ) + this.timelineMaxTime = maxTimeBoundFromJSONOrPositiveInfinity( + obj.timelineMaxTime + ) + // Todo: remove once we are more RAII. if (obj?.dimensions?.length) this.setDimensionsFromConfigs(obj.dimensions) diff --git a/packages/@ourworldindata/grapher/src/schema/defaultGrapherConfig.ts b/packages/@ourworldindata/grapher/src/schema/defaultGrapherConfig.ts index c45ec0d1224..01493285bff 100644 --- a/packages/@ourworldindata/grapher/src/schema/defaultGrapherConfig.ts +++ b/packages/@ourworldindata/grapher/src/schema/defaultGrapherConfig.ts @@ -23,7 +23,9 @@ export const defaultGrapherConfig = { maxTime: "latest", yAxis: { removePointsOutsideDomain: false, + min: "auto", scaleType: "linear", + max: "auto", canChangeScaleType: false, facetDomain: "shared", }, @@ -32,6 +34,7 @@ export const defaultGrapherConfig = { hasChartTab: true, hideLegend: false, hideLogo: false, + timelineMinTime: "earliest", hideTimeline: false, colorScale: { equalSizeBins: true, @@ -60,10 +63,13 @@ export const defaultGrapherConfig = { }, xAxis: { removePointsOutsideDomain: false, + min: "auto", scaleType: "linear", + max: "auto", canChangeScaleType: false, facetDomain: "shared", }, + timelineMaxTime: "latest", hideConnectedScatterLines: false, showNoDataArea: true, zoomToSelection: false, diff --git a/packages/@ourworldindata/grapher/src/schema/grapher-schema.005.yaml b/packages/@ourworldindata/grapher/src/schema/grapher-schema.005.yaml index 6f472f2a402..79ca82b4122 100644 --- a/packages/@ourworldindata/grapher/src/schema/grapher-schema.005.yaml +++ b/packages/@ourworldindata/grapher/src/schema/grapher-schema.005.yaml @@ -134,10 +134,15 @@ properties: type: boolean default: false timelineMinTime: - type: integer description: | The lowest year to show in the timeline. If this is set then the user is not able to see - any data before this year. Inferred from data if not provided. + any data before this year. If set to "earliest", then the earliest year in the data is used. + default: earliest + oneOf: + - type: number + - type: string + enum: + - earliest variantName: type: string description: Optional internal variant name for distinguishing charts with the same title @@ -423,10 +428,15 @@ properties: xAxis: $ref: "#/$defs/axis" timelineMaxTime: - type: integer description: | The highest year to show in the timeline. If this is set then the user is not able to see - any data after this year. Inferred from data if not provided. + any data after this year. If set to "latest", then the latest year in the data is used. + default: latest + oneOf: + - type: number + - type: string + enum: + - latest hideConnectedScatterLines: type: boolean default: false @@ -504,8 +514,13 @@ $defs: type: string description: Axis label min: - type: number - description: Minimum domain value of the axis. Inferred from data if not provided. + description: Minimum domain value of the axis. Inferred from data if set to "auto". + default: auto + oneOf: + - type: number + - type: string + enum: + - auto scaleType: type: string description: Toggle linear/logarithmic @@ -514,8 +529,13 @@ $defs: - linear - log max: - type: number - description: Maximum domain value of the axis. Inferred from data if not provided. + description: Maximum domain value of the axis. Inferred from data if set to "auto". + default: auto + oneOf: + - type: number + - type: string + enum: + - auto canChangeScaleType: type: boolean description: Allow user to change lin/log diff --git a/packages/@ourworldindata/types/src/grapherTypes/GrapherTypes.ts b/packages/@ourworldindata/types/src/grapherTypes/GrapherTypes.ts index 3c72fd1aeb3..a875400a223 100644 --- a/packages/@ourworldindata/types/src/grapherTypes/GrapherTypes.ts +++ b/packages/@ourworldindata/types/src/grapherTypes/GrapherTypes.ts @@ -132,6 +132,10 @@ export enum ChartTypeName { WorldMap = "WorldMap", } +export enum AxisMinMaxValueStr { + auto = "auto", +} + // We currently have the notion of "modes", where you can either select 1 entity, or select multiple entities, or not change the selection at all. // Todo: can we remove? export enum EntitySelectionMode { @@ -222,8 +226,8 @@ export interface TickFormattingOptions { export interface AxisConfigInterface { scaleType?: ScaleType label?: string - min?: number - max?: number + min?: number | AxisMinMaxValueStr.auto + max?: number | AxisMinMaxValueStr.auto canChangeScaleType?: boolean removePointsOutsideDomain?: boolean hideAxis?: boolean @@ -535,8 +539,8 @@ export interface GrapherInterface extends SortConfig { hideAnnotationFieldsInTitle?: AnnotationFieldsInTitle minTime?: TimeBound | TimeBoundValueStr maxTime?: TimeBound | TimeBoundValueStr - timelineMinTime?: Time - timelineMaxTime?: Time + timelineMinTime?: Time | TimeBoundValueStr + timelineMaxTime?: Time | TimeBoundValueStr dimensions?: OwidChartDimensionInterface[] addCountryMode?: EntitySelectionMode comparisonLines?: ComparisonLineConfig[] diff --git a/packages/@ourworldindata/types/src/index.ts b/packages/@ourworldindata/types/src/index.ts index ba118c9b489..ca2ccc42f92 100644 --- a/packages/@ourworldindata/types/src/index.ts +++ b/packages/@ourworldindata/types/src/index.ts @@ -99,6 +99,7 @@ export { type ChartRedirect, type DetailsMarker, GrapherWindowType, + AxisMinMaxValueStr, } from "./grapherTypes/GrapherTypes.js" export { From 8de194d445f732fa3fc56fd9293de7e72b9a4dec Mon Sep 17 00:00:00 2001 From: sophiamersmann Date: Fri, 20 Sep 2024 13:15:00 +0200 Subject: [PATCH 2/2] =?UTF-8?q?=E2=9C=A8=20(admin)=20add=20link/unlink=20b?= =?UTF-8?q?utton=20to=20time=20selection=20fields?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- adminSiteClient/EditorCustomizeTab.tsx | 223 ++++++++++++++++--------- adminSiteClient/Forms.tsx | 80 ++++++++- adminSiteClient/admin.scss | 29 ++++ 3 files changed, 250 insertions(+), 82 deletions(-) diff --git a/adminSiteClient/EditorCustomizeTab.tsx b/adminSiteClient/EditorCustomizeTab.tsx index eb27ebe3203..cdbc1cb260b 100644 --- a/adminSiteClient/EditorCustomizeTab.tsx +++ b/adminSiteClient/EditorCustomizeTab.tsx @@ -17,6 +17,7 @@ import { TextField, Button, RadioGroup, + BindAutoFloatExt, } from "./Forms.js" import { debounce, @@ -27,6 +28,8 @@ import { SortOrder, SortBy, SortConfig, + minTimeBoundFromJSONOrNegativeInfinity, + maxTimeBoundFromJSONOrPositiveInfinity, } from "@ourworldindata/utils" import { faPlus, faMinus } from "@fortawesome/free-solid-svg-icons" import { FontAwesomeIcon } from "@fortawesome/react-fontawesome/index.js" @@ -42,6 +45,73 @@ import { ErrorMessages } from "./ChartEditorTypes.js" const debounceOnLeadingEdge = (fn: (...args: any[]) => void) => debounce(fn, 0, { leading: true, trailing: false }) +@observer +class TimeField< + T extends { [field: string]: any }, + K extends Extract, +> extends React.Component<{ + field: K + store: T + label: string + defaultValue: number + parentValue: number + isInherited: boolean + allowLinking: boolean +}> { + private setValue(value: number) { + this.props.store[this.props.field] = value as any + } + + @computed get currentValue(): number | undefined { + return this.props.store[this.props.field] + } + + @action.bound onChange(value: number | undefined) { + this.setValue(value ?? this.props.defaultValue) + } + + @action.bound onBlur() { + if (this.currentValue === undefined) { + this.setValue(this.props.defaultValue) + } + } + + render() { + const { label, field, defaultValue } = this.props + + // the reset button resets the value to its default + const resetButton = { + onClick: action(() => this.setValue(defaultValue)), + disabled: this.currentValue === defaultValue, + } + + return this.props.allowLinking ? ( + store[field]} + writeFn={(store, newVal) => + (store[this.props.field] = newVal as any) + } + auto={this.props.parentValue} + isAuto={this.props.isInherited} + store={this.props.store} + onBlur={this.onBlur} + resetButton={resetButton} + /> + ) : ( + + ) + } +} + @observer export class ColorSchemeSelector extends React.Component<{ grapher: Grapher @@ -310,60 +380,6 @@ class TimelineSection< return this.props.editor.grapher } - @computed get minTime() { - return this.grapher.minTime - } - @computed get maxTime() { - return this.grapher.maxTime - } - - @computed get timelineMinTime() { - return this.grapher.timelineMinTime - } - @computed get timelineMaxTime() { - return this.grapher.timelineMaxTime - } - - @action.bound onMinTime(value: number | undefined) { - this.grapher.minTime = value ?? TimeBoundValue.negativeInfinity - } - - @action.bound onMaxTime(value: number | undefined) { - this.grapher.maxTime = value ?? TimeBoundValue.positiveInfinity - } - - @action.bound onBlurMinTime() { - if (this.minTime === undefined) { - this.grapher.minTime = TimeBoundValue.negativeInfinity - } - } - - @action.bound onBlurMaxTime() { - if (this.maxTime === undefined) { - this.grapher.maxTime = TimeBoundValue.positiveInfinity - } - } - - @action.bound onTimelineMinTime(value: number | undefined) { - this.grapher.timelineMinTime = value - } - - @action.bound onBlurTimelineMinTime() { - if (this.grapher.timelineMinTime === undefined) { - this.grapher.timelineMinTime = TimeBoundValue.negativeInfinity - } - } - - @action.bound onTimelineMaxTime(value: number | undefined) { - this.grapher.timelineMaxTime = value - } - - @action.bound onBlurTimelineMaxTime() { - if (this.grapher.timelineMaxTime === undefined) { - this.grapher.timelineMaxTime = TimeBoundValue.positiveInfinity - } - } - @action.bound onToggleHideTimeline(value: boolean) { this.grapher.hideTimeline = value || undefined } @@ -373,56 +389,77 @@ class TimelineSection< } render() { - const { features } = this.props.editor + const { editor } = this.props + const { features } = editor const { grapher } = this return (
{features.timeDomain && ( - )} - {features.timelineRange && ( - - )} @@ -538,6 +575,12 @@ export class EditorCustomizeTab< onValue={(value) => (yAxisConfig.min = value) } + resetButton={{ + onClick: () => + (yAxisConfig.min = undefined), + disabled: + yAxisConfig.min === undefined, + }} allowDecimal allowNegative /> @@ -547,6 +590,12 @@ export class EditorCustomizeTab< onValue={(value) => (yAxisConfig.max = value) } + resetButton={{ + onClick: () => + (yAxisConfig.max = undefined), + disabled: + yAxisConfig.max === undefined, + }} allowDecimal allowNegative /> @@ -611,6 +660,12 @@ export class EditorCustomizeTab< onValue={(value) => (xAxisConfig.min = value) } + resetButton={{ + onClick: () => + (xAxisConfig.min = undefined), + disabled: + xAxisConfig.min === undefined, + }} allowDecimal allowNegative /> @@ -620,6 +675,12 @@ export class EditorCustomizeTab< onValue={(value) => (xAxisConfig.max = value) } + resetButton={{ + onClick: () => + (xAxisConfig.max = undefined), + disabled: + xAxisConfig.max === undefined, + }} allowDecimal allowNegative /> diff --git a/adminSiteClient/Forms.tsx b/adminSiteClient/Forms.tsx index 3bf285e8236..52d4be12c40 100644 --- a/adminSiteClient/Forms.tsx +++ b/adminSiteClient/Forms.tsx @@ -286,6 +286,7 @@ interface NumberFieldProps { buttonContent?: React.ReactNode onButtonClick?: () => void buttonDisabled?: boolean + resetButton?: Omit } interface NumberFieldState { @@ -331,7 +332,44 @@ export class NumberField extends React.Component< }, } - return + if (props.resetButton) { + return ( + + + + ) + } else { + return + } + } +} + +interface WithResetButtonProps { + children: React.ReactElement + onClick: () => void + content?: React.ReactNode + disabled?: boolean +} + +class WithResetButton extends React.Component { + render() { + const { props } = this + + return ( +
+ {props.children} + +
+ ) } } @@ -991,6 +1029,7 @@ interface AutoFloatFieldProps { onValue: (value: number | undefined) => void onToggleAuto: (value: boolean) => void onBlur?: () => void + resetButton?: Omit } class AutoFloatField extends React.Component { @@ -1026,6 +1065,7 @@ class AutoFloatField extends React.Component { } onButtonClick={() => props.onToggleAuto(!props.isAuto)} buttonDisabled={props.isAuto} + resetButton={props.resetButton} /> ) } @@ -1117,6 +1157,44 @@ export class BindAutoFloat< } } +@observer +export class BindAutoFloatExt< + T extends Record, +> extends React.Component< + { + readFn: (x: T) => number + writeFn: (x: T, value: number | undefined) => void + store: T + auto?: number + } & Omit +> { + @action.bound onValue(value: number | undefined) { + this.props.writeFn(this.props.store, value) + } + + @action.bound onToggleAuto(value: boolean) { + this.props.writeFn( + this.props.store, + value ? this.props.auto : this.props.readFn(this.props.store) + ) + } + + render() { + const { readFn, auto, store, ...rest } = this.props + const currentReadValue = this.props.isAuto + ? auto ?? readFn(store) + : readFn(store) + return ( + + ) + } +} + @observer export class Modal extends React.Component<{ className?: string diff --git a/adminSiteClient/admin.scss b/adminSiteClient/admin.scss index 53c5b0c5636..9680ae2202f 100644 --- a/adminSiteClient/admin.scss +++ b/adminSiteClient/admin.scss @@ -647,6 +647,35 @@ $nav-height: 45px; } } +.WithResetButton { + display: flex; + flex-direction: column; + align-items: flex-start; + + .form-group { + margin-bottom: 0; + width: 100%; + } + + .ResetToDefaultButton { + padding: 0; + font-size: 0.8em; + color: inherit; + + &:disabled { + font-style: italic; + } + + &:not(:disabled) { + text-decoration: underline; + } + + &:hover { + text-decoration: none; + } + } +} + .ColorBox { width: 2em; height: 2em;