Skip to content

Commit

Permalink
Merge pull request #4382 from owid/disconnect-lines
Browse files Browse the repository at this point in the history
🎉 (line) allow to disconnect lines / TAS-775
  • Loading branch information
sophiamersmann authored Jan 13, 2025
2 parents 7f333de + 36139d6 commit e56bf5b
Show file tree
Hide file tree
Showing 6 changed files with 81 additions and 15 deletions.
15 changes: 15 additions & 0 deletions adminSiteClient/DimensionCard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,11 @@ export class DimensionCard<
this.onChange()
}

@action.bound onPlotMarkersOnly(value: boolean) {
this.props.dimension.display.plotMarkersOnlyInLineChart = value
this.onChange()
}

@action.bound onColor(color: string | undefined) {
this.props.dimension.display.color = color
this.onChange()
Expand Down Expand Up @@ -252,6 +257,16 @@ export class DimensionCard<
onValue={this.onIsProjection}
/>
)}
{grapher.isLineChart && (
<Toggle
label="Plot markers only"
value={
column.display
?.plotMarkersOnlyInLineChart ?? false
}
onValue={this.onPlotMarkersOnly}
/>
)}
<hr className="ui divider" />
</div>
)}
Expand Down
73 changes: 58 additions & 15 deletions packages/@ourworldindata/grapher/src/lineCharts/LineChart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ const VARIABLE_COLOR_STROKE_WIDTH = 2.5
// marker radius
const DEFAULT_MARKER_RADIUS = 1.8
const VARIABLE_COLOR_MARKER_RADIUS = 2.2
const DISCONNECTED_DOTS_MARKER_RADIUS = 2.6
// line outline
const DEFAULT_LINE_OUTLINE_WIDTH = 0.5
const VARIABLE_COLOR_LINE_OUTLINE_WIDTH = 1.0
Expand All @@ -151,10 +152,14 @@ class Lines extends React.Component<LinesProps> {
return this.props.lineStrokeWidth ?? DEFAULT_STROKE_WIDTH
}

@computed private get lineOutlineWidth(): number {
@computed private get outlineWidth(): number {
return this.props.lineOutlineWidth ?? DEFAULT_LINE_OUTLINE_WIDTH
}

@computed private get outlineColor(): string {
return this.props.backgroundColor ?? GRAPHER_BACKGROUND_DEFAULT
}

// Don't display point markers if there are very many of them for performance reasons
// Note that we're using circle elements instead of marker-mid because marker performance in Safari 10 is very poor for some reason
@computed private get hasMarkers(): boolean {
Expand All @@ -167,14 +172,29 @@ class Lines extends React.Component<LinesProps> {
return totalPoints < 500
}

@computed private get hasMarkersOnlySeries(): boolean {
return this.props.series.some((series) => series.plotMarkersOnly)
}

private seriesHasMarkers(series: RenderLineChartSeries): boolean {
if (series.hover.background || series.isProjection) return false
if (
series.hover.background ||
series.isProjection ||
// 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
}

private renderLine(series: RenderLineChartSeries): React.ReactElement {
private renderLine(
series: RenderLineChartSeries
): React.ReactElement | void {
const { hover, focus } = series

if (series.plotMarkersOnly) return

const seriesColor = series.placedPoints[0]?.color ?? DEFAULT_LINE_COLOR
const color =
!focus.background || hover.active
Expand All @@ -190,9 +210,8 @@ class Lines extends React.Component<LinesProps> {
hover.background && !focus.background ? GRAPHER_OPACITY_MUTE : 1

const showOutline = !focus.background || hover.active
const outlineColor =
this.props.backgroundColor ?? GRAPHER_BACKGROUND_DEFAULT
const outlineWidth = strokeWidth + this.lineOutlineWidth * 2
const outlineColor = this.outlineColor
const outlineWidth = strokeWidth + this.outlineWidth * 2

const outline = (
<LinePath
Expand Down Expand Up @@ -238,9 +257,12 @@ class Lines extends React.Component<LinesProps> {
const { horizontalAxis } = this.props.dualAxis
const { hover, focus } = series

// If the series only contains one point, then we will always want to
// show a marker/circle because we can't draw a line.
const forceMarkers = series.placedPoints.length === 1
const forceMarkers =
// If the series only contains one point, then we will always want to
// 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.plotMarkersOnly

// check if we should hide markers on the chart and series level
const hideMarkers = !this.hasMarkers || !this.seriesHasMarkers(series)
Expand All @@ -250,6 +272,13 @@ class Lines extends React.Component<LinesProps> {
const opacity =
hover.background && !focus.background ? GRAPHER_OPACITY_MUTE : 1

const outlineColor = series.plotMarkersOnly
? this.outlineColor
: undefined
const outlineWidth = series.plotMarkersOnly
? this.outlineWidth
: undefined

return (
<g id={makeIdForHumanConsumption("markers", series.seriesName)}>
{series.placedPoints.map((value, index) => {
Expand All @@ -268,6 +297,8 @@ class Lines extends React.Component<LinesProps> {
cy={value.y}
r={this.markerRadius}
fill={color}
stroke={outlineColor}
strokeWidth={outlineWidth}
opacity={opacity}
/>
)
Expand Down Expand Up @@ -521,9 +552,9 @@ export class LineChart
}

@computed private get markerRadius(): number {
return this.hasColorScale
? VARIABLE_COLOR_MARKER_RADIUS
: DEFAULT_MARKER_RADIUS
if (this.hasMarkersOnlySeries) return DISCONNECTED_DOTS_MARKER_RADIUS
if (this.hasColorScale) return VARIABLE_COLOR_MARKER_RADIUS
return DEFAULT_MARKER_RADIUS
}

@computed get selectionArray(): SelectionArray {
Expand Down Expand Up @@ -845,6 +876,10 @@ export class LineChart
return this.hasColorScale ? 700 : 400
}

@computed get hidePoints(): boolean {
return !!this.manager.hidePoints || !!this.manager.isStaticAndSmall
}

@computed get lineLegendX(): number {
return this.bounds.right - this.lineLegendWidth
}
Expand Down Expand Up @@ -975,7 +1010,7 @@ export class LineChart
dualAxis={this.dualAxis}
series={this.renderSeries}
multiColor={this.hasColorScale}
hidePoints={manager.hidePoints || manager.isStaticAndSmall}
hidePoints={this.hidePoints}
lineStrokeWidth={this.lineStrokeWidth}
lineOutlineWidth={this.lineOutlineWidth}
backgroundColor={this.manager.backgroundColor}
Expand Down Expand Up @@ -1285,6 +1320,7 @@ export class LineChart
points,
seriesName,
isProjection: column.isProjection,
plotMarkersOnly: column.display?.plotMarkersOnlyInLineChart,
color: seriesColor,
}
}
Expand All @@ -1298,6 +1334,10 @@ export class LineChart
)
}

@computed private get hasMarkersOnlySeries(): boolean {
return this.series.some((series) => series.plotMarkersOnly)
}

// TODO: remove, seems unused
@computed get allPoints(): LinePoint[] {
return this.series.flatMap((series) => series.points)
Expand Down Expand Up @@ -1341,7 +1381,7 @@ export class LineChart
}

@computed get renderSeries(): RenderLineChartSeries[] {
const series: RenderLineChartSeries[] = this.placedSeries.map(
let series: RenderLineChartSeries[] = this.placedSeries.map(
(series) => {
return {
...series,
Expand All @@ -1351,10 +1391,13 @@ export class LineChart
}
)

// 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
if (this.isHoverModeActive || this.isFocusModeActive) {
return sortBy(series, byHoverThenFocusState)
series = sortBy(series, byHoverThenFocusState)
}

return series
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ export interface PlacedPoint {

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

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

// todo: flatten onto the above
Expand Down
1 change: 1 addition & 0 deletions packages/@ourworldindata/utils/src/OwidVariable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ class OwidVariableDisplayConfigDefaults {
@observable includeInTable? = true
@observable tableDisplay?: OwidVariableDataTableConfigInterface
@observable color?: string = undefined
@observable plotMarkersOnlyInLineChart?: boolean = undefined
}

export class OwidVariableDisplayConfig
Expand Down

0 comments on commit e56bf5b

Please sign in to comment.