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

✨ add narrative-chart and figma-url to DI frontmatter / TAS-850 #4489

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

sophiamersmann
Copy link
Member

@sophiamersmann sophiamersmann commented Jan 27, 2025

Adds two new properties to the front matter of DIs.

  • narrative-chart is the name of a narrative chart. It's useful to know which narrative chart a DI is based on, similar to how we track the grapher-url a DI is based on.
  • figma-url is a link to a Figma page. Authors are encouraged to share a Figma link if they customised the chart, but currently paste the link into the comment section of the GDoc. We double-down on Figma as an important piece of infrastructure this cycle, so I think it's justified to track it properly.

Both properties will be surfaced in a new Data Insights index page we have in mind.


This is part 1 of 3 in a stack made with GitButler:

@owidbot
Copy link
Contributor

owidbot commented Jan 27, 2025

Quick links (staging server):

Site Dev Site Preview Admin Wizard Docs

Login: ssh owid@staging-site-gdoc-di-props

SVG tester:

Number of differences (default views): 0 ✅
Number of differences (all views): 0 ✅

Edited: 2025-01-27 09:51:46 UTC
Execution time: 1.21 seconds

@sophiamersmann sophiamersmann marked this pull request as ready for review January 27, 2025 17:05
@sophiamersmann sophiamersmann requested a review from ikesau January 27, 2025 17:06
Copy link
Member

@ikesau ikesau left a comment

Choose a reason for hiding this comment

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

this is a perfectly implemented PR, but I worry we're starting to overload these warnings so that authors will learn to ignore them 🙈

I think the checklist method works better for them, so hopefully your DI tool will make this a non-issue anyway because they'll have already filled everything, everytime, before hitting publish. 🙂

@ikesau
Copy link
Member

ikesau commented Jan 27, 2025

Oh, you might want to add these to the _omittableFields property on GdocDataInsight so that we don't send them to the client.

e.g. inspect _OWID_GDOC_PROPS.content["grapher-url"] on https://ourworldindata.org/data-insights/papua-new-guinea-has-more-living-languages-than-any-other-country

We probably don't want arbitrary figma URLs getting embedded in our HTML 🙂

@sophiamersmann
Copy link
Member Author

sophiamersmann commented Jan 28, 2025

this is a perfectly implemented PR, but I worry we're starting to overload these warnings so that authors will learn to ignore them 🙈

That's a good point! I'm almost inclined to remove all warnings – or maybe keep the warning for the narrative-chart field since that's the preferred flow (although there are good reasons to specify a grapher-url instead, e.g. when creating DIs from explorers or MDims). Mhhh.. I think you're right in saying that this is not the right place to prompt for this information, so I guess I'm in favour of removing all warnings, but I don't have a strong opinion.

@sophiamersmann sophiamersmann changed the title ✨ add narrative-chart and figma-url to DI frontmatter ✨ add narrative-chart and figma-url to DI frontmatter / TAS-850 Jan 28, 2025
Copy link

@sophiamersmann sophiamersmann force-pushed the gdoc-di-props branch 3 times, most recently from 9004e2f to aed91a1 Compare February 4, 2025 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
staging-viz Let SVG tester fail silently in CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants