From 0aece5ef745bb065c32a190f8c9db43872287c76 Mon Sep 17 00:00:00 2001 From: Sophia Mersmann Date: Mon, 13 Jan 2025 09:50:01 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=A8=20rename=20isConnected=20to=20plot?= =?UTF-8?q?MarkersOnlyInLineChart=20update?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- adminSiteClient/DimensionCard.tsx | 13 ++++--- .../grapher/src/lineCharts/LineChart.tsx | 34 +++++++++++-------- .../src/lineCharts/LineChartConstants.ts | 2 +- .../src/schema/grapher-schema.006.yaml | 4 +-- .../src/OwidVariableDisplayConfigInterface.ts | 2 +- .../@ourworldindata/utils/src/OwidVariable.ts | 2 +- 6 files changed, 32 insertions(+), 25 deletions(-) diff --git a/adminSiteClient/DimensionCard.tsx b/adminSiteClient/DimensionCard.tsx index d812d2ba16..87f69dbcdb 100644 --- a/adminSiteClient/DimensionCard.tsx +++ b/adminSiteClient/DimensionCard.tsx @@ -51,8 +51,8 @@ export class DimensionCard< this.onChange() } - @action.bound onIsConnected(value: boolean) { - this.props.dimension.display.isConnected = value + @action.bound onPlotMarkersOnly(value: boolean) { + this.props.dimension.display.plotMarkersOnlyInLineChart = value this.onChange() } @@ -259,9 +259,12 @@ export class DimensionCard< )} {grapher.isLineChart && ( )}
diff --git a/packages/@ourworldindata/grapher/src/lineCharts/LineChart.tsx b/packages/@ourworldindata/grapher/src/lineCharts/LineChart.tsx index 5033d637d3..8a87922921 100644 --- a/packages/@ourworldindata/grapher/src/lineCharts/LineChart.tsx +++ b/packages/@ourworldindata/grapher/src/lineCharts/LineChart.tsx @@ -172,17 +172,17 @@ class Lines extends React.Component { return totalPoints < 500 } - @computed private get hasDisconnectedSeries(): boolean { - return this.props.series.some((series) => !series.isConnected) + @computed private get hasMarkersOnlySeries(): boolean { + return this.props.series.some((series) => series.plotMarkersOnly) } private seriesHasMarkers(series: RenderLineChartSeries): boolean { if ( series.hover.background || series.isProjection || - // if the series is connected, but there is another one that isn't connected, - // don't show markers since the connected series is likely a smoothed version - (this.hasDisconnectedSeries && series.isConnected) + // if the series has a line, but there is another one that hasn't, then + // don't show markers since the plotted line is likely a smoothed version + (this.hasMarkersOnlySeries && !series.plotMarkersOnly) ) return false return !series.focus.background || series.hover.active @@ -193,7 +193,7 @@ class Lines extends React.Component { ): React.ReactElement | void { const { hover, focus } = series - if (!series.isConnected) return + if (series.plotMarkersOnly) return const seriesColor = series.placedPoints[0]?.color ?? DEFAULT_LINE_COLOR const color = @@ -262,7 +262,7 @@ class Lines extends React.Component { // show a marker/circle because we can't draw a line. series.placedPoints.length === 1 || // If no line is plotted, we'll always want to show markers - !series.isConnected + series.plotMarkersOnly // check if we should hide markers on the chart and series level const hideMarkers = !this.hasMarkers || !this.seriesHasMarkers(series) @@ -272,8 +272,12 @@ class Lines extends React.Component { const opacity = hover.background && !focus.background ? GRAPHER_OPACITY_MUTE : 1 - const outlineColor = !series.isConnected ? this.outlineColor : undefined - const outlineWidth = !series.isConnected ? this.outlineWidth : undefined + const outlineColor = series.plotMarkersOnly + ? this.outlineColor + : undefined + const outlineWidth = series.plotMarkersOnly + ? this.outlineWidth + : undefined return ( @@ -548,7 +552,7 @@ export class LineChart } @computed private get markerRadius(): number { - if (this.hasDisconnectedSeries) return DISCONNECTED_DOTS_MARKER_RADIUS + if (this.hasMarkersOnlySeries) return DISCONNECTED_DOTS_MARKER_RADIUS if (this.hasColorScale) return VARIABLE_COLOR_MARKER_RADIUS return DEFAULT_MARKER_RADIUS } @@ -1316,7 +1320,7 @@ export class LineChart points, seriesName, isProjection: column.isProjection, - isConnected: column.display?.isConnected ?? true, + plotMarkersOnly: column.display?.plotMarkersOnlyInLineChart, color: seriesColor, } } @@ -1330,8 +1334,8 @@ export class LineChart ) } - @computed private get hasDisconnectedSeries(): boolean { - return this.series.some((series) => !series.isConnected) + @computed private get hasMarkersOnlySeries(): boolean { + return this.series.some((series) => series.plotMarkersOnly) } // TODO: remove, seems unused @@ -1387,8 +1391,8 @@ export class LineChart } ) - // draw connected series on top of disconnected ones - series = sortBy(series, (series) => series.isConnected) + // draw lines on top of markers-only series + series = sortBy(series, (series) => !series.plotMarkersOnly) // sort by interaction state so that foreground series // are drawn on top of background series diff --git a/packages/@ourworldindata/grapher/src/lineCharts/LineChartConstants.ts b/packages/@ourworldindata/grapher/src/lineCharts/LineChartConstants.ts index 0da51b6884..749da005d8 100644 --- a/packages/@ourworldindata/grapher/src/lineCharts/LineChartConstants.ts +++ b/packages/@ourworldindata/grapher/src/lineCharts/LineChartConstants.ts @@ -23,7 +23,7 @@ export interface PlacedPoint { export interface LineChartSeries extends ChartSeries { isProjection?: boolean - isConnected?: boolean + plotMarkersOnly?: boolean points: LinePoint[] } diff --git a/packages/@ourworldindata/grapher/src/schema/grapher-schema.006.yaml b/packages/@ourworldindata/grapher/src/schema/grapher-schema.006.yaml index 4d47e18d50..18cc124752 100644 --- a/packages/@ourworldindata/grapher/src/schema/grapher-schema.006.yaml +++ b/packages/@ourworldindata/grapher/src/schema/grapher-schema.006.yaml @@ -266,11 +266,11 @@ properties: description: | Indicates if this time series is a forward projection (if yes then this is rendered differently in e.g. line charts) - isConnected: + plotMarkersOnlyInLineChart: type: boolean default: false description: | - Indicates if this time series should be connected with a line. Only relevant for line charts. + Indicates if data points should be connected with a line in a line chart name: type: string description: The display string for this variable diff --git a/packages/@ourworldindata/types/src/OwidVariableDisplayConfigInterface.ts b/packages/@ourworldindata/types/src/OwidVariableDisplayConfigInterface.ts index f44feaf8f9..6c7c0a182c 100644 --- a/packages/@ourworldindata/types/src/OwidVariableDisplayConfigInterface.ts +++ b/packages/@ourworldindata/types/src/OwidVariableDisplayConfigInterface.ts @@ -21,7 +21,7 @@ export interface OwidVariableDisplayConfigInterface { includeInTable?: boolean tableDisplay?: OwidVariableDataTableConfigInterface color?: string - isConnected?: boolean + plotMarkersOnlyInLineChart?: boolean } // todo: flatten onto the above diff --git a/packages/@ourworldindata/utils/src/OwidVariable.ts b/packages/@ourworldindata/utils/src/OwidVariable.ts index 74201954b6..7ffbb8e894 100644 --- a/packages/@ourworldindata/utils/src/OwidVariable.ts +++ b/packages/@ourworldindata/utils/src/OwidVariable.ts @@ -29,7 +29,7 @@ class OwidVariableDisplayConfigDefaults { @observable includeInTable? = true @observable tableDisplay?: OwidVariableDataTableConfigInterface @observable color?: string = undefined - @observable isConnected?: boolean = undefined + @observable plotMarkersOnlyInLineChart?: boolean = undefined } export class OwidVariableDisplayConfig