-
-
Notifications
You must be signed in to change notification settings - Fork 229
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
Conversation
Quick links (staging server):
Login:
SVG tester:Number of differences (default views): 0 ✅ Edited: 2025-01-13 22:01:06 UTC |
e9a303e
to
9af48d6
Compare
isOnHomepage = props?.content?.type === OwidGdocType.Homepage | ||
} | ||
ReactDOM.render( | ||
<SiteNavigation |
There was a problem hiding this comment.
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.
9af48d6
to
ad4d4b7
Compare
There was a problem hiding this 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?
ce8192c
to
445e50f
Compare
445e50f
to
ad88120
Compare
ad88120
to
1f7a2ae
Compare
@@ -0,0 +1,47 @@ | |||
import { useEffect, useRef } from "react" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a rename of SiteMobileCategory
- if you omit the commit where that change was made, you can see the actual changes to the file I made
Hey @mlbrgl - thanks for taking another look at this - the main difference is the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a couple of very minor comments, everything looks good!
site/gdocs/pages/Homepage.tsx
Outdated
className="homepage-topic__subtopic" | ||
key={`subarea-${name}`} | ||
> | ||
{name} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure about the design here but we had a ":" after subarea names.
Incidentally, subareas don't show on staging, but ok on dev (some rebaking/caching shenanigans?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah the tag graph on staging doesn't have the latest structure that prod does. It's a pain to re-import, but if I tore down and recreated this staging env it would use the prod data which is correct 👍
e94d617
to
2247f67
Compare
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: