Skip to content

Commit

Permalink
🎉 (line) allow to disconnect lines
Browse files Browse the repository at this point in the history
small change
  • Loading branch information
sophiamersmann committed Jan 13, 2025
1 parent c39e7b3 commit 3b638b0
Show file tree
Hide file tree
Showing 6 changed files with 74 additions and 15 deletions.
12 changes: 12 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 onIsConnected(value: boolean) {
this.props.dimension.display.isConnected = value
this.onChange()
}

@action.bound onColor(color: string | undefined) {
this.props.dimension.display.color = color
this.onChange()
Expand Down Expand Up @@ -252,6 +257,13 @@ export class DimensionCard<
onValue={this.onIsProjection}
/>
)}
{grapher.isLineChart && (
<Toggle
label="Is connected"
value={column.display?.isConnected ?? true}
onValue={this.onIsConnected}
/>
)}
<hr className="ui divider" />
</div>
)}
Expand Down
69 changes: 54 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 hasDisconnectedSeries(): boolean {
return this.props.series.some((series) => !series.isConnected)
}

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

// check if we should hide markers on the chart and series level
const hideMarkers = !this.hasMarkers || !this.seriesHasMarkers(series)
Expand All @@ -250,6 +272,9 @@ 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

return (
<g id={makeIdForHumanConsumption("markers", series.seriesName)}>
{series.placedPoints.map((value, index) => {
Expand All @@ -268,6 +293,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 +548,9 @@ export class LineChart
}

@computed private get markerRadius(): number {
return this.hasColorScale
? VARIABLE_COLOR_MARKER_RADIUS
: DEFAULT_MARKER_RADIUS
if (this.hasDisconnectedSeries) 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 +872,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 +1006,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 +1316,7 @@ export class LineChart
points,
seriesName,
isProjection: column.isProjection,
isConnected: column.display?.isConnected ?? true,
color: seriesColor,
}
}
Expand All @@ -1298,6 +1330,10 @@ export class LineChart
)
}

@computed private get hasDisconnectedSeries(): boolean {
return this.series.some((series) => !series.isConnected)
}

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

// draw connected series on top of disconnected ones
series = sortBy(series, (series) => series.isConnected)

// 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
isConnected?: 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)
isConnected:
type: boolean
default: false
description: |
Indicates if this time series should be connected with a line. Only relevant for line charts.
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
isConnected?: 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 isConnected?: boolean = undefined
}

export class OwidVariableDisplayConfig
Expand Down

0 comments on commit 3b638b0

Please sign in to comment.