diff --git a/adminSiteClient/ChartList.tsx b/adminSiteClient/ChartList.tsx index 521d71aa992..2bf5889ba9f 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,13 +18,14 @@ 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 + hasMapTab: boolean lastEditedAt: string lastEditedBy: string @@ -142,13 +147,13 @@ export class ChartList extends React.Component<{ } } -export function showChartType(chart: ChartListItem) { +export function showChartType(chart: ChartListItem): string { const chartType = chart.type ?? ChartTypeName.LineChart const displayType = ChartTypeName[chartType] ? startCase(ChartTypeName[chartType]) : "Unknown" - if (chart.tab === "map") { + if (chart.tab === GrapherTabOption.WorldMap) { if (chart.hasChartTab) return `Map + ${displayType}` else return "Map" } else { diff --git a/adminSiteClient/EditorBasicTab.tsx b/adminSiteClient/EditorBasicTab.tsx index 879b7ac36bb..67ced5871b5 100644 --- a/adminSiteClient/EditorBasicTab.tsx +++ b/adminSiteClient/EditorBasicTab.tsx @@ -11,6 +11,7 @@ import { observer } from "mobx-react" import { ChartTypeName, EntitySelectionMode, + GrapherTabOption, StackMode, } from "@ourworldindata/types" import { @@ -18,6 +19,7 @@ import { WorldEntityName, CONTINENTS_INDICATOR_ID, POPULATION_INDICATOR_ID_USED_IN_ADMIN, + allChartTypes, } from "@ourworldindata/grapher" import { DimensionProperty, @@ -147,7 +149,10 @@ class DimensionSlotView< () => this.grapher.isReady, () => { this.disposers.push( - reaction(() => this.grapher.type, this.updateDefaults), + reaction( + () => this.grapher.availableTabs, + this.updateDefaults + ), reaction( () => this.grapher.yColumnsFromDimensions.length, this.updateDefaults @@ -355,7 +360,20 @@ export class EditorBasicTab< @action.bound onChartTypeChange(value: string) { const { grapher } = this.props.editor - grapher.type = value as ChartTypeName + + const newChartType = value as ChartTypeName + const newTabs: ChartTypeName[] = (grapher.availableTabs ?? []).filter( + (tab) => tab !== ChartTypeName.WorldMap + ) + newTabs.push(newChartType) + grapher.availableTabs = newTabs + + if ( + grapher.tab !== GrapherTabOption.Table && + grapher.tab !== GrapherTabOption.WorldMap + ) { + grapher.tab = newChartType as unknown as GrapherTabOption + } if (grapher.isMarimekko) { grapher.hideRelativeToggle = false @@ -395,9 +413,6 @@ export class EditorBasicTab< render() { const { editor } = this.props const { grapher } = editor - const chartTypes = Object.keys(ChartTypeName).filter( - (chartType) => chartType !== ChartTypeName.WorldMap - ) const isIndicatorChart = isIndicatorChartEditorInstance(editor) @@ -407,9 +422,9 @@ export class EditorBasicTab<
({ + options={allChartTypes.map((key) => ({ value: key, label: startCase(key), }))} @@ -418,12 +433,43 @@ export class EditorBasicTab< (grapher.hasChartTab = value)} + onValue={(value) => { + if (value) { + // add line chart tab + grapher.availableTabs = + grapher.availableTabs ?? [] + grapher.availableTabs.push( + ChartTypeName.LineChart + ) + } else { + // remove all chart tabs + grapher.availableTabs = + grapher.availableTabs?.filter( + (tab) => + tab === ChartTypeName.WorldMap + ) + } + }} /> (grapher.hasMapTab = value)} + onValue={(value) => { + if (value) { + // add map tab + grapher.availableTabs = [ + ChartTypeName.WorldMap, + ...(grapher.availableTabs ?? []), + ] + } else { + // remove map tab + grapher.availableTabs = + grapher.availableTabs?.filter( + (tab) => + tab !== ChartTypeName.WorldMap + ) + } + }} />
diff --git a/adminSiteClient/EditorCustomizeTab.tsx b/adminSiteClient/EditorCustomizeTab.tsx index ae3cdf5bedc..6a01665b595 100644 --- a/adminSiteClient/EditorCustomizeTab.tsx +++ b/adminSiteClient/EditorCustomizeTab.tsx @@ -158,7 +158,7 @@ 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} invertedColorScheme={!!grapher.invertColorScheme} additionalOptions={[ { @@ -751,7 +751,7 @@ export class EditorCustomizeTab< {grapher.chartInstanceExceptMap.colorScale && ( >"$.hasMapTab" = "true"`) - tab = tab || GrapherTabOption.map + tab = tab || GrapherTabOption.WorldMap } else { if (params.type === "LineChart") { query = query.andWhereRaw( @@ -165,7 +166,8 @@ async function propsFromQueryParams( { type: params.type } ) } - tab = tab || GrapherTabOption.chart + // TODO + // tab = tab || GrapherTabOption.chart } } @@ -173,26 +175,30 @@ async function propsFromQueryParams( query = query.andWhereRaw( `cc.full->>'$.yAxis.canChangeScaleType' = "true" OR cc.full->>'$.xAxis.canChangeScaleType' = "true"` ) - tab = GrapherTabOption.chart + // TODO + // tab = GrapherTabOption.chart } if (params.comparisonLines) { query = query.andWhereRaw( `cc.full->'$.comparisonLines[0].yEquals' != ''` ) - tab = GrapherTabOption.chart + // TODO + // tab = GrapherTabOption.chart } if (params.stackMode) { query = query.andWhereRaw(`cc.full->'$.stackMode' = :stackMode`, { stackMode: params.stackMode, }) - tab = GrapherTabOption.chart + // TODO + // tab = GrapherTabOption.chart } if (params.relativeToggle) { query = query.andWhereRaw(`cc.full->>'$.hideRelativeToggle' = "false"`) - tab = GrapherTabOption.chart + // TODO + // tab = GrapherTabOption.chart } if (params.categoricalLegend) { @@ -202,7 +208,7 @@ async function propsFromQueryParams( query = query.andWhereRaw( `json_length(cc.full->'$.map.colorScale.customCategoryColors') > 1` ) - tab = GrapherTabOption.map + tab = GrapherTabOption.WorldMap } if (params.mixedTimeTypes) { @@ -242,13 +248,14 @@ async function propsFromQueryParams( query = query.andWhereRaw(`charts.id IN (${params.ids})`) } - 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"` - ) - } + // TODO + // if (tab === GrapherTabOption.WorldMap) { + // query = query.andWhereRaw(`cc.full->>"$.hasMapTab" = "true"`) + // } else if (tab === GrapherTabOption.chart) { + // query = query.andWhereRaw( + // `COALESCE(cc.full->>"$.hasChartTab", "true") = "true"` + // ) + // } if (datasetIds.length > 0) { const datasetIds = params.datasetIds diff --git a/baker/countryProfiles.tsx b/baker/countryProfiles.tsx index 3c774d1aed1..4b79ba1d0f0 100644 --- a/baker/countryProfiles.tsx +++ b/baker/countryProfiles.tsx @@ -16,7 +16,13 @@ import { CountryProfilePage, } from "../site/CountryProfilePage.js" import { SiteBaker } from "./SiteBaker.js" -import { countries, getCountryBySlug, JsonError } from "@ourworldindata/utils" +import { + countries, + getCountryBySlug, + JsonError, + getMainChartTypeFromConfig, + hasChartTabFromConfig, +} from "@ourworldindata/utils" import { renderToHtmlPage } from "./siteRenderers.js" import { dataAsDF } from "../db/model/Variable.js" import pl from "nodejs-polars" @@ -37,8 +43,9 @@ function bakeCache(cacheKey: any, retriever: () => T): T { } const checkShouldShowIndicator = (grapher: GrapherInterface) => - (grapher.hasChartTab ?? true) && - (grapher.type ?? "LineChart") === "LineChart" && + hasChartTabFromConfig(grapher) && + // TODO: should this be the intial chart type instead? + getMainChartTypeFromConfig(grapher) === "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/baker/updateChartEntities.ts b/baker/updateChartEntities.ts index 10264e28155..da0b0858db7 100644 --- a/baker/updateChartEntities.ts +++ b/baker/updateChartEntities.ts @@ -97,7 +97,7 @@ const obtainAvailableEntitiesForGrapherConfig = async ( // If the grapher has a chart tab, then the available entities there are the "most interesting" ones to us if (grapher.hasChartTab) { - grapher.tab = GrapherTabOption.chart + grapher.tab = grapher.firstChartType as unknown as GrapherTabOption // If the grapher allows for changing or multi-selecting entities, then let's index all entities the // user can choose from. Otherwise, we'll just use the default-selected entities. @@ -112,7 +112,7 @@ const obtainAvailableEntitiesForGrapherConfig = async ( return grapher.tableForSelection.availableEntityNames as string[] else return grapher.selectedEntityNames ?? [] } else if (grapher.hasMapTab) { - grapher.tab = GrapherTabOption.map + grapher.tab = GrapherTabOption.WorldMap // On a map tab, tableAfterAuthorTimelineAndActiveChartTransform contains all // mappable entities for which data is available return grapher.tableAfterAuthorTimelineAndActiveChartTransform 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/1731316808331-AddAvailableTabsToGrapherConfigs.ts b/db/migration/1731316808331-AddAvailableTabsToGrapherConfigs.ts new file mode 100644 index 00000000000..60fa929da92 --- /dev/null +++ b/db/migration/1731316808331-AddAvailableTabsToGrapherConfigs.ts @@ -0,0 +1,205 @@ +import { MigrationInterface, QueryRunner } from "typeorm" + +export class AddAvailableTabsToGrapherConfigs1731316808331 + implements MigrationInterface +{ + private async updateSchema( + queryRunner: QueryRunner, + newSchema: string + ): Promise { + await queryRunner.query( + ` + -- sql + UPDATE chart_configs + SET + patch = JSON_SET(patch, '$.$schema', ?), + full = JSON_SET(full, '$.$schema', ?) + `, + [newSchema, newSchema] + ) + } + + private async addAvailableTabsToGrapherConfigs( + queryRunner: QueryRunner + ): Promise { + // CASES: + // 1. hasMapTab=false, hasChartTab=false -> availableTabs is unset + // 2. hasMapTab=false, hasChartTab=true -> availableTabs=[chartType] + // 3. hasMapTab=true, hasChartTab=false -> availableTabs=["WorldMap"] + // 4. hasMapTab=true, hasChartTab=true -> availableTabs=["WorldMap", chartType] + + for (const configType of ["patch", "full"]) { + // CASE 2: hasMapTab=false, hasChartTab=true -> availableTabs=[chartType] + await queryRunner.query( + ` + -- sql + UPDATE chart_configs + SET ?? = JSON_SET( + ??, + '$.availableTabs', + JSON_ARRAY(COALESCE(?? ->> '$.type', 'LineChart')) + ) + WHERE + (?? ->> '$.hasMapTab' = 'false' OR ?? ->> '$.hasMapTab' IS NULL) + AND (?? ->> '$.hasChartTab' = 'true' OR ?? ->> '$.hasChartTab' IS NULL) + `, + [ + configType, + configType, + configType, + configType, + configType, + configType, + configType, + ] + ) + + // CASE 3: hasMapTab=true, hasChartTab=false -> availableTabs=["WorldMap"] + await queryRunner.query( + ` + -- sql + UPDATE chart_configs + SET ?? = JSON_SET( + ??, + '$.availableTabs', + JSON_ARRAY('WorldMap') + ) + WHERE + (?? ->> '$.hasMapTab' = 'true') + AND (?? ->> '$.hasChartTab' = 'false') + `, + [configType, configType, configType, configType] + ) + + // CASE 4: hasMapTab=true, hasChartTab=true -> availableTabs=["WorldMap", chartType] + await queryRunner.query( + ` + -- sql + UPDATE chart_configs + SET ?? = JSON_SET( + ??, + '$.availableTabs', + JSON_ARRAY('WorldMap', COALESCE(?? ->> '$.type', 'LineChart')) + ) + WHERE + (?? ->> '$.hasMapTab' = 'true') + AND (?? ->> '$.hasChartTab' = 'true' OR ?? ->> '$.hasChartTab' IS NULL) + `, + [ + configType, + configType, + configType, + configType, + configType, + configType, + ] + ) + } + } + + private async updateTabField(queryRunner: QueryRunner): Promise { + // CASES: + // 1. tab=map -> tab=WorldMap + // 2. tab=table -> tab=Table + // 3. tab=chart -> tab=LineChart/SlopeChart/ScatterPlot/etc. + + for (const configType of ["patch", "full"]) { + // CASE 1: tab=map -> tab=WorldMap + await queryRunner.query( + ` + -- sql + UPDATE chart_configs + SET ?? = JSON_SET(??, '$.tab', 'WorldMap') + WHERE ?? ->> '$.tab' = 'map' + `, + [configType, configType, configType] + ) + + // CASE 2: tab=table -> tab=Table + await queryRunner.query( + ` + -- sql + UPDATE chart_configs + SET ?? = JSON_SET(??, '$.tab', 'Table') + WHERE ?? ->> '$.tab' = 'table' + `, + [configType, configType, configType] + ) + + // CASE 3: tab=chart -> tab=LineChart/SlopeChart/ScatterPlot/etc. + await queryRunner.query( + ` + -- sql + UPDATE chart_configs + SET ?? = JSON_SET(??, '$.tab', COALESCE(?? ->> '$.type', 'LineChart')) + WHERE ?? ->> '$.tab' = 'chart' + `, + [configType, configType, configType, configType] + ) + } + } + + private async removeTypeHasMapTabAndHasChartTabFields( + queryRunner: QueryRunner + ): Promise { + await queryRunner.query(` + -- sql + UPDATE chart_configs + SET patch = JSON_REMOVE(patch, '$.type', '$.hasChartTab', '$.hasMapTab') + `) + } + + private async addDerivedChartTypeColumn( + queryRunner: QueryRunner + ): Promise { + // TODO: not future-proof + await queryRunner.query( + `-- sql + ALTER TABLE chart_configs + ADD COLUMN chartType VARCHAR(255) GENERATED ALWAYS AS + ( + CASE + -- Graphers with a line and a slope chart are considered to be of type LineChart + WHEN JSON_CONTAINS(full, '"LineChart"', '$.availableTabs') THEN 'LineChart' + WHEN JSON_CONTAINS(full, '"SlopeChart"', '$.availableTabs') THEN 'SlopeChart' + WHEN JSON_CONTAINS(full, '"ScatterPlot"', '$.availableTabs') THEN 'ScatterPlot' + WHEN JSON_CONTAINS(full, '"StackedArea"', '$.availableTabs') THEN 'StackedArea' + WHEN JSON_CONTAINS(full, '"StackedBar"', '$.availableTabs') THEN 'StackedBar' + WHEN JSON_CONTAINS(full, '"ScatterPlot"', '$.availableTabs') THEN 'ScatterPlot' + WHEN JSON_CONTAINS(full, '"DiscreteBar"', '$.availableTabs') THEN 'DiscreteBar' + WHEN JSON_CONTAINS(full, '"StackedDiscreteBar"', '$.availableTabs') THEN 'StackedDiscreteBar' + WHEN JSON_CONTAINS(full, '"Marimekko"', '$.availableTabs') THEN 'Marimekko' + ELSE NULL + END + ) + VIRTUAL AFTER slug; + ` + ) + } + + public async removeDerivedTypeColumn( + queryRunner: QueryRunner + ): Promise { + await queryRunner.query( + `-- sql + ALTER TABLE chart_configs + DROP COLUMN type; + ` + ) + } + + public async up(queryRunner: QueryRunner): Promise { + await this.addAvailableTabsToGrapherConfigs(queryRunner) + await this.updateTabField(queryRunner) + await this.removeTypeHasMapTabAndHasChartTabFields(queryRunner) + // await this.addDerivedChartTypeColumn(queryRunner) + await this.updateSchema( + queryRunner, + "https://files.ourworldindata.org/schemas/grapher-schema.007.json" + ) + } + + public async down(queryRunner: QueryRunner): Promise { + // TODO: implement down migration + } +} diff --git a/db/model/Chart.ts b/db/model/Chart.ts index 1f44e590f12..59270da853c 100644 --- a/db/model/Chart.ts +++ b/db/model/Chart.ts @@ -526,12 +526,12 @@ 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, - JSON_EXTRACT(chart_configs.full, "$.hasMapTab") = true AS hasMapTab, + chart_configs.chartType IS NOT NULL AS hasChartTab, + JSON_EXTRACT(chart_configs.full, "$.availableTabs.WorldMap") = true AS hasMapTab, JSON_EXTRACT(chart_configs.full, "$.isPublished") = true AS isPublished, charts.lastEditedAt, charts.lastEditedByUserId, diff --git a/db/model/Gdoc/GdocBase.ts b/db/model/Gdoc/GdocBase.ts index 4dfacfe69f0..9c7933d6090 100644 --- a/db/model/Gdoc/GdocBase.ts +++ b/db/model/Gdoc/GdocBase.ts @@ -934,7 +934,7 @@ export async function makeGrapherLinkedChart( const resolvedSlug = config.slug ?? "" const resolvedTitle = config.title ?? "" const resolvedUrl = `${BAKED_GRAPHER_URL}/${resolvedSlug}` - const tab = config.tab ?? GrapherTabOption.chart + const tab = config.tab const datapageIndicator = await getVariableOfDatapageIfApplicable(config) return { configType: ChartConfigType.Grapher, diff --git a/db/model/Variable.ts b/db/model/Variable.ts index 7dd6130a28e..7bd1c71e56a 100644 --- a/db/model/Variable.ts +++ b/db/model/Variable.ts @@ -7,6 +7,7 @@ import { omitUndefinedValues, mergeGrapherConfigs, diffGrapherConfigs, + getMainChartTypeFromConfig, } from "@ourworldindata/utils" import { getVariableDataRoute, @@ -878,7 +879,7 @@ export async function getVariableOfDatapageIfApplicable( // showing a data page. if ( yVariableIds.length === 1 && - (grapher.type !== ChartTypeName.ScatterPlot || + (getMainChartTypeFromConfig(grapher) !== ChartTypeName.ScatterPlot || xVariableIds.length === 0) ) { const variableId = yVariableIds[0] diff --git a/devTools/svgTester/utils.ts b/devTools/svgTester/utils.ts index 239466c9af7..906bd9b2cfe 100644 --- a/devTools/svgTester/utils.ts +++ b/devTools/svgTester/utils.ts @@ -1,5 +1,6 @@ import { ChartTypeName, GrapherTabOption } from "@ourworldindata/types" import { + getMainChartTypeFromConfig, MultipleOwidVariableDataDimensionsMap, OwidVariableMixedData, OwidVariableWithSourceAndDimension, @@ -205,7 +206,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 = + getMainChartTypeFromConfig(grapherConfig) ?? ChartTypeName.LineChart const queryStrings = options.shouldTestAllViews ? queryStringsByChartType[chartType] @@ -283,7 +285,9 @@ export async function findValidChartIds( const grapherConfig = await parseGrapherConfig(grapherId, { inDir, }) - const chartType = grapherConfig.type ?? ChartTypeName.LineChart + const chartType = + getMainChartTypeFromConfig(grapherConfig) ?? + ChartTypeName.LineChart if (chartTypes.includes(chartType)) { validChartIds.push(grapherId) } @@ -422,7 +426,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.currentTab, queryStr, md5: processSvgAndCalculateHash(svg), svgFilename: outFilename, diff --git a/packages/@ourworldindata/explorer/src/Explorer.jsdom.test.tsx b/packages/@ourworldindata/explorer/src/Explorer.jsdom.test.tsx index 644852c0684..12dc74f5766 100755 --- a/packages/@ourworldindata/explorer/src/Explorer.jsdom.test.tsx +++ b/packages/@ourworldindata/explorer/src/Explorer.jsdom.test.tsx @@ -30,26 +30,26 @@ describe(Explorer, () => { explorer.onChangeChoice("Gas")("All GHGs (CO₂eq)") - if (explorer.grapher) explorer.grapher.tab = GrapherTabOption.table + if (explorer.grapher) explorer.grapher.tab = GrapherTabOption.Table else throw Error("where's the grapher?") expect(explorer.queryParams.tab).toEqual("table") explorer.onChangeChoice("Gas")("CO₂") expect(explorer.queryParams.tab).toEqual("table") - explorer.grapher.tab = GrapherTabOption.chart + explorer.grapher.tab = GrapherTabOption.Table }) it("switches to first tab if current tab does not exist in new view", () => { const explorer = element.instance() as Explorer expect(explorer.queryParams.tab).toBeUndefined() - if (explorer.grapher) explorer.grapher.tab = GrapherTabOption.map + if (explorer.grapher) explorer.grapher.tab = GrapherTabOption.WorldMap else throw Error("where's the grapher?") expect(explorer.queryParams.tab).toEqual("map") explorer.onChangeChoice("Gas")("All GHGs (CO₂eq)") - expect(explorer.grapher.tab).toEqual("chart") + expect(explorer.grapher.tab).toEqual(GrapherTabOption.LineChart) expect(explorer.queryParams.tab).toEqual(undefined) }) diff --git a/packages/@ourworldindata/explorer/src/Explorer.sample.tsx b/packages/@ourworldindata/explorer/src/Explorer.sample.tsx index a15b805a6f5..9b534653f8b 100644 --- a/packages/@ourworldindata/explorer/src/Explorer.sample.tsx +++ b/packages/@ourworldindata/explorer/src/Explorer.sample.tsx @@ -1,6 +1,6 @@ import React from "react" import { DimensionProperty } from "@ourworldindata/utils" -import { GrapherTabOption } from "@ourworldindata/types" +import { GrapherTabOption, ChartTypeName } from "@ourworldindata/types" import { GrapherProgrammaticInterface } from "@ourworldindata/grapher" import { Explorer, ExplorerProps } from "./Explorer.js" @@ -54,7 +54,8 @@ export const SampleExplorerOfGraphers = (props?: Partial) => { property: DimensionProperty.y, }, ], - tab: GrapherTabOption.chart, + availableTabs: [ChartTypeName.LineChart], + tab: GrapherTabOption.LineChart, owidDataset: new Map([ [ 142609, diff --git a/packages/@ourworldindata/explorer/src/Explorer.tsx b/packages/@ourworldindata/explorer/src/Explorer.tsx index 65498e161e7..60863e1417d 100644 --- a/packages/@ourworldindata/explorer/src/Explorer.tsx +++ b/packages/@ourworldindata/explorer/src/Explorer.tsx @@ -443,18 +443,20 @@ export class Explorer time: this.grapher.timeParam, } - const previousTab = this.grapher.tab + const previousTab = this.grapher.currentTab 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.sortedAvailableTabs.includes(previousTab)) { + newGrapherParams.tab = previousTab + } else { + const tabsWithoutTable = this.grapher.sortedAvailableTabs.filter( + (tab) => tab !== GrapherTabOption.Table + ) + newGrapherParams.tab = tabsWithoutTable[0] ?? GrapherTabOption.Table + } this.grapher.populateFromQueryParams(newGrapherParams) @@ -591,6 +593,7 @@ export class Explorer const partialGrapherConfig = this.partialGrapherConfigsByVariableId.get(yVariableIdsList[0]) ?? {} + console.log(partialGrapherConfig) const config: GrapherProgrammaticInterface = { ...mergeGrapherConfigs( diff --git a/packages/@ourworldindata/explorer/src/ExplorerProgram.ts b/packages/@ourworldindata/explorer/src/ExplorerProgram.ts index e8f289462b1..ada1f87bd89 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,10 @@ interface ExplorerGrapherInterface extends GrapherInterface { relatedQuestionText?: string relatedQuestionUrl?: string mapTargetTime?: number + + type?: ChartTypeName + hasChartTab?: boolean + hasMapTab?: boolean } const ExplorerRootDef: CellDef = { @@ -379,6 +384,9 @@ export class ExplorerProgram extends GridProgram { if (!GrapherGrammar[key]) delete config[key] }) + // assume a line chart if no type is specified + if (!config.type) config.type = ChartTypeName.LineChart + return config } @@ -401,6 +409,19 @@ export class ExplorerProgram extends GridProgram { // assume config is valid against the latest schema mergedConfig.$schema = defaultGrapherConfig.$schema + // construct the available tabs from type, hasChartTab and hasMapTab + const { + type = ChartTypeName.LineChart, + hasChartTab = true, + hasMapTab = false, + } = this.explorerGrapherConfig + if (hasMapTab || hasChartTab) { + const availableTabs = [] + if (hasMapTab) availableTabs.push(ChartTypeName.WorldMap) + if (hasChartTab) availableTabs.push(type) + mergedConfig.availableTabs = availableTabs + } + return mergedConfig } diff --git a/packages/@ourworldindata/explorer/src/GrapherGrammar.ts b/packages/@ourworldindata/explorer/src/GrapherGrammar.ts index 5e235502b53..1d1bdbe5860 100644 --- a/packages/@ourworldindata/explorer/src/GrapherGrammar.ts +++ b/packages/@ourworldindata/explorer/src/GrapherGrammar.ts @@ -65,7 +65,7 @@ export const GrapherGrammar: Grammar = { keyword: "type", description: `The type of chart to show such as LineChart or ScatterPlot.`, terminalOptions: toTerminalOptions(Object.values(ChartTypeName)), - toGrapherObject: (value) => ({ type: value }), + toGrapherObject: () => ({}), // handled in code }, grapherId: { ...IntegerCellDef, @@ -84,7 +84,7 @@ export const GrapherGrammar: Grammar = { ...BooleanCellDef, keyword: "hasMapTab", description: "Show the map tab?", - toGrapherObject: (value) => ({ hasMapTab: value }), + toGrapherObject: () => ({}), // handled in code }, tab: { ...EnumCellDef, @@ -97,7 +97,8 @@ export const GrapherGrammar: Grammar = { ...BooleanCellDef, keyword: "hasChartTab", description: "Show the chart tab?", - toGrapherObject: (value) => ({ hasChartTab: value }), + // overwrites the given chart type if provided + toGrapherObject: () => ({}), // handled in code }, xSlug: { ...SlugDeclarationCellDef, diff --git a/packages/@ourworldindata/grapher/src/captionedChart/CaptionedChart.stories.tsx b/packages/@ourworldindata/grapher/src/captionedChart/CaptionedChart.stories.tsx index 63157d73e38..ccf152c3cae 100644 --- a/packages/@ourworldindata/grapher/src/captionedChart/CaptionedChart.stories.tsx +++ b/packages/@ourworldindata/grapher/src/captionedChart/CaptionedChart.stories.tsx @@ -48,7 +48,7 @@ export const StaticLineChartForExport = (): React.ReactElement => { } export const MapChart = (): React.ReactElement => ( - + ) export const StackedArea = (): React.ReactElement => ( { return !this.manager.isOnMapTab && hasStrategy } - @computed get chartTypeName(): ChartTypeName { + @computed get currentChartType(): ChartTypeName | undefined { const { manager } = this - return this.manager.isOnMapTab - ? ChartTypeName.WorldMap - : manager.typeExceptWhenLineChartAndSingleTimeThenWillBeBarChart ?? - manager.type ?? - ChartTypeName.LineChart + if (manager.isOnMapTab) return ChartTypeName.WorldMap + if (manager.isOnChartTab) { + return manager.isLineChartThatTurnedIntoDiscreteBar + ? ChartTypeName.DiscreteBar + : manager.currentChartType + } + return undefined } - renderChart(): React.ReactElement { - const { manager } = this + renderReadyChartOrMap(): React.ReactElement | void { + const { manager, currentChartType, containerElement } = this const bounds = this.boundsForChartArea - const ChartClass = - ChartComponentClassMap.get(this.chartTypeName) ?? DefaultChartClass + + if (!currentChartType) return // Todo: make FacetChart a chart type name? if (this.isFaceted) return ( ) + const ChartClass = + ChartComponentClassMap.get(currentChartType) ?? DefaultChartClass return ( ) } @@ -345,7 +348,7 @@ export class CaptionedChart extends React.Component { > {this.patterns} {this.manager.isReady - ? this.renderChart() + ? this.renderReadyChartOrMap() : this.renderLoadingIndicator()} @@ -607,7 +610,9 @@ export class StaticCaptionedChart extends CaptionedChart { We cannot render a table to svg, but would rather display nothing at all to avoid issues. See https://github.com/owid/owid-grapher/issues/3283 */} - {this.manager.isOnTableTab ? undefined : this.renderChart()} + {this.manager.isOnTableTab + ? undefined + : this.renderReadyChartOrMap()} { // keep in sync with ContentSwitcher.scss @@ -78,32 +74,81 @@ export class ContentSwitchers extends React.Component<{ }, 0) } - private tabIcon(tab: GrapherTabOption): React.ReactElement { + private getTabIcon(tab: GrapherTabOption): React.ReactElement { const { manager } = this switch (tab) { - case GrapherTabOption.table: + case GrapherTabOption.Table: return - case GrapherTabOption.map: + case GrapherTabOption.WorldMap: return - case GrapherTabOption.chart: + default: const chartIcon = manager.isLineChartThatTurnedIntoDiscreteBar ? chartIcons[ChartTypeName.DiscreteBar] - : chartIcons[this.chartType] + : chartIcons[tab] return chartIcon } } + private getTabTextLabel(tab: GrapherTabOption): string { + switch (tab) { + case GrapherTabOption.Table: + return "Table" + case GrapherTabOption.WorldMap: + return "Map" + case GrapherTabOption.LineChart: + return "Line" + case GrapherTabOption.SlopeChart: + return "Slope" + default: + return "Chart" + } + } + + private getTrackingKey(tab: GrapherTabOption): string { + switch (tab) { + case GrapherTabOption.Table: + return "chart_click_table" + case GrapherTabOption.WorldMap: + return "chart_click_map" + case GrapherTabOption.LineChart: + return "chart_click_line" + case GrapherTabOption.SlopeChart: + return "chart_click_slope" + default: + return "chart_click_chart" + } + } + + private getAriaLabel(tab: GrapherTabOption): string { + switch (tab) { + case GrapherTabOption.Table: + return "Table" + case GrapherTabOption.WorldMap: + return "Map" + case GrapherTabOption.LineChart: + return "Line chart" + case GrapherTabOption.SlopeChart: + return "Slope chart" + default: + return "Chart" + } + } + @computed private get tabLabels(): TabLabel[] { return this.availableTabs.map((tab) => ({ element: ( - {this.tabIcon(tab)} - {this.showTabLabels && {tab}} + {this.getTabIcon(tab)} + {this.showTabLabels && ( + + {this.getTabTextLabel(tab)} + + )} ), buttonProps: { - "data-track-note": "chart_click_" + tab, - "aria-label": tab, + "data-track-note": this.getTrackingKey(tab), + "aria-label": this.getAriaLabel(tab), }, })) } @@ -112,7 +157,9 @@ export class ContentSwitchers extends React.Component<{ const { manager, tabLabels } = this const activeIndex = - (manager.tab && this.availableTabs.indexOf(manager.tab)) ?? 0 + (manager.currentTab && + this.availableTabs.indexOf(manager.currentTab)) ?? + 0 return ( dim.display.isProjection @@ -259,9 +268,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 +396,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..294d85e2d28 100644 --- a/packages/@ourworldindata/grapher/src/controls/controlsRow/ControlsRow.tsx +++ b/packages/@ourworldindata/grapher/src/controls/controlsRow/ControlsRow.tsx @@ -2,7 +2,8 @@ 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 { ChartTypeName, GrapherTabOption } from "@ourworldindata/types" import { ContentSwitchers, ContentSwitchersManager } from "../ContentSwitchers" import { @@ -22,7 +23,7 @@ export interface ControlsRowManager MapProjectionMenuManager, SettingsMenuManager { sidePanelBounds?: Bounds - availableTabs?: GrapherTabOption[] + sortedAvailableTabs?: 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..fb6322c0241 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 + currentChartType?: ChartTypeName } @observer @@ -31,10 +31,10 @@ export class AbsRelToggle extends React.Component<{ } @computed get tooltip(): string { - const { type } = this.manager - return type === ScatterPlot + const { currentChartType } = this.manager + return currentChartType === ScatterPlot ? "Show the percentage change per year over the the selected time range." - : type === LineChart + : currentChartType === 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..52952ab5b70 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, + ChartTypeNameRecord, } from "@ourworldindata/types" import { TimeBoundValue, @@ -44,6 +45,7 @@ const TestGrapherConfig = (): { property: DimensionProperty variableId: any }[] + availableTabs: ChartTypeName[] } => { const table = SynthesizeGDPTable({ entityCount: 10 }) return { @@ -56,6 +58,7 @@ const TestGrapherConfig = (): { variableId: SampleColumnSlugs.GDP as any, }, ], + availableTabs: [ChartTypeName.LineChart], } } @@ -69,9 +72,12 @@ it("regression fix: container options are not serialized", () => { it("can get dimension slots", () => { const grapher = new Grapher() + expect(grapher.dimensionSlots.length).toBe(1) + + grapher.availableTabs = [ChartTypeName.LineChart] expect(grapher.dimensionSlots.length).toBe(2) - grapher.type = ChartTypeName.ScatterPlot + grapher.availableTabs = [ChartTypeName.ScatterPlot] expect(grapher.dimensionSlots.length).toBe(4) }) @@ -85,9 +91,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, - } + const input = { availableTabs: ["invalid" as any] } expect(new Grapher(input).toObject()).toEqual({ ...input, $schema: defaultGrapherConfig.$schema, @@ -98,12 +102,15 @@ it("a bad chart type does not crash grapher", () => { }) it("does not preserve defaults in the object (except for the schema)", () => { - expect(new Grapher({ tab: GrapherTabOption.chart }).toObject()).toEqual({ - $schema: defaultGrapherConfig.$schema, + // TODO + expect(new Grapher({ tab: GrapherTabOption.LineChart }).toObject()).toEqual( + { + $schema: defaultGrapherConfig.$schema, - // TODO: ideally, selectedEntityNames is not serialised for an empty object - selectedEntityNames: [], - }) + // TODO: ideally, selectedEntityNames is not serialised for an empty object + selectedEntityNames: [], + } + ) }) const unit = "% of children under 5" @@ -211,24 +218,27 @@ 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.availableTabs = [ChartTypeName.LineChart] expect(grapher.hasTimeline).toBeTruthy() - grapher.type = ChartTypeName.SlopeChart + grapher.availableTabs = [ChartTypeName.SlopeChart] expect(grapher.hasTimeline).toBeTruthy() - grapher.type = ChartTypeName.StackedArea + grapher.availableTabs = [ChartTypeName.StackedArea] expect(grapher.hasTimeline).toBeTruthy() - grapher.type = ChartTypeName.StackedBar + grapher.availableTabs = [ChartTypeName.StackedBar] expect(grapher.hasTimeline).toBeTruthy() - grapher.type = ChartTypeName.DiscreteBar + grapher.availableTabs = [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.availableTabs = [ + ChartTypeName.WorldMap, + ChartTypeName.LineChart, + ] expect(grapher.hasTimeline).toBeFalsy() - grapher.tab = GrapherTabOption.map + grapher.tab = GrapherTabOption.WorldMap expect(grapher.hasTimeline).toBeTruthy() grapher.map.hideTimeline = true expect(grapher.hasTimeline).toBeFalsy() @@ -289,6 +299,7 @@ const getGrapher = (): Grapher => ]), minTime: -5000, maxTime: 5000, + // availableTabs: [ChartTypeName.LineChart], }) function fromQueryParams( @@ -379,7 +390,7 @@ describe("authors can use maxTime", () => { const table = SynthesizeGDPTable({ timeRange: [2000, 2010] }) const grapher = new Grapher({ table, - type: ChartTypeName.DiscreteBar, + availableTabs: [ChartTypeName.DiscreteBar], selectedEntityNames: table.availableEntityNames, maxTime: 2005, ySlugs: "GDP", @@ -482,7 +493,7 @@ describe("urls", () => { isPublished: true, slug: "foo", bakedGrapherURL: "/grapher", - tab: GrapherTabOption.map, + tab: GrapherTabOption.WorldMap, }) expect(grapher.embedUrl).toEqual("/grapher/foo?tab=map") }) @@ -797,6 +808,7 @@ it("canChangeEntity reflects all available entities before transforms", () => { addCountryMode: EntitySelectionMode.SingleEntity, table, selectedEntityNames: table.sampleEntityName(1), + availableTabs: [ChartTypeName.LineChart], }) expect(grapher.canChangeEntity).toBe(true) }) @@ -834,7 +846,7 @@ describe("year parameter (applies to map only)", () => { }) it(`encode ${test.name}`, () => { const params = toQueryParams({ - tab: GrapherTabOption.map, + tab: GrapherTabOption.WorldMap, map: { time: test.param }, }) expect(params.time).toEqual(test.query) @@ -900,7 +912,7 @@ describe("year parameter (applies to map only)", () => { it(`encode ${test.name}`, () => { const grapher = getGrapher() grapher.updateFromObject({ - tab: GrapherTabOption.map, + tab: GrapherTabOption.WorldMap, map: { time: test.param }, }) const params = grapher.changedParams @@ -918,7 +930,7 @@ it("correctly identifies activeColumnSlugs", () => { `) const grapher = new Grapher({ table, - type: ChartTypeName.ScatterPlot, + availableTabs: [ChartTypeName.ScatterPlot], xSlug: "gdp", ySlugs: "child_mortality", colorSlug: "continent", @@ -955,9 +967,9 @@ it("considers map tolerance before using column tolerance", () => { const grapher = new Grapher({ table, - type: ChartTypeName.WorldMap, + availableTabs: [ChartTypeName.WorldMap, ChartTypeName.LineChart], ySlugs: "gdp", - tab: GrapherTabOption.map, + tab: GrapherTabOption.WorldMap, map: new MapConfig({ timeTolerance: 1, columnSlug: "gdp", time: 2002 }), }) @@ -1016,7 +1028,7 @@ describe("tableForSelection", () => { const grapher = new Grapher({ table, - type: ChartTypeName.ScatterPlot, + availableTabs: [ChartTypeName.ScatterPlot], excludedEntities: [3], xSlug: "x", ySlugs: "y", @@ -1052,7 +1064,7 @@ it("handles tolerance when there are gaps in ScatterPlot data", () => { const grapher = new Grapher({ table, - type: ChartTypeName.ScatterPlot, + availableTabs: [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..903929204d0 100644 --- a/packages/@ourworldindata/grapher/src/core/Grapher.stories.tsx +++ b/packages/@ourworldindata/grapher/src/core/Grapher.stories.tsx @@ -90,15 +90,15 @@ export const StackedArea = (): React.ReactElement => { export const MapFirst = (): React.ReactElement => { const model = { ...basics, - tab: GrapherTabOption.map, + tab: GrapherTabOption.WorldMap, } return } export const BlankGrapher = (): React.ReactElement => { const model = { - type: ChartTypeName.WorldMap, - tab: GrapherTabOption.map, + availableTabs: [GrapherTabOption.Table, GrapherTabOption.WorldMap], + tab: GrapherTabOption.WorldMap, table: BlankOwidTable(), hasMapTab: true, } diff --git a/packages/@ourworldindata/grapher/src/core/Grapher.tsx b/packages/@ourworldindata/grapher/src/core/Grapher.tsx index 20c99b3a5bc..4ed573848a1 100644 --- a/packages/@ourworldindata/grapher/src/core/Grapher.tsx +++ b/packages/@ourworldindata/grapher/src/core/Grapher.tsx @@ -5,8 +5,8 @@ import { computed, action, autorun, - runInAction, reaction, + runInAction, } from "mobx" import { bind } from "decko" import { @@ -14,7 +14,6 @@ import { isEqual, uniq, slugify, - identity, lowerCaseFirstLetterUnlessAbbreviation, isMobile, next, @@ -66,6 +65,7 @@ import { sortBy, extractDetailsFromSyntax, omit, + getTabPosition, } from "@ourworldindata/utils" import { MarkdownTextWrap, @@ -105,6 +105,7 @@ import { GrapherWindowType, Color, GRAPHER_QUERY_PARAM_KEYS, + ChartTypeNameRecord, } from "@ourworldindata/types" import { BlankOwidTable, @@ -129,6 +130,7 @@ import { isPopulationVariableETLPath, GRAPHER_BACKGROUND_BEIGE, GRAPHER_BACKGROUND_DEFAULT, + allChartTypes, } from "../core/GrapherConstants" import { defaultGrapherConfig } from "../schema/defaultGrapherConfig" import { loadVariableDataAndMetadata } from "./loadVariable" @@ -300,7 +302,6 @@ export interface GrapherProgrammaticInterface extends GrapherInterface { hideMapProjectionMenu?: boolean hideTableFilterToggle?: boolean forceHideAnnotationFieldsInTitle?: AnnotationFieldsInTitle - hasTableTab?: boolean hideShareButton?: boolean hideExploreTheDataButton?: boolean hideRelatedQuestion?: boolean @@ -350,7 +351,6 @@ export class Grapher SlopeChartManager { @observable.ref $schema = defaultGrapherConfig.$schema - @observable.ref type = ChartTypeName.LineChart @observable.ref id?: number = undefined @observable.ref version = 1 @observable.ref slug?: string = undefined @@ -364,6 +364,9 @@ export class Grapher @observable.ref internalNotes?: string = undefined @observable.ref originUrl?: string = undefined + @observable availableTabs?: ChartTypeName[] + @observable.ref tab?: GrapherTabOption + @observable hideAnnotationFieldsInTitle?: AnnotationFieldsInTitle = undefined @observable.ref minTime?: TimeBound = undefined @@ -384,9 +387,6 @@ 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 isPublished?: boolean = undefined @observable.ref baseColorScheme?: ColorSchemeName = undefined @observable.ref invertColorScheme?: boolean = undefined @@ -603,15 +603,31 @@ export class Grapher this.setDimensionsFromConfigs(obj.dimensions) } + private mapQueryParamToGrapherTab( + tab: string + ): GrapherTabOption | undefined { + switch (tab) { + case "table": + return GrapherTabOption.Table + case "map": + return GrapherTabOption.WorldMap + case "line": + return GrapherTabOption.LineChart + case "slope": + return GrapherTabOption.SlopeChart + case "chart": + return this.firstChartType as unknown as GrapherTabOption + default: + return undefined + } + } + @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.tab = tab + else console.error("Unexpected tab: " + params.tab) } // Set overlay if specified @@ -693,16 +709,73 @@ export class Grapher ) as TimeBounds } + @computed get currentTab(): GrapherTabOption { + if (this.tab) return this.tab + if (this.firstChartType) + return this.firstChartType as unknown as GrapherTabOption + if (this.hasMapTab) return GrapherTabOption.WorldMap + return GrapherTabOption.Table + } + + @computed get availableTabsSet(): Set { + return new Set(this.availableTabs ?? []) + } + + /** + * List of available tabs sorted as they appear in the UI, + * with the table tab always first + */ + @computed get sortedAvailableTabs(): GrapherTabOption[] { + const tabList = Array.from(this.availableTabsSet) + const sortedTabs = sortBy(tabList, (tab) => getTabPosition(tab)) + return [GrapherTabOption.Table, ...sortedTabs] as GrapherTabOption[] + } + + /** + * List of available chart types (without the map) + */ + @computed get chartTypes(): ChartTypeName[] { + return this.sortedAvailableTabs.filter( + (tab) => + tab !== GrapherTabOption.Table && + tab !== GrapherTabOption.WorldMap + ) as unknown as ChartTypeName[] + } + + /** + * The chart type that appears first in the UI + */ + @computed get firstChartType(): ChartTypeName | undefined { + return this.chartTypes[0] + } + + @computed get chartType(): ChartTypeName { + return this.firstChartType ?? ChartTypeName.LineChart + } + + @computed get currentChartType(): ChartTypeName | undefined { + if (!this.isOnChartTab) return undefined + return this.currentTab as unknown as ChartTypeName + } + + @computed get hasChartTab(): boolean { + return allChartTypes.some((type) => this.availableTabsSet.has(type)) + } + + @computed get hasMapTab(): boolean { + return this.availableTabsSet.has(ChartTypeName.WorldMap) + } + @computed get isOnChartTab(): boolean { - return this.tab === GrapherTabOption.chart + return allChartTypes.includes(this.currentTab as any) } @computed get isOnMapTab(): boolean { - return this.tab === GrapherTabOption.map + return this.currentTab === GrapherTabOption.WorldMap } @computed get isOnTableTab(): boolean { - return this.tab === GrapherTabOption.table + return this.currentTab === GrapherTabOption.Table } @computed get isOnChartOrMapTab(): boolean { @@ -813,7 +886,8 @@ export class Grapher // When Map becomes a first-class chart instance, we should drop this @computed get chartInstanceExceptMap(): ChartInterface { const chartTypeName = - this.typeExceptWhenLineChartAndSingleTimeThenWillBeBarChart + this.typeExceptWhenLineChartAndSingleTimeThenWillBeBarChart ?? + ChartTypeName.LineChart // TODO const ChartClass = ChartComponentClassMap.get(chartTypeName) ?? DefaultChartClass @@ -953,8 +1027,7 @@ export class Grapher properties: [ // might be missing for charts within explorers or mdims ["slug", this.slug ?? "missing-slug"], - ["chartType", this.type], - ["tab", this.tab], + ["tab", this.currentTab], ], }, } @@ -1276,8 +1349,8 @@ export class Grapher ) const disposers = [ autorun(() => { - if (!this.availableTabs.includes(this.tab)) - runInAction(() => (this.tab = this.availableTabs[0])) + if (!this.sortedAvailableTabs.includes(this.currentTab)) + runInAction(() => (this.tab = this.sortedAvailableTabs[0])) }), autorun(() => { const validDimensions = this.validDimensions @@ -1479,14 +1552,6 @@ export class Grapher }) } - @computed get availableTabs(): GrapherTabOption[] { - return [ - this.hasTableTab && GrapherTabOption.table, - this.hasMapTab && GrapherTabOption.map, - this.hasChartTab && GrapherTabOption.chart, - ].filter(identity) as GrapherTabOption[] - } - @computed get currentSubtitle(): string { const subtitle = this.subtitle if (subtitle !== undefined) return subtitle @@ -1505,7 +1570,7 @@ export class Grapher return !!( !this.forceHideAnnotationFieldsInTitle?.entity && - this.tab === GrapherTabOption.chart && + this.isOnChartTab && (seriesStrategy !== SeriesStrategy.entity || !this.showLegend) && selectedEntityNames.length === 1 && (showEntityAnnotation || @@ -1577,23 +1642,20 @@ export class Grapher // we don't have more than one distinct time point in our data, so it doesn't make sense to show a timeline if (this.times.length <= 1) return false - switch (this.tab) { + switch (this.currentTab) { // the map tab has its own `hideTimeline` option - case GrapherTabOption.map: + case GrapherTabOption.WorldMap: return !this.map.hideTimeline - // use the chart-level `hideTimeline` option - case GrapherTabOption.chart: - return !this.hideTimeline - // use the chart-level `hideTimeline` option for the table, with some exceptions - case GrapherTabOption.table: + case GrapherTabOption.Table: // always show the timeline for charts that plot time on the x-axis if (this.hasTimeDimension) return true return !this.hideTimeline + // use the chart-level `hideTimeline` option default: - return false + return !this.hideTimeline } } @@ -1862,36 +1924,38 @@ export class Grapher } @computed - get typeExceptWhenLineChartAndSingleTimeThenWillBeBarChart(): ChartTypeName { + get typeExceptWhenLineChartAndSingleTimeThenWillBeBarChart(): + | ChartTypeName + | undefined { // 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.firstChartType } @computed get isLineChart(): boolean { - return this.type === ChartTypeName.LineChart + return this.firstChartType === ChartTypeName.LineChart } @computed get isScatter(): boolean { - return this.type === ChartTypeName.ScatterPlot + return this.firstChartType === ChartTypeName.ScatterPlot } @computed get isStackedArea(): boolean { - return this.type === ChartTypeName.StackedArea + return this.firstChartType === ChartTypeName.StackedArea } @computed get isSlopeChart(): boolean { - return this.type === ChartTypeName.SlopeChart + return this.firstChartType === ChartTypeName.SlopeChart } @computed get isDiscreteBar(): boolean { - return this.type === ChartTypeName.DiscreteBar + return this.firstChartType === ChartTypeName.DiscreteBar } @computed get isStackedBar(): boolean { - return this.type === ChartTypeName.StackedBar + return this.firstChartType === ChartTypeName.StackedBar } @computed get isMarimekko(): boolean { - return this.type === ChartTypeName.Marimekko + return this.firstChartType === ChartTypeName.Marimekko } @computed get isStackedDiscreteBar(): boolean { - return this.type === ChartTypeName.StackedDiscreteBar + return this.firstChartType === ChartTypeName.StackedDiscreteBar } @computed get isLineChartThatTurnedIntoDiscreteBar(): boolean { @@ -2370,7 +2434,7 @@ export class Grapher } @action.bound private toggleTabCommand(): void { - this.tab = next(this.availableTabs, this.tab) + this.tab = next(this.sortedAvailableTabs, this.currentTab) } @action.bound private togglePlayingCommand(): void { @@ -3131,9 +3195,26 @@ export class Grapher debounceMode = false + // TODO: also uses line and slope for Graphers with a single chart – is that ok? + // TODO: should we get rid of chart completely? in that case, URLs break when chart type changes + private tabToQueryParam(tab: GrapherTabOption): string { + switch (tab) { + case GrapherTabOption.Table: + return "table" + case GrapherTabOption.WorldMap: + return "map" + case GrapherTabOption.LineChart: + return "line" + case GrapherTabOption.SlopeChart: + return "slope" + default: + return "chart" + } + } + @computed.struct get allParams(): GrapherQueryParams { const params: GrapherQueryParams = {} - params.tab = this.tab + params.tab = this.tabToQueryParam(this.currentTab) params.xScale = this.xAxis.scaleType params.yScale = this.yAxis.scaleType params.stackMode = this.stackMode diff --git a/packages/@ourworldindata/grapher/src/core/GrapherConstants.ts b/packages/@ourworldindata/grapher/src/core/GrapherConstants.ts index 3fabd87cb2a..4d20a0d213b 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" @@ -93,9 +94,6 @@ export const grapherInterfaceWithHiddenControlsOnly: GrapherProgrammaticInterfac }, } -export const grapherInterfaceWithHiddenTabsOnly: GrapherProgrammaticInterface = - { - hasChartTab: false, - hasMapTab: false, - hasTableTab: false, - } +export const allChartTypes = Object.values(ChartTypeName).filter( + (type) => type !== ChartTypeName.WorldMap +) diff --git a/packages/@ourworldindata/grapher/src/core/GrapherWithChartTypes.jsdom.test.tsx b/packages/@ourworldindata/grapher/src/core/GrapherWithChartTypes.jsdom.test.tsx index b82ca6f925e..dac33672993 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, + availableTabs: [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..94dd893a162 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, + availableTabs: [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..c65dd877e4e 100644 --- a/packages/@ourworldindata/grapher/src/core/LegacyToOwidTable.ts +++ b/packages/@ourworldindata/grapher/src/core/LegacyToOwidTable.ts @@ -37,6 +37,7 @@ import { OwidChartDimensionInterface, OwidVariableType, memoize, + getMainChartTypeFromConfig, } from "@ourworldindata/utils" import { isContinentsVariableId } from "./GrapherConstants" @@ -198,9 +199,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 = getMainChartTypeFromConfig(grapherConfig) 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..e0ece919f36 100755 --- a/packages/@ourworldindata/grapher/src/dataTable/DataTable.jsdom.test.tsx +++ b/packages/@ourworldindata/grapher/src/dataTable/DataTable.jsdom.test.tsx @@ -70,8 +70,8 @@ describe("when you select a range of years", () => { let view: ReactWrapper beforeAll(() => { const grapher = childMortalityGrapher({ - type: ChartTypeName.LineChart, - tab: GrapherTabOption.table, + availableTabs: [ChartTypeName.LineChart], + tab: GrapherTabOption.Table, }) grapher.timelineHandleTimeBounds = [1950, 2019] @@ -158,7 +158,7 @@ describe("when the table has no filter toggle", () => { const grapher = LifeExpectancyGrapher({ selectedEntityNames: ["World"], hideEntityControls: true, // no filter toggle - hasMapTab: true, + availableTabs: [ChartTypeName.WorldMap], }) const view = Enzyme.mount() const rows = view.find("tbody tr:not(.title)") @@ -169,7 +169,6 @@ describe("when the table has no filter toggle", () => { const grapher = LifeExpectancyGrapher({ selectedEntityNames: [], hideEntityControls: true, // no filter toggle - hasMapTab: false, }) const view = Enzyme.mount() const rows = view.find("tbody tr:not(.title)") diff --git a/packages/@ourworldindata/grapher/src/dataTable/DataTable.sample.ts b/packages/@ourworldindata/grapher/src/dataTable/DataTable.sample.ts index 5bf81c3a0bc..30fda027687 100644 --- a/packages/@ourworldindata/grapher/src/dataTable/DataTable.sample.ts +++ b/packages/@ourworldindata/grapher/src/dataTable/DataTable.sample.ts @@ -37,8 +37,7 @@ export const childMortalityGrapher = ( }, ] return new Grapher({ - hasMapTab: true, - tab: GrapherTabOption.map, + tab: GrapherTabOption.WorldMap, dimensions, ...props, owidDataset: createOwidTestDataset([ @@ -85,7 +84,7 @@ export const GrapherWithIncompleteData = ( }, ] return new Grapher({ - tab: GrapherTabOption.table, + tab: GrapherTabOption.Table, dimensions, ...props, owidDataset: createOwidTestDataset([{ metadata, data }]), @@ -125,7 +124,7 @@ export const GrapherWithAggregates = ( }, ] return new Grapher({ - tab: GrapherTabOption.table, + tab: GrapherTabOption.Table, dimensions, ...props, owidDataset: createOwidTestDataset([ @@ -169,7 +168,7 @@ export const GrapherWithMultipleVariablesAndMultipleYears = ( } return new Grapher({ - tab: GrapherTabOption.table, + tab: GrapherTabOption.Table, dimensions, ...props, owidDataset: createOwidTestDataset([ diff --git a/packages/@ourworldindata/grapher/src/dataTable/DataTable.stories.tsx b/packages/@ourworldindata/grapher/src/dataTable/DataTable.stories.tsx index e0279bfaf88..41dfb90f232 100644 --- a/packages/@ourworldindata/grapher/src/dataTable/DataTable.stories.tsx +++ b/packages/@ourworldindata/grapher/src/dataTable/DataTable.stories.tsx @@ -88,8 +88,8 @@ export const FromLegacy = (): React.ReactElement => { export const FromLegacyWithTimeRange = (): React.ReactElement => { const grapher = childMortalityGrapher({ - type: ChartTypeName.LineChart, - tab: GrapherTabOption.chart, + availableTabs: [GrapherTabOption.Table, GrapherTabOption.LineChart], + tab: GrapherTabOption.LineChart, }) grapher.startHandleTimeBound = 1950 grapher.endHandleTimeBound = 2019 diff --git a/packages/@ourworldindata/grapher/src/index.ts b/packages/@ourworldindata/grapher/src/index.ts index 203bb60db3a..deb58f14f7a 100644 --- a/packages/@ourworldindata/grapher/src/index.ts +++ b/packages/@ourworldindata/grapher/src/index.ts @@ -23,9 +23,9 @@ export { WorldEntityName, Patterns, grapherInterfaceWithHiddenControlsOnly, - grapherInterfaceWithHiddenTabsOnly, CONTINENTS_INDICATOR_ID, POPULATION_INDICATOR_ID_USED_IN_ADMIN, + allChartTypes, } from "./core/GrapherConstants" export { getVariableDataRoute, diff --git a/packages/@ourworldindata/grapher/src/mapCharts/MapChart.sample.ts b/packages/@ourworldindata/grapher/src/mapCharts/MapChart.sample.ts index a118a1d6969..b741d73d328 100644 --- a/packages/@ourworldindata/grapher/src/mapCharts/MapChart.sample.ts +++ b/packages/@ourworldindata/grapher/src/mapCharts/MapChart.sample.ts @@ -1,10 +1,11 @@ import { DimensionProperty } from "@ourworldindata/utils" import { GrapherProgrammaticInterface } from "../core/Grapher" -import { GrapherTabOption } from "@ourworldindata/types" +import { ChartTypeName, GrapherTabOption } from "@ourworldindata/types" +// TODO: rewrite export const legacyMapGrapher: GrapherProgrammaticInterface = { - hasMapTab: true, - tab: GrapherTabOption.map, + availableTabs: [ChartTypeName.WorldMap], + tab: GrapherTabOption.WorldMap, map: { timeTolerance: 5, }, diff --git a/packages/@ourworldindata/grapher/src/mapCharts/MapChart.tsx b/packages/@ourworldindata/grapher/src/mapCharts/MapChart.tsx index b8d9343a8fc..9a72be96eaa 100644 --- a/packages/@ourworldindata/grapher/src/mapCharts/MapChart.tsx +++ b/packages/@ourworldindata/grapher/src/mapCharts/MapChart.tsx @@ -289,7 +289,7 @@ export class MapChart if (!ev.shiftKey) { this.selectionArray.setSelectedEntities([entityName]) - this.manager.tab = GrapherTabOption.chart + this.manager.tab = GrapherTabOption.LineChart if ( this.manager.isLineChartThatTurnedIntoDiscreteBar && this.manager.hasTimeline diff --git a/packages/@ourworldindata/grapher/src/mapCharts/MapChartConstants.ts b/packages/@ourworldindata/grapher/src/mapCharts/MapChartConstants.ts index 3e974d98c62..9cbe370e700 100644 --- a/packages/@ourworldindata/grapher/src/mapCharts/MapChartConstants.ts +++ b/packages/@ourworldindata/grapher/src/mapCharts/MapChartConstants.ts @@ -57,7 +57,6 @@ export interface MapChartManager extends ChartManager { mapColumnSlug?: ColumnSlug mapIsClickable?: boolean tab?: GrapherTabOption // Used to switch to chart tab on map click - type?: ChartTypeName // Used to determine the "Click to select" text in MapTooltip isLineChartThatTurnedIntoDiscreteBar?: boolean // Used to determine whether to reset the timeline on map click hasTimeline?: boolean // Used to determine whether to reset the timeline on map click resetHandleTimeBounds?: () => void // Used to reset the timeline on map click diff --git a/packages/@ourworldindata/grapher/src/schema/defaultGrapherConfig.ts b/packages/@ourworldindata/grapher/src/schema/defaultGrapherConfig.ts index 1243ba4e985..a208acf6d25 100644 --- a/packages/@ourworldindata/grapher/src/schema/defaultGrapherConfig.ts +++ b/packages/@ourworldindata/grapher/src/schema/defaultGrapherConfig.ts @@ -4,17 +4,18 @@ import { GrapherInterface } from "@ourworldindata/types" -export const latestSchemaVersion = "006" as const +export const latestSchemaVersion = "007" as const export const outdatedSchemaVersions = [ "001", "002", "003", "004", "005", + "006", ] as const export const defaultGrapherConfig = { - $schema: "https://files.ourworldindata.org/schemas/grapher-schema.006.json", + $schema: "https://files.ourworldindata.org/schemas/grapher-schema.007.json", map: { projection: "World", hideTimeline: false, @@ -37,9 +38,7 @@ export const defaultGrapherConfig = { canChangeScaleType: false, facetDomain: "shared", }, - tab: "chart", matchingEntitiesOnly: false, - hasChartTab: true, hideLegend: false, hideLogo: false, timelineMinTime: "earliest", @@ -60,8 +59,6 @@ export const defaultGrapherConfig = { facettingLabelByYVariables: "metric", addCountryMode: "add-country", compareEndPointsOnly: false, - type: "LineChart", - hasMapTab: false, stackMode: "absolute", minTime: "earliest", hideAnnotationFieldsInTitle: { diff --git a/packages/@ourworldindata/grapher/src/schema/grapher-schema.006.yaml b/packages/@ourworldindata/grapher/src/schema/grapher-schema.007.yaml similarity index 97% rename from packages/@ourworldindata/grapher/src/schema/grapher-schema.006.yaml rename to packages/@ourworldindata/grapher/src/schema/grapher-schema.007.yaml index f906da32f22..f14e75a1a25 100644 --- a/packages/@ourworldindata/grapher/src/schema/grapher-schema.006.yaml +++ b/packages/@ourworldindata/grapher/src/schema/grapher-schema.007.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.006.json" +$id: "https://files.ourworldindata.org/schemas/grapher-schema.007.json" required: - $schema - dimensions @@ -14,17 +14,46 @@ 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.006.json" + default: "https://files.ourworldindata.org/schemas/grapher-schema.007.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.006.json" + const: "https://files.ourworldindata.org/schemas/grapher-schema.007.json" id: type: integer description: Internal DB id. Useful internally for OWID but not required if just using grapher directly. minimum: 0 + availableTabs: + type: array + description: The tabs that are available for selection. The Table tab is always present. + items: + type: string + enum: + - WorldMap + - LineChart + - ScatterPlot + - StackedArea + - DiscreteBar + - StackedDiscreteBar + - SlopeChart + - StackedBar + - Marimekko + tab: + type: string + description: The tab that is shown initially. Defaults to the chart tab if available. + enum: + - Table + - WorldMap + - LineChart + - ScatterPlot + - StackedArea + - DiscreteBar + - StackedDiscreteBar + - SlopeChart + - StackedBar + - Marimekko map: type: object description: Configuration of the world map chart @@ -111,22 +140,10 @@ properties: If not provided, a default is chosen based on the chart type. yAxis: $ref: "#/$defs/axis" - tab: - type: string - description: The tab that is shown initially - default: chart - enum: - - chart - - map - - table matchingEntitiesOnly: 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 @@ -369,23 +386,6 @@ 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 - hasMapTab: - type: boolean - default: false - description: Indicates if the map tab should be shown stackMode: type: string description: Stack mode. Only absolute and relative are actively used. diff --git a/packages/@ourworldindata/grapher/src/schema/migrations/migrations.ts b/packages/@ourworldindata/grapher/src/schema/migrations/migrations.ts index 7b9b1858c84..871f0190dd6 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 = ( @@ -72,6 +73,37 @@ const migrateFrom005To006 = ( return config } +const migrateFrom006To007 = ( + config: AnyConfigWithValidSchema +): AnyConfigWithValidSchema => { + const { + type = ChartTypeName.LineChart, + hasChartTab = true, + hasMapTab = false, + } = config + + // update availableTabs + const availableTabs: Record = {} + if (hasMapTab || hasChartTab) { + if (hasMapTab) availableTabs.WorldMap = true + if (hasChartTab) availableTabs[type] = true + config.availableTabs = availableTabs + } + + // update tab + if (config.tab === "chart") config.tab = type + else if (config.tab === "map") config.tab = "WorldMap" + else if (config.tab === "table") config.tab = "Table" + + // remove old fields + delete config.type + delete config.hasMapTab + delete config.hasChartTab + + config.$schema = createSchemaForVersion("007") + return config +} + export const runMigration = ( config: AnyConfigWithValidSchema ): AnyConfigWithValidSchema => { @@ -83,5 +115,6 @@ export const runMigration = ( .with("003", () => migrateFrom003To004(config)) .with("004", () => migrateFrom004To005(config)) .with("005", () => migrateFrom005To006(config)) + .with("006", () => migrateFrom006To007(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..8205950952f 100644 --- a/packages/@ourworldindata/grapher/src/stackedCharts/MarimekkoChart.jsdom.test.tsx +++ b/packages/@ourworldindata/grapher/src/stackedCharts/MarimekkoChart.jsdom.test.tsx @@ -3,8 +3,8 @@ import { Bounds, ColumnTypeNames } from "@ourworldindata/utils" import { OwidTable } from "@ourworldindata/core-table" import { DefaultColorScheme } from "../color/CustomSchemes" -import { Grapher } from "../core/Grapher" -import { ChartTypeName } from "@ourworldindata/types" +import { Grapher, GrapherProgrammaticInterface } from "../core/Grapher" +import { ChartTypeName, GrapherInterface } from "@ourworldindata/types" import { MarimekkoChart } from "./MarimekkoChart" import { BarShape, PlacedItem } from "./MarimekkoChartConstants" it("can filter years correctly", () => { @@ -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, + availableTabs: [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, + availableTabs: [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, + availableTabs: [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, + availableTabs: [ChartTypeName.Marimekko], table, selection: table.availableEntityNames, ySlugs: "percentBelow2USD percentBelow10USD", diff --git a/packages/@ourworldindata/types/src/grapherTypes/GrapherTypes.ts b/packages/@ourworldindata/types/src/grapherTypes/GrapherTypes.ts index 5954b1e985c..1094a13253b 100644 --- a/packages/@ourworldindata/types/src/grapherTypes/GrapherTypes.ts +++ b/packages/@ourworldindata/types/src/grapherTypes/GrapherTypes.ts @@ -119,6 +119,7 @@ export enum ToleranceStrategy { backwards = "backwards", forwards = "forwards", } + export enum ChartTypeName { LineChart = "LineChart", ScatterPlot = "ScatterPlot", @@ -132,6 +133,23 @@ export enum ChartTypeName { WorldMap = "WorldMap", } +export type ChartTypeNameRecord = Partial> + +// ChartTypeNames + Table +// TODO: turn into a non-enum type? +export enum GrapherTabOption { + Table = "Table", + WorldMap = "WorldMap", + LineChart = "LineChart", + ScatterPlot = "ScatterPlot", + StackedArea = "StackedArea", + DiscreteBar = "DiscreteBar", + StackedDiscreteBar = "StackedDiscreteBar", + SlopeChart = "SlopeChart", + StackedBar = "StackedBar", + Marimekko = "Marimekko", +} + export enum AxisMinMaxValueStr { auto = "auto", } @@ -172,12 +190,6 @@ export type SeriesName = string export type SeriesColorMap = Map -export enum GrapherTabOption { - chart = "chart", - map = "map", - table = "table", -} - export interface RelatedQuestionsConfig { text?: string url?: string @@ -528,7 +540,6 @@ export interface MapConfigInterface { // under the same rendering conditions it ought to remain visually identical export interface GrapherInterface extends SortConfig { $schema?: string - type?: ChartTypeName id?: number version?: number slug?: string @@ -536,6 +547,8 @@ export interface GrapherInterface extends SortConfig { subtitle?: string sourceDesc?: string note?: string + availableTabs?: ChartTypeName[] + tab?: GrapherTabOption hideAnnotationFieldsInTitle?: AnnotationFieldsInTitle minTime?: TimeBound | TimeBoundValueStr maxTime?: TimeBound | TimeBoundValueStr @@ -556,9 +569,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 relatedQuestion?: RelatedQuestionsConfig details?: DetailDictionary internalNotes?: string @@ -647,10 +657,10 @@ 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", "id", "version", "slug", + "availableTabs", "title", "subtitle", "sourceDesc", @@ -673,8 +683,6 @@ export const grapherKeysToSerialize = [ "hideTimeline", "zoomToSelection", "showYearLabels", - "hasChartTab", - "hasMapTab", "tab", "internalNotes", "variantName", diff --git a/packages/@ourworldindata/types/src/index.ts b/packages/@ourworldindata/types/src/index.ts index 5decf69e009..2f4674a6207 100644 --- a/packages/@ourworldindata/types/src/index.ts +++ b/packages/@ourworldindata/types/src/index.ts @@ -75,6 +75,7 @@ export { ColorSchemeName, colorScaleConfigDefaults, ChartTypeName, + type ChartTypeNameRecord, GrapherTabOption, StackMode, EntitySelectionMode, diff --git a/packages/@ourworldindata/utils/src/Util.ts b/packages/@ourworldindata/utils/src/Util.ts index f62bdc5bb40..00dc19f5bb7 100644 --- a/packages/@ourworldindata/utils/src/Util.ts +++ b/packages/@ourworldindata/utils/src/Util.ts @@ -177,6 +177,7 @@ import { GrapherInterface, ChartTypeName, DimensionProperty, + GrapherTabOption, } from "@ourworldindata/types" import { PointVector } from "./PointVector.js" import React from "react" @@ -1963,23 +1964,6 @@ export function traverseObjects>( return result } -export function getParentVariableIdFromChartConfig( - config: GrapherInterface -): number | undefined { - const { type = ChartTypeName.LineChart, dimensions } = config - - if (type === ChartTypeName.ScatterPlot) return undefined - if (!dimensions) return undefined - - const yVariableIds = dimensions - .filter((d) => d.property === DimensionProperty.y) - .map((d) => d.variableId) - - if (yVariableIds.length !== 1) return undefined - - return yVariableIds[0] -} - // Page numbers are 0-indexed - you'll have to +1 to them when rendering export function getPaginationPageNumbers( currentPageNumber: number, diff --git a/packages/@ourworldindata/utils/src/grapherConfigUtils.test.ts b/packages/@ourworldindata/utils/src/grapherConfigUtils.test.ts index 882b0d74326..6dbef8ef821 100644 --- a/packages/@ourworldindata/utils/src/grapherConfigUtils.test.ts +++ b/packages/@ourworldindata/utils/src/grapherConfigUtils.test.ts @@ -243,14 +243,14 @@ describe(diffGrapherConfigs, () => { it("drops redundant entries", () => { expect( diffGrapherConfigs( - { tab: GrapherTabOption.map }, - { tab: GrapherTabOption.map } + { tab: GrapherTabOption.WorldMap }, + { tab: GrapherTabOption.WorldMap } ) ).toEqual({}) expect( diffGrapherConfigs( - { tab: GrapherTabOption.chart, title: "Chart" }, - { tab: GrapherTabOption.chart, title: "Reference chart" } + { tab: GrapherTabOption.LineChart, title: "Chart" }, + { tab: GrapherTabOption.LineChart, title: "Reference chart" } ) ).toEqual({ title: "Chart" }) }) @@ -260,7 +260,7 @@ describe(diffGrapherConfigs, () => { diffGrapherConfigs( { title: "Chart", - tab: GrapherTabOption.chart, + tab: GrapherTabOption.LineChart, map: { projection: MapProjectionName.World, hideTimeline: true, @@ -268,7 +268,7 @@ describe(diffGrapherConfigs, () => { }, { title: "Reference chart", - tab: GrapherTabOption.chart, + tab: GrapherTabOption.LineChart, map: { projection: MapProjectionName.World, hideTimeline: false, @@ -279,14 +279,14 @@ describe(diffGrapherConfigs, () => { expect( diffGrapherConfigs( { - tab: GrapherTabOption.chart, + tab: GrapherTabOption.LineChart, map: { projection: MapProjectionName.World, hideTimeline: true, }, }, { - tab: GrapherTabOption.chart, + tab: GrapherTabOption.LineChart, map: { projection: MapProjectionName.World, hideTimeline: true, @@ -300,11 +300,11 @@ describe(diffGrapherConfigs, () => { expect( diffGrapherConfigs( { - tab: GrapherTabOption.chart, + tab: GrapherTabOption.LineChart, title: "Chart", subtitle: undefined, }, - { tab: GrapherTabOption.chart, title: "Reference chart" } + { tab: GrapherTabOption.LineChart, title: "Reference chart" } ) ).toEqual({ title: "Chart" }) }) @@ -361,12 +361,12 @@ describe(diffGrapherConfigs, () => { it("is idempotent", () => { const config: GrapherInterface = { - tab: GrapherTabOption.chart, + tab: GrapherTabOption.LineChart, title: "Chart", subtitle: undefined, } const reference: GrapherInterface = { - tab: GrapherTabOption.chart, + tab: GrapherTabOption.LineChart, title: "Reference chart", } const diffedOnce = diffGrapherConfigs(config, reference) @@ -378,12 +378,12 @@ describe(diffGrapherConfigs, () => { describe("diff+merge", () => { it("are consistent", () => { const config: GrapherInterface = { - tab: GrapherTabOption.chart, + tab: GrapherTabOption.LineChart, title: "Chart", subtitle: "Chart subtitle", } const reference: GrapherInterface = { - tab: GrapherTabOption.chart, + tab: GrapherTabOption.LineChart, title: "Reference chart", } const diffedAndMerged = mergeGrapherConfigs( diff --git a/packages/@ourworldindata/utils/src/grapherConfigUtils.ts b/packages/@ourworldindata/utils/src/grapherConfigUtils.ts index 2a8db8e0120..df5779295c0 100644 --- a/packages/@ourworldindata/utils/src/grapherConfigUtils.ts +++ b/packages/@ourworldindata/utils/src/grapherConfigUtils.ts @@ -1,4 +1,8 @@ -import { GrapherInterface } from "@ourworldindata/types" +import { + ChartTypeName, + DimensionProperty, + GrapherInterface, +} from "@ourworldindata/types" import { isEqual, mergeWith, @@ -10,6 +14,7 @@ import { omitEmptyObjectsRecursive, traverseObjects, isEmpty, + sortBy, } from "./Util" const REQUIRED_KEYS = ["$schema", "dimensions"] @@ -91,3 +96,52 @@ export function diffGrapherConfigs( return { ...diffed, ...keep } } + +export function getTabPosition(tab: ChartTypeName): number { + switch (tab) { + case ChartTypeName.WorldMap: + return 1 + case ChartTypeName.LineChart: + return 2 + default: + return 3 + } +} + +export function hasChartTabFromConfig(config: GrapherInterface): boolean { + const { availableTabs = [] } = config + const firstChartType = availableTabs.find( + (tab) => tab !== ChartTypeName.WorldMap + ) + return firstChartType !== undefined +} + +export function getMainChartTypeFromConfig( + config: GrapherInterface +): ChartTypeName | undefined { + const { availableTabs = [] } = config + const sortedTabs = sortBy(availableTabs, (tab) => getTabPosition(tab)) + const firstChartType = sortedTabs.find( + (tab) => tab !== ChartTypeName.WorldMap + ) + return firstChartType +} + +export function getParentVariableIdFromChartConfig( + config: GrapherInterface +): number | undefined { + const { dimensions } = config + + const chartType = getMainChartTypeFromConfig(config) + if (chartType === ChartTypeName.ScatterPlot) return undefined + + if (!dimensions) return undefined + + const yVariableIds = dimensions + .filter((d) => d.property === DimensionProperty.y) + .map((d) => d.variableId) + + if (yVariableIds.length !== 1) return undefined + + return yVariableIds[0] +} diff --git a/packages/@ourworldindata/utils/src/index.ts b/packages/@ourworldindata/utils/src/index.ts index eacf00917fa..6a33af8bd4e 100644 --- a/packages/@ourworldindata/utils/src/index.ts +++ b/packages/@ourworldindata/utils/src/index.ts @@ -123,7 +123,6 @@ export { createTagGraph, formatInlineList, lazy, - getParentVariableIdFromChartConfig, } from "./Util.js" export { @@ -336,6 +335,10 @@ export { isAndroid, isIOS } from "./BrowserUtils.js" export { diffGrapherConfigs, mergeGrapherConfigs, + getMainChartTypeFromConfig, + getParentVariableIdFromChartConfig, + hasChartTabFromConfig, + getTabPosition, } from "./grapherConfigUtils.js" export { diff --git a/site/DataPageV2Content.tsx b/site/DataPageV2Content.tsx index 8cb1c996938..a9f8b6932c9 100644 --- a/site/DataPageV2Content.tsx +++ b/site/DataPageV2Content.tsx @@ -247,9 +247,9 @@ export const DataPageV2Content = ({ Explore charts that include this data
- + /> */}
) : null} diff --git a/site/gdocs/components/Chart.tsx b/site/gdocs/components/Chart.tsx index dbc57a3f973..892961d4aac 100644 --- a/site/gdocs/components/Chart.tsx +++ b/site/gdocs/components/Chart.tsx @@ -2,7 +2,6 @@ import React, { useRef } from "react" import { useEmbedChart } from "../../hooks.js" import { grapherInterfaceWithHiddenControlsOnly, - grapherInterfaceWithHiddenTabsOnly, GrapherProgrammaticInterface, } from "@ourworldindata/grapher" import { @@ -12,6 +11,8 @@ import { identity, Url, merge, + excludeNull, + GrapherTabOption, } from "@ourworldindata/utils" import { ChartConfigType } from "@ourworldindata/types" import { renderSpans, useLinkedChart } from "../utils.js" @@ -52,19 +53,24 @@ export default function Chart({ let customizedChartConfig: GrapherProgrammaticInterface = {} const isCustomized = d.title || d.subtitle if (!isExplorer && isCustomized) { + // TODO: add back support for fine-grained control over tabs? + 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 displayedControls = excludeNull( + controls.map(mapControlKeywordToGrapherConfig) + ) as GrapherProgrammaticInterface[] + + const allControlsHidden = grapherInterfaceWithHiddenControlsOnly + const allTabsHidden = { availableTabs: [] } customizedChartConfig = merge( {}, - !showAllControls ? grapherInterfaceWithHiddenControlsOnly : {}, - !showAllTabs ? grapherInterfaceWithHiddenTabsOnly : {}, - ...listOfPartialGrapherConfigs, + !showAllControls ? allControlsHidden : {}, + !showAllTabs ? allTabsHidden : {}, + ...displayedControls, { hideRelatedQuestion: true, hideShareButton: true, // always hidden since the original chart would be shared, not the customized one @@ -134,12 +140,10 @@ export default function Chart({ ) } -const mapKeywordToGrapherConfig = ( - keyword: ChartControlKeyword | ChartTabKeyword +const mapControlKeywordToGrapherConfig = ( + keyword: ChartControlKeyword ): GrapherProgrammaticInterface | null => { switch (keyword) { - // controls - case ChartControlKeyword.relativeToggle: return { hideRelativeToggle: false } @@ -173,17 +177,6 @@ const mapKeywordToGrapherConfig = ( case ChartControlKeyword.tableFilterToggle: return { hideTableFilterToggle: false } - // tabs - - case ChartTabKeyword.chart: - return { hasChartTab: true } - - case ChartTabKeyword.map: - return { hasMapTab: true } - - case ChartTabKeyword.table: - return { hasTableTab: true } - default: return null } diff --git a/site/gdocs/components/KeyIndicatorCollection.tsx b/site/gdocs/components/KeyIndicatorCollection.tsx index 8539a5a70b3..1277857bd3a 100644 --- a/site/gdocs/components/KeyIndicatorCollection.tsx +++ b/site/gdocs/components/KeyIndicatorCollection.tsx @@ -25,10 +25,15 @@ import { Button } from "@ourworldindata/components" // keep in sync with $duration in KeyIndicatorCollection.scss const HEIGHT_ANIMATION_DURATION_IN_SECONDS = 0.4 -const tabIconMap: Record = { - [GrapherTabOption.chart]: faChartLine, - [GrapherTabOption.map]: faEarthAmericas, - [GrapherTabOption.table]: faTable, +function getTabIcon(tab: GrapherTabOption): IconDefinition { + switch (tab) { + case GrapherTabOption.Table: + return faTable + case GrapherTabOption.WorldMap: + return faEarthAmericas + default: + return faChartLine + } } export default function KeyIndicatorCollection({ @@ -241,13 +246,15 @@ function KeyIndicatorHeader({ if (!linkedChart) return null if (!linkedIndicator) return null + // TODO: translate old query params to new ones const { queryParams } = Url.fromURL(linkedChart.resolvedUrl) const tabFromQueryParams = queryParams.tab && isValidGrapherTab(queryParams.tab) ? queryParams.tab : undefined + // TODO const activeTab = - tabFromQueryParams || linkedChart.tab || GrapherTabOption.chart + tabFromQueryParams || linkedChart.tab || GrapherTabOption.LineChart // || GrapherTabOption.chart const source = block.source || linkedIndicator.attributionShort @@ -255,7 +262,7 @@ function KeyIndicatorHeader({
diff --git a/site/multiembedder/MultiEmbedder.tsx b/site/multiembedder/MultiEmbedder.tsx index 7e1639f85e9..4c44eedefed 100644 --- a/site/multiembedder/MultiEmbedder.tsx +++ b/site/multiembedder/MultiEmbedder.tsx @@ -210,18 +210,19 @@ class MultiEmbedder { : {} // make sure the tab of the active pane is visible - if (figureConfigAttr) { - const activeTab = - 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 - } + // TODO: translate query params... construct config + // if (figureConfigAttr) { + // const activeTab = + // 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( {}, // merge mutates the first argument