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

Implement thumbnails for multidimensional data pages #4475

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 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
27 changes: 24 additions & 3 deletions functions/_common/grapherRenderer.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { svg2png, initialize as initializeSvg2Png } from "svg2png-wasm"
import { TimeLogger } from "./timeLogger.js"
import { png } from "itty-router"
import { png, StatusError } from "itty-router"

import svg2png_wasm from "../../node_modules/svg2png-wasm/svg2png_wasm_bg.wasm"

Expand All @@ -11,7 +11,8 @@ import LatoBold from "../_common/fonts/LatoLatin-Bold.ttf.bin"
import PlayfairSemiBold from "../_common/fonts/PlayfairDisplayLatin-SemiBold.ttf.bin"
import { Env } from "./env.js"
import { ImageOptions, extractOptions } from "./imageOptions.js"
import { GrapherIdentifier, initGrapher } from "./grapherTools.js"
import { GrapherIdentifier, initGrapher, MultiDimSlug } from "./grapherTools.js"
import { Grapher } from "@ourworldindata/grapher"

declare global {
// eslint-disable-next-line no-var
Expand Down Expand Up @@ -62,7 +63,27 @@ async function fetchAndRenderGrapherToSvg(
env: Env
) {
const grapherLogger = new TimeLogger("grapher")
const grapher = await initGrapher(identifier, options, searchParams, env)
let grapher: Grapher
try {
grapher = await initGrapher(identifier, options, searchParams, env)
} catch (e) {
if (
identifier.type === "slug" &&
e instanceof StatusError &&
e.status === 404
) {
// Normal graphers and multi-dims have the same URL namespace, but
// we have no way of knowing which of them was requested, so we try
// again with a multi-dim identifier.
const multiDimId: MultiDimSlug = {
type: "multi-dim-slug",
id: identifier.id,
}
grapher = await initGrapher(multiDimId, options, searchParams, env)
} else {
throw e
}
}

grapherLogger.log("initGrapher")
const promises = []
Expand Down
52 changes: 45 additions & 7 deletions functions/_common/grapherTools.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
import { Grapher } from "@ourworldindata/grapher"
import {
GrapherInterface,
MultiDimDataPageConfigEnriched,
R2GrapherConfigDirectory,
} from "@ourworldindata/types"
import { excludeUndefined, Bounds } from "@ourworldindata/utils"
import {
excludeUndefined,
Bounds,
searchParamsToMultiDimView,
} from "@ourworldindata/utils"
import { StatusError } from "itty-router"
import { Env } from "./env.js"
import { fetchFromR2, grapherBaseUrl } from "./grapherRenderer.js"
Expand Down Expand Up @@ -83,11 +88,30 @@ export async function fetchUnparsedGrapherConfig(
return fetchFromR2(requestUrl, etag, fallbackUrl)
}

export async function fetchGrapherConfig(
identifier: GrapherIdentifier,
env: Env,
async function fetchMultiDimGrapherConfig(
multiDimConfig: MultiDimDataPageConfigEnriched,
searchParams: URLSearchParams,
env: Env
) {
const view = searchParamsToMultiDimView(multiDimConfig, searchParams)
const response = await fetchUnparsedGrapherConfig(
{ type: "uuid", id: view.fullConfigId },
env
)
return await response.json()
}

export async function fetchGrapherConfig({
identifier,
env,
etag,
searchParams,
}: {
identifier: GrapherIdentifier
env: Env
etag?: string
): Promise<FetchGrapherConfigResult> {
searchParams?: URLSearchParams
}): Promise<FetchGrapherConfigResult> {
const fetchResponse = await fetchUnparsedGrapherConfig(
identifier,
env,
Expand All @@ -113,7 +137,17 @@ export async function fetchGrapherConfig(
}
}

const grapherConfig: GrapherInterface = await fetchResponse.json()
const config = await fetchResponse.json()
let grapherConfig: GrapherInterface
if (identifier.type === "multi-dim-slug") {
grapherConfig = await fetchMultiDimGrapherConfig(
config as MultiDimDataPageConfigEnriched,
searchParams,
env
)
} else {
grapherConfig = config
}
console.log("grapher title", grapherConfig.title)
return {
grapherConfig,
Expand All @@ -127,7 +161,11 @@ export async function initGrapher(
searchParams: URLSearchParams,
env: Env
): Promise<Grapher> {
const grapherConfigResponse = await fetchGrapherConfig(identifier, env)
const grapherConfigResponse = await fetchGrapherConfig({
identifier,
env,
searchParams,
})

if (grapherConfigResponse.status === 404) {
// we throw 404 errors instad of returning a 404 response so that the router
Expand Down
8 changes: 4 additions & 4 deletions functions/grapher/by-uuid/[uuid].ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,11 @@ async function handleConfigRequest(
const shouldCache = searchParams.get("nocache") === null
console.log("Preparing json response for uuid ", uuid)

const grapherPageResp = await fetchGrapherConfig(
{ type: "uuid", id: uuid },
const grapherPageResp = await fetchGrapherConfig({
identifier: { type: "uuid", id: uuid },
env,
etag
)
etag,
})

if (grapherPageResp.status === 304) {
return new Response(null, { status: 304 })
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,5 @@ export interface MultiDimDataPageProps {
primaryTopic?: PrimaryTopic
relatedResearchCandidates: DataPageRelatedResearch[]
imageMetadata: Record<string, ImageMetadata>
initialQueryStr?: string
isPreviewing?: boolean
}
24 changes: 24 additions & 0 deletions packages/@ourworldindata/utils/src/MultiDimDataPageConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -217,3 +217,27 @@ export const extractMultiDimChoicesFromQueryStr = (

return dimensionChoices
}

export function searchParamsToMultiDimView(
config: MultiDimDataPageConfigEnriched,
searchParams: URLSearchParams
): ViewEnriched {
const mdimConfig = MultiDimDataPageConfig.fromObject(config)
let dimensions: MultiDimDimensionChoices
if (searchParams.size > 0) {
dimensions = extractMultiDimChoicesFromQueryStr(
searchParams.toString(),
mdimConfig
)
} else {
// Get the default dimensions.
dimensions = mdimConfig.filterToAvailableChoices({}).selectedChoices
}
const view = mdimConfig.findViewByDimensions(dimensions)
if (!view) {
throw new Error(
`No view found for dimensions ${JSON.stringify(dimensions)}`
)
}
return view
}
1 change: 1 addition & 0 deletions packages/@ourworldindata/utils/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -350,4 +350,5 @@ export {
MultiDimDataPageConfig,
extractMultiDimChoicesFromQueryStr,
multiDimStateToQueryStr,
searchParamsToMultiDimView,
} from "./MultiDimDataPageConfig.js"
15 changes: 10 additions & 5 deletions site/multiDim/MultiDimDataPage.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import urljoin from "url-join"
import { Head } from "../Head.js"
import { IFrameDetector } from "../IframeDetector.js"
import { SiteHeader } from "../SiteHeader.js"
Expand All @@ -18,7 +19,6 @@ export function MultiDimDataPage({
primaryTopic,
relatedResearchCandidates,
imageMetadata,
initialQueryStr,
isPreviewing,
}: MultiDimDataPageProps) {
const canonicalUrl = `${baseGrapherUrl}/${slug}`
Expand All @@ -31,19 +31,24 @@ export function MultiDimDataPage({
relatedResearchCandidates,
imageMetadata,
tagToSlugMap,
initialQueryStr,
}
// Due to thumbnails not taking into account URL parameters, they are often inaccurate on
// social media. We decided to remove them and use a single thumbnail for all charts.
// See https://github.com/owid/owid-grapher/issues/1086
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh? We do show chart previews based on query params

See rewriteMetaTags

image image

So at least one of us must be confused 😅

Copy link
Member

@ikesau ikesau Jan 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once we've resolved this, I'll actually test the PR 👍

(Though, I might need a refresher on how to get MDD's running locally 🙂)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, that's pretty cool that we do that. I copy-pasted the existing code from the data page, so I'll remove those comments in both places then. And, it works the same for mdims out of the box:

http://staging-site-mdim-thumbnails/grapher/mdd-demo-poverty?povertyLine=30&metric=share

Copy link
Contributor Author

@rakyi rakyi Jan 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Testing mdims locally is quite complicated. You need a relatively fresh copy of the DB. The previews are available at an admin sub-path, e.g. http://localhost:3030/admin/grapher/mdd-demo-poverty (even if the mdim is not published). Let me know if you'll need help with it.

const imageUrl: string = urljoin(baseUrl, "default-grapher-thumbnail.png")
const imageWidth = "1200"
const imageHeight = "628"
return (
<Html>
<Head
canonicalUrl={canonicalUrl}
// pageTitle={pageTitle}
// pageDesc={pageDesc}
// imageUrl={imageUrl}
imageUrl={imageUrl}
baseUrl={baseUrl}
>
{/* <meta property="og:image:width" content={imageWidth} />
<meta property="og:image:height" content={imageHeight} /> */}
<meta property="og:image:width" content={imageWidth} />
<meta property="og:image:height" content={imageHeight} />
<IFrameDetector />
<noscript>
<style>{`
Expand Down
18 changes: 5 additions & 13 deletions site/multiembedder/MultiEmbedder.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,9 @@ import {
Url,
GRAPHER_TAB_OPTIONS,
merge,
MultiDimDataPageConfig,
extractMultiDimChoicesFromQueryStr,
fetchWithRetry,
ChartViewInfo,
searchParamsToMultiDimView,
} from "@ourworldindata/utils"
import { action } from "mobx"
import ReactDOM from "react-dom"
Expand Down Expand Up @@ -246,20 +245,13 @@ class MultiEmbedder {
const { queryStr, slug } = embedUrl

const mdimConfigUrl = `${MULTI_DIM_DYNAMIC_CONFIG_URL}/${slug}.json`
const mdimJsonConfig = await fetchWithRetry(mdimConfigUrl).then((res) =>
const multiDimConfig = await fetchWithRetry(mdimConfigUrl).then((res) =>
res.json()
)
const mdimConfig = MultiDimDataPageConfig.fromObject(mdimJsonConfig)
const dimensions = extractMultiDimChoicesFromQueryStr(
queryStr,
mdimConfig
const view = searchParamsToMultiDimView(
multiDimConfig,
new URLSearchParams(queryStr)
)
const view = mdimConfig.findViewByDimensions(dimensions)
if (!view) {
throw new Error(
`No view found for dimensions ${JSON.stringify(dimensions)}`
)
}

const configUrl = `${GRAPHER_DYNAMIC_CONFIG_URL}/by-uuid/${view.fullConfigId}.config.json`

Expand Down
Loading