Skip to content

Commit

Permalink
🎉 (slope) feature parity with line charts / TAS-708 (#4192)
Browse files Browse the repository at this point in the history
Adds line chart features to slope charts.

Designs are implemented in a later PR. For now, I removed all custom rendering logic and used the line chart's line legend for labelling. The details of the layout, the slope style, the entity and value labels will change in a future PR, so no need to review the rendering. This also includes rendering bugs like dots overflowing, misaligned labels, etc. 

### Summary

- Switched from showing all entities by default to only plotting the selected ones
- Added support for multiple y-dimensions (behaves like line charts, i.e. multiple entities and columns can be plotted at the same time)
- Added support for specifying a missing data strategy
- Added support for relative mode
- Added support for "entity name attributions" (the explanations that sometimes appear next to a label in the line legend)
- Added tooltips
- Enabled timeline animation for slope charts
- Switched to the labelling algorithm used by line charts
- Dropped support for focusing lines (will be added back later in a different form)
- Dropped support for a categorical colour dimension (could be a nice to have but not strictly necessary if only a small subset of entities are rendered at a time)
- Migrated all existing slope charts (changes have been reviewed by Fiona)

**Might be addressed later**
- Faceting! (would be nice but not strictly necessary, so will look into it maybe later)

**Not addressed**
- Some line charts have a numerical colour dimension, but slope charts ignore the colour channel for now. It wouldn't be very useful in slope charts and we have so few line charts that use a colour channel that I don't feel it's worth the effort

### Testing

I enabled slope tabs for all line charts on staging for testing.

**Example charts:**
- With relative mode toggled: http://staging-site-redesign-slopes-viz/grapher/gdp-per-capita-worldbank?tab=slope&stackMode=relative
- With daily data and entity annotations: http://staging-site-redesign-slopes-viz/grapher/daily-tests-per-thousand-people-smoothed-7-day?tab=slope&time=2021-04-27..2021-11-25
- With No Data section: http://staging-site-redesign-slopes-viz/grapher/ratio-of-female-to-male-labor-force-participation-rates-slopes?time=1960..latest&country=AGO~OWID_WRL
- Without any plottable entities: http://staging-site-redesign-slopes-viz/grapher/ratio-of-female-to-male-labor-force-participation-rates-slopes?country=AGO~OWID_WRL~AUT
- With missing data strategy set to "hide": http://staging-site-redesign-slopes-viz/grapher/production-vs-consumption-co2-emissions?tab=slope&country=~MKD
- With multiple y-columns: http://staging-site-redesign-slopes-viz/grapher/antibiotic-usage-by-surveillance-category-time?tab=slope&country=~CIV
- With multiple columns where some columns don't have data for the selected entity (note that they don't appear in the No Data section): http://staging-site-redesign-slopes-viz/grapher/consumption-of-inhaled-antibiotics-by-type?tab=slope&time=earliest..latest
- With faceted line chart that is a bar chart initially: http://staging-site-redesign-slopes-viz/grapher/antibiotic-usage-by-surveillance-category
- Without legend: http://staging-site-redesign-slopes-viz/grapher/number-of-countries-reporting-data-on-vaccinations

### Bugs

- http://staging-site-redesign-slopes-viz/grapher/consumption-of-inhaled-antibiotics-by-type?country=~BLR
  - Reproduce by switching to the slope chart tab, then switch back
  - Not sure how to fix this
  - This is a side effect of modifying the start/end time when switching from a line to a slope chart – not sure that's worth it, we could also just remove that
  - Even if there is no bug, it's a bit weird to go from discrete bar chart, to slope chart, to line chart (example: http://staging-site-redesign-slopes-viz/grapher/consumption-of-inhaled-antibiotics-by-type)
  - Not sure what the right thing to do here is. Curious what you think!
  • Loading branch information
sophiamersmann authored Dec 11, 2024
1 parent 8124296 commit 4219a0b
Show file tree
Hide file tree
Showing 25 changed files with 1,409 additions and 1,304 deletions.
9 changes: 3 additions & 6 deletions adminSiteClient/EditorBasicTab.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ class DimensionSlotView<
const { selection } = grapher
const { availableEntityNames, availableEntityNameSet } = selection

if (grapher.isScatter || grapher.isSlopeChart || grapher.isMarimekko) {
if (grapher.isScatter || grapher.isMarimekko) {
// chart types that display all entities by default shouldn't select any by default
selection.clearSelection()
} else if (
Expand Down Expand Up @@ -372,8 +372,8 @@ export class EditorBasicTab<
grapher.stackMode = StackMode.relative
}

// Give scatterplots and slope charts a default color dimension if they don't have one
if (grapher.isScatter || grapher.isSlopeChart) {
// Give scatterplots a default color and size dimensions
if (grapher.isScatter) {
const hasColor = grapher.dimensions.find(
(d) => d.property === DimensionProperty.color
)
Expand All @@ -382,10 +382,7 @@ export class EditorBasicTab<
variableId: CONTINENTS_INDICATOR_ID,
property: DimensionProperty.color,
})
}

// Give scatterplots a default size dimension if they don't have one
if (grapher.isScatter) {
const hasSize = grapher.dimensions.find(
(d) => d.property === DimensionProperty.size
)
Expand Down
8 changes: 5 additions & 3 deletions adminSiteClient/EditorFeatures.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ export class EditorFeatures {
@computed get hideLegend() {
return (
this.grapher.isLineChart ||
this.grapher.isSlopeChart ||
this.grapher.isStackedArea ||
this.grapher.isStackedDiscreteBar
)
Expand All @@ -77,6 +78,7 @@ export class EditorFeatures {
this.grapher.isStackedBar ||
this.grapher.isStackedDiscreteBar ||
this.grapher.isLineChart ||
this.grapher.isSlopeChart ||
this.grapher.isScatter ||
this.grapher.isMarimekko
)
Expand Down Expand Up @@ -118,9 +120,9 @@ export class EditorFeatures {
return true
}

// for line charts, specifying a missing data strategy only makes sense
// for line and slope charts, specifying a missing data strategy only makes sense
// if there are multiple entities
if (this.grapher.isLineChart) {
if (this.grapher.isLineChart || this.grapher.isSlopeChart) {
return (
this.grapher.canChangeEntity ||
this.grapher.canSelectMultipleEntities
Expand All @@ -132,7 +134,7 @@ export class EditorFeatures {

@computed get showChangeInPrefixToggle() {
return (
this.grapher.isLineChart &&
(this.grapher.isLineChart || this.grapher.isSlopeChart) &&
(this.grapher.isRelativeMode || this.grapher.canToggleRelativeMode)
)
}
Expand Down
2 changes: 1 addition & 1 deletion baker/updateChartEntities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ const obtainAvailableEntitiesForGrapherConfig = async (

// In these chart types, an unselected entity is still shown
const chartTypeShowsUnselectedEntities =
grapher.isScatter || grapher.isSlopeChart || grapher.isMarimekko
grapher.isScatter || grapher.isMarimekko

if (canChangeEntities || chartTypeShowsUnselectedEntities)
return grapher.tableForSelection.availableEntityNames as string[]
Expand Down
12 changes: 11 additions & 1 deletion packages/@ourworldindata/core-table/src/CoreTableColumns.ts
Original file line number Diff line number Diff line change
Expand Up @@ -305,12 +305,22 @@ export abstract class AbstractCoreColumn<JS_TYPE extends PrimitiveType> {
@imemo get displayName(): string {
return (
this.display?.name ??
this.def.presentation?.titlePublic ?? // this is a bit of an unusual fallback - if display.name is not given, titlePublic is the next best thing before name
// this is a bit of an unusual fallback - if display.name is not given, titlePublic is the next best thing before name
this.def.presentation?.titlePublic ??
this.name ??
""
)
}

@imemo get nonEmptyDisplayName(): string {
return (
this.display?.name ||
// this is a bit of an unusual fallback - if display.name is not given, titlePublic is the next best thing before name
this.def.presentation?.titlePublic ||
this.nonEmptyName
)
}

@imemo get titlePublicOrDisplayName(): IndicatorTitleWithFragments {
return this.def.presentation?.titlePublic
? {
Expand Down
89 changes: 89 additions & 0 deletions packages/@ourworldindata/core-table/src/OwidTable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,95 @@ export class OwidTable extends CoreTable<OwidRow, OwidColumnDef> {
)
}

// Drop _all rows_ for an entity if all columns have at least one invalid or missing value for that entity.
dropEntitiesThatHaveSomeMissingOrErrorValueInAllColumns(
columnSlugs: ColumnSlug[]
): this {
const indexesByEntityName = this.rowIndicesByEntityName
const uniqTimes = new Set(this.allTimes)

// entity names to iterate over
const entityNamesToIterateOver = new Set(indexesByEntityName.keys())

// set of entities we want to keep
const entityNamesToKeep = new Set<string>()

// total number of entities
const entityCount = entityNamesToIterateOver.size

// helper function to generate operation name
const makeOpName = (entityNamesToKeep: Set<EntityName>): string => {
const entityNamesToDrop = differenceOfSets([
this.availableEntityNameSet,
entityNamesToKeep,
])
const droppedEntitiesStr =
entityNamesToDrop.size > 0
? [...entityNamesToDrop].join(", ")
: "(None)"
return `Drop entities that have some missing or error value in all column: ${columnSlugs.join(", ")}.\nDropped entities: ${droppedEntitiesStr}`
}

// Optimization: if there is a column that has a valid data entry for
// every entity and every time, we are done
for (let i = 0; i <= columnSlugs.length; i++) {
const slug = columnSlugs[i]
const col = this.get(slug)

if (
col.numValues === entityCount * uniqTimes.size &&
col.numErrorValues === 0
) {
const entityNamesToKeep = new Set(indexesByEntityName.keys())

return this.columnFilter(
this.entityNameSlug,
(rowEntityName) =>
entityNamesToKeep.has(rowEntityName as string),
makeOpName(entityNamesToKeep)
)
}
}

for (let i = 0; i <= columnSlugs.length; i++) {
const slug = columnSlugs[i]
const col = this.get(slug)

for (const entityName of entityNamesToIterateOver) {
const indicesForEntityName = indexesByEntityName.get(entityName)
if (!indicesForEntityName)
throw new Error("Unexpected: entity not found in index map")

// Optimization: If the column is missing values for the entity,
// we know we can't make a decision yet, so we skip this entity
if (indicesForEntityName.length < uniqTimes.size) continue

// Optimization: We don't care about the number of valid/error
// values, we just need to know if there is at least one invalid value
const hasSomeInvalidValueForEntityInCol =
indicesForEntityName.some(
(index) =>
!isNotErrorValue(
col.valuesIncludingErrorValues[index]
)
)

// Optimization: If all values are valid, we know we want to keep this entity,
// so we remove it from the entities to iterate over
if (!hasSomeInvalidValueForEntityInCol) {
entityNamesToKeep.add(entityName)
entityNamesToIterateOver.delete(entityName)
}
}
}

return this.columnFilter(
this.entityNameSlug,
(rowEntityName) => entityNamesToKeep.has(rowEntityName as string),
makeOpName(entityNamesToKeep)
)
}

private sumsByTime(columnSlug: ColumnSlug): Map<number, number> {
const timeValues = this.timeColumn.values
const values = this.get(columnSlug).values as number[]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -409,11 +409,14 @@ export class DiscreteBarChart
<g id={makeIdForHumanConsumption("entity-labels")}>
{this.placedSeries.map((series) => {
return (
series.label &&
series.label.render(
series.entityLabelX,
series.barY - series.label.height / 2,
{ textProps: style }
series.label && (
<React.Fragment key={series.seriesName}>
{series.label.render(
series.entityLabelX,
series.barY - series.label.height / 2,
{ textProps: style }
)}
</React.Fragment>
)
)
})}
Expand Down Expand Up @@ -990,6 +993,7 @@ function makeProjectedDataPattern(color: string): React.ReactElement {
const size = 7
return (
<pattern
key={color}
id={makeProjectedDataPatternId(color)}
patternUnits="userSpaceOnUse"
width={size}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ export interface CaptionedChartManager
isOnMapTab?: boolean
isOnTableTab?: boolean
activeChartType?: GrapherChartType
isLineChartThatTurnedIntoDiscreteBar?: boolean
isLineChartThatTurnedIntoDiscreteBarActive?: boolean
showEntitySelectionToggle?: boolean
isExportingForSocialMedia?: boolean

Expand Down Expand Up @@ -197,7 +197,7 @@ export class CaptionedChart extends React.Component<CaptionedChartProps> {
if (manager.isOnTableTab) return undefined
if (manager.isOnMapTab) return GRAPHER_MAP_TYPE
if (manager.isOnChartTab) {
return manager.isLineChartThatTurnedIntoDiscreteBar
return manager.isLineChartThatTurnedIntoDiscreteBarActive
? GRAPHER_CHART_TYPES.DiscreteBar
: manager.activeChartType
}
Expand Down
15 changes: 14 additions & 1 deletion packages/@ourworldindata/grapher/src/chart/ChartUtils.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React from "react"
import { Box, getCountryByName } from "@ourworldindata/utils"
import { areSetsEqual, Box, getCountryByName } from "@ourworldindata/utils"
import {
SeriesStrategy,
EntityName,
Expand All @@ -15,6 +15,7 @@ import {
GRAPHER_SIDE_PANEL_CLASS,
GRAPHER_TIMELINE_CLASS,
GRAPHER_SETTINGS_CLASS,
validChartTypeCombinations,
} from "../core/GrapherConstants"

export const autoDetectYColumnSlugs = (manager: ChartManager): string[] => {
Expand Down Expand Up @@ -175,3 +176,15 @@ export function mapChartTypeNameToQueryParam(
return GRAPHER_TAB_QUERY_PARAMS.marimekko
}
}

export function findValidChartTypeCombination(
chartTypes: GrapherChartType[]
): GrapherChartType[] | undefined {
const chartTypeSet = new Set(chartTypes)
for (const validCombination of validChartTypeCombinations) {
const validCombinationSet = new Set(validCombination)
if (areSetsEqual(chartTypeSet, validCombinationSet))
return validCombination
}
return undefined
}
19 changes: 15 additions & 4 deletions packages/@ourworldindata/grapher/src/controls/ContentSwitchers.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export interface ContentSwitchersManager {
activeTab?: GrapherTabName
hasMultipleChartTypes?: boolean
setTab: (tab: GrapherTabName) => void
onTabChange: (oldTab: GrapherTabName, newTab: GrapherTabName) => void
isNarrow?: boolean
isMedium?: boolean
isLineChartThatTurnedIntoDiscreteBar?: boolean
Expand Down Expand Up @@ -112,8 +113,10 @@ export class ContentSwitchers extends React.Component<{
}

@action.bound setTab(tabIndex: number): void {
const oldTab = this.manager.activeTab
const newTab = this.availableTabs[tabIndex]
this.manager.setTab(newTab)
this.manager.onTabChange?.(oldTab!, newTab)
}

render(): React.ReactElement {
Expand Down Expand Up @@ -175,9 +178,11 @@ function TabIcon({
case GRAPHER_TAB_NAMES.WorldMap:
return <FontAwesomeIcon icon={faEarthAmericas} />
default:
const chartIcon = isLineChartThatTurnedIntoDiscreteBar
? chartIcons[GRAPHER_CHART_TYPES.DiscreteBar]
: chartIcons[tab]
const chartIcon =
tab === GRAPHER_TAB_NAMES.LineChart &&
isLineChartThatTurnedIntoDiscreteBar
? chartIcons[GRAPHER_CHART_TYPES.DiscreteBar]
: chartIcons[tab]
return chartIcon
}
}
Expand All @@ -193,9 +198,15 @@ function makeTabLabelText(
if (tab === GRAPHER_TAB_NAMES.WorldMap) return "Map"
if (!options.hasMultipleChartTypes) return "Chart"

if (
tab === GRAPHER_TAB_NAMES.LineChart &&
options.isLineChartThatTurnedIntoDiscreteBar
)
return "Bar"

switch (tab) {
case GRAPHER_TAB_NAMES.LineChart:
return options.isLineChartThatTurnedIntoDiscreteBar ? "Bar" : "Line"
return "Line"
case GRAPHER_TAB_NAMES.SlopeChart:
return "Slope"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ const {
StackedDiscreteBar,
StackedBar,
Marimekko,
SlopeChart,
} = GRAPHER_CHART_TYPES

export interface SettingsMenuManager
Expand Down Expand Up @@ -170,6 +171,7 @@ export class SettingsMenu extends React.Component<{
ScatterPlot,
LineChart,
Marimekko,
SlopeChart,
].includes(this.chartType as any)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
} from "@ourworldindata/types"
import { LabeledSwitch } from "@ourworldindata/components"

const { LineChart, ScatterPlot } = GRAPHER_CHART_TYPES
const { LineChart, ScatterPlot, SlopeChart } = GRAPHER_CHART_TYPES

export interface AbsRelToggleManager {
stackMode?: StackMode
Expand Down Expand Up @@ -38,7 +38,7 @@ export class AbsRelToggle extends React.Component<{
const { activeChartType } = this.manager
return activeChartType === ScatterPlot
? "Show the percentage change per year over the the selected time range."
: activeChartType === LineChart
: activeChartType === LineChart || activeChartType === SlopeChart
? "Show proportional changes over time or actual values in their original units."
: "Show values as their share of the total or as actual values in their original units."
}
Expand Down
Loading

0 comments on commit 4219a0b

Please sign in to comment.