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

✨ use dynamic thumbnails in GrapherImage #3746

Closed
wants to merge 3 commits into from
Closed
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
41 changes: 30 additions & 11 deletions site/GrapherImage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,29 +4,48 @@ import {
DEFAULT_GRAPHER_HEIGHT,
DEFAULT_GRAPHER_WIDTH,
} from "@ourworldindata/grapher"
import { BAKED_GRAPHER_EXPORTS_BASE_URL } from "../settings/clientSettings.js"
import { GRAPHER_DYNAMIC_THUMBNAIL_URL } from "../settings/clientSettings.js"
import { Url } from "@ourworldindata/utils"

export default function GrapherImage({
alt,
slug,
noFormatting,
}: {
export default function GrapherImage(props: {
url: string
alt?: string
noFormatting?: boolean
}): JSX.Element
export default function GrapherImage(props: {
slug: string
queryString?: string
alt?: string
noFormatting?: boolean
}): JSX.Element
export default function GrapherImage(props: {
url?: string
slug?: string
queryString?: string
alt?: string
noFormatting?: boolean
}) {
let slug: string = ""
let queryString: string = ""
if (props.url) {
const url = Url.fromURL(props.url)
slug = url.slug!
queryString = url.queryStr
} else {
slug = props.slug!
queryString = props.queryString ?? ""
}

return (
<img
className="GrapherImage"
// TODO: use the CF worker to render these previews so that we can show non-default configurations of the chart
// https://github.com/owid/owid-grapher/issues/3661
src={`${BAKED_GRAPHER_EXPORTS_BASE_URL}/${slug}.svg`}
alt={alt}
src={`${GRAPHER_DYNAMIC_THUMBNAIL_URL}/${slug}${queryString}`}
alt={props.alt}
width={DEFAULT_GRAPHER_WIDTH}
height={DEFAULT_GRAPHER_HEIGHT}
loading="lazy"
data-no-lightbox
data-no-img-formatting={noFormatting}
data-no-img-formatting={props.noFormatting}
/>
)
}
1 change: 1 addition & 0 deletions site/InteractionNotice.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ export const INTERACTIVE_ICON_SVG = `<svg aria-hidden="true" focusable="false" d
<path fill="currentColor" opacity="0.6" d="M239.76,234.78A27.5,27.5,0,0,1,217,192a87.76,87.76,0,1,0-145.9,0A27.5,27.5,0,1,1,25.37,222.6,142.17,142.17,0,0,1,1.24,143.17C1.24,64.45,65.28.41,144,.41s142.76,64,142.76,142.76a142.17,142.17,0,0,1-24.13,79.43A27.47,27.47,0,0,1,239.76,234.78Z" transform="translate(0 -0.41)"/>
</svg>`

// shouldProgressiveEmbed is hardcoded to true, so this will only show if JS is disabled
export default function InteractionNotice() {
return (
<div className="interactionNotice">
Expand Down
4 changes: 2 additions & 2 deletions site/blocks/RelatedCharts.jsdom.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import Enzyme from "enzyme"
import Adapter from "@wojtekmaj/enzyme-adapter-react-17"
import {
BAKED_BASE_URL,
BAKED_GRAPHER_EXPORTS_BASE_URL,
GRAPHER_DYNAMIC_THUMBNAIL_URL,
} from "../../settings/clientSettings.js"
import { KeyChartLevel } from "@ourworldindata/utils"
Enzyme.configure({ adapter: new Adapter() })
Expand Down Expand Up @@ -37,7 +37,7 @@ it("renders active chart links and loads respective chart on click", () => {
.find("li")
.first()
.find(
`img[src="${BAKED_GRAPHER_EXPORTS_BASE_URL}/${charts[1].slug}.svg"]`
`img[src="${GRAPHER_DYNAMIC_THUMBNAIL_URL}/${charts[1].slug}"]`
)
).toHaveLength(1)

Expand Down
6 changes: 6 additions & 0 deletions site/css/content.scss
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,12 @@ figure[data-explorer-src] {
}
}

// Doesn't make sense to show the notice if JavaScript is disabled
// As the grapher/data page will also be disabled
.js-disabled .interactionNotice {
display: none;
}

/*******************************************************************************
* Tables
*/
Expand Down
11 changes: 4 additions & 7 deletions site/gdocs/components/Chart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ export default function Chart({

const url = Url.fromURL(d.url)
const resolvedUrl = linkedChart.resolvedUrl
const resolvedSlug = Url.fromURL(resolvedUrl).slug
const isExplorer = url.isExplorer
const hasControls = url.queryParams.hideControls !== "true"
const isExplorerWithControls = isExplorer && hasControls
Expand Down Expand Up @@ -116,12 +115,10 @@ export default function Chart({
{isExplorer ? (
<div className="js--show-warning-block-if-js-disabled" />
) : (
resolvedSlug && (
<a href={resolvedUrl} target="_blank" rel="noopener">
<GrapherImage slug={resolvedSlug} alt={d.title} />
<InteractionNotice />
</a>
)
<a href={resolvedUrl} target="_blank" rel="noopener">
<GrapherImage url={resolvedUrl} alt={d.title} />
<InteractionNotice />
</a>
)}
</figure>
{d.caption ? (
Expand Down