adds beta docs to learn - #114
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
👋 Codeowner Review RequestThe following codeowners have been identified for the changed files: Team reviewers: @nodejs/nodejs-website Please review the changes when you have a chance. Thank you! 🙏 |
There was a problem hiding this comment.
🟡 Not ready to approve
The new NavBar entry uses a label i18n key that is not defined/used elsewhere in this repo and is inconsistent with existing text fields, likely causing an incorrect or missing link label.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
This PR adds a new top-level navigation link to the Node.js Beta documentation site in site.json, intended to surface “Beta Docs” prominently in the Learn site NavBar.
Changes:
- Add a new NavBar item linking to
https://beta.docs.nodejs.org/ - Mark the new link as opening in a new tab and visually accented
File summaries
| File | Description |
|---|---|
| site.json | Adds a new navigation entry for the Beta docs site in the main NavBar list. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
| { | ||
| "link": "https://beta.docs.nodejs.org/", | ||
| "label": "components.containers.navBar.links.betaDocs", | ||
| "target": "_blank", | ||
| "accent": true |
There was a problem hiding this comment.
🟡 Not ready to approve
The new nav item uses label instead of text, which is likely to render the link with no visible text given how navItems are passed through.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Comments suppressed due to low confidence (1)
site.json:21
- This new nav entry uses
label, but the rest ofsite.jsonnavigation items usetext, andcomponents/Navigation/index.jsxpassesnavItems={navigation}directly into@node-core/ui-componentsNavBar(no label->text mapping). As a result, this item will likely render with an empty label. Usetext(or add mapping code, but that would be a separate change).
"label": "components.containers.navBar.links.betaDocs",
- Files reviewed: 2/3 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
There was a problem hiding this comment.
🟡 Not ready to approve
The new nav entry uses label instead of text, which likely prevents the Beta Docs link text from rendering with the current NavBar integration.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Comments suppressed due to low confidence (1)
site.json:23
components/Navigationpassesnavigationdirectly into@node-core/ui-components/Containers/NavBar, which expects each item to provide atextfield (as the other entries insite.jsondo). Usinglabelhere will likely result in the Beta Docs item rendering without any visible text (and the i18n key isn’t resolved in this repo).
"link": "https://beta.docs.nodejs.org/",
"label": "components.containers.navBar.links.betaDocs",
"target": "_blank",
"accent": true
- Files reviewed: 2/3 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
| }, | ||
| { | ||
| "link": "https://beta.docs.nodejs.org/", | ||
| "label": "components.containers.navBar.links.betaDocs", |
There was a problem hiding this comment.
| "label": "components.containers.navBar.links.betaDocs", | |
| "text": "Beta Docs", |
Need to see if we need to update ui-components or any other aspect of what made the nav accent.
cross-ref nodejs/nodejs.org#8998