Skip to content

adds beta docs to learn - #114

Draft
bmuenzenmeyer wants to merge 3 commits into
mainfrom
learn-beta-docs
Draft

adds beta docs to learn#114
bmuenzenmeyer wants to merge 3 commits into
mainfrom
learn-beta-docs

Conversation

@bmuenzenmeyer

@bmuenzenmeyer bmuenzenmeyer commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

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

Copilot AI review requested due to automatic review settings July 30, 2026 22:22
@vercel

vercel Bot commented Jul 30, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
nodejs-learn Ready Ready Preview Jul 30, 2026 10:30pm

Request Review

@github-actions

Copy link
Copy Markdown

👋 Codeowner Review Request

The 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! 🙏

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 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.

Comment thread site.json
Comment on lines +19 to +23
{
"link": "https://beta.docs.nodejs.org/",
"label": "components.containers.navBar.links.betaDocs",
"target": "_blank",
"accent": true

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 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 of site.json navigation items use text, and components/Navigation/index.jsx passes navItems={navigation} directly into @node-core/ui-components NavBar (no label->text mapping). As a result, this item will likely render with an empty label. Use text (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.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 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/Navigation passes navigation directly into @node-core/ui-components/Containers/NavBar, which expects each item to provide a text field (as the other entries in site.json do). Using label here 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.

Comment thread site.json
},
{
"link": "https://beta.docs.nodejs.org/",
"label": "components.containers.navBar.links.betaDocs",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
"label": "components.containers.navBar.links.betaDocs",
"text": "Beta Docs",

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