Skip to content

Commit

Permalink
🚧 use list for available tabs
Browse files Browse the repository at this point in the history
  • Loading branch information
@sophiamersmann
sophiamersmann committed Nov 12, 2024
1 parent ef4a4ef commit f5bd209
Show file tree
Hide file tree
Showing 22 changed files with 184 additions and 227 deletions.
46 changes: 29 additions & 17 deletions adminSiteClient/EditorBasicTab.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import {
CONTINENTS_INDICATOR_ID,
POPULATION_INDICATOR_ID_USED_IN_ADMIN,
allChartTypes,
allChartTypesDisabled,
} from "@ourworldindata/grapher"
import {
DimensionProperty,
Expand Down Expand Up @@ -363,11 +362,11 @@ export class EditorBasicTab<
const { grapher } = this.props.editor

const newChartType = value as ChartTypeName
grapher.availableTabs = {
...grapher.availableTabs,
...allChartTypesDisabled,
[newChartType]: true,
}
const newTabs: ChartTypeName[] = (grapher.availableTabs ?? []).filter(
(tab) => tab !== ChartTypeName.WorldMap
)
newTabs.push(newChartType)
grapher.availableTabs = newTabs

if (
grapher.tab !== GrapherTabOption.Table &&
Expand Down Expand Up @@ -435,28 +434,41 @@ export class EditorBasicTab<
label="Chart tab"
value={grapher.hasChartTab}
onValue={(value) => {
// TODO: remove false?
if (value) {
// add line chart tab
grapher.availableTabs =
grapher.availableTabs ?? {}
grapher.availableTabs[
grapher.availableTabs ?? []
grapher.availableTabs.push(
ChartTypeName.LineChart
] = true
)
} else {
grapher.availableTabs = {
...grapher.availableTabs,
...allChartTypesDisabled,
}
// remove all chart tabs
grapher.availableTabs =
grapher.availableTabs?.filter(
(tab) =>
tab === ChartTypeName.WorldMap
)
}
}}
/>
<Toggle
label="Map tab"
value={grapher.hasMapTab}
onValue={(value) => {
grapher.availableTabs =
grapher.availableTabs ?? {}
grapher.availableTabs.WorldMap = 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
)
}
}}
/>
</FieldsRow>
Expand Down
17 changes: 8 additions & 9 deletions adminSiteClient/VariableEditPage.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -688,23 +688,22 @@ class VariableEditor extends React.Component<{
@computed private get grapherConfig(): GrapherInterface {
const { variable } = this.props
const grapherConfig = variable.grapherConfig
if (grapherConfig)
if (grapherConfig) {
let { availableTabs = [] } = grapherConfig
if (!availableTabs.includes(ChartTypeName.WorldMap)) {
availableTabs = [ChartTypeName.WorldMap, ...availableTabs]
}
return {
...grapherConfig,
availableTabs: {
...grapherConfig.availableTabs,
[ChartTypeName.WorldMap]: true,
},
availableTabs,
tab: GrapherTabOption.WorldMap,
}
else
} else
return {
yAxis: { min: 0 },
map: { columnSlug: this.props.variable.id.toString() },
tab: GrapherTabOption.WorldMap,
availableTabs: {
[ChartTypeName.WorldMap]: true,
},
availableTabs: [ChartTypeName.WorldMap],
dimensions: [
{
property: DimensionProperty.y,
Expand Down
50 changes: 20 additions & 30 deletions db/migration/1731316808331-AddAvailableTabsToGrapherConfigs.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,26 +24,20 @@ export class AddAvailableTabsToGrapherConfigs1731316808331
): Promise<void> {
// CASES:
// 1. hasMapTab=false, hasChartTab=false -> availableTabs is unset
// 2. hasMapTab=false, hasChartTab=true -> availableTabs={ [chartType]: true }
// 3. hasMapTab=true, hasChartTab=false -> availableTabs={ WorldMap: true }
// 4. hasMapTab=true, hasChartTab=true -> availableTabs={ WorldMap: true, [chartType]: true }
// 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"]) {
// We could run a single query that updates all configs at once,
// but then some values in `availableTabs` would be set to 'false'.
// That shouldn't be a problem (and over time we'll probably see
// configs where chart types are set to false), but for now I like
// to keep it clean.

// CASE 2: hasMapTab=false, hasChartTab=true -> availableTabs={ [chartType]: true }
// CASE 2: hasMapTab=false, hasChartTab=true -> availableTabs=[chartType]
await queryRunner.query(
`
-- sql
UPDATE chart_configs
SET ?? = JSON_SET(
??,
'$.availableTabs',
JSON_OBJECT(COALESCE(?? ->> '$.type', 'LineChart'), TRUE)
JSON_ARRAY(COALESCE(?? ->> '$.type', 'LineChart'))
)
WHERE
(?? ->> '$.hasMapTab' = 'false' OR ?? ->> '$.hasMapTab' IS NULL)
Expand All @@ -60,15 +54,15 @@ export class AddAvailableTabsToGrapherConfigs1731316808331
]
)

// CASE 3: hasMapTab=true, hasChartTab=false -> availableTabs={ WorldMap: true }
// CASE 3: hasMapTab=true, hasChartTab=false -> availableTabs=["WorldMap"]
await queryRunner.query(
`
-- sql
UPDATE chart_configs
SET ?? = JSON_SET(
??,
'$.availableTabs',
JSON_OBJECT('WorldMap', TRUE)
JSON_ARRAY('WorldMap')
)
WHERE
(?? ->> '$.hasMapTab' = 'true')
Expand All @@ -77,18 +71,15 @@ export class AddAvailableTabsToGrapherConfigs1731316808331
[configType, configType, configType, configType]
)

// CASE 4: hasMapTab=true, hasChartTab=true -> availableTabs={ WorldMap: true, [chartType]: true }
// CASE 4: hasMapTab=true, hasChartTab=true -> availableTabs=["WorldMap", chartType]
await queryRunner.query(
`
-- sql
UPDATE chart_configs
SET ?? = JSON_SET(
??,
??,
'$.availableTabs',
JSON_OBJECT(
'WorldMap', TRUE,
COALESCE(?? ->> '$.type', 'LineChart'), TRUE
)
JSON_ARRAY('WorldMap', COALESCE(?? ->> '$.type', 'LineChart'))
)
WHERE
(?? ->> '$.hasMapTab' = 'true')
Expand Down Expand Up @@ -168,17 +159,16 @@ export class AddAvailableTabsToGrapherConfigs1731316808331
ADD COLUMN chartType VARCHAR(255) GENERATED ALWAYS AS
(
CASE
WHEN full ->> '$.availableTabs' IS NULL OR JSON_LENGTH(full, '$.availableTabs') = 0 THEN NULL
-- Graphers with a line and a slope chart are considered to be of type LineChart
WHEN full ->> '$.availableTabs.LineChart' = 'true' THEN 'LineChart'
WHEN full ->> '$.availableTabs.SlopeChart' = 'true' THEN 'SlopeChart'
WHEN full ->> '$.availableTabs.ScatterPlot' = 'true' THEN 'ScatterPlot'
WHEN full ->> '$.availableTabs.StackedArea' = 'true' THEN 'StackedArea'
WHEN full ->> '$.availableTabs.StackedBar' = 'true' THEN 'StackedBar'
WHEN full ->> '$.availableTabs.ScatterPlot' = 'true' THEN 'ScatterPlot'
WHEN full ->> '$.availableTabs.DiscreteBar' = 'true' THEN 'DiscreteBar'
WHEN full ->> '$.availableTabs.StackedDiscreteBar' = 'true' THEN 'StackedDiscreteBar'
WHEN full ->> '$.availableTabs.Marimekko' = 'true' THEN 'Marimekko'
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
)
Expand All @@ -202,7 +192,7 @@ export class AddAvailableTabsToGrapherConfigs1731316808331
await this.addAvailableTabsToGrapherConfigs(queryRunner)
await this.updateTabField(queryRunner)
await this.removeTypeHasMapTabAndHasChartTabFields(queryRunner)
await this.addDerivedChartTypeColumn(queryRunner)
// await this.addDerivedChartTypeColumn(queryRunner)
await this.updateSchema(
queryRunner,
"https://files.ourworldindata.org/schemas/grapher-schema.007.json"
Expand Down
4 changes: 2 additions & 2 deletions packages/@ourworldindata/explorer/src/Explorer.sample.tsx
View file
Original file line number Diff line number Diff line change
@@ -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"

Expand Down Expand Up @@ -54,7 +54,7 @@ export const SampleExplorerOfGraphers = (props?: Partial<ExplorerProps>) => {
property: DimensionProperty.y,
},
],
// TODO
availableTabs: [ChartTypeName.LineChart],
tab: GrapherTabOption.LineChart,
owidDataset: new Map([
[
Expand Down
13 changes: 13 additions & 0 deletions packages/@ourworldindata/explorer/src/ExplorerProgram.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -409,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
}

Expand Down
9 changes: 3 additions & 6 deletions packages/@ourworldindata/explorer/src/GrapherGrammar.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import {
IndicatorIdOrEtlPathCellDef,
GrapherCellDef,
} from "./gridLang/GridLangConstants.js"
import { allChartTypesDisabled } from "@ourworldindata/grapher"

const toTerminalOptions = (keywords: string[]): CellDef[] => {
return keywords.map((keyword) => ({
Expand Down Expand Up @@ -66,7 +65,7 @@ export const GrapherGrammar: Grammar<GrapherCellDef> = {
keyword: "type",
description: `The type of chart to show such as LineChart or ScatterPlot.`,
terminalOptions: toTerminalOptions(Object.values(ChartTypeName)),
toGrapherObject: (value) => ({ availableTabs: { [value]: true } }),
toGrapherObject: () => ({}), // handled in code
},
grapherId: {
...IntegerCellDef,
Expand All @@ -85,9 +84,7 @@ export const GrapherGrammar: Grammar<GrapherCellDef> = {
...BooleanCellDef,
keyword: "hasMapTab",
description: "Show the map tab?",
toGrapherObject: (value) => ({
availableTabs: { [ChartTypeName.WorldMap]: value },
}),
toGrapherObject: () => ({}), // handled in code
},
tab: {
...EnumCellDef,
Expand All @@ -101,7 +98,7 @@ export const GrapherGrammar: Grammar<GrapherCellDef> = {
keyword: "hasChartTab",
description: "Show the chart tab?",
// overwrites the given chart type if provided
toGrapherObject: (value) => ({ availableTabs: allChartTypesDisabled }),
toGrapherObject: () => ({}), // handled in code
},
xSlug: {
...SlugDeclarationCellDef,
Expand Down
11 changes: 0 additions & 11 deletions packages/@ourworldindata/grapher/src/chart/ChartUtils.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,14 +113,3 @@ export function getShortNameForEntity(entityName: string): string | undefined {
const country = getCountryByName(entityName)
return country?.shortName
}

export function getTabPosition(tab: ChartTypeName): number {
switch (tab) {
case ChartTypeName.WorldMap:
return 1
case ChartTypeName.LineChart:
return 2
default:
return 3
}
}
Loading

0 comments on commit f5bd209

Please sign in to comment.