🎉 update header nav to new taxonomy #3178
Conversation
WalkthroughThe recent update involves refining the website's navigation structure by renaming categories and entries within the Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- site/SiteNavigation.tsx (26 hunks)
Additional comments: 14
site/SiteNavigation.tsx (14)
- 299-299: Ensure the new category "Geography of the World Population" aligns with the intended taxonomy updates. The name and slug should accurately reflect the content under this category.
- 324-337: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [317-334]
The addition of "Health Risks" subcategory with entries like "Lead Pollution", "Alcohol Consumption", and "Opioids, Cocaine, Cannabis, and Other Illicit Drugs" is noted. Verify these entries are correctly categorized and the titles are appropriately chosen to reflect the content.
- 378-394: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [362-408]
The "Infectious Diseases" subcategory has been updated with entries like "Coronavirus Pandemic (COVID-19)", "Pandemics", "Mpox (monkeypox)", and "Smallpox". Ensure these updates are in line with the latest research insights and the titles are consistent with the terminology used in the field.
- 415-425: The "Health Institutions and Interventions" subcategory now includes "Vaccination", "Healthcare Spending", and "Eradication of Diseases". Confirm these additions and ensure the titles accurately represent the content and are consistent with the site's taxonomy.
- 402-458: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [431-471]
Updates to the "Life and Death" subcategory, including new entries like "Causes of Death", "Mental Health", and "Global Health", are observed. Verify the appropriateness of these entries within this subcategory and check for consistency in naming.
- 480-514: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [483-541]
The "Energy and Environment" category has been expanded with subcategories like "Energy Systems", "Waste and Pollution", and "Climate and Air". Ensure these subcategories and their entries are correctly organized and named to reflect the updated taxonomy.
- 576-588: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [579-597]
The addition of the "Environment and Ecosystems" subcategory with entries such as "Natural Disasters" and "Animal Welfare" is noted. Confirm these updates align with the site's content strategy and the entries are accurately categorized.
- 620-659: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [613-668]
The "Food and Agriculture" category updates, including the "Nutrition" and "Food Production" subcategories with entries like "Food Prices" and "Animal Welfare", are observed. Verify these changes accurately reflect the intended updates to the site's navigation taxonomy.
- 714-729: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [717-743]
Updates to the "Poverty and Economic Development" category, specifically the "Public Sector" subcategory with entries like "State Capacity" and "Corruption", are noted. Ensure these updates are consistent with the site's content and the taxonomy updates.
- 754-757: The addition of "Economic Inequality" to the "Poverty and Prosperity" subcategory is observed. Confirm this entry is appropriately named and categorized according to the updated taxonomy.
- 820-919: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [787-1034]
The "Violence and War" category has been updated with entries like "War and Peace", "Nuclear Weapons", and "Biological and Chemical Weapons". Ensure these updates are in line with the site's content strategy and the entries are accurately categorized and named.
- 852-875: The "Innovation and Technological Change" category now includes entries such as "Artificial Intelligence" and "Space Exploration and Satellites". Verify these additions and ensure they are correctly categorized and named to reflect the updated taxonomy.
- 820-919: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [881-949]
Updates to the "Living Conditions, Community, and Wellbeing" category, including the "Housing and Infrastructure" and "Relationships" subcategories with entries like "Access to Energy" and "Trust", are observed. Confirm these updates align with the intended taxonomy changes and the entries are accurately categorized.
- 971-999: The "Human Rights and Democracy" category has been updated with entries like "Human Rights", "Democracy", and "Violence Against Children and Children’s Rights". Ensure these updates are consistent with the site's content strategy and the entries are accurately categorized and named.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 4
Configuration used: CodeRabbit UI
Files ignored due to path filters (3)
devTools/navigationTest/tsconfig.jsonis excluded by:!**/*.jsonpackage.jsonis excluded by:!**/*.jsontsconfig.jsonis excluded by:!**/*.json
Files selected for processing (2)
- devTools/navigationTest/navigationTest.ts (1 hunks)
- site/SiteNavigation.tsx (27 hunks)
Files skipped from review as they are similar to previous changes (1)
- site/SiteNavigation.tsx
Additional comments: 1
devTools/navigationTest/navigationTest.ts (1)
- 22-26: Using the "HEAD" method is efficient for checking URL validity without downloading the entire resource. However, ensure that the server supports HEAD requests for all URLs being tested.
| // Test all the slugs in the SiteNavigationStatic object and makes sure | ||
| // https://ourworldindata.org/{slug} returns a 200 | ||
|
|
||
| import { SiteNavigationStatic } from "../../site/SiteNavigation.js" |
There was a problem hiding this comment.
Consider using TypeScript's type annotations for SiteNavigationStatic to ensure type safety and clarity in the code.
| const testSiteNavigation = async () => { | ||
| const slugs = SiteNavigationStatic.categories | ||
| .map((category) => { | ||
| const categorySlugs = category.entries | ||
| .map((entry) => entry.slug) | ||
| .concat( | ||
| (category.subcategories?.length && | ||
| category.subcategories.flatMap((subcategory) => | ||
| subcategory.entries.map((entry) => entry.slug) | ||
| )) || | ||
| [] | ||
| ) | ||
| return categorySlugs | ||
| }) | ||
| .flat() | ||
|
|
||
| let promises = slugs.map((slug) => { | ||
| return fetch(`https://ourworldindata.org/${slug}`, { | ||
| method: "HEAD", | ||
| }) | ||
| }) | ||
|
|
||
| const responses: Response[] = await Promise.all(promises) | ||
| if (responses.some((response) => !response.ok)) { | ||
| console.error( | ||
| "❌ One or more fetches failed: ", | ||
| responses | ||
| .filter((response) => !response.ok) | ||
| .map((response) => response.url) | ||
| ) | ||
| return | ||
| } | ||
| console.log("✅ All fetches completed") | ||
| } |
There was a problem hiding this comment.
- The function
testSiteNavigationlacks error handling for network issues or exceptions thrown byfetch. Wrap the fetch call in a try-catch block to handle these cases. - The function does not return any value indicating success or failure, which might be useful for automated testing or integration into a larger test suite.
| const responses: Response[] = await Promise.all(promises) | ||
| if (responses.some((response) => !response.ok)) { | ||
| console.error( | ||
| "❌ One or more fetches failed: ", | ||
| responses | ||
| .filter((response) => !response.ok) | ||
| .map((response) => response.url) | ||
| ) | ||
| return | ||
| } |
There was a problem hiding this comment.
- The error logging could be more informative by including the status code and status text.
- Returning early from the function upon failure prevents any further processing or cleanup that might be necessary. Consider how this behavior integrates with the broader testing framework or CI/CD pipeline.
| console.log("✅ All fetches completed") | ||
| } | ||
|
|
||
| testSiteNavigation() |
There was a problem hiding this comment.
Invoking testSiteNavigation directly in the script makes it less modular and harder to integrate into automated testing environments. Consider exporting the function and moving the invocation to a separate script or test runner.
Updates the nav to the new structure as agreed upon by the D&R team.
Here's a preview:
new.header.nav.mp4
And a screenshot of the tree as an unordered list, from a small observable util I made
Summary by CodeRabbit