Merge pull request #2794 from owid/refactor-grapher-core-column-def

Sources modal: simplify CoreColumnDef
owid · Oct 27, 2023 · 5081d4b · 5081d4b
2 parents bb6300d + 4022482
commit 5081d4b
Show file tree

Hide file tree

Showing 7 changed files with 37 additions and 40 deletions.
diff --git a/packages/@ourworldindata/core-table/src/CoreColumnDef.ts b/packages/@ourworldindata/core-table/src/CoreColumnDef.ts
@@ -3,6 +3,8 @@ import {
     OwidVariableDisplayConfigInterface,
     ToleranceStrategy,
     OwidOrigin,
+    OwidSource,
+    OwidVariablePresentation,
 } from "@ourworldindata/utils"
 import { CoreValueType, Color } from "./CoreTableConstants.js"
 
@@ -62,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
@@ -76,16 +75,12 @@ 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
-    attribution?: string
+    source?: OwidSource
     timespanFromMetadata?: string
 
+    // Metadata v2
     origins?: OwidOrigin[]
+    presentation?: OwidVariablePresentation
 
     // Dataset information
     datasetId?: number

diff --git a/packages/@ourworldindata/core-table/src/CoreTableColumns.ts b/packages/@ourworldindata/core-table/src/CoreTableColumns.ts
@@ -409,15 +409,7 @@ export abstract class AbstractCoreColumn<JS_TYPE extends PrimitiveType> {
     }
 
     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/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
@@ -2092,6 +2093,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(),

diff --git 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
@@ -588,20 +586,10 @@ 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,
-        titleVariant,
-        attributionShort,
+        presentation,
         owidVariableId: variable.id,
         type: isContinent
             ? ColumnTypeNames.Continent

diff --git 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<DownloadModalProps> {
         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
@@ -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 (
             <div key={slug} className="datasource-wrapper">
                 <h2>
@@ -239,11 +245,11 @@ export class SourcesModal extends React.Component<{
                                 </td>
                             </tr>
                         ) : null}
-                        {source.link ? (
+                        {sourceLink ? (
                             <tr>
                                 <td>Link</td>
                                 <td>
-                                    <HtmlOrMarkdownText text={source.link} />
+                                    <HtmlOrMarkdownText text={sourceLink} />
                                 </td>
                             </tr>
                         ) : null}

diff --git 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.