diff --git a/adminSiteClient/BulkGrapherConfigEditor.tsx b/adminSiteClient/BulkGrapherConfigEditor.tsx index c4109f42cbc..378d00998cf 100644 --- a/adminSiteClient/BulkGrapherConfigEditor.tsx +++ b/adminSiteClient/BulkGrapherConfigEditor.tsx @@ -79,7 +79,7 @@ const bulkChartEditorColumnSets: ColumnSet[] = [ label: "Common", kind: "specificColumns", columns: [ - "/type", + "/chartTypes", "/hasMapTab", "/title", "/subtitle", @@ -110,9 +110,8 @@ const bulkChartEditorColumnSets: ColumnSet[] = [ "/baseColorScheme", "/map/colorScale", "/colorScale", - "/hasChartTab", "/hasMapTab", - "/type", + "/chartTypes", ], }, ] 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..d484ef7842a 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 @@ -346,6 +349,8 @@ export class EditorBasicTab< database: EditorDatabase errorMessagesForDimensions: ErrorMessagesForDimensions }> { + private chartTypeOptionNone = "None" + @action.bound private updateParentConfig() { const { editor } = this.props if (isChartEditorInstance(editor)) { @@ -355,7 +360,9 @@ export class EditorBasicTab< @action.bound onChartTypeChange(value: string) { const { grapher } = this.props.editor - grapher.type = value as ChartTypeName + + grapher.chartTypes = + value === this.chartTypeOptionNone ? [] : [value as ChartTypeName] if (grapher.isMarimekko) { grapher.hideRelativeToggle = false @@ -392,38 +399,48 @@ export class EditorBasicTab< this.updateParentConfig() } - render() { - const { editor } = this.props - const { grapher } = editor - const chartTypes = Object.keys(ChartTypeName).filter( + @computed private get chartTypeOptions(): { + value: string + label: string + }[] { + const allChartTypes = Object.keys(ChartTypeName).filter( (chartType) => chartType !== ChartTypeName.WorldMap ) + const chartTypeOptions = allChartTypes.map((key) => ({ + value: key, + label: startCase(key), + })) + + return [ + ...chartTypeOptions, + { value: this.chartTypeOptionNone, label: "No chart tab" }, + ] + } + + render() { + const { editor } = this.props + const { grapher } = editor const isIndicatorChart = isIndicatorChartEditorInstance(editor) return (
{isIndicatorChart && } -
+
({ - value: key, - label: startCase(key), - }))} + options={this.chartTypeOptions} /> - (grapher.hasChartTab = value)} - /> (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->>"$.subtitle" AS subtitle, chart_configs.full->>"$.sourceDesc" AS sourceDesc, chart_configs.full->>"$.note" AS note, - 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->>"$.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 e3c08990370..120673d8706 100644 --- a/adminSiteServer/app.test.tsx +++ b/adminSiteServer/app.test.tsx @@ -58,6 +58,7 @@ import { import path from "path" import fs from "fs" import { omitUndefinedValues } from "@ourworldindata/utils" +import { latestGrapherConfigSchema } from "@ourworldindata/grapher" const ADMIN_SERVER_HOST = "localhost" const ADMIN_SERVER_PORT = 8765 @@ -192,11 +193,10 @@ async function makeRequestAgainstAdminApi( describe("OwidAdminApp", () => { const testChartConfig = { - $schema: - "https://files.ourworldindata.org/schemas/grapher-schema.005.json", + $schema: latestGrapherConfigSchema, slug: "test-chart", title: "Test chart", - type: "LineChart", + chartTypes: ["LineChart"], } it("should be able to create an app", () => { @@ -311,16 +311,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: latestGrapherConfigSchema, hasMapTab: true, note: "Indicator note", selectedEntityNames: ["France", "Italy", "Spain"], hideRelativeToggle: false, } const testVariableConfigAdmin = { - $schema: - "https://files.ourworldindata.org/schemas/grapher-schema.005.json", + $schema: latestGrapherConfigSchema, title: "Admin title", subtitle: "Admin subtitle", } @@ -331,17 +329,15 @@ describe("OwidAdminApp: indicator-level chart configs", () => { id: otherVariableId, } const otherTestVariableConfig = { - $schema: - "https://files.ourworldindata.org/schemas/grapher-schema.005.json", + $schema: latestGrapherConfigSchema, note: "Other indicator note", } const testChartConfig = { - $schema: - "https://files.ourworldindata.org/schemas/grapher-schema.005.json", + $schema: latestGrapherConfigSchema, slug: "test-chart", title: "Test chart", - type: "Marimekko", + chartTypes: ["Marimekko"], selectedEntityNames: [], hideRelativeToggle: false, dimensions: [ @@ -352,8 +348,7 @@ describe("OwidAdminApp: indicator-level chart configs", () => { ], } const testMultiDimConfig = { - grapherConfigSchema: - "https://files.ourworldindata.org/schemas/grapher-schema.005.json", + grapherConfigSchema: latestGrapherConfigSchema, title: { title: "Energy use", titleVariant: "by energy source", @@ -613,14 +608,13 @@ describe("OwidAdminApp: indicator-level chart configs", () => { ) expect(fullConfig).toEqual({ - $schema: - "https://files.ourworldindata.org/schemas/grapher-schema.005.json", + $schema: latestGrapherConfigSchema, id: chartId, isPublished: false, version: 1, slug: "test-chart", title: "Test chart", - type: "Marimekko", + chartTypes: ["Marimekko"], selectedEntityNames: [], hideRelativeToggle: false, dimensions: [{ variableId, property: "y" }], @@ -634,14 +628,13 @@ describe("OwidAdminApp: indicator-level chart configs", () => { `/charts/${chartId}.patchConfig.json` ) expect(patchConfig).toEqual({ - $schema: - "https://files.ourworldindata.org/schemas/grapher-schema.005.json", + $schema: latestGrapherConfigSchema, id: chartId, version: 1, isPublished: false, slug: "test-chart", title: "Test chart", - type: "Marimekko", + chartTypes: ["Marimekko"], selectedEntityNames: [], dimensions: [{ variableId, property: "y" }], // note that `hideRelativeToggle` is not included @@ -671,8 +664,7 @@ describe("OwidAdminApp: indicator-level chart configs", () => { `/charts/${chartId}.config.json` ) expect(fullConfigAfterDelete).toEqual({ - $schema: - "https://files.ourworldindata.org/schemas/grapher-schema.005.json", + $schema: latestGrapherConfigSchema, id: chartId, version: 1, isPublished: false, @@ -680,7 +672,7 @@ describe("OwidAdminApp: indicator-level chart configs", () => { selectedEntityNames: [], slug: "test-chart", title: "Test chart", - type: "Marimekko", + chartTypes: ["Marimekko"], }) // fetch the patch config and verify it's diffed correctly @@ -688,14 +680,13 @@ describe("OwidAdminApp: indicator-level chart configs", () => { `/charts/${chartId}.patchConfig.json` ) expect(patchConfigAfterDelete).toEqual({ - $schema: - "https://files.ourworldindata.org/schemas/grapher-schema.005.json", + $schema: latestGrapherConfigSchema, id: chartId, version: 1, isPublished: false, slug: "test-chart", title: "Test chart", - type: "Marimekko", + chartTypes: ["Marimekko"], selectedEntityNames: [], dimensions: [ { diff --git a/adminSiteServer/multiDim.ts b/adminSiteServer/multiDim.ts index 82dc38e5f76..2d5d8fced82 100644 --- a/adminSiteServer/multiDim.ts +++ b/adminSiteServer/multiDim.ts @@ -1,7 +1,10 @@ import { uniq } from "lodash" import { uuidv7 } from "uuidv7" -import { migrateGrapherConfigToLatestVersion } from "@ourworldindata/grapher" +import { + defaultGrapherConfig, + migrateGrapherConfigToLatestVersion, +} from "@ourworldindata/grapher" import { Base64String, ChartConfigsTableName, @@ -290,8 +293,7 @@ export async function createMultiDimConfig( const variableId = view.indicators.y[0] // Main config for each view. const mainGrapherConfig: GrapherInterface = { - $schema: - "https://files.ourworldindata.org/schemas/grapher-schema.005.json", + $schema: defaultGrapherConfig.$schema, dimensions: MultiDimDataPageConfig.viewToDimensionsConfig(view), selectedEntityNames: config.defaultSelection ?? [], slug, 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..a4310434488 --- /dev/null +++ b/db/migration/1731431457062-AddTypesFieldToConfigs.ts @@ -0,0 +1,246 @@ +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 addChartTypesFieldToConfigs( + 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 + ) + STORED AFTER slug; + ` + ) + } + + private async removeTypeAndHasChartTabFields( + queryRunner: QueryRunner + ): Promise { + await queryRunner.query(` + -- sql + UPDATE chart_configs + SET patch = JSON_REMOVE(patch, '$.type', '$.hasChartTab') + `) + } + + private async removeDerivedTypeColumn( + queryRunner: QueryRunner + ): Promise { + await queryRunner.query( + `-- sql + ALTER TABLE chart_configs + DROP COLUMN chartType; + ` + ) + } + + private async addTypeAndHasChartTabFields( + queryRunner: QueryRunner + ): Promise { + for (const configType of ["patch", "full"]) { + await queryRunner.query( + ` + -- sql + UPDATE chart_configs + SET ?? = JSON_SET(??, '$.type', ?? ->> '$.chartTypes[0]') + WHERE ?? ->> '$.chartTypes' IS NOT NULL + `, + [configType, configType, configType, configType] + ) + await queryRunner.query( + ` + -- sql + UPDATE chart_configs + SET ?? = JSON_SET(??, '$.hasChartTab', FALSE) + WHERE + ?? ->> '$.chartTypes' IS NOT NULL + AND JSON_LENGTH(?? ->> '$.chartTypes') = 0 + `, + [configType, configType, configType, configType] + ) + } + } + + private async removeChartTypesFieldFromConfigs( + queryRunner: QueryRunner + ): Promise { + for (const configType of ["patch", "full"]) { + await queryRunner.query( + ` + -- sql + UPDATE chart_configs + SET ?? = JSON_REMOVE(??, '$.chartTypes') + `, + [configType, configType] + ) + } + } + + private async updateChartsXParentsViewToUseTypeColumn( + queryRunner: QueryRunner + ): Promise { + // same as the current definition, but uses the new `type` column + // instead of `full ->> '$.type'` + await queryRunner.query(`-- sql + ALTER VIEW charts_x_parents AS ( + WITH y_dimensions AS ( + SELECT + * + FROM + chart_dimensions + WHERE + property = 'y' + ), + single_y_indicator_charts AS ( + SELECT + c.id as chartId, + cc.patch as patchConfig, + -- should only be one + max(yd.variableId) as variableId + FROM + charts c + JOIN chart_configs cc ON cc.id = c.configId + JOIN y_dimensions yd ON c.id = yd.chartId + WHERE + -- scatter plots can't inherit settings + cc.chartType != 'ScatterPlot' + GROUP BY + c.id + HAVING + -- restrict to single y-variable charts + COUNT(distinct yd.variableId) = 1 + ) + SELECT + variableId, + chartId + FROM + single_y_indicator_charts + ORDER BY + variableId + ) + `) + } + + private async updateChartsXParentsViewToUseTypeField( + queryRunner: QueryRunner + ): Promise { + await queryRunner.query(`-- sql + ALTER VIEW charts_x_parents AS ( + WITH y_dimensions AS ( + SELECT + * + FROM + chart_dimensions + WHERE + property = 'y' + ), + single_y_indicator_charts AS ( + SELECT + c.id as chartId, + cc.patch as patchConfig, + -- should only be one + max(yd.variableId) as variableId + FROM + charts c + JOIN chart_configs cc ON cc.id = c.configId + JOIN y_dimensions yd ON c.id = yd.chartId + WHERE + -- scatter plots can't inherit settings + cc.full ->> '$.type' != 'ScatterPlot' + GROUP BY + c.id + HAVING + -- restrict to single y-variable charts + COUNT(distinct yd.variableId) = 1 + ) + SELECT + variableId, + chartId + FROM + single_y_indicator_charts + ORDER BY + variableId + ) + `) + } + + public async up(queryRunner: QueryRunner): Promise { + await this.addChartTypesFieldToConfigs(queryRunner) + await this.removeTypeAndHasChartTabFields(queryRunner) + await this.addDerivedChartTypeColumn(queryRunner) + await this.updateChartsXParentsViewToUseTypeColumn(queryRunner) + await this.updateSchema(queryRunner, "006") + } + + public async down(queryRunner: QueryRunner): Promise { + await this.updateChartsXParentsViewToUseTypeField(queryRunner) + await this.removeDerivedTypeColumn(queryRunner) + await this.addTypeAndHasChartTabFields(queryRunner) + await this.removeChartTypesFieldFromConfigs(queryRunner) + await this.updateSchema(queryRunner, "007") + } +} diff --git a/db/model/Chart.ts b/db/model/Chart.ts index 1f44e590f12..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 COALESCE(cc.full ->> "$.type", 'LineChart') = ? + 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 7dd6130a28e..dacd7ed1ebd 100644 --- a/db/model/Variable.ts +++ b/db/model/Variable.ts @@ -878,7 +878,7 @@ export async function getVariableOfDatapageIfApplicable( // showing a data page. if ( yVariableIds.length === 1 && - (grapher.type !== ChartTypeName.ScatterPlot || + (grapher.chartTypes?.[0] !== ChartTypeName.ScatterPlot || xVariableIds.length === 0) ) { const variableId = yVariableIds[0] diff --git a/devTools/schemaProcessor/columns.json b/devTools/schemaProcessor/columns.json index 75a5b3579a4..f9d3f306d59 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": "/chartTypes", + "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 5c2c93f8688..7cffdb8c34a 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 9d7e1af2e4b..748b4e333c0 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/barCharts/DiscreteBarChart.tsx b/packages/@ourworldindata/grapher/src/barCharts/DiscreteBarChart.tsx index be1fb0538b7..144ed286209 100644 --- a/packages/@ourworldindata/grapher/src/barCharts/DiscreteBarChart.tsx +++ b/packages/@ourworldindata/grapher/src/barCharts/DiscreteBarChart.tsx @@ -730,7 +730,7 @@ export class DiscreteBarChart // an all-blue color scheme (singleColorDenim). const defaultColorScheme = this.defaultBaseColorScheme const colorScheme = this.manager.baseColorScheme ?? defaultColorScheme - return this.manager.isLineChart + return this.manager.isOnLineChartTab ? ColorSchemes.get(defaultColorScheme) : ColorSchemes.get(colorScheme) } diff --git a/packages/@ourworldindata/grapher/src/barCharts/DiscreteBarChartConstants.ts b/packages/@ourworldindata/grapher/src/barCharts/DiscreteBarChartConstants.ts index d1fe2ab665d..ed08064e768 100644 --- a/packages/@ourworldindata/grapher/src/barCharts/DiscreteBarChartConstants.ts +++ b/packages/@ourworldindata/grapher/src/barCharts/DiscreteBarChartConstants.ts @@ -28,7 +28,7 @@ export interface PlacedDiscreteBarSeries extends DiscreteBarSeries { export interface DiscreteBarChartManager extends ChartManager { showYearLabels?: boolean endTime?: Time - isLineChart?: boolean + isOnLineChartTab?: boolean } export const BACKGROUND_COLOR = "#fff" diff --git a/packages/@ourworldindata/grapher/src/captionedChart/CaptionedChart.tsx b/packages/@ourworldindata/grapher/src/captionedChart/CaptionedChart.tsx index 502c56a156c..7e5b8482a13 100644 --- a/packages/@ourworldindata/grapher/src/captionedChart/CaptionedChart.tsx +++ b/packages/@ourworldindata/grapher/src/captionedChart/CaptionedChart.tsx @@ -36,9 +36,9 @@ import { EntityName, ChartTypeName, FacetStrategy, - GrapherTabOption, RelatedQuestionsConfig, Color, + GrapherTabName, } from "@ourworldindata/types" import { DataTable, DataTableManager } from "../dataTable/DataTable" import { @@ -76,11 +76,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 @@ -189,27 +189,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, containerElement } = 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 ( ) @@ -218,7 +224,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 c6285eabed2..4ba869a8132 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" @@ -120,3 +125,54 @@ export function isTargetOutsideElement( document.contains(targetNode) ) } + +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/Controls.scss b/packages/@ourworldindata/grapher/src/controls/Controls.scss index 87031973b05..b58d50d26d9 100644 --- a/packages/@ourworldindata/grapher/src/controls/Controls.scss +++ b/packages/@ourworldindata/grapher/src/controls/Controls.scss @@ -85,7 +85,7 @@ nav.controlsRow .chart-controls { } } -@container grapher (max-width:550px) { +@container grapher (max-width:575px) { // collapse the Settings toggle down to just an icon on small screens nav.controlsRow .settings-menu button.menu-toggle { min-height: $control-row-height; @@ -105,7 +105,7 @@ nav.controlsRow .chart-controls { } } -@container grapher (max-width:475px) { +@container grapher (max-width:525px) { // hide the entity name in the Edit/Select/Switch button nav.controlsRow .entity-selection-menu button.menu-toggle { label span { diff --git a/packages/@ourworldindata/grapher/src/controls/SettingsMenu.tsx b/packages/@ourworldindata/grapher/src/controls/SettingsMenu.tsx index daedbd24493..200bafc0967 100644 --- a/packages/@ourworldindata/grapher/src/controls/SettingsMenu.tsx +++ b/packages/@ourworldindata/grapher/src/controls/SettingsMenu.tsx @@ -67,7 +67,7 @@ export interface SettingsMenuManager hideTableFilterToggle?: boolean // chart state - type: ChartTypeName + activeChartType?: ChartTypeName isRelativeMode?: boolean selection?: SelectionArray | EntityName[] canChangeAddOrHighlightEntities?: boolean @@ -104,6 +104,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 } @@ -111,7 +115,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 } @@ -126,15 +130,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 ) } @@ -142,16 +146,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, @@ -160,7 +164,7 @@ export class SettingsMenu extends React.Component<{ ScatterPlot, LineChart, Marimekko, - ].includes(type) + ].includes(this.chartType) } @computed get showFacetControl(): boolean { @@ -169,7 +173,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 @@ -184,7 +187,7 @@ export class SettingsMenu extends React.Component<{ StackedBar, StackedDiscreteBar, LineChart, - ].includes(type) + ].includes(this.chartType) const hasProjection = filledDimensions.some( (dim) => dim.display.isProjection @@ -261,9 +264,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 { @@ -390,10 +392,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 a2c5f9d38c2..a01b56ab0b7 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 { @@ -25,7 +25,6 @@ export interface ControlsRowManager MapProjectionMenuManager, SettingsMenuManager { sidePanelBounds?: Bounds - availableTabs?: GrapherTabOption[] showEntitySelectionToggle?: boolean } 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 36e05f38c92..fd221f0eacd 100755 --- a/packages/@ourworldindata/grapher/src/core/Grapher.jsdom.test.ts +++ b/packages/@ourworldindata/grapher/src/core/Grapher.jsdom.test.ts @@ -9,6 +9,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 8f065684949..41574adb404 100644 --- a/packages/@ourworldindata/grapher/src/core/Grapher.tsx +++ b/packages/@ourworldindata/grapher/src/core/Grapher.tsx @@ -67,6 +67,7 @@ import { extractDetailsFromSyntax, omit, isTouchDevice, + areSetsEqual, } from "@ourworldindata/utils" import { MarkdownTextWrap, @@ -107,6 +108,8 @@ import { Color, GRAPHER_QUERY_PARAM_KEYS, GrapherTooltipAnchor, + GrapherTabName, + GrapherTabQueryParam, } from "@ourworldindata/types" import { BlankOwidTable, @@ -133,6 +136,7 @@ import { GRAPHER_FRAME_PADDING_HORIZONTAL, GRAPHER_FRAME_PADDING_VERTICAL, latestGrapherConfigSchema, + validChartTypeCombinations, } from "../core/GrapherConstants" import { loadVariableDataAndMetadata } from "./loadVariable" import Cookies from "js-cookie" @@ -194,6 +198,8 @@ import { ScatterPlotManager } from "../scatterCharts/ScatterPlotChartConstants" import { autoDetectSeriesStrategy, autoDetectYColumnSlugs, + mapChartTypeNameToQueryParam, + mapQueryParamToChartTypeName, } from "../chart/ChartUtils" import classnames from "classnames" import { GrapherAnalytics } from "./GrapherAnalytics" @@ -305,6 +311,7 @@ export interface GrapherProgrammaticInterface extends GrapherInterface { hideTableFilterToggle?: boolean forceHideAnnotationFieldsInTitle?: AnnotationFieldsInTitle hasTableTab?: boolean + hideChartTabs?: boolean hideShareButton?: boolean hideExploreTheDataButton?: boolean hideRelatedQuestion?: boolean @@ -354,7 +361,7 @@ export class Grapher SlopeChartManager { @observable.ref $schema = latestGrapherConfigSchema - @observable.ref type = ChartTypeName.LineChart + @observable.ref chartTypes = [ChartTypeName.LineChart] @observable.ref id?: number = undefined @observable.ref version = 1 @observable.ref slug?: string = undefined @@ -388,9 +395,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 @@ -613,13 +620,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 @@ -701,6 +705,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.chartType 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 | undefined { + return this.validChartTypes[0] + } + + @computed get hasChartTab(): boolean { + return this.validChartTypes.length > 0 + } + @computed get isOnChartTab(): boolean { return this.tab === GrapherTabOption.chart } @@ -728,7 +755,7 @@ export class Grapher @computed get showLegend(): boolean { // hide the legend for stacked bar charts // if the legend only ever shows a single entity - if (this.isStackedBar) { + if (this.isOnStackedBarTab) { const seriesStrategy = this.chartInstance.seriesStrategy || autoDetectSeriesStrategy(this, true) @@ -757,10 +784,9 @@ export class Grapher // Depending on the chart type, the criteria for being able to select an entity are // different; e.g. for scatterplots, the entity needs to (1) not be excluded and // (2) needs to have data for the x and y dimension. - let table = - this.isScatter || this.isSlopeChart - ? this.tableAfterAuthorTimelineAndActiveChartTransform - : this.inputTable + let table = this.isScatter + ? this.tableAfterAuthorTimelineAndActiveChartTransform + : this.inputTable if (!this.isReady) return table @@ -955,7 +981,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], ], }, @@ -1212,7 +1238,7 @@ export class Grapher @computed get isSingleTimeScatterAnimationActive(): boolean { return ( this.isTimelineAnimationActive && - this.isScatter && + this.isOnScatterTab && !this.isRelativeMode && !!this.areHandlesOnSameTimeBeforeAnimation ) @@ -1269,6 +1295,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 { @@ -1280,8 +1319,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 @@ -1483,12 +1522,37 @@ export class Grapher }) } - @computed get availableTabs(): GrapherTabOption[] { + @computed get validChartTypes(): ChartTypeName[] { + const { chartTypes } = this + + // all single-chart Graphers are valid + 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 validChartTypeSet(): Set { + return new Set(this.validChartTypes) + } + + @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 { @@ -1527,9 +1591,9 @@ export class Grapher (this.hasTimeline && // chart types that refer to the current time only in the timeline (this.isLineChartThatTurnedIntoDiscreteBar || - this.isDiscreteBar || - this.isStackedDiscreteBar || - this.isMarimekko || + this.isOnDiscreteBarTab || + this.isOnStackedDiscreteBarTab || + this.isOnMarimekkoTab || this.isOnMapTab))) ) } @@ -1539,7 +1603,7 @@ export class Grapher !this.hideAnnotationFieldsInTitle?.changeInPrefix return ( !this.forceHideAnnotationFieldsInTitle?.changeInPrefix && - this.isLineChart && + this.isOnLineChartTab && this.isRelativeMode && showChangeInPrefix ) @@ -1867,35 +1931,34 @@ export class Grapher @computed get typeExceptWhenLineChartAndSingleTimeThenWillBeBarChart(): ChartTypeName { - // 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.activeChartType ?? ChartTypeName.LineChart } @computed get isLineChart(): boolean { - return this.type === ChartTypeName.LineChart + return this.chartType === ChartTypeName.LineChart || !this.chartType } @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 { @@ -1925,6 +1988,38 @@ export class Grapher return closestMinTime !== undefined && closestMinTime === closestMaxTime } + @computed get isOnLineChartTab(): boolean { + return this.activeChartType === ChartTypeName.LineChart + } + @computed get isOnScatterTab(): boolean { + return this.activeChartType === ChartTypeName.ScatterPlot + } + @computed get isOnStackedAreaTab(): boolean { + return this.activeChartType === ChartTypeName.StackedArea + } + @computed get isOnSlopeChartTab(): boolean { + return this.activeChartType === ChartTypeName.SlopeChart + } + @computed get isOnDiscreteBarTab(): boolean { + return this.activeChartType === ChartTypeName.DiscreteBar + } + @computed get isOnStackedBarTab(): boolean { + return this.activeChartType === ChartTypeName.StackedBar + } + @computed get isOnMarimekkoTab(): boolean { + return this.activeChartType === ChartTypeName.Marimekko + } + @computed get isOnStackedDiscreteBarTab(): boolean { + return this.activeChartType === ChartTypeName.StackedDiscreteBar + } + + @computed get hasLineChart(): boolean { + return this.validChartTypeSet.has(ChartTypeName.LineChart) + } + @computed get hasSlopeChart(): boolean { + return this.validChartTypeSet.has(ChartTypeName.SlopeChart) + } + @computed get supportsMultipleYColumns(): boolean { return !(this.isScatter || this.isSlopeChart) } @@ -2037,14 +2132,14 @@ export class Grapher @computed get mapIsClickable(): boolean { return ( this.hasChartTab && - (this.isLineChart || this.isScatter) && + (this.hasLineChart || this.isScatter) && !isMobile() ) } @computed get relativeToggleLabel(): string { - if (this.isScatter) return "Display average annual change" - else if (this.isLineChart) return "Display relative change" + if (this.isOnScatterTab) return "Display average annual change" + else if (this.isOnLineChartTab) return "Display relative change" return "Display relative values" } @@ -2378,7 +2473,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 { @@ -2928,7 +3023,6 @@ export class Grapher return this.frameBounds.width <= 420 } - // SemiNarrow charts shorten their button labels to fit within the controls row @computed get isSemiNarrow(): boolean { if (this.isStatic) return false return this.frameBounds.width <= 550 @@ -3162,9 +3256,58 @@ export class Grapher debounceMode = false + private mapQueryParamToGrapherTab(tab: string): GrapherTabName | undefined { + const { + chartType: defaultChartType, + validChartTypeSet, + hasMapTab, + } = this + + if (tab === GrapherTabQueryParam.Table) { + return GrapherTabName.Table + } + if (tab === GrapherTabQueryParam.WorldMap) { + return GrapherTabName.WorldMap + } + + 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 (validChartTypeSet.has(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 @@ -3331,7 +3474,7 @@ export class Grapher } @computed get disablePlay(): boolean { - return this.isSlopeChart + return this.isOnSlopeChartTab } @computed get animationEndTime(): Time { @@ -3376,7 +3519,7 @@ export class Grapher @computed get canChangeEntity(): boolean { return ( this.hasChartTab && - !this.isScatter && + !this.isOnScatterTab && !this.canSelectMultipleEntities && this.addCountryMode === EntitySelectionMode.SingleEntity && this.numSelectableEntityNames > 1 @@ -3387,11 +3530,11 @@ export class Grapher return ( this.hasChartTab && this.canSelectMultipleEntities && - (this.isLineChart || - this.isStackedArea || - this.isStackedBar || - this.isDiscreteBar || - this.isStackedDiscreteBar) + (this.isOnLineChartTab || + this.isOnStackedAreaTab || + this.isOnStackedBarTab || + this.isOnDiscreteBarTab || + this.isOnStackedDiscreteBarTab) ) } @@ -3500,6 +3643,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 82dbc2ab89b..3921dfd04b0 100644 --- a/packages/@ourworldindata/grapher/src/core/GrapherConstants.ts +++ b/packages/@ourworldindata/grapher/src/core/GrapherConstants.ts @@ -1,4 +1,5 @@ import { defaultGrapherConfig } from "../schema/defaultGrapherConfig.js" +import { ChartTypeName } from "@ourworldindata/types" import type { GrapherProgrammaticInterface } from "./Grapher" export const GRAPHER_EMBEDDED_FIGURE_ATTR = "data-grapher-src" @@ -82,7 +83,7 @@ export enum Patterns { noDataPatternForMapChart = "noDataPatternForMapChart", } -export const grapherInterfaceWithHiddenControlsOnly: GrapherProgrammaticInterface = +export const grapherInterfaceWithHiddenControls: GrapherProgrammaticInterface = { hideRelativeToggle: true, hideTimeline: true, @@ -100,9 +101,17 @@ export const grapherInterfaceWithHiddenControlsOnly: GrapherProgrammaticInterfac }, } -export const grapherInterfaceWithHiddenTabsOnly: GrapherProgrammaticInterface = - { - hasChartTab: false, - hasMapTab: false, - hasTableTab: false, - } +export const grapherInterfaceWithHiddenTabs: GrapherProgrammaticInterface = { + hasMapTab: false, + hasTableTab: false, + hideChartTabs: true, +} + +/** + * Chart type combinations that are currently supported. + * + * This also determines the order of chart types in the UI. + */ +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 0b51f881722..81efb82e166 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, latestGrapherConfigSchema, diff --git a/packages/@ourworldindata/grapher/src/schema/defaultGrapherConfig.ts b/packages/@ourworldindata/grapher/src/schema/defaultGrapherConfig.ts index dd0f68334e2..79da7c8c82a 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", + chartTypes: ["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..67ae88895c5 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 + chartTypes: + 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 05c80c40fb0..3a7a1c23b43 100644 --- a/packages/@ourworldindata/types/src/grapherTypes/GrapherTypes.ts +++ b/packages/@ourworldindata/types/src/grapherTypes/GrapherTypes.ts @@ -172,12 +172,54 @@ export type SeriesName = string export type SeriesColorMap = Map +/** + * Grapher tab specified in the config that determines the default tab to show. + * If `chart` is selected and Grapher has more than one chart tab, then the + * first chart tab will be active. + */ export enum GrapherTabOption { chart = "chart", map = "map", table = "table", } +/** + * Internal tab names used in Grapher. + */ +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", +} + +/** + * Valid values for the `tab` query parameter in Grapher. + */ +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 interface RelatedQuestionsConfig { text: string url: string @@ -535,7 +577,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 @@ -563,7 +605,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[] @@ -654,7 +695,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", @@ -680,7 +721,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 914e651385b..347263145cf 100644 --- a/packages/@ourworldindata/types/src/index.ts +++ b/packages/@ourworldindata/types/src/index.ts @@ -75,6 +75,8 @@ export { ColorSchemeName, ChartTypeName, GrapherTabOption, + GrapherTabName, + GrapherTabQueryParam, StackMode, EntitySelectionMode, ScatterPointLabelStrategy, diff --git a/packages/@ourworldindata/utils/src/Util.ts b/packages/@ourworldindata/utils/src/Util.ts index f62bdc5bb40..ccdd684a6f8 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 ): 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..b4a239370c6 100644 --- a/site/gdocs/components/Chart.tsx +++ b/site/gdocs/components/Chart.tsx @@ -1,17 +1,17 @@ import React, { useRef } from "react" import { useEmbedChart } from "../../hooks.js" import { - grapherInterfaceWithHiddenControlsOnly, - grapherInterfaceWithHiddenTabsOnly, + grapherInterfaceWithHiddenControls, + grapherInterfaceWithHiddenTabs, GrapherProgrammaticInterface, } from "@ourworldindata/grapher" import { ChartControlKeyword, ChartTabKeyword, EnrichedBlockChart, - identity, Url, merge, + excludeUndefined, } from "@ourworldindata/utils" import { ChartConfigType } from "@ourworldindata/types" import { renderSpans, useLinkedChart } from "../utils.js" @@ -54,17 +54,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 +143,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 +180,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(