From 986f7465e7f24ae7da3aa94aec62000f6ddb0d59 Mon Sep 17 00:00:00 2001 From: sophiamersmann Date: Mon, 16 Oct 2023 13:40:01 +0000 Subject: [PATCH 1/6] refactor(grapher): simplify CoreColumnDef --- .../@ourworldindata/core-table/src/CoreColumnDef.ts | 9 +++------ .../@ourworldindata/core-table/src/CoreTableColumns.ts | 10 +--------- .../grapher/src/core/LegacyToOwidTable.ts | 9 +-------- .../grapher/src/modal/DownloadModal.tsx | 8 +++++++- .../@ourworldindata/grapher/src/modal/SourcesModal.tsx | 10 ++++++++-- 5 files changed, 20 insertions(+), 26 deletions(-) diff --git a/packages/@ourworldindata/core-table/src/CoreColumnDef.ts b/packages/@ourworldindata/core-table/src/CoreColumnDef.ts index 8e15ede4c92..ccccb0a21f5 100644 --- a/packages/@ourworldindata/core-table/src/CoreColumnDef.ts +++ b/packages/@ourworldindata/core-table/src/CoreColumnDef.ts @@ -3,6 +3,7 @@ import { OwidVariableDisplayConfigInterface, ToleranceStrategy, OwidOrigin, + OwidSource, } from "@ourworldindata/utils" import { CoreValueType, Color } from "./CoreTableConstants.js" @@ -76,15 +77,11 @@ export interface CoreColumnDef extends ColumnColorScale { color?: Color // A column can have a fixed color for use in charts where the columns are series // Source information used for display only - sourceName?: string - sourceLink?: string - dataPublishedBy?: string - dataPublisherSource?: string - retrievedDate?: string - additionalInfo?: string + source?: OwidSource attribution?: string timespanFromMetadata?: string + // Origins origins?: OwidOrigin[] // Dataset information diff --git a/packages/@ourworldindata/core-table/src/CoreTableColumns.ts b/packages/@ourworldindata/core-table/src/CoreTableColumns.ts index 87fa17cf7ed..53954bc640f 100644 --- a/packages/@ourworldindata/core-table/src/CoreTableColumns.ts +++ b/packages/@ourworldindata/core-table/src/CoreTableColumns.ts @@ -409,15 +409,7 @@ export abstract class AbstractCoreColumn { } get source(): OwidSource { - const { def } = this - return { - name: def.sourceName, - link: def.sourceLink, - dataPublishedBy: def.dataPublishedBy, - dataPublisherSource: def.dataPublisherSource, - retrievedDate: def.retrievedDate, - additionalInfo: def.additionalInfo, - } + return this.def.source ?? {} } // todo: remove. should not be on coretable diff --git a/packages/@ourworldindata/grapher/src/core/LegacyToOwidTable.ts b/packages/@ourworldindata/grapher/src/core/LegacyToOwidTable.ts index 8c17ba8b8eb..066bae2144e 100644 --- a/packages/@ourworldindata/grapher/src/core/LegacyToOwidTable.ts +++ b/packages/@ourworldindata/grapher/src/core/LegacyToOwidTable.ts @@ -588,15 +588,8 @@ const columnDefFromOwidVariable = ( datasetName, display, nonRedistributable, - sourceLink: - source?.link ?? - (origins && origins.length > 0 ? origins[0].urlMain : undefined), - sourceName: source?.name, - dataPublishedBy: source?.dataPublishedBy, - dataPublisherSource: source?.dataPublisherSource, + source, timespanFromMetadata: timespan, - retrievedDate: source?.retrievedDate, - additionalInfo: source?.additionalInfo, origins, attribution, titlePublic, diff --git a/packages/@ourworldindata/grapher/src/modal/DownloadModal.tsx b/packages/@ourworldindata/grapher/src/modal/DownloadModal.tsx index 38c38a0a487..17ef44cda73 100644 --- a/packages/@ourworldindata/grapher/src/modal/DownloadModal.tsx +++ b/packages/@ourworldindata/grapher/src/modal/DownloadModal.tsx @@ -163,7 +163,13 @@ export class DownloadModal extends React.Component { const def = this.nonRedistributableColumn?.def as | OwidColumnDef | undefined - return def?.sourceLink + if (!def) return undefined + return ( + def.source?.link ?? + (def.origins && def.origins.length > 0 + ? def.origins[0].urlMain + : undefined) + ) } @action.bound private onPngDownload(): void { diff --git a/packages/@ourworldindata/grapher/src/modal/SourcesModal.tsx b/packages/@ourworldindata/grapher/src/modal/SourcesModal.tsx index 5f7ddc0070b..3c7f4fffadc 100644 --- a/packages/@ourworldindata/grapher/src/modal/SourcesModal.tsx +++ b/packages/@ourworldindata/grapher/src/modal/SourcesModal.tsx @@ -106,6 +106,12 @@ export class SourcesModal extends React.Component<{ ...citationFull, ] + const sourceLink = + source?.link ?? + (column.def.origins && column.def.origins.length > 0 + ? column.def.origins[0].urlMain + : undefined) + return (

@@ -239,11 +245,11 @@ export class SourcesModal extends React.Component<{ ) : null} - {source.link ? ( + {sourceLink ? ( Link - + ) : null} From 63fbfdfd1f01949d2b82e03c4ebda059ef26de5a Mon Sep 17 00:00:00 2001 From: sophiamersmann Date: Mon, 16 Oct 2023 13:58:53 +0000 Subject: [PATCH 2/6] enhance(data-page): enable keyboard shortcuts --- site/GrapherFigureView.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/site/GrapherFigureView.tsx b/site/GrapherFigureView.tsx index fa9e9075320..c29ea3a0653 100644 --- a/site/GrapherFigureView.tsx +++ b/site/GrapherFigureView.tsx @@ -45,6 +45,7 @@ export class GrapherFigureView extends React.Component<{ grapher: Grapher }> { dataApiUrl: DATA_API_URL, dataApiUrlForAdmin: this.context?.admin?.settings?.DATA_API_FOR_ADMIN_UI, // passed this way because clientSettings are baked and need a recompile to be updated + enableKeyboardShortcuts: true, } return ( // They key= in here makes it so that the chart is re-loaded when the slug changes. From 40a4e0bc07e2187ac99e7e9fe5306d4650e58692 Mon Sep 17 00:00:00 2001 From: sophiamersmann Date: Mon, 16 Oct 2023 13:59:45 +0000 Subject: [PATCH 3/6] enhance(grapher): add keyboard shortcut to open the sources modal --- packages/@ourworldindata/grapher/src/core/Grapher.tsx | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/@ourworldindata/grapher/src/core/Grapher.tsx b/packages/@ourworldindata/grapher/src/core/Grapher.tsx index 8111bc4f5f3..f7ed8158aca 100644 --- a/packages/@ourworldindata/grapher/src/core/Grapher.tsx +++ b/packages/@ourworldindata/grapher/src/core/Grapher.tsx @@ -2092,6 +2092,14 @@ export class Grapher title: `Toggle full-screen mode`, category: "Chart", }, + { + combo: "s", + fn: (): void => { + this.isSourcesModalOpen = !this.isSourcesModalOpen + }, + title: `Toggle sources modal`, + category: "Chart", + }, { combo: "esc", fn: (): void => this.clearErrors(), From daa807c5e3660f4ab807ebccc0449937b0cbb6e7 Mon Sep 17 00:00:00 2001 From: sophiamersmann Date: Tue, 17 Oct 2023 06:53:54 +0000 Subject: [PATCH 4/6] refactor(grapher): add presentation property to CoreColumnDef --- packages/@ourworldindata/core-table/src/CoreColumnDef.ts | 8 +++----- packages/@ourworldindata/grapher/src/core/Grapher.tsx | 7 ++++--- .../grapher/src/core/LegacyToOwidTable.ts | 9 ++------- .../@ourworldindata/grapher/src/modal/SourcesModal.tsx | 2 +- 4 files changed, 10 insertions(+), 16 deletions(-) diff --git a/packages/@ourworldindata/core-table/src/CoreColumnDef.ts b/packages/@ourworldindata/core-table/src/CoreColumnDef.ts index ccccb0a21f5..dcd2570352c 100644 --- a/packages/@ourworldindata/core-table/src/CoreColumnDef.ts +++ b/packages/@ourworldindata/core-table/src/CoreColumnDef.ts @@ -4,6 +4,7 @@ import { ToleranceStrategy, OwidOrigin, OwidSource, + OwidVariablePresentation, } from "@ourworldindata/utils" import { CoreValueType, Color } from "./CoreTableConstants.js" @@ -63,9 +64,6 @@ export interface CoreColumnDef extends ColumnColorScale { // Column information used for display only name?: string // The display name for the column - titlePublic?: string // The Metadata V2 display title for the variable - titleVariant?: string // The Metadata V2 title disambiguation fragment for the variant (e.g. "projected") - attributionShort?: string // The Metadata V2 title disambiguation fragment for the producer description?: string descriptionShort?: string descriptionProcessing?: string @@ -78,11 +76,11 @@ export interface CoreColumnDef extends ColumnColorScale { // Source information used for display only source?: OwidSource - attribution?: string timespanFromMetadata?: string - // Origins + // Metadata v2 origins?: OwidOrigin[] + presentation?: Omit // Dataset information datasetId?: number diff --git a/packages/@ourworldindata/grapher/src/core/Grapher.tsx b/packages/@ourworldindata/grapher/src/core/Grapher.tsx index f7ed8158aca..529b9f86da1 100644 --- a/packages/@ourworldindata/grapher/src/core/Grapher.tsx +++ b/packages/@ourworldindata/grapher/src/core/Grapher.tsx @@ -1516,14 +1516,15 @@ export class Grapher @computed private get defaultSourcesLine(): string { const attributions = this.columnsWithSourcesCondensed.flatMap( (column) => { + const { presentation = {} } = column.def // if the variable metadata specifies an attribution on the // variable level then this is preferred over assembling it from // the source and origins if ( - column.def.attribution !== undefined && - column.def.attribution !== "" + presentation.attribution !== undefined && + presentation.attribution !== "" ) - return [column.def.attribution] + return [presentation.attribution] else { const originFragments = getOriginAttributionFragments( column.def.origins diff --git a/packages/@ourworldindata/grapher/src/core/LegacyToOwidTable.ts b/packages/@ourworldindata/grapher/src/core/LegacyToOwidTable.ts index 066bae2144e..1e39b54b6e6 100644 --- a/packages/@ourworldindata/grapher/src/core/LegacyToOwidTable.ts +++ b/packages/@ourworldindata/grapher/src/core/LegacyToOwidTable.ts @@ -563,11 +563,9 @@ const columnDefFromOwidVariable = ( display, timespan, nonRedistributable, + presentation, } = variable - const { attribution, titlePublic, titleVariant, attributionShort } = - variable.presentation || {} - // Without this the much used var 123 appears as "Countries Continent". We could rename in Grapher but not sure the effects of that. const isContinent = variable.id === 123 const name = isContinent ? "Continent" : variable.name @@ -591,10 +589,7 @@ const columnDefFromOwidVariable = ( source, timespanFromMetadata: timespan, origins, - attribution, - titlePublic, - titleVariant, - attributionShort, + presentation, owidVariableId: variable.id, type: isContinent ? ColumnTypeNames.Continent diff --git a/packages/@ourworldindata/grapher/src/modal/SourcesModal.tsx b/packages/@ourworldindata/grapher/src/modal/SourcesModal.tsx index 3c7f4fffadc..d19a2f918ad 100644 --- a/packages/@ourworldindata/grapher/src/modal/SourcesModal.tsx +++ b/packages/@ourworldindata/grapher/src/modal/SourcesModal.tsx @@ -63,7 +63,7 @@ export class SourcesModal extends React.Component<{ // made when the CoreColumn is filled from the Variable metadata // in columnDefFromOwidVariable in packages/@ourworldindata/grapher/src/core/LegacyToOwidTable.ts const { slug, source, def } = column - const { datasetId, coverage } = def as OwidColumnDef + const { datasetId, coverage, presentation } = def as OwidColumnDef // there will not be a datasetId for explorers that define the FASTT in TSV const editUrl = From a2e6fc55bd5f08acbd104f6136193abcf00fa5be Mon Sep 17 00:00:00 2001 From: sophiamersmann Date: Wed, 25 Oct 2023 15:08:01 +0000 Subject: [PATCH 5/6] chore: make eslint happy --- packages/@ourworldindata/grapher/src/modal/SourcesModal.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@ourworldindata/grapher/src/modal/SourcesModal.tsx b/packages/@ourworldindata/grapher/src/modal/SourcesModal.tsx index d19a2f918ad..3c7f4fffadc 100644 --- a/packages/@ourworldindata/grapher/src/modal/SourcesModal.tsx +++ b/packages/@ourworldindata/grapher/src/modal/SourcesModal.tsx @@ -63,7 +63,7 @@ export class SourcesModal extends React.Component<{ // made when the CoreColumn is filled from the Variable metadata // in columnDefFromOwidVariable in packages/@ourworldindata/grapher/src/core/LegacyToOwidTable.ts const { slug, source, def } = column - const { datasetId, coverage, presentation } = def as OwidColumnDef + const { datasetId, coverage } = def as OwidColumnDef // there will not be a datasetId for explorers that define the FASTT in TSV const editUrl = From 402248244c3b25c04e0f7d3ede39b7858da68239 Mon Sep 17 00:00:00 2001 From: sophiamersmann Date: Fri, 27 Oct 2023 10:12:24 +0000 Subject: [PATCH 6/6] refactor(core-column): allow access to all presentation properties --- packages/@ourworldindata/core-table/src/CoreColumnDef.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@ourworldindata/core-table/src/CoreColumnDef.ts b/packages/@ourworldindata/core-table/src/CoreColumnDef.ts index dcd2570352c..f0b5ffe6aeb 100644 --- a/packages/@ourworldindata/core-table/src/CoreColumnDef.ts +++ b/packages/@ourworldindata/core-table/src/CoreColumnDef.ts @@ -80,7 +80,7 @@ export interface CoreColumnDef extends ColumnColorScale { // Metadata v2 origins?: OwidOrigin[] - presentation?: Omit + presentation?: OwidVariablePresentation // Dataset information datasetId?: number