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

✨ Site Nav from tag graph #4439

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

✨ Site Nav from tag graph #4439

wants to merge 8 commits into from

Conversation

ikesau
Copy link
Member

@ikesau ikesau commented Jan 11, 2025

Removes the hardcoded SiteNavigationStatic object that powered the site nav and replaces it with a JSON file representation of the tag graph from the database.

As a reminder, the tag graph shape is:

interface TagGraphNode {
    children: TagGraphNode[]
    id: number
    isTopic: boolean
    name: string
    path: number[]
    slug: string | null
    weight: number
}

@owidbot
Copy link
Contributor

owidbot commented Jan 11, 2025

Quick links (staging server):

Site Dev Site Preview Admin Wizard Docs

Login: ssh owid@staging-site-sitenav-from-tag-graph

SVG tester:

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

Edited: 2025-01-13 22:01:06 UTC
Execution time: 1.25 seconds

@ikesau ikesau force-pushed the sitenav-from-tag-graph branch from e9a303e to 9af48d6 Compare January 13, 2025 21:49
isOnHomepage = props?.content?.type === OwidGdocType.Homepage
}
ReactDOM.render(
<SiteNavigation
Copy link
Member Author

Choose a reason for hiding this comment

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

Technically we don't need to fetch the topicTagGraph.json when we're on the homepage, because it's already embedded in the homepage gdoc's homepageMetadata, but seeing as the JSON request is ~4kB transferred, I think it's okay to not optimize.

Later, we should see if it's possible to get this file and dods.json to cache.

@ikesau ikesau requested a review from mlbrgl January 13, 2025 22:01
@ikesau ikesau marked this pull request as ready for review January 13, 2025 22:01
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.

2 participants