diff --git a/adminSiteClient/BulkGrapherConfigEditor.tsx b/adminSiteClient/BulkGrapherConfigEditor.tsx index c4109f42cbc..a3ffbc0bc3d 100644 --- a/adminSiteClient/BulkGrapherConfigEditor.tsx +++ b/adminSiteClient/BulkGrapherConfigEditor.tsx @@ -110,7 +110,6 @@ const bulkChartEditorColumnSets: ColumnSet[] = [ "/baseColorScheme", "/map/colorScale", "/colorScale", - "/hasChartTab", "/hasMapTab", "/type", ], diff --git a/adminSiteClient/ChartList.tsx b/adminSiteClient/ChartList.tsx index 521d71aa992..dccdf863c14 100644 --- a/adminSiteClient/ChartList.tsx +++ b/adminSiteClient/ChartList.tsx @@ -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" @@ -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 @@ -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 { diff --git a/adminSiteClient/EditorBasicTab.tsx b/adminSiteClient/EditorBasicTab.tsx index 879b7ac36bb..3c506fa12e3 100644 --- a/adminSiteClient/EditorBasicTab.tsx +++ b/adminSiteClient/EditorBasicTab.tsx @@ -147,7 +147,10 @@ class DimensionSlotView< () => this.grapher.isReady, () => { this.disposers.push( - reaction(() => this.grapher.type, this.updateDefaults), + reaction( + () => this.grapher.validChartTypes, + this.updateDefaults + ), reaction( () => this.grapher.yColumnsFromDimensions.length, this.updateDefaults @@ -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 @@ -395,7 +400,7 @@ export class EditorBasicTab< render() { const { editor } = this.props const { grapher } = editor - const chartTypes = Object.keys(ChartTypeName).filter( + const allChartTypes = Object.keys(ChartTypeName).filter( (chartType) => chartType !== ChartTypeName.WorldMap ) @@ -407,9 +412,9 @@ export class EditorBasicTab<
({ + options={allChartTypes.map((key) => ({ value: key, label: startCase(key), }))} @@ -418,12 +423,18 @@ export class EditorBasicTab< (grapher.hasChartTab = value)} + onValue={(shouldHaveChartTab) => + (grapher.chartTypes = shouldHaveChartTab + ? [ChartTypeName.LineChart] + : []) + } /> (grapher.hasMapTab = value)} + onValue={(shouldHaveMapTab) => + (grapher.hasMapTab = shouldHaveMapTab) + } />
diff --git a/adminSiteClient/EditorCustomizeTab.tsx b/adminSiteClient/EditorCustomizeTab.tsx index ae3cdf5bedc..2949b38a5d0 100644 --- a/adminSiteClient/EditorCustomizeTab.tsx +++ b/adminSiteClient/EditorCustomizeTab.tsx @@ -6,6 +6,7 @@ import { ColorSchemeName, FacetAxisDomain, FacetStrategy, + ChartTypeName, } from "@ourworldindata/types" import { Grapher } from "@ourworldindata/grapher" import { @@ -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.chartType ?? + ChartTypeName.LineChart + } invertedColorScheme={!!grapher.invertColorScheme} additionalOptions={[ { @@ -751,7 +755,7 @@ export class EditorCustomizeTab< {grapher.chartInstanceExceptMap.colorScale && ( { chart_configs.full->>"$.variantName" AS variantName, chart_configs.full->>"$.isPublished" AS isPublished, chart_configs.full->>"$.tab" AS tab, - JSON_EXTRACT(chart_configs.full, "$.hasChartTab") = true AS hasChartTab, + chart_configs.chartType IS NOT NULL AS hasChartTab, JSON_EXTRACT(chart_configs.full, "$.hasMapTab") = true AS hasMapTab, chart_configs.full->>"$.originUrl" AS originUrl, charts.lastEditedAt, diff --git a/adminSiteServer/app.test.tsx b/adminSiteServer/app.test.tsx index b3fd43c8f7b..e93e0442ac8 100644 --- a/adminSiteServer/app.test.tsx +++ b/adminSiteServer/app.test.tsx @@ -58,12 +58,15 @@ import { import path from "path" import fs from "fs" import { omitUndefinedValues } from "@ourworldindata/utils" +import { defaultGrapherConfig } from "@ourworldindata/grapher" const ADMIN_SERVER_HOST = "localhost" const ADMIN_SERVER_PORT = 8765 const ADMIN_URL = `http://${ADMIN_SERVER_HOST}:${ADMIN_SERVER_PORT}/admin/api` +const currentSchema = defaultGrapherConfig.$schema + jest.setTimeout(10000) // wait for up to 10s for the app server to start let testKnexInstance: Knex | undefined = undefined let serverKnexInstance: Knex | undefined = undefined @@ -192,8 +195,7 @@ async function makeRequestAgainstAdminApi( describe("OwidAdminApp", () => { const testChartConfig = { - $schema: - "https://files.ourworldindata.org/schemas/grapher-schema.005.json", + $schema: currentSchema, slug: "test-chart", title: "Test chart", type: "LineChart", @@ -325,16 +327,14 @@ describe("OwidAdminApp: indicator-level chart configs", () => { display: '{ "unit": "kg", "shortUnit": "kg" }', } const testVariableConfigETL = { - $schema: - "https://files.ourworldindata.org/schemas/grapher-schema.005.json", + $schema: currentSchema, hasMapTab: true, note: "Indicator note", selectedEntityNames: ["France", "Italy", "Spain"], hideRelativeToggle: false, } const testVariableConfigAdmin = { - $schema: - "https://files.ourworldindata.org/schemas/grapher-schema.005.json", + $schema: currentSchema, title: "Admin title", subtitle: "Admin subtitle", } @@ -345,14 +345,12 @@ describe("OwidAdminApp: indicator-level chart configs", () => { id: otherVariableId, } const otherTestVariableConfig = { - $schema: - "https://files.ourworldindata.org/schemas/grapher-schema.005.json", + $schema: currentSchema, note: "Other indicator note", } const testChartConfig = { - $schema: - "https://files.ourworldindata.org/schemas/grapher-schema.005.json", + $schema: currentSchema, slug: "test-chart", title: "Test chart", type: "Marimekko", @@ -366,8 +364,7 @@ describe("OwidAdminApp: indicator-level chart configs", () => { ], } const testMultiDimConfig = { - grapherConfigSchema: - "https://files.ourworldindata.org/schemas/grapher-schema.005.json", + grapherConfigSchema: currentSchema, title: { title: "Energy use", titleVariant: "by energy source", @@ -621,7 +618,7 @@ describe("OwidAdminApp: indicator-level chart configs", () => { expect(parentConfig).toEqual(mergedGrapherConfig) // fetch the full config of the chart and verify that it's been merged - // with the indicator config and the default config + // with the indicator config const fullConfig = await fetchJsonFromAdminApi( `/charts/${chartId}.config.json` ) @@ -640,8 +637,7 @@ describe("OwidAdminApp: indicator-level chart configs", () => { `/charts/${chartId}.patchConfig.json` ) expect(patchConfig).toEqual({ - $schema: - "https://files.ourworldindata.org/schemas/grapher-schema.005.json", + $schema: currentSchema, id: chartId, version: 1, isPublished: false, @@ -696,8 +692,7 @@ describe("OwidAdminApp: indicator-level chart configs", () => { `/charts/${chartId}.patchConfig.json` ) expect(patchConfigAfterDelete).toEqual({ - $schema: - "https://files.ourworldindata.org/schemas/grapher-schema.005.json", + $schema: currentSchema, id: chartId, version: 1, isPublished: false, diff --git a/adminSiteServer/testPageRouter.tsx b/adminSiteServer/testPageRouter.tsx index b0e5a5ac063..9d88e4735ea 100644 --- a/adminSiteServer/testPageRouter.tsx +++ b/adminSiteServer/testPageRouter.tsx @@ -153,17 +153,11 @@ async function propsFromQueryParams( tab = tab || GrapherTabOption.map } else { if (params.type === "LineChart") { - query = query.andWhereRaw( - `( - cc.full->"$.type" = "LineChart" - OR cc.full->"$.type" IS NULL - ) AND COALESCE(cc.full->>"$.hasChartTab", "true") = "true"` - ) + query = query.andWhereRaw(`cc.chartType = "LineChart"`) } else { - query = query.andWhereRaw( - `cc.full->"$.type" = :type AND COALESCE(cc.full->>"$.hasChartTab", "true") = "true"`, - { type: params.type } - ) + query = query.andWhereRaw(`cc.chartType = :type`, { + type: params.type, + }) } tab = tab || GrapherTabOption.chart } @@ -245,9 +239,7 @@ async function propsFromQueryParams( if (tab === GrapherTabOption.map) { query = query.andWhereRaw(`cc.full->>"$.hasMapTab" = "true"`) } else if (tab === GrapherTabOption.chart) { - query = query.andWhereRaw( - `COALESCE(cc.full->>"$.hasChartTab", "true") = "true"` - ) + query = query.andWhereRaw(`cc.chartType IS NOT NULL`) } if (datasetIds.length > 0) { diff --git a/baker/countryProfiles.tsx b/baker/countryProfiles.tsx index 3c774d1aed1..358b762606a 100644 --- a/baker/countryProfiles.tsx +++ b/baker/countryProfiles.tsx @@ -36,9 +36,12 @@ function bakeCache(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) diff --git a/db/migration/1661264304751-MigrateSelectedData.ts b/db/migration/1661264304751-MigrateSelectedData.ts index 2779864cb98..5c8e73382ce 100644 --- a/db/migration/1661264304751-MigrateSelectedData.ts +++ b/db/migration/1661264304751-MigrateSelectedData.ts @@ -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 /** * Migrate the legacy `selectedData` and get rid of it. diff --git a/db/migration/1731431457062-AddTypesFieldToConfigs.ts b/db/migration/1731431457062-AddTypesFieldToConfigs.ts new file mode 100644 index 00000000000..faf175b1c00 --- /dev/null +++ b/db/migration/1731431457062-AddTypesFieldToConfigs.ts @@ -0,0 +1,111 @@ +import { MigrationInterface, QueryRunner } from "typeorm" + +export class AddTypesFieldToConfigs1731431457062 implements MigrationInterface { + private async updateSchema( + queryRunner: QueryRunner, + newVersion: `${number}${number}${number}` + ): Promise { + 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 { + 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 { + await queryRunner.query( + `-- sql + ALTER TABLE chart_configs + ADD COLUMN chartType 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 { + await queryRunner.query(` + -- sql + UPDATE chart_configs + SET patch = JSON_REMOVE(patch, '$.type', '$.hasChartTab') + `) + } + + public async removeDerivedTypeColumn( + queryRunner: QueryRunner + ): Promise { + await queryRunner.query( + `-- sql + ALTER TABLE chart_configs + DROP COLUMN chartType; + ` + ) + } + + public async up(queryRunner: QueryRunner): Promise { + await this.addTypesFieldToConfigs(queryRunner) + await this.removeTypeAndHasChartTabFields(queryRunner) + await this.addDerivedChartTypeColumn(queryRunner) + await this.updateSchema(queryRunner, "006") + } + + public async down(queryRunner: QueryRunner): Promise { + // TODO: implement down migration + } +} diff --git a/db/model/Chart.ts b/db/model/Chart.ts index 637f3f8ccaf..e665f6301c0 100644 --- a/db/model/Chart.ts +++ b/db/model/Chart.ts @@ -506,7 +506,7 @@ export interface OldChartFieldList { id: number title: string slug: string - type: string + type?: string internalNotes: string variantName: string isPublished: boolean @@ -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.chartType 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.chartType 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, @@ -567,9 +567,8 @@ export const getMostViewedGrapherIdsByChartType = async ( JOIN chart_configs cc ON slug = SUBSTRING_INDEX(a.url, '/', -1) JOIN charts c ON c.configId = cc.id WHERE a.url LIKE "https://ourworldindata.org/grapher/%" - AND cc.full ->> "$.type" = ? + AND cc.chartType = ? AND cc.full ->> "$.isPublished" = "true" - and (cc.full ->> "$.hasChartTab" = "true" or cc.full ->> "$.hasChartTab" is null) ORDER BY a.views_365d DESC LIMIT ? `, diff --git a/db/model/Variable.ts b/db/model/Variable.ts index 14a3406f66a..c711945abb4 100644 --- a/db/model/Variable.ts +++ b/db/model/Variable.ts @@ -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] diff --git a/devTools/schemaProcessor/columns.json b/devTools/schemaProcessor/columns.json index 75a5b3579a4..c0a60e7ab5e 100644 --- a/devTools/schemaProcessor/columns.json +++ b/devTools/schemaProcessor/columns.json @@ -248,12 +248,6 @@ "default": false, "editor": "checkbox" }, - { - "type": "boolean", - "pointer": "/hasChartTab", - "default": true, - "editor": "checkbox" - }, { "type": "boolean", "pointer": "/hideLegend", @@ -631,20 +625,9 @@ "editor": "textfield" }, { - "type": "string", - "pointer": "/type", - "default": "LineChart", - "editor": "dropdown", - "enumOptions": [ - "LineChart", - "ScatterPlot", - "StackedArea", - "DiscreteBar", - "StackedDiscreteBar", - "SlopeChart", - "StackedBar", - "Marimekko" - ] + "type": "array", + "pointer": "/types", + "editor": "primitiveListEditor" }, { "type": "boolean", diff --git a/devTools/svgTester/utils.ts b/devTools/svgTester/utils.ts index 239466c9af7..724a41a0492 100644 --- a/devTools/svgTester/utils.ts +++ b/devTools/svgTester/utils.ts @@ -1,4 +1,8 @@ -import { ChartTypeName, GrapherTabOption } from "@ourworldindata/types" +import { + ChartTypeName, + GrapherTabName, + GrapherTabOption, +} from "@ourworldindata/types" import { MultipleOwidVariableDataDimensionsMap, OwidVariableMixedData, @@ -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 @@ -205,7 +209,8 @@ export async function findChartViewsToGenerate( const grapherConfig = await parseGrapherConfig(chartId, { inDir }) const slug = grapherConfig.slug ?? chartId.toString() - const chartType = grapherConfig.type ?? ChartTypeName.LineChart + const chartType = + grapherConfig.chartTypes?.[0] ?? ChartTypeName.LineChart const queryStrings = options.shouldTestAllViews ? queryStringsByChartType[chartType] @@ -283,7 +288,8 @@ export async function findValidChartIds( const grapherConfig = await parseGrapherConfig(grapherId, { inDir, }) - const chartType = grapherConfig.type ?? ChartTypeName.LineChart + const chartType = + grapherConfig.chartTypes?.[0] ?? ChartTypeName.LineChart if (chartTypes.includes(chartType)) { validChartIds.push(grapherId) } @@ -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, diff --git a/packages/@ourworldindata/explorer/src/Explorer.tsx b/packages/@ourworldindata/explorer/src/Explorer.tsx index 65498e161e7..c0595ae7285 100644 --- a/packages/@ourworldindata/explorer/src/Explorer.tsx +++ b/packages/@ourworldindata/explorer/src/Explorer.tsx @@ -8,7 +8,8 @@ import { TableSlug, GrapherInterface, GrapherQueryParams, - GrapherTabOption, + GrapherTabQueryParam, + GrapherTabName, } from "@ourworldindata/types" import { OwidTable, @@ -443,18 +444,26 @@ export class Explorer time: this.grapher.timeParam, } - const previousTab = this.grapher.tab + const previousTab = this.grapher.activeTab this.updateGrapherFromExplorer() - // preserve the previous tab if that's still available in the new view; - // and use the first tab otherwise, ignoring the table - const tabsWithoutTable = this.grapher.availableTabs.filter( - (tab) => tab !== GrapherTabOption.table - ) - newGrapherParams.tab = this.grapher.availableTabs.includes(previousTab) - ? previousTab - : tabsWithoutTable[0] ?? GrapherTabOption.table + if (this.grapher.availableTabs.includes(previousTab)) { + // preserve the previous tab if that's still available in the new view + newGrapherParams.tab = + this.grapher.mapGrapherTabToQueryParam(previousTab) + } else if (this.grapher.validChartTypes.length > 0) { + // otherwise, switch to the first chart tab + newGrapherParams.tab = this.grapher.mapGrapherTabToQueryParam( + this.grapher.validChartTypes[0] as unknown as GrapherTabName + ) + } else if (this.grapher.hasMapTab) { + // or switch to the map, if there is one + newGrapherParams.tab = GrapherTabQueryParam.WorldMap + } else { + // if everything fails, switch to the table tab that is always available + newGrapherParams.tab = GrapherTabQueryParam.Table + } this.grapher.populateFromQueryParams(newGrapherParams) diff --git a/packages/@ourworldindata/explorer/src/ExplorerProgram.ts b/packages/@ourworldindata/explorer/src/ExplorerProgram.ts index 5535566d998..608382dff89 100644 --- a/packages/@ourworldindata/explorer/src/ExplorerProgram.ts +++ b/packages/@ourworldindata/explorer/src/ExplorerProgram.ts @@ -8,6 +8,7 @@ import { FacetAxisDomain, GrapherInterface, AxisMinMaxValueStr, + ChartTypeName, } from "@ourworldindata/types" import { CoreTable, @@ -64,6 +65,7 @@ interface ExplorerGrapherInterface extends GrapherInterface { relatedQuestionText?: string relatedQuestionUrl?: string mapTargetTime?: number + type?: ChartTypeName | "None" } const ExplorerRootDef: CellDef = { diff --git a/packages/@ourworldindata/explorer/src/GrapherGrammar.ts b/packages/@ourworldindata/explorer/src/GrapherGrammar.ts index cc2579ea360..bbcb295f6a6 100644 --- a/packages/@ourworldindata/explorer/src/GrapherGrammar.ts +++ b/packages/@ourworldindata/explorer/src/GrapherGrammar.ts @@ -63,9 +63,14 @@ export const GrapherGrammar: Grammar = { type: { ...StringCellDef, keyword: "type", - description: `The type of chart to show such as LineChart or ScatterPlot.`, - terminalOptions: toTerminalOptions(Object.values(ChartTypeName)), - toGrapherObject: (value) => ({ type: value }), + description: `The type of chart to show such as LineChart or ScatterPlot. If set to None, then the chart tab is hidden.`, + terminalOptions: toTerminalOptions([ + ...Object.values(ChartTypeName), + "None", + ]), + toGrapherObject: (value) => ({ + chartTypes: value === "None" ? [] : [value], + }), }, grapherId: { ...IntegerCellDef, @@ -93,12 +98,6 @@ export const GrapherGrammar: Grammar = { terminalOptions: toTerminalOptions(Object.values(GrapherTabOption)), toGrapherObject: (value) => ({ tab: value }), }, - hasChartTab: { - ...BooleanCellDef, - keyword: "hasChartTab", - description: "Show the chart tab?", - toGrapherObject: (value) => ({ hasChartTab: value }), - }, xSlug: { ...SlugDeclarationCellDef, description: "ColumnSlug for the xAxis", diff --git a/packages/@ourworldindata/grapher/src/captionedChart/CaptionedChart.tsx b/packages/@ourworldindata/grapher/src/captionedChart/CaptionedChart.tsx index 9aaffebffd6..d7ba2a7f0b4 100644 --- a/packages/@ourworldindata/grapher/src/captionedChart/CaptionedChart.tsx +++ b/packages/@ourworldindata/grapher/src/captionedChart/CaptionedChart.tsx @@ -38,6 +38,7 @@ import { GrapherTabOption, RelatedQuestionsConfig, Color, + GrapherTabName, } from "@ourworldindata/types" import { DataTable, DataTableManager } from "../dataTable/DataTable" import { @@ -77,11 +78,11 @@ export interface CaptionedChartManager backgroundColor?: string // state - tab?: GrapherTabOption + activeTab?: GrapherTabName isOnMapTab?: boolean isOnTableTab?: boolean - type: ChartTypeName - typeExceptWhenLineChartAndSingleTimeThenWillBeBarChart?: ChartTypeName + activeChartType?: ChartTypeName + isLineChartThatTurnedIntoDiscreteBar?: boolean showEntitySelectionToggle?: boolean isExportingForSocialMedia?: boolean @@ -199,27 +200,33 @@ export class CaptionedChart extends React.Component { return !this.manager.isOnMapTab && hasStrategy } - @computed get chartTypeName(): ChartTypeName { + @computed get activeChartType(): ChartTypeName | undefined { const { manager } = this - return this.manager.isOnMapTab - ? ChartTypeName.WorldMap - : manager.typeExceptWhenLineChartAndSingleTimeThenWillBeBarChart ?? - manager.type ?? - ChartTypeName.LineChart + if (manager.isOnTableTab) return undefined + if (manager.isOnMapTab) return ChartTypeName.WorldMap + if (manager.isOnChartTab) { + return manager.isLineChartThatTurnedIntoDiscreteBar + ? ChartTypeName.DiscreteBar + : manager.activeChartType + } + return undefined } - renderChart(): React.ReactElement { - const { manager } = this + renderChart(): React.ReactElement | void { + const { manager, activeChartType } = this + + if (!activeChartType) return + const bounds = this.boundsForChartArea const ChartClass = - ChartComponentClassMap.get(this.chartTypeName) ?? DefaultChartClass + ChartComponentClassMap.get(activeChartType) ?? DefaultChartClass // Todo: make FacetChart a chart type name? if (this.isFaceted) return ( ) @@ -228,7 +235,7 @@ export class CaptionedChart extends React.Component { ) } diff --git a/packages/@ourworldindata/grapher/src/chart/ChartUtils.tsx b/packages/@ourworldindata/grapher/src/chart/ChartUtils.tsx index 7a17f9980dd..4a84f2b9721 100644 --- a/packages/@ourworldindata/grapher/src/chart/ChartUtils.tsx +++ b/packages/@ourworldindata/grapher/src/chart/ChartUtils.tsx @@ -1,6 +1,11 @@ import React from "react" import { Box, getCountryByName } from "@ourworldindata/utils" -import { SeriesStrategy, EntityName } from "@ourworldindata/types" +import { + SeriesStrategy, + EntityName, + GrapherTabQueryParam, + ChartTypeName, +} from "@ourworldindata/types" import { LineChartSeries } from "../lineCharts/LineChartConstants" import { SelectionArray } from "../selection/SelectionArray" import { ChartManager } from "./ChartManager" @@ -107,3 +112,54 @@ export function getShortNameForEntity(entityName: string): string | undefined { const country = getCountryByName(entityName) return country?.shortName } + +export function mapQueryParamToChartTypeName( + chartTab: string +): ChartTypeName | undefined { + switch (chartTab) { + case GrapherTabQueryParam.LineChart: + return ChartTypeName.LineChart + case GrapherTabQueryParam.SlopeChart: + return ChartTypeName.SlopeChart + case GrapherTabQueryParam.ScatterPlot: + return ChartTypeName.ScatterPlot + case GrapherTabQueryParam.StackedArea: + return ChartTypeName.StackedArea + case GrapherTabQueryParam.StackedBar: + return ChartTypeName.StackedBar + case GrapherTabQueryParam.DiscreteBar: + return ChartTypeName.DiscreteBar + case GrapherTabQueryParam.StackedDiscreteBar: + return ChartTypeName.StackedDiscreteBar + case GrapherTabQueryParam.Marimekko: + return ChartTypeName.Marimekko + default: + return undefined + } +} + +export function mapChartTypeNameToQueryParam( + chartType: ChartTypeName +): GrapherTabQueryParam { + switch (chartType) { + case ChartTypeName.LineChart: + return GrapherTabQueryParam.LineChart + case ChartTypeName.SlopeChart: + return GrapherTabQueryParam.SlopeChart + case ChartTypeName.ScatterPlot: + return GrapherTabQueryParam.ScatterPlot + case ChartTypeName.StackedArea: + return GrapherTabQueryParam.StackedArea + case ChartTypeName.StackedBar: + return GrapherTabQueryParam.StackedBar + case ChartTypeName.DiscreteBar: + return GrapherTabQueryParam.DiscreteBar + case ChartTypeName.StackedDiscreteBar: + return GrapherTabQueryParam.StackedDiscreteBar + case ChartTypeName.Marimekko: + return GrapherTabQueryParam.Marimekko + // TODO: remove once stricter typed + default: + return GrapherTabQueryParam.LineChart + } +} diff --git a/packages/@ourworldindata/grapher/src/controls/ContentSwitchers.tsx b/packages/@ourworldindata/grapher/src/controls/ContentSwitchers.tsx index 051ba277ffc..6a9087943a1 100644 --- a/packages/@ourworldindata/grapher/src/controls/ContentSwitchers.tsx +++ b/packages/@ourworldindata/grapher/src/controls/ContentSwitchers.tsx @@ -1,20 +1,21 @@ import React from "react" -import { computed } from "mobx" +import { action, computed } from "mobx" import { observer } from "mobx-react" import classnames from "classnames" import { FontAwesomeIcon } from "@fortawesome/react-fontawesome/index.js" import { faTable, faEarthAmericas } from "@fortawesome/free-solid-svg-icons" -import { ChartTypeName, GrapherTabOption } from "@ourworldindata/types" +import { ChartTypeName, GrapherTabName } from "@ourworldindata/types" import { chartIcons } from "./ChartIcons" -import { Bounds, capitalize } from "@ourworldindata/utils" +import { Bounds } from "@ourworldindata/utils" import { TabLabel, Tabs } from "../tabs/Tabs.js" export interface ContentSwitchersManager { - availableTabs?: GrapherTabOption[] - tab?: GrapherTabOption + availableTabs?: GrapherTabName[] + activeTab?: GrapherTabName + hasMultipleChartTypes?: boolean + setTab: (tab: GrapherTabName) => void isNarrow?: boolean isMedium?: boolean - type: ChartTypeName isLineChartThatTurnedIntoDiscreteBar?: boolean } @@ -43,7 +44,7 @@ export class ContentSwitchers extends React.Component<{ return this.props.manager } - @computed private get availableTabs(): GrapherTabOption[] { + @computed private get availableTabs(): GrapherTabName[] { return this.manager.availableTabs || [] } @@ -55,10 +56,6 @@ export class ContentSwitchers extends React.Component<{ return !this.manager.isNarrow } - @computed private get chartType(): ChartTypeName { - return this.manager.type ?? ChartTypeName.LineChart - } - @computed get width(): number { return this.availableTabs.reduce((totalWidth, tab) => { // keep in sync with ContentSwitcher.scss @@ -68,7 +65,12 @@ export class ContentSwitchers extends React.Component<{ let tabWidth = 2 * outerPadding + ICON_WIDTH if (this.showTabLabels) { - const labelWidth = Bounds.forText(capitalize(tab), { + const tabLabel = makeTabLabelText(tab, { + hasMultipleChartTypes: this.manager.hasMultipleChartTypes, + isLineChartThatTurnedIntoDiscreteBar: + this.manager.isLineChartThatTurnedIntoDiscreteBar, + }) + const labelWidth = Bounds.forText(tabLabel, { fontSize: TAB_FONT_SIZE, }).width tabWidth += labelWidth + ICON_PADDING @@ -78,28 +80,18 @@ export class ContentSwitchers extends React.Component<{ }, 0) } - private tabIcon(tab: GrapherTabOption): React.ReactElement { - const { manager } = this - switch (tab) { - case GrapherTabOption.table: - return - case GrapherTabOption.map: - return - case GrapherTabOption.chart: - const chartIcon = manager.isLineChartThatTurnedIntoDiscreteBar - ? chartIcons[ChartTypeName.DiscreteBar] - : chartIcons[this.chartType] - return chartIcon - } - } - @computed private get tabLabels(): TabLabel[] { return this.availableTabs.map((tab) => ({ element: ( - - {this.tabIcon(tab)} - {this.showTabLabels && {tab}} - + ), buttonProps: { "data-track-note": "chart_click_" + tab, @@ -108,24 +100,114 @@ export class ContentSwitchers extends React.Component<{ })) } - render(): React.ReactElement { - const { manager, tabLabels } = this + @computed private get activeTabIndex(): number { + const { activeTab } = this.manager + if (!activeTab) return 0 + return this.availableTabs.indexOf(activeTab) ?? 0 + } - const activeIndex = - (manager.tab && this.availableTabs.indexOf(manager.tab)) ?? 0 + @action.bound setTab(tabIndex: number): void { + const newTab = this.availableTabs[tabIndex] + this.manager.setTab(newTab) + } + render(): React.ReactElement { return ( - (manager.tab = this.availableTabs[index]) - } + labels={this.tabLabels} + activeIndex={this.activeTabIndex} + setActiveIndex={this.setTab} /> ) } } + +function ContentSwitcherTab({ + tab, + showLabel, + hasMultipleChartTypes, + isLineChartThatTurnedIntoDiscreteBar, +}: { + tab: GrapherTabName + showLabel?: boolean + hasMultipleChartTypes?: boolean + isLineChartThatTurnedIntoDiscreteBar?: boolean +}): React.ReactElement { + return ( + + + {showLabel && ( + + {makeTabLabelText(tab, { + isLineChartThatTurnedIntoDiscreteBar, + hasMultipleChartTypes, + })} + + )} + + ) +} + +function TabIcon({ + tab, + isLineChartThatTurnedIntoDiscreteBar, +}: { + tab: GrapherTabName + isLineChartThatTurnedIntoDiscreteBar?: boolean +}): React.ReactElement { + switch (tab) { + case GrapherTabName.Table: + return + case GrapherTabName.WorldMap: + return + default: + const chartIcon = isLineChartThatTurnedIntoDiscreteBar + ? chartIcons[ChartTypeName.DiscreteBar] + : chartIcons[tab as unknown as ChartTypeName] + return chartIcon + } +} + +function makeTabLabelText( + tab: GrapherTabName, + options: { + isLineChartThatTurnedIntoDiscreteBar?: boolean + hasMultipleChartTypes?: boolean + } +): string { + if (tab === GrapherTabName.Table) return "Table" + if (tab === GrapherTabName.WorldMap) return "Map" + if (!options.hasMultipleChartTypes) return "Chart" + + switch (tab) { + case GrapherTabName.LineChart: + return options.isLineChartThatTurnedIntoDiscreteBar ? "Bar" : "Line" + case GrapherTabName.SlopeChart: + return "Slope" + + // chart type labels are preliminary + case GrapherTabName.ScatterPlot: + return "Scatter" + case GrapherTabName.StackedArea: + return "Stacked area" + case GrapherTabName.StackedBar: + return "Stacked bar" + case GrapherTabName.DiscreteBar: + return "Bar" + case GrapherTabName.StackedDiscreteBar: + return "Stacked bar" + case GrapherTabName.Marimekko: + return "Marimekko" + default: + return "Chart" + } +} diff --git a/packages/@ourworldindata/grapher/src/controls/SettingsMenu.tsx b/packages/@ourworldindata/grapher/src/controls/SettingsMenu.tsx index 53cfcae4414..7613fde4322 100644 --- a/packages/@ourworldindata/grapher/src/controls/SettingsMenu.tsx +++ b/packages/@ourworldindata/grapher/src/controls/SettingsMenu.tsx @@ -64,7 +64,7 @@ export interface SettingsMenuManager hideTableFilterToggle?: boolean // chart state - type: ChartTypeName + activeChartType?: ChartTypeName isRelativeMode?: boolean selection?: SelectionArray | EntityName[] canChangeAddOrHighlightEntities?: boolean @@ -101,6 +101,10 @@ export class SettingsMenu extends React.Component<{ return test.showSettingsMenuToggle } + @computed get chartType(): ChartTypeName { + return this.manager.activeChartType ?? ChartTypeName.LineChart + } + @computed get maxWidth(): number { return this.props.maxWidth ?? DEFAULT_BOUNDS.width } @@ -108,7 +112,7 @@ export class SettingsMenu extends React.Component<{ @computed get showYScaleToggle(): boolean | undefined { if (this.manager.hideYScaleToggle) return false if (this.manager.isRelativeMode) return false - if ([StackedArea, StackedBar].includes(this.manager.type)) return false // We currently do not have these charts with log scale + if ([StackedArea, StackedBar].includes(this.chartType)) return false // We currently do not have these charts with log scale return this.manager.yAxis.canChangeScaleType } @@ -123,15 +127,15 @@ export class SettingsMenu extends React.Component<{ return ( !this.manager.hideFacetYDomainToggle && this.manager.facetStrategy !== FacetStrategy.none && - this.manager.type !== StackedDiscreteBar + this.chartType !== StackedDiscreteBar ) } @computed get showZoomToggle(): boolean { - const { type, hideZoomToggle } = this.manager + const { hideZoomToggle } = this.manager return ( !hideZoomToggle && - type === ScatterPlot && + this.chartType === ScatterPlot && this.selectionArray.hasSelection ) } @@ -139,16 +143,16 @@ export class SettingsMenu extends React.Component<{ @computed get showNoDataAreaToggle(): boolean { return ( !this.manager.hideNoDataAreaToggle && - this.manager.type === Marimekko && + this.chartType === Marimekko && this.manager.xColumnSlug !== undefined ) } @computed get showAbsRelToggle(): boolean { - const { type, canToggleRelativeMode, hasTimeline, xOverrideTime } = + const { canToggleRelativeMode, hasTimeline, xOverrideTime } = this.manager if (!canToggleRelativeMode) return false - if (type === ScatterPlot) + if (this.chartType === ScatterPlot) return xOverrideTime === undefined && !!hasTimeline return [ StackedArea, @@ -157,7 +161,7 @@ export class SettingsMenu extends React.Component<{ ScatterPlot, LineChart, Marimekko, - ].includes(type) + ].includes(this.chartType) } @computed get showFacetControl(): boolean { @@ -166,7 +170,6 @@ export class SettingsMenu extends React.Component<{ availableFacetStrategies, hideFacetControl, isOnTableTab, - type, } = this.manager // if there's no choice to be made, don't display a lone button @@ -181,7 +184,7 @@ export class SettingsMenu extends React.Component<{ StackedBar, StackedDiscreteBar, LineChart, - ].includes(type) + ].includes(this.chartType) const hasProjection = filledDimensions.some( (dim) => dim.display.isProjection @@ -259,9 +262,8 @@ export class SettingsMenu extends React.Component<{ return this.props.manager } - @computed get chartType(): string { - const { type } = this.manager - return type.replace(/([A-Z])/g, " $1") + @computed get chartTypeLabel(): string { + return this.chartType.replace(/([A-Z])/g, " $1") } @computed get selectionArray(): SelectionArray { @@ -388,10 +390,10 @@ export class SettingsMenu extends React.Component<{ } @computed get menuContents(): JSX.Element { - const { manager, chartType } = this + const { manager, chartTypeLabel } = this const { isOnTableTab } = manager - const menuTitle = `${isOnTableTab ? "Table" : chartType} settings` + const menuTitle = `${isOnTableTab ? "Table" : chartTypeLabel} settings` return (
diff --git a/packages/@ourworldindata/grapher/src/controls/controlsRow/ControlsRow.tsx b/packages/@ourworldindata/grapher/src/controls/controlsRow/ControlsRow.tsx index 0ae2db8b681..5cc1f330cb5 100644 --- a/packages/@ourworldindata/grapher/src/controls/controlsRow/ControlsRow.tsx +++ b/packages/@ourworldindata/grapher/src/controls/controlsRow/ControlsRow.tsx @@ -2,7 +2,7 @@ import React from "react" import { computed } from "mobx" import { observer } from "mobx-react" -import { Bounds, DEFAULT_BOUNDS, GrapherTabOption } from "@ourworldindata/utils" +import { Bounds, DEFAULT_BOUNDS } from "@ourworldindata/utils" import { ContentSwitchers, ContentSwitchersManager } from "../ContentSwitchers" import { @@ -22,7 +22,6 @@ export interface ControlsRowManager MapProjectionMenuManager, SettingsMenuManager { sidePanelBounds?: Bounds - availableTabs?: GrapherTabOption[] showEntitySelectionToggle?: boolean framePaddingHorizontal?: number framePaddingVertical?: number diff --git a/packages/@ourworldindata/grapher/src/controls/settings/AbsRelToggle.tsx b/packages/@ourworldindata/grapher/src/controls/settings/AbsRelToggle.tsx index 7db655a3e87..fd9e6c17a55 100644 --- a/packages/@ourworldindata/grapher/src/controls/settings/AbsRelToggle.tsx +++ b/packages/@ourworldindata/grapher/src/controls/settings/AbsRelToggle.tsx @@ -9,7 +9,7 @@ const { LineChart, ScatterPlot } = ChartTypeName export interface AbsRelToggleManager { stackMode?: StackMode relativeToggleLabel?: string - type: ChartTypeName + activeChartType?: ChartTypeName } @observer @@ -31,10 +31,10 @@ export class AbsRelToggle extends React.Component<{ } @computed get tooltip(): string { - const { type } = this.manager - return type === ScatterPlot + const { activeChartType } = this.manager + return activeChartType === ScatterPlot ? "Show the percentage change per year over the the selected time range." - : type === LineChart + : activeChartType === LineChart ? "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." } diff --git a/packages/@ourworldindata/grapher/src/core/Grapher.jsdom.test.ts b/packages/@ourworldindata/grapher/src/core/Grapher.jsdom.test.ts index c8ad4c18e69..956e6dd4e00 100755 --- a/packages/@ourworldindata/grapher/src/core/Grapher.jsdom.test.ts +++ b/packages/@ourworldindata/grapher/src/core/Grapher.jsdom.test.ts @@ -10,6 +10,7 @@ import { GrapherQueryParams, LegacyGrapherInterface, LegacyGrapherQueryParams, + GrapherTabName, } from "@ourworldindata/types" import { TimeBoundValue, @@ -71,7 +72,7 @@ it("can get dimension slots", () => { const grapher = new Grapher() expect(grapher.dimensionSlots.length).toBe(2) - grapher.type = ChartTypeName.ScatterPlot + grapher.chartTypes = [ChartTypeName.ScatterPlot] expect(grapher.dimensionSlots.length).toBe(4) }) @@ -86,7 +87,7 @@ it("an empty Grapher serializes to an object that includes only the schema", () it("a bad chart type does not crash grapher", () => { const input = { - type: "fff" as any, + chartTypes: ["fff" as any], } expect(new Grapher(input).toObject()).toEqual({ ...input, @@ -211,22 +212,22 @@ it("can generate a url with country selection even if there is no entity code", describe("hasTimeline", () => { it("charts with timeline", () => { const grapher = new Grapher(legacyConfig) - grapher.type = ChartTypeName.LineChart + grapher.chartTypes = [ChartTypeName.LineChart] expect(grapher.hasTimeline).toBeTruthy() - grapher.type = ChartTypeName.SlopeChart + grapher.chartTypes = [ChartTypeName.SlopeChart] expect(grapher.hasTimeline).toBeTruthy() - grapher.type = ChartTypeName.StackedArea + grapher.chartTypes = [ChartTypeName.StackedArea] expect(grapher.hasTimeline).toBeTruthy() - grapher.type = ChartTypeName.StackedBar + grapher.chartTypes = [ChartTypeName.StackedBar] expect(grapher.hasTimeline).toBeTruthy() - grapher.type = ChartTypeName.DiscreteBar + grapher.chartTypes = [ChartTypeName.DiscreteBar] expect(grapher.hasTimeline).toBeTruthy() }) it("map tab has timeline even if chart doesn't", () => { const grapher = new Grapher(legacyConfig) grapher.hideTimeline = true - grapher.type = ChartTypeName.LineChart + grapher.chartTypes = [ChartTypeName.LineChart] expect(grapher.hasTimeline).toBeFalsy() grapher.tab = GrapherTabOption.map expect(grapher.hasTimeline).toBeTruthy() @@ -379,7 +380,7 @@ describe("authors can use maxTime", () => { const table = SynthesizeGDPTable({ timeRange: [2000, 2010] }) const grapher = new Grapher({ table, - type: ChartTypeName.DiscreteBar, + chartTypes: [ChartTypeName.DiscreteBar], selectedEntityNames: table.availableEntityNames, maxTime: 2005, ySlugs: "GDP", @@ -509,6 +510,73 @@ describe("urls", () => { grapher.populateFromQueryParams(url.queryParams) expect(grapher.selection.selectedEntityNames).toEqual(["usa", "canada"]) }) + + it("parses tab=table correctly", () => { + const grapher = new Grapher() + grapher.populateFromQueryParams({ tab: "table" }) + expect(grapher.activeTab).toEqual(GrapherTabName.Table) + }) + + it("parses tab=map correctly", () => { + const grapher = new Grapher() + grapher.populateFromQueryParams({ tab: "map" }) + expect(grapher.activeTab).toEqual(GrapherTabName.WorldMap) + }) + + it("parses tab=chart correctly", () => { + const grapher = new Grapher({ chartTypes: [ChartTypeName.ScatterPlot] }) + grapher.populateFromQueryParams({ tab: "chart" }) + expect(grapher.activeTab).toEqual(GrapherTabName.ScatterPlot) + }) + + it("parses tab=line and tab=slope correctly", () => { + const grapher = new Grapher({ + chartTypes: [ChartTypeName.LineChart, ChartTypeName.SlopeChart], + }) + grapher.populateFromQueryParams({ tab: "line" }) + expect(grapher.activeTab).toEqual(GrapherTabName.LineChart) + grapher.populateFromQueryParams({ tab: "slope" }) + expect(grapher.activeTab).toEqual(GrapherTabName.SlopeChart) + }) + + it("switches to the first chart tab if the given chart isn't available", () => { + const grapher = new Grapher({ + chartTypes: [ChartTypeName.LineChart, ChartTypeName.SlopeChart], + }) + grapher.populateFromQueryParams({ tab: "bar" }) + expect(grapher.activeTab).toEqual(GrapherTabName.LineChart) + }) + + it("switches to the map tab if no chart is available", () => { + const grapher = new Grapher({ chartTypes: [], hasMapTab: true }) + grapher.populateFromQueryParams({ tab: "line" }) + expect(grapher.activeTab).toEqual(GrapherTabName.WorldMap) + }) + + it("switches to the table tab if it's the only tab available", () => { + const grapher = new Grapher({ chartTypes: [] }) + grapher.populateFromQueryParams({ tab: "line" }) + expect(grapher.activeTab).toEqual(GrapherTabName.Table) + }) + + it("adds tab=chart to the URL if there is a single chart tab", () => { + const grapher = new Grapher({ + hasMapTab: true, + tab: GrapherTabOption.map, + }) + grapher.setTab(GrapherTabName.LineChart) + expect(grapher.changedParams.tab).toEqual("chart") + }) + + it("adds the chart type name as tab query param if there are multiple chart tabs", () => { + const grapher = new Grapher({ + chartTypes: [ChartTypeName.LineChart, ChartTypeName.SlopeChart], + hasMapTab: true, + tab: GrapherTabOption.map, + }) + grapher.setTab(GrapherTabName.LineChart) + expect(grapher.changedParams.tab).toEqual("line") + }) }) describe("time domain tests", () => { @@ -918,7 +986,7 @@ it("correctly identifies activeColumnSlugs", () => { `) const grapher = new Grapher({ table, - type: ChartTypeName.ScatterPlot, + chartTypes: [ChartTypeName.ScatterPlot], xSlug: "gdp", ySlugs: "child_mortality", colorSlug: "continent", @@ -955,9 +1023,9 @@ it("considers map tolerance before using column tolerance", () => { const grapher = new Grapher({ table, - type: ChartTypeName.WorldMap, ySlugs: "gdp", tab: GrapherTabOption.map, + hasMapTab: true, map: new MapConfig({ timeTolerance: 1, columnSlug: "gdp", time: 2002 }), }) @@ -1016,7 +1084,7 @@ describe("tableForSelection", () => { const grapher = new Grapher({ table, - type: ChartTypeName.ScatterPlot, + chartTypes: [ChartTypeName.ScatterPlot], excludedEntities: [3], xSlug: "x", ySlugs: "y", @@ -1052,7 +1120,7 @@ it("handles tolerance when there are gaps in ScatterPlot data", () => { const grapher = new Grapher({ table, - type: ChartTypeName.ScatterPlot, + chartTypes: [ChartTypeName.ScatterPlot], xSlug: "x", ySlugs: "y", minTime: 1999, diff --git a/packages/@ourworldindata/grapher/src/core/Grapher.stories.tsx b/packages/@ourworldindata/grapher/src/core/Grapher.stories.tsx index a5d3b43b6bc..84248560305 100644 --- a/packages/@ourworldindata/grapher/src/core/Grapher.stories.tsx +++ b/packages/@ourworldindata/grapher/src/core/Grapher.stories.tsx @@ -49,7 +49,7 @@ export const Line = (): React.ReactElement => export const SlopeChart = (): React.ReactElement => { const model = { - type: ChartTypeName.SlopeChart, + chartTypes: [ChartTypeName.SlopeChart], ...basics, } return @@ -57,7 +57,7 @@ export const SlopeChart = (): React.ReactElement => { export const ScatterPlot = (): React.ReactElement => { const model = { - type: ChartTypeName.ScatterPlot, + chartTypes: [ChartTypeName.ScatterPlot], ...basics, } return @@ -65,7 +65,7 @@ export const ScatterPlot = (): React.ReactElement => { export const DiscreteBar = (): React.ReactElement => { const model = { - type: ChartTypeName.DiscreteBar, + chartTypes: [ChartTypeName.DiscreteBar], ...basics, } return @@ -73,7 +73,7 @@ export const DiscreteBar = (): React.ReactElement => { export const StackedBar = (): React.ReactElement => { const model = { - type: ChartTypeName.StackedBar, + chartTypes: [ChartTypeName.StackedBar], ...basics, } return @@ -81,7 +81,7 @@ export const StackedBar = (): React.ReactElement => { export const StackedArea = (): React.ReactElement => { const model = { - type: ChartTypeName.StackedArea, + chartTypes: [ChartTypeName.StackedArea], ...basics, } return @@ -97,7 +97,6 @@ export const MapFirst = (): React.ReactElement => { export const BlankGrapher = (): React.ReactElement => { const model = { - type: ChartTypeName.WorldMap, tab: GrapherTabOption.map, table: BlankOwidTable(), hasMapTab: true, @@ -115,7 +114,7 @@ export const NoMap = (): React.ReactElement => { export const Faceting = (): React.ReactElement => { const model = { - type: ChartTypeName.StackedArea, + chartTypes: [ChartTypeName.StackedArea], facet: FacetStrategy.entity, ...basics, } @@ -161,7 +160,7 @@ class PerfGrapher extends React.Component {
diff --git a/packages/@ourworldindata/grapher/src/core/Grapher.tsx b/packages/@ourworldindata/grapher/src/core/Grapher.tsx index cf38c20e544..eaf418865fd 100644 --- a/packages/@ourworldindata/grapher/src/core/Grapher.tsx +++ b/packages/@ourworldindata/grapher/src/core/Grapher.tsx @@ -66,6 +66,7 @@ import { sortBy, extractDetailsFromSyntax, omit, + areSetsEqual, } from "@ourworldindata/utils" import { MarkdownTextWrap, @@ -105,6 +106,8 @@ import { GrapherWindowType, Color, GRAPHER_QUERY_PARAM_KEYS, + GrapherTabName, + GrapherTabQueryParam, } from "@ourworldindata/types" import { BlankOwidTable, @@ -129,6 +132,7 @@ import { isPopulationVariableETLPath, GRAPHER_BACKGROUND_BEIGE, GRAPHER_BACKGROUND_DEFAULT, + validChartTypeCombinations, } from "../core/GrapherConstants" import { defaultGrapherConfig } from "../schema/defaultGrapherConfig" import { loadVariableDataAndMetadata } from "./loadVariable" @@ -191,6 +195,8 @@ import { ScatterPlotManager } from "../scatterCharts/ScatterPlotChartConstants" import { autoDetectSeriesStrategy, autoDetectYColumnSlugs, + mapChartTypeNameToQueryParam, + mapQueryParamToChartTypeName, } from "../chart/ChartUtils" import classnames from "classnames" import { GrapherAnalytics } from "./GrapherAnalytics" @@ -301,6 +307,7 @@ export interface GrapherProgrammaticInterface extends GrapherInterface { hideTableFilterToggle?: boolean forceHideAnnotationFieldsInTitle?: AnnotationFieldsInTitle hasTableTab?: boolean + hideChartTabs?: boolean hideShareButton?: boolean hideExploreTheDataButton?: boolean hideRelatedQuestion?: boolean @@ -350,7 +357,7 @@ export class Grapher SlopeChartManager { @observable.ref $schema = defaultGrapherConfig.$schema - @observable.ref type = ChartTypeName.LineChart + @observable.ref chartTypes = [ChartTypeName.LineChart] @observable.ref id?: number = undefined @observable.ref version = 1 @observable.ref slug?: string = undefined @@ -384,9 +391,9 @@ export class Grapher @observable.ref hideScatterLabels?: boolean = undefined @observable.ref zoomToSelection?: boolean = undefined @observable.ref showYearLabels?: boolean = undefined // Always show year in labels for bar charts - @observable.ref hasChartTab = true @observable.ref hasMapTab = false @observable.ref tab = GrapherTabOption.chart + @observable.ref chartTab?: ChartTypeName // TODO: remove map from ChartTypeName @observable.ref isPublished?: boolean = undefined @observable.ref baseColorScheme?: ColorSchemeName = undefined @observable.ref invertColorScheme?: boolean = undefined @@ -605,13 +612,10 @@ export class Grapher @action.bound populateFromQueryParams(params: GrapherQueryParams): void { // Set tab if specified - const tab = params.tab - if (tab) { - if (this.availableTabs.includes(tab as any)) { - this.tab = tab as GrapherTabOption - } else { - console.error("Unexpected tab: " + tab) - } + if (params.tab) { + const tab = this.mapQueryParamToGrapherTab(params.tab) + if (tab) this.setTab(tab) + else console.error("Unexpected tab: " + params.tab) } // Set overlay if specified @@ -693,6 +697,29 @@ export class Grapher ) as TimeBounds } + @computed get activeTab(): GrapherTabName { + if (this.tab === GrapherTabOption.table) return GrapherTabName.Table + if (this.tab === GrapherTabOption.map) return GrapherTabName.WorldMap + if (this.chartTab) return this.chartTab as unknown as GrapherTabName + return ( + (this.validChartTypes[0] as unknown as GrapherTabName) ?? + GrapherTabName.LineChart + ) + } + + @computed get activeChartType(): ChartTypeName | undefined { + if (!this.isOnChartTab) return undefined + return this.activeTab as unknown as ChartTypeName + } + + @computed get chartType(): ChartTypeName { + return this.validChartTypes[0] ?? ChartTypeName.LineChart + } + + @computed get hasChartTab(): boolean { + return this.validChartTypes.length > 0 + } + @computed get isOnChartTab(): boolean { return this.tab === GrapherTabOption.chart } @@ -953,7 +980,7 @@ export class Grapher properties: [ // might be missing for charts within explorers or mdims ["slug", this.slug ?? "missing-slug"], - ["chartType", this.type], + ["chartTypes", this.validChartTypes], ["tab", this.tab], ], }, @@ -1267,6 +1294,19 @@ export class Grapher this.disposers.forEach((dispose) => dispose()) } + @action.bound setTab(newTab: GrapherTabName): void { + if (newTab === GrapherTabName.Table) { + this.tab = GrapherTabOption.table + this.chartTab = undefined + } else if (newTab === GrapherTabName.WorldMap) { + this.tab = GrapherTabOption.map + this.chartTab = undefined + } else { + this.tab = GrapherTabOption.chart + this.chartTab = newTab as unknown as ChartTypeName + } + } + // todo: can we remove this? // I believe these states can only occur during editing. @action.bound private ensureValidConfigWhenEditing(): void { @@ -1278,8 +1318,8 @@ export class Grapher ) const disposers = [ autorun(() => { - if (!this.availableTabs.includes(this.tab)) - runInAction(() => (this.tab = this.availableTabs[0])) + if (!this.availableTabs.includes(this.activeTab)) + runInAction(() => this.setTab(this.availableTabs[0])) }), autorun(() => { const validDimensions = this.validDimensions @@ -1481,12 +1521,32 @@ export class Grapher }) } - @computed get availableTabs(): GrapherTabOption[] { + @computed get validChartTypes(): ChartTypeName[] { + const { chartTypes } = this + + if (chartTypes.length <= 1) return chartTypes + + const chartTypeSet = new Set(chartTypes) + for (const validCombination of validChartTypeCombinations) { + const validCombinationSet = new Set(validCombination) + if (areSetsEqual(chartTypeSet, validCombinationSet)) + return validCombination + } + + // if the given combination is not valid, then ignore all but the first chart type + return chartTypes.slice(0, 1) + } + + @computed get availableTabs(): GrapherTabName[] { return [ - this.hasTableTab && GrapherTabOption.table, - this.hasMapTab && GrapherTabOption.map, - this.hasChartTab && GrapherTabOption.chart, - ].filter(identity) as GrapherTabOption[] + this.hasTableTab && GrapherTabName.Table, + this.hasMapTab && GrapherTabName.WorldMap, + ...this.validChartTypes, + ].filter(identity) as GrapherTabName[] + } + + @computed get hasMultipleChartTypes(): boolean { + return this.validChartTypes.length > 1 } @computed get currentSubtitle(): string { @@ -1868,32 +1928,32 @@ export class Grapher // Switch to bar chart if a single year is selected. Todo: do we want to do this? return this.isLineChartThatTurnedIntoDiscreteBar ? ChartTypeName.DiscreteBar - : this.type + : this.chartType ?? ChartTypeName.LineChart } @computed get isLineChart(): boolean { - return this.type === ChartTypeName.LineChart + return this.chartType === ChartTypeName.LineChart } @computed get isScatter(): boolean { - return this.type === ChartTypeName.ScatterPlot + return this.chartType === ChartTypeName.ScatterPlot } @computed get isStackedArea(): boolean { - return this.type === ChartTypeName.StackedArea + return this.chartType === ChartTypeName.StackedArea } @computed get isSlopeChart(): boolean { - return this.type === ChartTypeName.SlopeChart + return this.chartType === ChartTypeName.SlopeChart } @computed get isDiscreteBar(): boolean { - return this.type === ChartTypeName.DiscreteBar + return this.chartType === ChartTypeName.DiscreteBar } @computed get isStackedBar(): boolean { - return this.type === ChartTypeName.StackedBar + return this.chartType === ChartTypeName.StackedBar } @computed get isMarimekko(): boolean { - return this.type === ChartTypeName.Marimekko + return this.chartType === ChartTypeName.Marimekko } @computed get isStackedDiscreteBar(): boolean { - return this.type === ChartTypeName.StackedDiscreteBar + return this.chartType === ChartTypeName.StackedDiscreteBar } @computed get isLineChartThatTurnedIntoDiscreteBar(): boolean { @@ -2372,7 +2432,7 @@ export class Grapher } @action.bound private toggleTabCommand(): void { - this.tab = next(this.availableTabs, this.tab) + this.setTab(next(this.availableTabs, this.activeTab)) } @action.bound private togglePlayingCommand(): void { @@ -3140,9 +3200,55 @@ export class Grapher debounceMode = false + private mapQueryParamToGrapherTab(tab: string): GrapherTabName | undefined { + const { validChartTypes, hasMapTab } = this + + if (tab === GrapherTabQueryParam.Table) { + return GrapherTabName.Table + } + if (tab === GrapherTabQueryParam.WorldMap) { + return GrapherTabName.WorldMap + } + + const defaultChartType = validChartTypes[0] + if (tab === GrapherTabQueryParam.Chart) { + if (defaultChartType) { + return defaultChartType as unknown as GrapherTabName + } else if (hasMapTab) { + return GrapherTabName.WorldMap + } else { + return GrapherTabName.Table + } + } + + const chartTypeName = mapQueryParamToChartTypeName(tab) + + if (!chartTypeName) return undefined + + if (validChartTypes.includes(chartTypeName)) { + return chartTypeName as unknown as GrapherTabName + } else if (defaultChartType) { + return defaultChartType as unknown as GrapherTabName + } else if (hasMapTab) { + return GrapherTabName.WorldMap + } else { + return GrapherTabName.Table + } + } + + mapGrapherTabToQueryParam(tab: GrapherTabName): string { + if (tab === GrapherTabName.Table) return GrapherTabQueryParam.Table + if (tab === GrapherTabName.WorldMap) + return GrapherTabQueryParam.WorldMap + + if (!this.hasMultipleChartTypes) return GrapherTabQueryParam.Chart + + return mapChartTypeNameToQueryParam(tab as unknown as ChartTypeName) + } + @computed.struct get allParams(): GrapherQueryParams { const params: GrapherQueryParams = {} - params.tab = this.tab + params.tab = this.mapGrapherTabToQueryParam(this.activeTab) params.xScale = this.xAxis.scaleType params.yScale = this.yAxis.scaleType params.stackMode = this.stackMode @@ -3473,6 +3579,7 @@ export class Grapher changeInPrefix: false, } @observable hasTableTab = true + @observable hideChartTabs = false @observable hideShareButton = false @observable hideExploreTheDataButton = true @observable hideRelatedQuestion = false diff --git a/packages/@ourworldindata/grapher/src/core/GrapherConstants.ts b/packages/@ourworldindata/grapher/src/core/GrapherConstants.ts index 3fabd87cb2a..2c80e8c26f4 100644 --- a/packages/@ourworldindata/grapher/src/core/GrapherConstants.ts +++ b/packages/@ourworldindata/grapher/src/core/GrapherConstants.ts @@ -1,3 +1,4 @@ +import { ChartTypeName } from "@ourworldindata/types" import type { GrapherProgrammaticInterface } from "./Grapher" export const GRAPHER_EMBEDDED_FIGURE_ATTR = "data-grapher-src" @@ -75,7 +76,7 @@ export enum Patterns { noDataPatternForMapChart = "noDataPatternForMapChart", } -export const grapherInterfaceWithHiddenControlsOnly: GrapherProgrammaticInterface = +export const grapherInterfaceWithHiddenControls: GrapherProgrammaticInterface = { hideRelativeToggle: true, hideTimeline: true, @@ -93,9 +94,12 @@ export const grapherInterfaceWithHiddenControlsOnly: GrapherProgrammaticInterfac }, } -export const grapherInterfaceWithHiddenTabsOnly: GrapherProgrammaticInterface = - { - hasChartTab: false, - hasMapTab: false, - hasTableTab: false, - } +export const grapherInterfaceWithHiddenTabs: GrapherProgrammaticInterface = { + hasMapTab: false, + hasTableTab: false, + hideChartTabs: true, +} + +export const validChartTypeCombinations = [ + [ChartTypeName.LineChart, ChartTypeName.SlopeChart], +] diff --git a/packages/@ourworldindata/grapher/src/core/GrapherWithChartTypes.jsdom.test.tsx b/packages/@ourworldindata/grapher/src/core/GrapherWithChartTypes.jsdom.test.tsx index b82ca6f925e..467fa2af3c4 100755 --- a/packages/@ourworldindata/grapher/src/core/GrapherWithChartTypes.jsdom.test.tsx +++ b/packages/@ourworldindata/grapher/src/core/GrapherWithChartTypes.jsdom.test.tsx @@ -50,7 +50,7 @@ const basicGrapherConfig: GrapherProgrammaticInterface = { describe("grapher and discrete bar charts", () => { const grapher = new Grapher({ - type: ChartTypeName.DiscreteBar, + chartTypes: [ChartTypeName.DiscreteBar], ...basicGrapherConfig, }) expect(grapher.chartInstance.series.length).toBeGreaterThan(0) diff --git a/packages/@ourworldindata/grapher/src/core/LegacyToOwidTable.test.ts b/packages/@ourworldindata/grapher/src/core/LegacyToOwidTable.test.ts index 29c8fcf6302..c292f2ad16a 100755 --- a/packages/@ourworldindata/grapher/src/core/LegacyToOwidTable.test.ts +++ b/packages/@ourworldindata/grapher/src/core/LegacyToOwidTable.test.ts @@ -466,7 +466,7 @@ describe(legacyToOwidTableAndDimensions, () => { it("joins targetTime", () => { const scatterLegacyGrapherConfig = { ...legacyGrapherConfig, - type: ChartTypeName.ScatterPlot, + chartTypes: [ChartTypeName.ScatterPlot], } const { table } = legacyToOwidTableAndDimensions( diff --git a/packages/@ourworldindata/grapher/src/core/LegacyToOwidTable.ts b/packages/@ourworldindata/grapher/src/core/LegacyToOwidTable.ts index dad8561d05c..19e1bd42d86 100644 --- a/packages/@ourworldindata/grapher/src/core/LegacyToOwidTable.ts +++ b/packages/@ourworldindata/grapher/src/core/LegacyToOwidTable.ts @@ -198,9 +198,10 @@ export const legacyToOwidTableAndDimensions = ( // We do this by dropping the column. We interpolate before which adds an originalTime // column which can be used to recover the time. const targetTime = dimension?.targetYear + const chartType = grapherConfig.chartTypes?.[0] if ( - (grapherConfig.type === ChartTypeName.ScatterPlot || - grapherConfig.type === ChartTypeName.Marimekko) && + (chartType === ChartTypeName.ScatterPlot || + chartType === ChartTypeName.Marimekko) && isNumber(targetTime) ) { variableTable = variableTable diff --git a/packages/@ourworldindata/grapher/src/dataTable/DataTable.jsdom.test.tsx b/packages/@ourworldindata/grapher/src/dataTable/DataTable.jsdom.test.tsx index 0825a56dece..c52f0c833af 100755 --- a/packages/@ourworldindata/grapher/src/dataTable/DataTable.jsdom.test.tsx +++ b/packages/@ourworldindata/grapher/src/dataTable/DataTable.jsdom.test.tsx @@ -70,7 +70,7 @@ describe("when you select a range of years", () => { let view: ReactWrapper beforeAll(() => { const grapher = childMortalityGrapher({ - type: ChartTypeName.LineChart, + chartTypes: [ChartTypeName.LineChart], tab: GrapherTabOption.table, }) grapher.timelineHandleTimeBounds = [1950, 2019] diff --git a/packages/@ourworldindata/grapher/src/index.ts b/packages/@ourworldindata/grapher/src/index.ts index 203bb60db3a..21fb90a2ce4 100644 --- a/packages/@ourworldindata/grapher/src/index.ts +++ b/packages/@ourworldindata/grapher/src/index.ts @@ -22,8 +22,8 @@ export { ThereWasAProblemLoadingThisChart, WorldEntityName, Patterns, - grapherInterfaceWithHiddenControlsOnly, - grapherInterfaceWithHiddenTabsOnly, + grapherInterfaceWithHiddenControls, + grapherInterfaceWithHiddenTabs, CONTINENTS_INDICATOR_ID, POPULATION_INDICATOR_ID_USED_IN_ADMIN, } from "./core/GrapherConstants" diff --git a/packages/@ourworldindata/grapher/src/schema/defaultGrapherConfig.ts b/packages/@ourworldindata/grapher/src/schema/defaultGrapherConfig.ts index dd0f68334e2..cb53a48545a 100644 --- a/packages/@ourworldindata/grapher/src/schema/defaultGrapherConfig.ts +++ b/packages/@ourworldindata/grapher/src/schema/defaultGrapherConfig.ts @@ -4,11 +4,17 @@ import { GrapherInterface } from "@ourworldindata/types" -export const latestSchemaVersion = "005" as const -export const outdatedSchemaVersions = ["001", "002", "003", "004"] as const +export const latestSchemaVersion = "006" as const +export const outdatedSchemaVersions = [ + "001", + "002", + "003", + "004", + "005", +] as const export const defaultGrapherConfig = { - $schema: "https://files.ourworldindata.org/schemas/grapher-schema.005.json", + $schema: "https://files.ourworldindata.org/schemas/grapher-schema.006.json", map: { projection: "World", hideTimeline: false, @@ -33,7 +39,6 @@ export const defaultGrapherConfig = { }, tab: "chart", matchingEntitiesOnly: false, - hasChartTab: true, hideLegend: false, hideLogo: false, timelineMinTime: "earliest", @@ -54,7 +59,7 @@ export const defaultGrapherConfig = { facettingLabelByYVariables: "metric", addCountryMode: "add-country", compareEndPointsOnly: false, - type: "LineChart", + types: ["LineChart"], hasMapTab: false, stackMode: "absolute", minTime: "earliest", diff --git a/packages/@ourworldindata/grapher/src/schema/grapher-schema.005.yaml b/packages/@ourworldindata/grapher/src/schema/grapher-schema.006.yaml similarity index 97% rename from packages/@ourworldindata/grapher/src/schema/grapher-schema.005.yaml rename to packages/@ourworldindata/grapher/src/schema/grapher-schema.006.yaml index 4465a68f6c0..b6b77c94245 100644 --- a/packages/@ourworldindata/grapher/src/schema/grapher-schema.005.yaml +++ b/packages/@ourworldindata/grapher/src/schema/grapher-schema.006.yaml @@ -1,7 +1,7 @@ $schema: "http://json-schema.org/draft-07/schema#" # if you update the required keys, make sure that the mergeGrapherConfigs and # diffGrapherConfigs functions both reflect this change -$id: "https://files.ourworldindata.org/schemas/grapher-schema.005.json" +$id: "https://files.ourworldindata.org/schemas/grapher-schema.006.json" required: - $schema - dimensions @@ -14,13 +14,13 @@ properties: type: string description: Url of the concrete schema version to use to validate this document format: uri - default: "https://files.ourworldindata.org/schemas/grapher-schema.005.json" + default: "https://files.ourworldindata.org/schemas/grapher-schema.006.json" # for now, we only validate configs in our database using this schema. # since we expect all configs in our database to be valid against the latest schema, # we restrict the $schema field to a single value, the latest schema version. # if we ever need to validate configs against multiple schema versions, # we can remove this constraint. - const: "https://files.ourworldindata.org/schemas/grapher-schema.005.json" + const: "https://files.ourworldindata.org/schemas/grapher-schema.006.json" id: type: integer description: Internal DB id. Useful internally for OWID but not required if just using grapher directly. @@ -123,10 +123,6 @@ properties: type: boolean default: false description: Exclude entities that do not belong in any color group - hasChartTab: - type: boolean - default: true - description: Whether to show the (non-map) chart tab hideLegend: type: boolean default: false @@ -368,19 +364,21 @@ properties: title: type: string description: Big title text of the chart - type: - type: string - description: Which type of chart should be shown (hasMapChart can be used to always also show a map chart) - default: LineChart - enum: - - LineChart - - ScatterPlot - - StackedArea - - DiscreteBar - - StackedDiscreteBar - - SlopeChart - - StackedBar - - Marimekko + types: + type: array + description: Which chart types should be shown + default: ["LineChart"] + items: + type: string + enum: + - LineChart + - ScatterPlot + - StackedArea + - DiscreteBar + - StackedDiscreteBar + - SlopeChart + - StackedBar + - Marimekko hasMapTab: type: boolean default: false diff --git a/packages/@ourworldindata/grapher/src/schema/migrations/migrations.ts b/packages/@ourworldindata/grapher/src/schema/migrations/migrations.ts index 7a4fefa30f0..2a860c0cb9a 100644 --- a/packages/@ourworldindata/grapher/src/schema/migrations/migrations.ts +++ b/packages/@ourworldindata/grapher/src/schema/migrations/migrations.ts @@ -15,6 +15,7 @@ import { getSchemaVersion, isLatestVersion, } from "./helpers" +import { ChartTypeName } from "@ourworldindata/types" // see https://github.com/owid/owid-grapher/commit/26f2a0d1790c71bdda7e12f284ca552945d2f6ef const migrateFrom001To002 = ( @@ -60,6 +61,23 @@ const migrateFrom004To005 = ( return config } +const migrateFrom005To006 = ( + config: AnyConfigWithValidSchema +): AnyConfigWithValidSchema => { + const { type = ChartTypeName.LineChart, hasChartTab = true } = config + + // add types field + if (!hasChartTab) config.chartTypes = [] + else if (type !== ChartTypeName.LineChart) config.chartTypes = [type] + + // remove deprecated fields + delete config.type + delete config.hasChartTab + + config.$schema = createSchemaForVersion("006") + return config +} + export const runMigration = ( config: AnyConfigWithValidSchema ): AnyConfigWithValidSchema => { @@ -70,5 +88,6 @@ export const runMigration = ( .with("002", () => migrateFrom002To003(config)) .with("003", () => migrateFrom003To004(config)) .with("004", () => migrateFrom004To005(config)) + .with("005", () => migrateFrom005To006(config)) .exhaustive() } diff --git a/packages/@ourworldindata/grapher/src/stackedCharts/MarimekkoChart.jsdom.test.tsx b/packages/@ourworldindata/grapher/src/stackedCharts/MarimekkoChart.jsdom.test.tsx index c4650faccd6..306f5376524 100644 --- a/packages/@ourworldindata/grapher/src/stackedCharts/MarimekkoChart.jsdom.test.tsx +++ b/packages/@ourworldindata/grapher/src/stackedCharts/MarimekkoChart.jsdom.test.tsx @@ -23,7 +23,7 @@ it("can filter years correctly", () => { // TODO: why is it ySlugs and xSlug here instead of yColumnSlugs and xColumnSlug? Unify when we have config migrations? const manager = { - type: ChartTypeName.Marimekko, + chartTypes: [ChartTypeName.Marimekko], table, selection: table.availableEntityNames, ySlugs: "percentBelow2USD", @@ -133,7 +133,7 @@ it("shows no data points at the end", () => { // TODO: why is it ySlugs and xSlug here instead of yColumnSlugs and xColumnSlug? Unify when we have config migrations? const manager = { - type: ChartTypeName.Marimekko, + chartTypes: [ChartTypeName.Marimekko], table, selection: table.availableEntityNames, ySlugs: "percentBelow2USD", @@ -233,7 +233,7 @@ test("interpolation works as expected", () => { // TODO: why is it ySlugs and xSlug here instead of yColumnSlugs and xColumnSlug? Unify when we have config migrations? const manager = { - type: ChartTypeName.Marimekko, + chartTypes: [ChartTypeName.Marimekko], table, selection: table.availableEntityNames, ySlugs: "percentBelow2USD", @@ -344,7 +344,7 @@ it("can deal with y columns with missing values", () => { // TODO: why is it ySlugs and xSlug here instead of yColumnSlugs and xColumnSlug? Unify when we have config migrations? const manager = { - type: ChartTypeName.Marimekko, + chartTypes: [ChartTypeName.Marimekko], table, selection: table.availableEntityNames, ySlugs: "percentBelow2USD percentBelow10USD", diff --git a/packages/@ourworldindata/types/src/dbTypes/ChartConfigs.ts b/packages/@ourworldindata/types/src/dbTypes/ChartConfigs.ts index e3b91105b86..d6ad7227811 100644 --- a/packages/@ourworldindata/types/src/dbTypes/ChartConfigs.ts +++ b/packages/@ourworldindata/types/src/dbTypes/ChartConfigs.ts @@ -1,5 +1,8 @@ import { Base64String, JsonString } from "../domainTypes/Various.js" -import { GrapherInterface } from "../grapherTypes/GrapherTypes.js" +import { + ChartTypeName, + GrapherInterface, +} from "../grapherTypes/GrapherTypes.js" export const ChartConfigsTableName = "chart_configs" export interface DbInsertChartConfig { @@ -8,6 +11,7 @@ export interface DbInsertChartConfig { full: JsonString fullMd5?: Base64String slug?: string | null + chartType?: ChartTypeName | null // TODO: exclude WorldMap createdAt?: Date updatedAt?: Date | null } diff --git a/packages/@ourworldindata/types/src/grapherTypes/GrapherTypes.ts b/packages/@ourworldindata/types/src/grapherTypes/GrapherTypes.ts index 618c07588d8..82b060b58b6 100644 --- a/packages/@ourworldindata/types/src/grapherTypes/GrapherTypes.ts +++ b/packages/@ourworldindata/types/src/grapherTypes/GrapherTypes.ts @@ -178,6 +178,37 @@ export enum GrapherTabOption { table = "table", } +export enum GrapherTabQueryParam { + Chart = "chart", + Table = "table", + WorldMap = "map", + + // chart types + LineChart = "line", + ScatterPlot = "scatter", + StackedArea = "stacked-area", + DiscreteBar = "discrete-bar", + StackedDiscreteBar = "stacked-discrete-bar", + SlopeChart = "slope", + StackedBar = "stacked-bar", + Marimekko = "marimekko", +} + +export enum GrapherTabName { + Table = "Table", + WorldMap = "WorldMap", + + // chart types + LineChart = "LineChart", + ScatterPlot = "ScatterPlot", + StackedArea = "StackedArea", + DiscreteBar = "DiscreteBar", + StackedDiscreteBar = "StackedDiscreteBar", + SlopeChart = "SlopeChart", + StackedBar = "StackedBar", + Marimekko = "Marimekko", +} + export interface RelatedQuestionsConfig { text: string url: string @@ -528,7 +559,7 @@ export interface MapConfigInterface { // under the same rendering conditions it ought to remain visually identical export interface GrapherInterface extends SortConfig { $schema?: string - type?: ChartTypeName + chartTypes?: ChartTypeName[] id?: number version?: number slug?: string @@ -556,7 +587,6 @@ export interface GrapherInterface extends SortConfig { hideTimeline?: boolean zoomToSelection?: boolean showYearLabels?: boolean // Always show year in labels for bar charts - hasChartTab?: boolean hasMapTab?: boolean tab?: GrapherTabOption relatedQuestions?: RelatedQuestionsConfig[] @@ -647,7 +677,7 @@ export const GRAPHER_QUERY_PARAM_KEYS: (keyof LegacyGrapherQueryParams)[] = [ // Another approach we may want to try is this: https://github.com/mobxjs/serializr export const grapherKeysToSerialize = [ "$schema", - "type", + "chartTypes", "id", "version", "slug", @@ -673,7 +703,6 @@ export const grapherKeysToSerialize = [ "hideTimeline", "zoomToSelection", "showYearLabels", - "hasChartTab", "hasMapTab", "tab", "internalNotes", diff --git a/packages/@ourworldindata/types/src/index.ts b/packages/@ourworldindata/types/src/index.ts index 5decf69e009..ea93c38d22b 100644 --- a/packages/@ourworldindata/types/src/index.ts +++ b/packages/@ourworldindata/types/src/index.ts @@ -76,6 +76,8 @@ export { colorScaleConfigDefaults, ChartTypeName, GrapherTabOption, + GrapherTabName, + GrapherTabQueryParam, StackMode, EntitySelectionMode, ScatterPointLabelStrategy, diff --git a/packages/@ourworldindata/utils/src/Util.ts b/packages/@ourworldindata/utils/src/Util.ts index 5c93748b3fd..33a97b41f9e 100644 --- a/packages/@ourworldindata/utils/src/Util.ts +++ b/packages/@ourworldindata/utils/src/Util.ts @@ -926,6 +926,9 @@ export const differenceOfSets = (sets: Set[]): Set => { return diff } +export const areSetsEqual = (setA: Set, setB: Set): boolean => + setA.size === setB.size && [...setA].every((value) => setB.has(value)) + /** Tests whether the first argument is a strict subset of the second. The arguments do not have to be sets yet, they can be any iterable. Sets will be created by the function internally */ export function isSubsetOf( @@ -1966,9 +1969,10 @@ export function traverseObjects>( export function getParentVariableIdFromChartConfig( config: GrapherInterface // could be a patch config ): number | undefined { - const { type = ChartTypeName.LineChart, dimensions } = config + const { chartTypes, dimensions } = config - if (type === ChartTypeName.ScatterPlot) return undefined + const chartType = chartTypes?.[0] ?? ChartTypeName.LineChart + if (chartType === ChartTypeName.ScatterPlot) return undefined if (!dimensions) return undefined const yVariableIds = dimensions diff --git a/packages/@ourworldindata/utils/src/index.ts b/packages/@ourworldindata/utils/src/index.ts index eacf00917fa..75f99afdd16 100644 --- a/packages/@ourworldindata/utils/src/index.ts +++ b/packages/@ourworldindata/utils/src/index.ts @@ -62,6 +62,7 @@ export { intersectionOfSets, unionOfSets, differenceOfSets, + areSetsEqual, isSubsetOf, intersection, sortByUndefinedLast, diff --git a/site/gdocs/components/Chart.tsx b/site/gdocs/components/Chart.tsx index dbc57a3f973..c4070b96c38 100644 --- a/site/gdocs/components/Chart.tsx +++ b/site/gdocs/components/Chart.tsx @@ -1,8 +1,8 @@ import React, { useRef } from "react" import { useEmbedChart } from "../../hooks.js" import { - grapherInterfaceWithHiddenControlsOnly, - grapherInterfaceWithHiddenTabsOnly, + grapherInterfaceWithHiddenControls, + grapherInterfaceWithHiddenTabs, GrapherProgrammaticInterface, } from "@ourworldindata/grapher" import { @@ -12,6 +12,7 @@ import { identity, Url, merge, + excludeUndefined, } from "@ourworldindata/utils" import { ChartConfigType } from "@ourworldindata/types" import { renderSpans, useLinkedChart } from "../utils.js" @@ -54,17 +55,26 @@ export default function Chart({ if (!isExplorer && isCustomized) { const controls: ChartControlKeyword[] = d.controls || [] const tabs: ChartTabKeyword[] = d.tabs || [] + const showAllControls = controls.includes(ChartControlKeyword.all) const showAllTabs = tabs.includes(ChartTabKeyword.all) - const listOfPartialGrapherConfigs = [...controls, ...tabs] - .map(mapKeywordToGrapherConfig) - .filter(identity) as GrapherProgrammaticInterface[] + + const allControlsHidden = grapherInterfaceWithHiddenControls + const allTabsHidden = grapherInterfaceWithHiddenTabs + + const enabledControls = excludeUndefined( + controls.map(mapControlKeywordToGrapherConfig) + ) + const enabledTabs = excludeUndefined( + tabs.map(mapTabKeywordToGrapherConfig) + ) customizedChartConfig = merge( {}, - !showAllControls ? grapherInterfaceWithHiddenControlsOnly : {}, - !showAllTabs ? grapherInterfaceWithHiddenTabsOnly : {}, - ...listOfPartialGrapherConfigs, + !showAllControls ? allControlsHidden : {}, + !showAllTabs ? allTabsHidden : {}, + ...enabledControls, + ...enabledTabs, { hideRelatedQuestion: true, hideShareButton: true, // always hidden since the original chart would be shared, not the customized one @@ -134,12 +144,10 @@ export default function Chart({ ) } -const mapKeywordToGrapherConfig = ( - keyword: ChartControlKeyword | ChartTabKeyword -): GrapherProgrammaticInterface | null => { +const mapControlKeywordToGrapherConfig = ( + keyword: ChartControlKeyword +): GrapherProgrammaticInterface | undefined => { switch (keyword) { - // controls - case ChartControlKeyword.relativeToggle: return { hideRelativeToggle: false } @@ -173,18 +181,25 @@ const mapKeywordToGrapherConfig = ( case ChartControlKeyword.tableFilterToggle: return { hideTableFilterToggle: false } - // tabs + default: + return undefined + } +} - case ChartTabKeyword.chart: - return { hasChartTab: true } +const mapTabKeywordToGrapherConfig = ( + keyword: ChartTabKeyword +): GrapherProgrammaticInterface | undefined => { + switch (keyword) { + case ChartTabKeyword.table: + return { hasTableTab: true } case ChartTabKeyword.map: return { hasMapTab: true } - case ChartTabKeyword.table: - return { hasTableTab: true } + case ChartTabKeyword.chart: + return { hideChartTabs: false } default: - return null + return undefined } } diff --git a/site/multiembedder/MultiEmbedder.tsx b/site/multiembedder/MultiEmbedder.tsx index 7e1639f85e9..b9c6d085164 100644 --- a/site/multiembedder/MultiEmbedder.tsx +++ b/site/multiembedder/MultiEmbedder.tsx @@ -211,16 +211,10 @@ class MultiEmbedder { // make sure the tab of the active pane is visible if (figureConfigAttr) { - const activeTab = - queryParams.tab || - grapherPageConfig.tab || + localConfig.tab = + queryParams.tab ?? + grapherPageConfig.tab ?? GrapherTabOption.chart - if (activeTab === GrapherTabOption.chart) - localConfig.hasChartTab = true - if (activeTab === GrapherTabOption.map) - localConfig.hasMapTab = true - if (activeTab === GrapherTabOption.table) - localConfig.hasTableTab = true } const config = merge(