Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🎉 (line) allow to disconnect lines / TAS-775 #4382

Merged
merged 2 commits into from
Jan 13, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading