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

feat(gdocs): enable narrative charts inside key insights #4473

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

marcelgerber
Copy link
Member

@marcelgerber marcelgerber commented Jan 21, 2025

Based on a feature request from Slack: https://owid.slack.com/archives/C0149NKLZAM/p1737397453015989

You can preview this feature on the staging server here: http://staging-site-key-insights-narrative-chart/admin/gdocs/15ofwDSH2K_SEjQ7aZsLd99r6aGczV9vEz-uQzcHH0nY/preview

I didn't implement htmlToEnriched for this, since I assume it's only been used for WP -> gdocs conversion. Am I right in that?

CleanShot 2025-01-21 at 21 50 38@2x

@owidbot
Copy link
Contributor

owidbot commented Jan 21, 2025

Quick links (staging server):

Site Dev Site Preview Admin Wizard Docs

Login: ssh owid@staging-site-key-insights-narrative-chart

SVG tester:

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

Edited: 2025-01-21 20:50:38 UTC
Execution time: 1.13 seconds

@marcelgerber marcelgerber requested a review from ikesau January 21, 2025 20:51
@marcelgerber marcelgerber marked this pull request as ready for review January 21, 2025 20:53
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.

Cool!

I also noticed 2 (minor) things that probably should have been done in the original narrative charts PR. You can change them here if you'd like, or create a ticket/follow up PR.

"linkedChartViews" should be in GDOC_DIFF_OMITTABLE_PROPERTIES

It probably shouldn't be possible to publish a gdoc that's linking to an invalid narrative chart? You'll need another case in GdocBase.validate and a function to fetch all the names of the published narrative charts added to the place in GdocBase.validate where we fetch the published graphers/explorers

Then you can test it by making sure it renders in the settings drawer and you should be good to go! 🙂

})
}
if (rawInsight.url && rawInsight.filename) {
if (
Number(!!rawInsight.url) +
Copy link
Member

Choose a reason for hiding this comment

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

nice 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants