Skip to content

Commit

Permalink
Merge pull request #3016 from owid/change-display-name-vs-title-publi…
Browse files Browse the repository at this point in the history
…c-fallbacks

🔨 change fallback chain for display.name and titlePublic
  • Loading branch information
danyx23 authored Dec 21, 2023
2 parents b872b8a + ec5ae70 commit 107ebe4
Show file tree
Hide file tree
Showing 13 changed files with 159 additions and 36 deletions.
2 changes: 1 addition & 1 deletion baker/GrapherBaker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ export async function renderDataPageV2({
slug = await getSlugForTopicTag(firstTopicTag)
} catch (error) {
logErrorAndMaybeSendToBugsnag(
`Datapage with variableId "${variableId}" and title "${datapageData.title}" is using "${firstTopicTag}" as its primary tag, which we are unable to resolve to a tag in the grapher DB`
`Datapage with variableId "${variableId}" and title "${datapageData.title.title}" is using "${firstTopicTag}" as its primary tag, which we are unable to resolve to a tag in the grapher DB`
)
}
let gdoc: GdocPost | null = null
Expand Down
21 changes: 15 additions & 6 deletions datapage/Datapage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
getAttributionFragmentsFromVariable,
getLastUpdatedFromVariable,
getNextUpdateFromVariable,
omitUndefinedValues,
} from "@ourworldindata/utils"
import { ExplorerProgram } from "../explorer/ExplorerProgram.js"
import { GdocPost } from "../db/model/Gdoc/GdocPost.js"
Expand All @@ -24,12 +25,20 @@ export const getDatapageDataV2 = async (
const nextUpdate = getNextUpdateFromVariable(variableMetadata)
const datapageJson: DataPageDataV2 = {
status: "draft",
title:
variableMetadata.presentation?.titlePublic ??
partialGrapherConfig.title ??
variableMetadata.display?.name ??
variableMetadata.name ??
"",
title: variableMetadata.presentation?.titlePublic
? omitUndefinedValues({
title: variableMetadata.presentation?.titlePublic,
attributionShort:
variableMetadata.presentation?.attributionShort,
titleVariant: variableMetadata.presentation?.titleVariant,
})
: {
title:
partialGrapherConfig.title ??
variableMetadata.display?.name ??
variableMetadata.name ??
"",
},
description: variableMetadata.description,
descriptionShort: variableMetadata.descriptionShort,
descriptionFromProducer: variableMetadata.descriptionFromProducer,
Expand Down
18 changes: 15 additions & 3 deletions packages/@ourworldindata/core-table/src/CoreTableColumns.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
PrimitiveType,
imemo,
ToleranceStrategy,
IndicatorTitleWithFragments,
} from "@ourworldindata/utils"
import { CoreTable } from "./CoreTable.js"
import { Time, JsTypes, CoreValueType } from "./CoreTableConstants.js"
Expand Down Expand Up @@ -266,11 +267,22 @@ export abstract class AbstractCoreColumn<JS_TYPE extends PrimitiveType> {
}

@imemo get displayName(): string {
return this.display?.name ?? this.name ?? ""
return (
this.display?.name ??
this.def.presentation?.titlePublic ?? // this is a bit of an unusual fallback - if display.name is not given, titlePublic is the next best thing before name
this.name ??
""
)
}

@imemo get nonEmptyDisplayName(): string {
return this.display?.name || this.nonEmptyName
@imemo get titlePublicOrDisplayName(): IndicatorTitleWithFragments {
return this.def.presentation?.titlePublic
? {
title: this.def.presentation?.titlePublic,
attributionShort: this.def.presentation?.attributionShort,
titleVariant: this.def.presentation?.titleVariant,
}
: { title: this.display?.name || this.name || "" }
}

@imemo get datasetId(): number | undefined {
Expand Down
15 changes: 11 additions & 4 deletions packages/@ourworldindata/grapher/src/core/Grapher.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1475,7 +1475,7 @@ export class Grapher
// sort y-columns by their display name
const sortedYColumnSlugs = sortBy(
yColumnSlugs,
(slug) => this.inputTable.get(slug).nonEmptyDisplayName
(slug) => this.inputTable.get(slug).titlePublicOrDisplayName
)

const columnSlugs = excludeUndefined([
Expand Down Expand Up @@ -1590,7 +1590,10 @@ export class Grapher

if (this.isScatter)
return this.axisDimensions
.map((dimension) => dimension.column.displayName)
.map(
(dimension) =>
dimension.column.titlePublicOrDisplayName.title
)
.join(" vs. ")

const uniqueDatasetNames = uniq(
Expand All @@ -1603,9 +1606,13 @@ export class Grapher
return uniqueDatasetNames[0]

if (yColumns.length === 2)
return yColumns.map((col) => col.displayName).join(" and ")
return yColumns
.map((col) => col.titlePublicOrDisplayName.title)
.join(" and ")

return yColumns.map((col) => col.displayName).join(", ")
return yColumns
.map((col) => col.titlePublicOrDisplayName.title)
.join(", ")
}

@computed get displayTitle(): string {
Expand Down
5 changes: 5 additions & 0 deletions packages/@ourworldindata/grapher/src/dataTable/DataTable.scss
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@
overflow: auto;
border: $cell-border;
}

.title-fragments {
color: $light-text;
font-weight: 500;
}
}

table {
Expand Down
39 changes: 35 additions & 4 deletions packages/@ourworldindata/grapher/src/dataTable/DataTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ import {
maxBy,
minBy,
excludeUndefined,
IndicatorTitleWithFragments,
joinTitleFragments,
} from "@ourworldindata/utils"
import { makeSelectionArray } from "../chart/ChartUtils"
import { SelectionArray } from "../selection/SelectionArray"
Expand Down Expand Up @@ -306,7 +308,15 @@ export class DataTable extends React.Component<{

const dimensionHeaderText = (
<React.Fragment>
<div className="name">{upperFirst(display.columnName)}</div>
<div className="name">
{upperFirst(display.columnName.title)}{" "}
<span className="title-fragments">
{joinTitleFragments(
display.columnName.attributionShort,
display.columnName.titleVariant
)}
</span>
</div>
<div className="description">
<span className="unit">{display.unit}</span>{" "}
<span className="divider">
Expand Down Expand Up @@ -539,9 +549,29 @@ export class DataTable extends React.Component<{
@computed private get tableCaption(): JSX.Element | null {
if (this.hasDimensionHeaders) return null
const singleDimension = this.displayDimensions[0]
const titleFragments = (singleDimension.display.columnName
.attributionShort ||
singleDimension.display.columnName.titleVariant) && (
<>
<span className="title-fragments">
{joinTitleFragments(
singleDimension.display.columnName.attributionShort,
singleDimension.display.columnName.titleVariant
)}
</span>
</>
)
const separator =
(singleDimension.display.columnName.attributionShort ||
singleDimension.display.columnName.titleVariant) &&
singleDimension.display.unit
? " – "
: " "

return singleDimension ? (
<div className="caption">
{singleDimension.display.columnName}{" "}
{singleDimension.display.columnName.title} {titleFragments}
{separator}
{singleDimension.display.unit && (
<span className="unit">{singleDimension.display.unit}</span>
)}
Expand Down Expand Up @@ -855,7 +885,8 @@ export class DataTable extends React.Component<{
const coreTableColumn = d.sourceColumn
const unit =
coreTableColumn.unit === "%" ? "percent" : coreTableColumn.unit
const columnName = coreTableColumn.displayName

const columnName = coreTableColumn.titlePublicOrDisplayName

return {
coreTableColumn,
Expand Down Expand Up @@ -1084,7 +1115,7 @@ interface DataTableDimension {
columns: DataTableColumn[]
coreTableColumn: CoreColumn
sortable: boolean
display: { columnName: string; unit?: string }
display: { columnName: IndicatorTitleWithFragments; unit?: string }
}

interface DataTableRow {
Expand Down
5 changes: 5 additions & 0 deletions packages/@ourworldindata/grapher/src/modal/SourcesModal.scss
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,11 @@
color: $dark-text;
}

.title-fragments {
color: $light-text;
font-size: 1.25rem;
margin-left: 8px;
}
a {
color: inherit;
text-decoration: underline;
Expand Down
22 changes: 18 additions & 4 deletions packages/@ourworldindata/grapher/src/modal/SourcesModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import {
DisplaySource,
prepareSourcesForDisplay,
OwidSource,
IndicatorTitleWithFragments,
joinTitleFragments,
} from "@ourworldindata/utils"
import {
IndicatorSources,
Expand Down Expand Up @@ -103,7 +105,9 @@ export class SourcesModal extends React.Component<
}

@computed private get tabLabels(): string[] {
return this.columns.map((column) => column.nonEmptyDisplayName)
return this.columns.map(
(column) => column.titlePublicOrDisplayName.title
)
}

private renderSource(column: CoreColumn | undefined): JSX.Element | null {
Expand Down Expand Up @@ -282,8 +286,8 @@ export class Source extends React.Component<{
return this.def.source ?? {}
}

@computed private get title(): string {
return this.column.nonEmptyDisplayName
@computed private get title(): IndicatorTitleWithFragments {
return this.column.titlePublicOrDisplayName
}

@computed private get editUrl(): string | undefined {
Expand Down Expand Up @@ -348,7 +352,17 @@ export class Source extends React.Component<{
protected renderTitle(): JSX.Element {
return (
<h2>
{this.title}{" "}
{this.title.title}{" "}
{(this.title.attributionShort || this.title.titleVariant) && (
<>
<span className="title-fragments">
{joinTitleFragments(
this.title.attributionShort,
this.title.titleVariant
)}
</span>{" "}
</>
)}
{this.editUrl && (
<a href={this.editUrl} target="_blank" rel="noopener">
<FontAwesomeIcon icon={faPencilAlt} />
Expand Down
22 changes: 22 additions & 0 deletions packages/@ourworldindata/utils/src/OwidVariable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,28 @@ export interface OwidVariableWithSource {
// metadataPath
}

export interface IndicatorTitleWithFragments {
title: string
attributionShort?: string
titleVariant?: string
}

export function joinTitleFragments(
attributionShort: string | undefined,
titleVariant: string | undefined
): string | undefined {
if (attributionShort && titleVariant && attributionShort !== titleVariant) {
return `${titleVariant}${attributionShort}`
}
if (attributionShort) {
return attributionShort
}
if (titleVariant) {
return titleVariant
}
return undefined
}

export interface OwidLicense {
name: string
url: string
Expand Down
2 changes: 2 additions & 0 deletions packages/@ourworldindata/utils/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -533,6 +533,8 @@ export {
type OwidEntityKey,
type OwidLicense,
type OwidProcessingLevel,
type IndicatorTitleWithFragments,
joinTitleFragments,
} from "./OwidVariable.js"

export {
Expand Down
7 changes: 5 additions & 2 deletions packages/@ourworldindata/utils/src/owidTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@ import { Static, Type } from "@sinclair/typebox"
import { gdocUrlRegex } from "./GdocsConstants.js"
import { OwidOrigin } from "./OwidOrigin.js"
import { OwidSource } from "./OwidSource.js"
import { OwidProcessingLevel } from "./OwidVariable.js"
import {
OwidProcessingLevel,
IndicatorTitleWithFragments,
} from "./OwidVariable.js"
import { PostRowRaw } from "./dbTypes/Posts.js"

// todo: remove when we ditch Year and YearIsDay
Expand Down Expand Up @@ -1570,7 +1573,7 @@ export interface PrimaryTopic {

export interface DataPageDataV2 {
status: "published" | "draft"
title: string
title: IndicatorTitleWithFragments
titleVariant?: string
attributionShort?: string
topicTagsLinks?: string[]
Expand Down
2 changes: 1 addition & 1 deletion site/DataPageV2.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export const DataPageV2 = (props: {
faqEntries,
tagToSlugMap,
} = props
const pageTitle = grapher?.title ?? datapageData.title
const pageTitle = grapher?.title ?? datapageData.title.title
const canonicalUrl = grapher?.slug
? urljoin(baseGrapherUrl, grapher.slug as string)
: ""
Expand Down
Loading

0 comments on commit 107ebe4

Please sign in to comment.