Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🎉 (grapher) support multiple chart types / TAS-694 #4159

Merged
merged 1 commit into from
Nov 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions adminSiteClient/BulkGrapherConfigEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ const bulkChartEditorColumnSets: ColumnSet[] = [
label: "Common",
kind: "specificColumns",
columns: [
"/type",
"/chartTypes",
"/hasMapTab",
"/title",
"/subtitle",
Expand Down Expand Up @@ -110,9 +110,8 @@ const bulkChartEditorColumnSets: ColumnSet[] = [
"/baseColorScheme",
"/map/colorScale",
"/colorScale",
"/hasChartTab",
"/hasMapTab",
"/type",
"/chartTypes",
],
},
]
Expand Down
20 changes: 14 additions & 6 deletions adminSiteClient/ChartList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@ import { observer } from "mobx-react"
import { runInAction, observable } from "mobx"
import { bind } from "decko"
import { AdminAppContext, AdminAppContextType } from "./AdminAppContext.js"
import { ChartTypeName, GrapherInterface } from "@ourworldindata/types"
import {
ChartTypeName,
GrapherInterface,
GrapherTabOption,
} from "@ourworldindata/types"
import { startCase, DbChartTagJoin } from "@ourworldindata/utils"
import { References, getFullReferencesCount } from "./ChartEditor.js"
import { ChartRow } from "./ChartRow.js"
Expand All @@ -14,14 +18,15 @@ export interface ChartListItem {
id: GrapherInterface["id"]
title: GrapherInterface["title"]
slug: GrapherInterface["slug"]
type: GrapherInterface["type"]
internalNotes: GrapherInterface["internalNotes"]
variantName: GrapherInterface["variantName"]
isPublished: GrapherInterface["isPublished"]
tab: GrapherInterface["tab"]
hasChartTab: GrapherInterface["hasChartTab"]
hasMapTab: GrapherInterface["hasMapTab"]

type?: ChartTypeName
hasChartTab: boolean

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

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

if (!chartType) return "Map"

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

if (chart.tab === "map") {
if (chart.tab === GrapherTabOption.map) {
if (chart.hasChartTab) return `Map + ${displayType}`
else return "Map"
} else {
Expand Down
53 changes: 35 additions & 18 deletions adminSiteClient/EditorBasicTab.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,10 @@ class DimensionSlotView<
() => this.grapher.isReady,
() => {
this.disposers.push(
reaction(() => this.grapher.type, this.updateDefaults),
reaction(
() => this.grapher.validChartTypes,
this.updateDefaults
),
reaction(
() => this.grapher.yColumnsFromDimensions.length,
this.updateDefaults
Expand Down Expand Up @@ -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)) {
Expand All @@ -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
Expand Down Expand Up @@ -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 (
<div className="EditorBasicTab">
{isIndicatorChart && <IndicatorChartInfo editor={editor} />}

<Section name="Type of chart">
<Section name="Tabs">
<SelectField
value={grapher.type}
label="Type of chart"
value={grapher.chartType ?? this.chartTypeOptionNone}
onValue={this.onChartTypeChange}
options={chartTypes.map((key) => ({
value: key,
label: startCase(key),
}))}
options={this.chartTypeOptions}
/>
<FieldsRow>
<Toggle
label="Chart tab"
value={grapher.hasChartTab}
onValue={(value) => (grapher.hasChartTab = value)}
/>
<Toggle
label="Map tab"
value={grapher.hasMapTab}
onValue={(value) => (grapher.hasMapTab = value)}
onValue={(shouldHaveMapTab) =>
(grapher.hasMapTab = shouldHaveMapTab)
}
/>
</FieldsRow>
</Section>
Expand Down
8 changes: 6 additions & 2 deletions adminSiteClient/EditorCustomizeTab.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
ColorSchemeName,
FacetAxisDomain,
FacetStrategy,
ChartTypeName,
} from "@ourworldindata/types"
import { Grapher } from "@ourworldindata/grapher"
import {
Expand Down Expand Up @@ -158,7 +159,10 @@ export class ColorSchemeSelector extends React.Component<{
value={grapher.baseColorScheme}
onChange={this.onChange}
onBlur={this.onBlur}
chartType={this.props.grapher.type}
chartType={
this.props.grapher.chartType ??
ChartTypeName.LineChart
}
invertedColorScheme={!!grapher.invertColorScheme}
additionalOptions={[
{
Expand Down Expand Up @@ -751,7 +755,7 @@ export class EditorCustomizeTab<
{grapher.chartInstanceExceptMap.colorScale && (
<EditorColorScaleSection
scale={grapher.chartInstanceExceptMap.colorScale}
chartType={grapher.type}
chartType={grapher.chartType ?? ChartTypeName.LineChart}
showLineChartColors={grapher.isLineChart}
features={{
visualScaling: true,
Expand Down
6 changes: 4 additions & 2 deletions adminSiteClient/GrapherConfigGridEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -512,7 +512,9 @@ export class GrapherConfigGridEditor extends React.Component<GrapherConfigGridEd
return (
<EditorColorScaleSection
scale={colorScale}
chartType={grapher.type}
chartType={
grapher.chartType ?? ChartTypeName.LineChart
}
features={{
visualScaling: true,
legendDescription: false,
Expand Down Expand Up @@ -1096,7 +1098,7 @@ export class GrapherConfigGridEditor extends React.Component<GrapherConfigGridEd

async getFieldDefinitions() {
const json = await fetch(
"https://files.ourworldindata.org/schemas/grapher-schema.005.json"
"https://files.ourworldindata.org/schemas/grapher-schema.006.json"
).then((response) => response.json())
const fieldDescriptions = extractFieldDescriptionsFromSchema(json)
runInAction(() => {
Expand Down
5 changes: 2 additions & 3 deletions adminSiteClient/VariablesAnnotationPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ const variableAnnotationsColumnSets: ColumnSet[] = [
columns: [
"name",
"datasetname",
"/type",
"/chartTypes",
"/hasMapTab",
"/title",
"/subtitle",
Expand Down Expand Up @@ -126,9 +126,8 @@ const variableAnnotationsColumnSets: ColumnSet[] = [
"/baseColorScheme",
"/map/colorScale",
"/colorScale",
"/hasChartTab",
"/hasMapTab",
"/type",
"/chartTypes",
],
},
]
Expand Down
4 changes: 2 additions & 2 deletions adminSiteServer/apiRouter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -727,12 +727,12 @@ getRouteWithROTransaction(apiRouter, "/charts.csv", async (req, res, trx) => {
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,
Expand Down
43 changes: 17 additions & 26 deletions adminSiteServer/app.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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", () => {
Expand Down Expand Up @@ -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",
}
Expand All @@ -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: [
Expand All @@ -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",
Expand Down Expand Up @@ -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" }],
Expand All @@ -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
Expand Down Expand Up @@ -671,31 +664,29 @@ 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,
dimensions: [{ property: "y", variableId: 1 }],
selectedEntityNames: [],
slug: "test-chart",
title: "Test chart",
type: "Marimekko",
chartTypes: ["Marimekko"],
})

// fetch the patch config and verify it's diffed correctly
const patchConfigAfterDelete = await fetchJsonFromAdminApi(
`/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: [
{
Expand Down
Loading