chore(ui): merge SideBar and ProgressionSideBar #7840

avivkeller · 2025-06-06T15:28:50Z

Description

Currently, we have both ProgressionSideBar and SideBar, however, they are very similar, so the code duplication is unnecessarily complex.

Validation

The Stories should look the same, or very similar.

Related Issues

Check List

I have read the Contributing Guidelines and made commit messages that follow the guideline.
I have run pnpm format to ensure the code follows the style guide.
I have run pnpm test to check if all tests are passing.
I have run pnpm build to check if the website builds without errors.
I've covered new added functionality with unit tests if necessary.

vercel · 2025-06-06T15:28:54Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
nodejs-org	✅ Ready (Inspect)	Visit Preview	Jun 6, 2025 9:29pm

Copilot

Pull Request Overview

This PR consolidates the existing SideBar and ProgressionSideBar components into one SideBar with an optional showProgressionIcons flag, and removes the old ProgressionSidebar code.

Introduced showProgressionIcons prop across SideBar, SidebarGroup, and SidebarItem
Removed all Common/ProgressionSidebar files and updated usages to pass the new prop
Updated CSS and Storybook stories to support the unified component

Reviewed Changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
packages/ui-components/Containers/Sidebar/index.tsx	Added `showProgressionIcons` prop and propagation
packages/ui-components/Containers/Sidebar/index.stories.tsx	Refactored stories and added `Progression` story
packages/ui-components/Containers/Sidebar/index.module.css	Tweaked border CSS
packages/ui-components/Containers/Sidebar/SidebarItem/index.tsx	Added `showProgressionIcon` prop and classNames
packages/ui-components/Containers/Sidebar/SidebarItem/index.module.css	Renamed selectors, added SVG styling
packages/ui-components/Containers/Sidebar/SidebarGroup/index.tsx	Added `showProgressionIcons` prop to groups
packages/ui-components/Containers/Sidebar/ProgressionIcon/index.tsx	Renamed icon component
apps/site/layouts/Learn.tsx	Switched to `SideBar` and passed progression flag
apps/site/components/withSidebar.tsx	Propagated `showProgressionIcons` through HOC
(Removed) Common/ProgressionSidebar and related files	Deleted legacy progression sidebar implementation

Comments suppressed due to low confidence (4)

packages/ui-components/Containers/Sidebar/SidebarItem/index.tsx:16

[nitpick] The prop is named showProgressionIcon here but showProgressionIcons elsewhere. Consider using a single consistent name across components to avoid confusion.

showProgressionIcon?: boolean;

packages/ui-components/Containers/Sidebar/index.tsx:15

The showProgressionIcons flag adds a new rendering path, but there are no unit tests covering its true/false cases. Add tests for both scenarios to ensure the new icon behavior works as expected.

showProgressionIcons?: boolean;

packages/ui-components/Containers/Sidebar/index.tsx:15

[nitpick] The new showProgressionIcons prop isn't documented in JSDoc comments or Storybook controls. Please add documentation or update the prop table to explain its purpose and default behavior.

showProgressionIcons?: boolean;

packages/ui-components/Containers/Sidebar/SidebarItem/index.module.css:47

The CSS rule uses &:not(.progression) > .active outside of a nested context, which may produce invalid selectors. It should be nested under .item or rewritten as .item:not(.progression) > .active.

&:not(.progression) > .active {

packages/ui-components/Containers/Sidebar/SidebarItem/index.module.css

codecov · 2025-06-06T15:30:25Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.44%. Comparing base (ed840c2) to head (30bdca9).
Report is 2 commits behind head on main.

✅ All tests successful. No failed tests found.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7840      +/-   ##
==========================================
- Coverage   75.46%   75.44%   -0.02%     
==========================================
  Files         101      101              
  Lines        8305     8305              
  Branches      218      218              
==========================================
- Hits         6267     6266       -1     
- Misses       2036     2037       +1     
  Partials        2        2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

github-actions · 2025-06-06T21:00:53Z

Lighthouse Results

URL	Performance	Accessibility	Best Practices	SEO	Report
/en	🟢 99	🟢 100	🟢 100	🟢 91	🔗
/en/about	🟢 99	🟢 96	🟢 100	🟠 82	🔗
/en/about/previous-releases	🟢 99	🟢 96	🟢 100	🟠 83	🔗
/en/download	🟢 97	🟢 100	🟢 100	🟢 91	🔗
/en/blog	🟢 99	🟢 100	🟢 96	🟢 92	🔗

AugustinMauroy · 2025-06-06T21:09:56Z

FYI, sidebar may have drop down on it like for api docs. Could you add stories to unsure it's possible.

Figma ref:

avivkeller · 2025-06-06T21:17:15Z

FYI, sidebar may have drop down on it like for api docs. Could you add stories to unsure it's possible.

I planned to open a follow up allowing for children, but it shouldn't be too difficult to just do it here (it certainly makes my life a bit easier, I'll get on that ASAP)

AugustinMauroy · 2025-06-06T21:18:48Z

FYI, sidebar may have drop down on it like for api docs. Could you add stories to unsure it's possible.

I planned to open a follow up allowing for children, but it shouldn't be too difficult to just do it here (it certainly makes my life a bit easier, I'll get on that ASAP)

If you implement that here or in the next pr, both will work fine.

AugustinMauroy

LGMT ! thanks aviv

avivkeller · 2025-06-06T21:21:04Z

Also just FWIW for the select specifically, we know it'll be fine, since we more-or-less use it already on mobile layouts

avivkeller · 2025-06-06T21:28:20Z

@AugustinMauroy PTAL at the latest commit. I plan to pass the <Select /> write where the empty space is in the image below:

AugustinMauroy · 2025-06-06T22:05:44Z

that's can work !

avivkeller · 2025-06-06T22:08:14Z

@nodejs/nodejs-website Requesting fast track so I can update api-docs-tooling accordingly. Merge when ready

ovflowd · 2025-06-07T12:34:21Z

I don't think a PR with 19 file changes should be fast-tracked; Please let's avoid fast-tracking PRs with large changes, I wanted to review this but now feel that I cannot anymore because it got merged LOL.

ovflowd · 2025-06-07T12:34:50Z

apps/site/components/withSidebar.tsx

+  const mappedSidebarItems =
+    // If there's only a single navigation key, use it's sub-items
+    // as our navigation.
+    (navKeys.length === 1 ? sideNavigation[0][1].items : sideNavigation).map(


This could benefit from useMemo

ovflowd

Overall cool code.

avivkeller · 2025-06-07T12:36:20Z

I don't think a PR with 19 file changes should be fast-tracked; Please let's avoid fast-tracking PRs with large changes, I wanted to review this but now feel that I cannot anymore because it got merged LOL.

Yea, that's on me. It also looks like the css has a little bug, so I need to patch it up today.

ovflowd · 2025-06-07T12:39:26Z

I don't think a PR with 19 file changes should be fast-tracked; Please let's avoid fast-tracking PRs with large changes, I wanted to review this but now feel that I cannot anymore because it got merged LOL.

Yea, that's on me. It also looks like the css has a little bug, so I need to patch it up today.

Happens! Let's just be mindful of these rules 🙈

chore(ui): merge SideBar and ProgressionSideBar

3ccf4b0

Copilot AI review requested due to automatic review settings June 6, 2025 15:28

avivkeller requested a review from a team as a code owner June 6, 2025 15:28

avivkeller added the github_actions:pull-request Trigger Pull Request Checks label Jun 6, 2025

avivkeller deployed to Storybook June 6, 2025 15:29 — with GitHub Actions View deployment

github-actions bot removed the github_actions:pull-request Trigger Pull Request Checks label Jun 6, 2025

Copilot AI reviewed Jun 6, 2025

View reviewed changes

packages/ui-components/Containers/Sidebar/SidebarItem/index.module.css Show resolved Hide resolved

fix purity issue

7f3da1f

vercel bot deployed to Preview June 6, 2025 15:43 View deployment

avivkeller marked this pull request as draft June 6, 2025 16:05

fix styling and crash

88c43d4

avivkeller marked this pull request as ready for review June 6, 2025 16:32

vercel bot deployed to Preview June 6, 2025 16:34 View deployment

fixup! fix styling and crash

e05582c

vercel bot deployed to Preview June 6, 2025 16:38 View deployment

AugustinMauroy added the github_actions:pull-request Trigger Pull Request Checks label Jun 6, 2025

AugustinMauroy deployed to Storybook June 6, 2025 21:00 — with GitHub Actions View deployment

github-actions bot removed the github_actions:pull-request Trigger Pull Request Checks label Jun 6, 2025

AugustinMauroy approved these changes Jun 6, 2025

View reviewed changes

allow passing children

30bdca9

vercel bot deployed to Preview June 6, 2025 21:29 View deployment

avivkeller added the github_actions:pull-request Trigger Pull Request Checks label Jun 6, 2025

avivkeller deployed to Storybook June 6, 2025 21:31 — with GitHub Actions View deployment

github-actions bot removed the github_actions:pull-request Trigger Pull Request Checks label Jun 6, 2025

avivkeller added the fast-track Fast Tracking PRs label Jun 6, 2025

avivkeller enabled auto-merge June 6, 2025 22:15

avivkeller added the github_actions:pull-request Trigger Pull Request Checks label Jun 6, 2025

avivkeller deployed to Storybook June 6, 2025 22:15 — with GitHub Actions View deployment

github-actions bot removed the github_actions:pull-request Trigger Pull Request Checks label Jun 6, 2025

avivkeller added this pull request to the merge queue Jun 6, 2025

avivkeller removed this pull request from the merge queue due to a manual request Jun 6, 2025

avivkeller added this pull request to the merge queue Jun 6, 2025

Merged via the queue into main with commit 0bb5d49 Jun 6, 2025
30 checks passed

avivkeller deleted the avivkeller/chore/ui/sidebar branch June 6, 2025 22:25

ovflowd reviewed Jun 7, 2025

View reviewed changes

avivkeller mentioned this pull request Jun 7, 2025

fix(ui): sidebar css fixes #7843

Merged

Uh oh!

chore(ui): merge SideBar and ProgressionSideBar #7840

chore(ui): merge SideBar and ProgressionSideBar #7840

Uh oh!

Conversation

avivkeller commented Jun 6, 2025

Description

Validation

Related Issues

Check List

Uh oh!

vercel bot commented Jun 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

codecov bot commented Jun 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions bot commented Jun 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AugustinMauroy commented Jun 6, 2025

Uh oh!

avivkeller commented Jun 6, 2025

Uh oh!

AugustinMauroy commented Jun 6, 2025

Uh oh!

AugustinMauroy left a comment

Choose a reason for hiding this comment

Uh oh!

avivkeller commented Jun 6, 2025

Uh oh!

avivkeller commented Jun 6, 2025

Uh oh!

AugustinMauroy commented Jun 6, 2025

Uh oh!

avivkeller commented Jun 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ovflowd commented Jun 7, 2025

Uh oh!

ovflowd Jun 7, 2025

Choose a reason for hiding this comment

Uh oh!

ovflowd left a comment

Choose a reason for hiding this comment

Uh oh!

avivkeller commented Jun 7, 2025

Uh oh!

ovflowd commented Jun 7, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

vercel bot commented Jun 6, 2025 •

edited

Loading

codecov bot commented Jun 6, 2025 •

edited

Loading

github-actions bot commented Jun 6, 2025 •

edited

Loading

avivkeller commented Jun 6, 2025 •

edited

Loading