Skip to content

Commit

Permalink
✨ (grapher) support multiple chart types
Browse files Browse the repository at this point in the history
  • Loading branch information
sophiamersmann committed Nov 13, 2024
1 parent 1e07f96 commit c56c672
Show file tree
Hide file tree
Showing 36 changed files with 794 additions and 235 deletions.
20 changes: 14 additions & 6 deletions adminSiteClient/ChartList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@ import { observer } from "mobx-react"
import { runInAction, observable } from "mobx"
import { bind } from "decko"
import { AdminAppContext, AdminAppContextType } from "./AdminAppContext.js"
import { ChartTypeName, GrapherInterface } from "@ourworldindata/types"
import {
ChartTypeName,
GrapherInterface,
GrapherTabOption,
} from "@ourworldindata/types"
import { startCase, DbChartTagJoin } from "@ourworldindata/utils"
import { References, getFullReferencesCount } from "./ChartEditor.js"
import { ChartRow } from "./ChartRow.js"
Expand All @@ -14,14 +18,15 @@ export interface ChartListItem {
id: GrapherInterface["id"]
title: GrapherInterface["title"]
slug: GrapherInterface["slug"]
type: GrapherInterface["type"]
internalNotes: GrapherInterface["internalNotes"]
variantName: GrapherInterface["variantName"]
isPublished: GrapherInterface["isPublished"]
tab: GrapherInterface["tab"]
hasChartTab: GrapherInterface["hasChartTab"]
hasMapTab: GrapherInterface["hasMapTab"]

type?: ChartTypeName
hasChartTab: boolean

lastEditedAt: string
lastEditedBy: string
publishedAt: string
Expand Down Expand Up @@ -142,13 +147,16 @@ export class ChartList extends React.Component<{
}
}

export function showChartType(chart: ChartListItem) {
const chartType = chart.type ?? ChartTypeName.LineChart
export function showChartType(chart: ChartListItem): string {
const chartType = chart.type

if (!chartType) return "Map"

const displayType = ChartTypeName[chartType]
? startCase(ChartTypeName[chartType])
: "Unknown"

if (chart.tab === "map") {
if (chart.tab === GrapherTabOption.map) {
if (chart.hasChartTab) return `Map + ${displayType}`
else return "Map"
} else {
Expand Down
21 changes: 16 additions & 5 deletions adminSiteClient/EditorBasicTab.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,10 @@ class DimensionSlotView<
() => this.grapher.isReady,
() => {
this.disposers.push(
reaction(() => this.grapher.type, this.updateDefaults),
reaction(
() => this.grapher.chartTypes,
this.updateDefaults
),
reaction(
() => this.grapher.yColumnsFromDimensions.length,
this.updateDefaults
Expand Down Expand Up @@ -355,7 +358,9 @@ export class EditorBasicTab<

@action.bound onChartTypeChange(value: string) {
const { grapher } = this.props.editor
grapher.type = value as ChartTypeName

const newChartType = value as ChartTypeName
grapher.chartTypes = [newChartType]

if (grapher.isMarimekko) {
grapher.hideRelativeToggle = false
Expand Down Expand Up @@ -407,7 +412,7 @@ export class EditorBasicTab<

<Section name="Type of chart">
<SelectField
value={grapher.type}
value={grapher.mainChartType}
onValue={this.onChartTypeChange}
options={chartTypes.map((key) => ({
value: key,
Expand All @@ -418,12 +423,18 @@ export class EditorBasicTab<
<Toggle
label="Chart tab"
value={grapher.hasChartTab}
onValue={(value) => (grapher.hasChartTab = value)}
onValue={(shouldHaveChartTab) =>
(grapher.chartTypes = shouldHaveChartTab
? [ChartTypeName.LineChart]
: [])
}
/>
<Toggle
label="Map tab"
value={grapher.hasMapTab}
onValue={(value) => (grapher.hasMapTab = value)}
onValue={(shouldHaveMapTab) =>
(grapher.hasMapTab = shouldHaveMapTab)
}
/>
</FieldsRow>
</Section>
Expand Down
10 changes: 8 additions & 2 deletions adminSiteClient/EditorCustomizeTab.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
ColorSchemeName,
FacetAxisDomain,
FacetStrategy,
ChartTypeName,
} from "@ourworldindata/types"
import { Grapher } from "@ourworldindata/grapher"
import {
Expand Down Expand Up @@ -158,7 +159,10 @@ export class ColorSchemeSelector extends React.Component<{
value={grapher.baseColorScheme}
onChange={this.onChange}
onBlur={this.onBlur}
chartType={this.props.grapher.type}
chartType={
this.props.grapher.mainChartType ??
ChartTypeName.LineChart
}
invertedColorScheme={!!grapher.invertColorScheme}
additionalOptions={[
{
Expand Down Expand Up @@ -751,7 +755,9 @@ export class EditorCustomizeTab<
{grapher.chartInstanceExceptMap.colorScale && (
<EditorColorScaleSection
scale={grapher.chartInstanceExceptMap.colorScale}
chartType={grapher.type}
chartType={
grapher.mainChartType ?? ChartTypeName.LineChart
}
showLineChartColors={grapher.isLineChart}
features={{
visualScaling: true,
Expand Down
5 changes: 4 additions & 1 deletion adminSiteClient/GrapherConfigGridEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -512,7 +512,10 @@ export class GrapherConfigGridEditor extends React.Component<GrapherConfigGridEd
return (
<EditorColorScaleSection
scale={colorScale}
chartType={grapher.type}
chartType={
grapher.mainChartType ??
ChartTypeName.LineChart
}
features={{
visualScaling: true,
legendDescription: false,
Expand Down
7 changes: 5 additions & 2 deletions baker/countryProfiles.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,12 @@ function bakeCache<T>(cacheKey: any, retriever: () => T): T {
return result
}

const hasChartTab = (grapher: GrapherInterface): boolean =>
!grapher.chartTypes || grapher.chartTypes.length > 0

const checkShouldShowIndicator = (grapher: GrapherInterface) =>
(grapher.hasChartTab ?? true) &&
(grapher.type ?? "LineChart") === "LineChart" &&
hasChartTab(grapher) &&
(grapher.chartTypes?.[0] ?? "LineChart") === "LineChart" &&
grapher.dimensions?.length === 1

// Find the charts that will be shown on the country profile page (if they have that country)
Expand Down
4 changes: 3 additions & 1 deletion db/migration/1661264304751-MigrateSelectedData.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ import { MigrationInterface, QueryRunner } from "typeorm"

import { entityNameById } from "./data/entityNameById.js"

import { ChartTypeName, GrapherInterface } from "@ourworldindata/types"
import { ChartTypeName } from "@ourworldindata/types"

type GrapherInterface = Record<string, any>

/**
* Migrate the legacy `selectedData` and get rid of it.
Expand Down
111 changes: 111 additions & 0 deletions db/migration/1731431457062-AddTypesFieldToConfigs.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
import { MigrationInterface, QueryRunner } from "typeorm"

export class AddTypesFieldToConfigs1731431457062 implements MigrationInterface {
private async updateSchema(
queryRunner: QueryRunner,
newVersion: `${number}${number}${number}`
): Promise<void> {
const schema = `https://files.ourworldindata.org/schemas/grapher-schema.${newVersion}.json`
await queryRunner.query(
`
-- sql
UPDATE chart_configs
SET
patch = JSON_SET(patch, '$.$schema', ?),
full = JSON_SET(full, '$.$schema', ?)
`,
[schema, schema]
)
}

private async addTypesFieldToConfigs(
queryRunner: QueryRunner
): Promise<void> {
for (const configType of ["patch", "full"]) {
// if hasChartTab is true, set the types field to the current type
await queryRunner.query(
`
-- sql
UPDATE chart_configs
SET ?? = JSON_SET(
??,
'$.chartTypes',
JSON_ARRAY(?? ->> '$.type')
)
WHERE
COALESCE(?? ->> '$.hasChartTab', 'true') = 'true'
AND ?? ->> '$.type' IS NOT NULL
`,
[configType, configType, configType, configType, configType]
)

// if hasChartTab is false, set the types field to an empty array
await queryRunner.query(
`
-- sql
UPDATE chart_configs
SET ?? = JSON_SET(
??,
'$.chartTypes',
JSON_ARRAY()
)
WHERE ?? ->> '$.hasChartTab' = 'false'
`,
[configType, configType, configType]
)
}
}

private async addDerivedChartTypeColumn(
queryRunner: QueryRunner
): Promise<void> {
await queryRunner.query(
`-- sql
ALTER TABLE chart_configs
ADD COLUMN type VARCHAR(255) GENERATED ALWAYS AS
(
CASE
-- if types is unset, the type defaults to line chart
WHEN full ->> '$.chartTypes' IS NULL THEN 'LineChart'
-- else, the chart type listed first is considered the "main" type
-- (might be null for Graphers without a chart tab)
ELSE full ->> '$.chartTypes[0]'
END
)
VIRTUAL AFTER slug;
`
)
}

private async removeTypeAndHasChartTabFields(
queryRunner: QueryRunner
): Promise<void> {
await queryRunner.query(`
-- sql
UPDATE chart_configs
SET patch = JSON_REMOVE(patch, '$.type', '$.hasChartTab')
`)
}

public async removeDerivedTypeColumn(
queryRunner: QueryRunner
): Promise<void> {
await queryRunner.query(
`-- sql
ALTER TABLE chart_configs
DROP COLUMN type;
`
)
}

public async up(queryRunner: QueryRunner): Promise<void> {
await this.addTypesFieldToConfigs(queryRunner)
await this.removeTypeAndHasChartTabFields(queryRunner)
await this.addDerivedChartTypeColumn(queryRunner)
await this.updateSchema(queryRunner, "006")
}

public async down(queryRunner: QueryRunner): Promise<void> {

Check warning on line 108 in db/migration/1731431457062-AddTypesFieldToConfigs.ts

View workflow job for this annotation

GitHub Actions / eslint

'queryRunner' is defined but never used. Allowed unused args must match /^_/u

Check warning on line 108 in db/migration/1731431457062-AddTypesFieldToConfigs.ts

View workflow job for this annotation

GitHub Actions / eslint

'queryRunner' is defined but never used. Allowed unused args must match /^_/u
// TODO: implement down migration
}
}
6 changes: 3 additions & 3 deletions db/model/Chart.ts
Original file line number Diff line number Diff line change
Expand Up @@ -506,7 +506,7 @@ export interface OldChartFieldList {
id: number
title: string
slug: string
type: string
type?: string
internalNotes: string
variantName: string
isPublished: boolean
Expand All @@ -526,11 +526,11 @@ export const oldChartFieldList = `
charts.id,
chart_configs.full->>"$.title" AS title,
chart_configs.full->>"$.slug" AS slug,
chart_configs.full->>"$.type" AS type,
chart_configs.type AS type,
chart_configs.full->>"$.internalNotes" AS internalNotes,
chart_configs.full->>"$.variantName" AS variantName,
chart_configs.full->>"$.tab" AS tab,
JSON_EXTRACT(chart_configs.full, "$.hasChartTab") = true AS hasChartTab,
chart_configs.type IS NOT NULL AS hasChartTab,
JSON_EXTRACT(chart_configs.full, "$.hasMapTab") = true AS hasMapTab,
JSON_EXTRACT(chart_configs.full, "$.isPublished") = true AS isPublished,
charts.lastEditedAt,
Expand Down
2 changes: 1 addition & 1 deletion db/model/Variable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -880,7 +880,7 @@ export async function getVariableOfDatapageIfApplicable(
// showing a data page.
if (
yVariableIds.length === 1 &&
(grapher.type !== ChartTypeName.ScatterPlot ||
(!grapher.chartTypes?.includes(ChartTypeName.ScatterPlot) ||
xVariableIds.length === 0)
) {
const variableId = yVariableIds[0]
Expand Down
22 changes: 14 additions & 8 deletions devTools/svgTester/utils.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
import { ChartTypeName, GrapherTabOption } from "@ourworldindata/types"
import {
ChartTypeName,
GrapherTabName,
GrapherTabOption,
} from "@ourworldindata/types"
import {
MultipleOwidVariableDataDimensionsMap,
OwidVariableMixedData,
Expand Down Expand Up @@ -89,7 +93,7 @@ export type SvgRenderPerformance = {
export type SvgRecord = {
chartId: number
slug: string
chartType: ChartTypeName | GrapherTabOption | undefined
chartType: GrapherTabName | undefined
queryStr?: string
md5: string
svgFilename: string
Expand Down Expand Up @@ -205,10 +209,11 @@ export async function findChartViewsToGenerate(
const grapherConfig = await parseGrapherConfig(chartId, { inDir })

const slug = grapherConfig.slug ?? chartId.toString()
const chartType = grapherConfig.type ?? ChartTypeName.LineChart
const mainChartType =
grapherConfig.chartTypes?.[0] ?? ChartTypeName.LineChart

const queryStrings = options.shouldTestAllViews
? queryStringsByChartType[chartType]
? queryStringsByChartType[mainChartType]
: options.queryStr
? [options.queryStr]
: [undefined]
Expand All @@ -217,7 +222,7 @@ export async function findChartViewsToGenerate(
chartsToProcess.push({
id: chartId,
slug: slug,
type: chartType,
type: mainChartType,
queryStr,
})
}
Expand Down Expand Up @@ -283,8 +288,9 @@ export async function findValidChartIds(
const grapherConfig = await parseGrapherConfig(grapherId, {
inDir,
})
const chartType = grapherConfig.type ?? ChartTypeName.LineChart
if (chartTypes.includes(chartType)) {
const mainChartType =
grapherConfig.chartTypes?.[0] ?? ChartTypeName.LineChart
if (chartTypes.includes(mainChartType)) {
validChartIds.push(grapherId)
}
}
Expand Down Expand Up @@ -422,7 +428,7 @@ export async function renderSvg(
const svgRecord = {
chartId: configAndData.config.id!,
slug: configAndData.config.slug!,
chartType: grapher.tab === "chart" ? grapher.type : grapher.tab,
chartType: grapher.activeTab,
queryStr,
md5: processSvgAndCalculateHash(svg),
svgFilename: outFilename,
Expand Down
Loading

0 comments on commit c56c672

Please sign in to comment.