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

✨ deduplicate unit in axis labels / TAS-810 #4443

Merged
merged 1 commit into from
Jan 21, 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
6 changes: 0 additions & 6 deletions packages/@ourworldindata/core-table/src/CoreTableColumns.ts
Original file line number Diff line number Diff line change
Expand Up @@ -269,12 +269,6 @@ export abstract class AbstractCoreColumn<JS_TYPE extends PrimitiveType> {
return undefined
}

@imemo get displayUnit(): string | undefined {
return this.unit && this.unit !== this.shortUnit
? this.unit.replace(/^\((.*)\)$/, "$1")
: undefined
}

// Returns a map where the key is a series slug such as "name" and the value is a set
// of all the unique values that this column has for that particular series.
getUniqueValuesGroupedBy(
Expand Down
26 changes: 16 additions & 10 deletions packages/@ourworldindata/grapher/src/axis/Axis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import { AxisConfig, AxisManager } from "./AxisConfig"
import { MarkdownTextWrap } from "@ourworldindata/components"
import { ColumnTypeMap, CoreColumn } from "@ourworldindata/core-table"
import { GRAPHER_FONT_SCALE_12 } from "../core/GrapherConstants.js"
import { makeAxisLabel } from "../chart/ChartUtils"

interface TickLabelPlacement {
value: number
Expand Down Expand Up @@ -503,21 +504,26 @@ abstract class AbstractAxis {
this.axisManager?.detailsOrderedByReference,
}

const displayUnit = this.formatColumn?.displayUnit
if (displayUnit) {
const axisLabel = makeAxisLabel({
label: this.label,
unit: this.formatColumn?.unit,
shortUnit: this.formatColumn?.shortUnit,
})

if (axisLabel.unit) {
return MarkdownTextWrap.fromFragments({
main: { text: this.label, bold: true },
secondary: { text: `(${displayUnit})` },
main: { text: axisLabel.mainLabel, bold: true },
secondary: { text: axisLabel.unit },
newLine: "avoid-wrap",
textWrapProps,
})
} else {
return new MarkdownTextWrap({
text: this.label,
fontWeight: 700,
...textWrapProps,
})
}

return new MarkdownTextWrap({
text: axisLabel.mainLabel,
fontWeight: 700,
...textWrapProps,
})
}

@computed get labelHeight(): number {
Expand Down
32 changes: 32 additions & 0 deletions packages/@ourworldindata/grapher/src/chart/ChartUtils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -228,3 +228,35 @@ export function byHoverThenFocusState(series: {
// background series rank lowest
return 1
}

export function makeAxisLabel({
label,
unit,
shortUnit,
}: {
label: string
unit?: string
shortUnit?: string
}): {
mainLabel: string // shown in bold
unit?: string // shown in normal weight, usually in parens
} {
const displayUnit = unit && unit !== shortUnit ? unit : undefined
const unitInParens = displayUnit ? `(${displayUnit})` : undefined

if (unitInParens) {
// extract text in parens at the end of the label,
// e.g. "Population (millions)" is split into "Population " and "(millions)"
const [_fullMatch, untrimmedMainLabelText, labelTextInParens] =
label.trim().match(/^(.*?)(\([^()]*\))?$/) ?? []
const mainLabelText = untrimmedMainLabelText.trim()

// don't show unit twice if it's contained in the label
const displayLabel =
labelTextInParens === unitInParens ? mainLabelText : label

return { mainLabel: displayLabel, unit: unitInParens }
}

return { mainLabel: label }
}
8 changes: 0 additions & 8 deletions packages/@ourworldindata/grapher/src/tooltip/Tooltip.scss
Original file line number Diff line number Diff line change
Expand Up @@ -117,14 +117,6 @@
.unit {
font-weight: $medium;
font-style: normal;

&::before {
content: "(";
}

&::after {
content: ")";
}
}
}

Expand Down
28 changes: 16 additions & 12 deletions packages/@ourworldindata/grapher/src/tooltip/TooltipContents.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
TooltipValueRangeProps,
TooltipContext,
} from "./TooltipProps"
import { makeAxisLabel } from "../chart/ChartUtils.js"

export const NO_DATA_COLOR = "#999"

Expand Down Expand Up @@ -132,22 +133,25 @@ class Variable extends React.Component<{

if (column.isMissing || column.name === "time") return null

const { displayUnit, displayName } = column,
displayNotice =
uniq((notice ?? []).filter((t) => t !== undefined))
.map((time) =>
typeof time === "number"
? column.formatTime(time)
: time
)
.join("\u2013") || null
const { mainLabel: label, unit } = makeAxisLabel({
label: column.displayName,
unit: column.unit,
shortUnit: column.shortUnit,
})

const displayNotice =
uniq((notice ?? []).filter((t) => t !== undefined))
.map((time) =>
typeof time === "number" ? column.formatTime(time) : time
)
.join("\u2013") || null

return (
<div className="variable">
<div className="definition">
{displayName && <span className="name">{displayName}</span>}
{displayUnit && displayUnit.length > 1 && (
<span className="unit">{displayUnit}</span>
{label && <span className="name">{label}</span>}
{unit && unit.length > 1 && (
<span className="unit">{unit}</span>
)}
</div>
<div className="values" style={{ color }}>
Expand Down
Loading