Skip to content

Commit

Permalink
🔨 rename isConnected to plotMarkersOnlyInLineChart
Browse files Browse the repository at this point in the history
update
  • Loading branch information
sophiamersmann committed Jan 13, 2025
1 parent 3b638b0 commit 0aece5e
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 25 deletions.
13 changes: 8 additions & 5 deletions adminSiteClient/DimensionCard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}

Expand Down Expand Up @@ -259,9 +259,12 @@ export class DimensionCard<
)}
{grapher.isLineChart && (
<Toggle
label="Is connected"
value={column.display?.isConnected ?? true}
onValue={this.onIsConnected}
label="Plot markers only"
value={
column.display
?.plotMarkersOnlyInLineChart ?? false
}
onValue={this.onPlotMarkersOnly}
/>
)}
<hr className="ui divider" />
Expand Down
34 changes: 19 additions & 15 deletions packages/@ourworldindata/grapher/src/lineCharts/LineChart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -172,17 +172,17 @@ class Lines extends React.Component<LinesProps> {
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
Expand All @@ -193,7 +193,7 @@ class Lines extends React.Component<LinesProps> {
): 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 =
Expand Down Expand Up @@ -262,7 +262,7 @@ class Lines extends React.Component<LinesProps> {
// 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)
Expand All @@ -272,8 +272,12 @@ class Lines extends React.Component<LinesProps> {
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 (
<g id={makeIdForHumanConsumption("markers", series.seriesName)}>
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -1316,7 +1320,7 @@ export class LineChart
points,
seriesName,
isProjection: column.isProjection,
isConnected: column.display?.isConnected ?? true,
plotMarkersOnly: column.display?.plotMarkersOnlyInLineChart,
color: seriesColor,
}
}
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export interface PlacedPoint {

export interface LineChartSeries extends ChartSeries {
isProjection?: boolean
isConnected?: boolean
plotMarkersOnly?: boolean
points: LinePoint[]
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export interface OwidVariableDisplayConfigInterface {
includeInTable?: boolean
tableDisplay?: OwidVariableDataTableConfigInterface
color?: string
isConnected?: boolean
plotMarkersOnlyInLineChart?: boolean
}

// todo: flatten onto the above
Expand Down
2 changes: 1 addition & 1 deletion packages/@ourworldindata/utils/src/OwidVariable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 0aece5e

Please sign in to comment.