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 12 commits into
base: master
Choose a base branch
from
Open

✨ Site Nav from tag graph #4439

wants to merge 12 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
@ikesau ikesau force-pushed the sitenav-from-tag-graph branch from 9af48d6 to ad4d4b7 Compare January 16, 2025 19:11
Copy link
Member

@mlbrgl mlbrgl left a comment

Choose a reason for hiding this comment

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

nice to see the merging of tag and topics taking shape more visibly!

It seems like the order of tags in the graph is a bit arbitrary at the moment on live, are we waiting for authors to reorder before merging?

site/gdocs/pages/Homepage.tsx Outdated Show resolved Hide resolved
db/db.ts Outdated Show resolved Hide resolved
packages/@ourworldindata/types/src/gdocTypes/Gdoc.ts Outdated Show resolved Hide resolved
db/model/Post.ts Outdated Show resolved Hide resolved
site/SiteNavigationTopics.tsx Outdated Show resolved Hide resolved
@ikesau ikesau force-pushed the sitenav-from-tag-graph branch from ad4d4b7 to ce8192c Compare January 20, 2025 20:48
@ikesau ikesau force-pushed the sitenav-from-tag-graph branch from ce8192c to 445e50f Compare January 24, 2025 23: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.

3 participants