feat(banner): add `x` for closing by avivkeller · Pull Request #9008 · nodejs/nodejs.org

avivkeller · 2026-07-14T19:13:09Z

Makes the banner close-able.

Note: This just updated ui-components, and doesn't touch Next.js + cookie handling

vercel · 2026-07-14T19:13:15Z

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

Project	Deployment	Actions	Updated (UTC)
nodejs-org	Ready	Preview	Jul 15, 2026 7:35am

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

cursor · 2026-07-14T19:13:17Z

PR Summary

Low Risk
UI-only behavior using client localStorage and a static inline script; no auth, cookies, or server data changes.

Overview
Site banners can be dismissed with an × control, and that choice is remembered across visits via localStorage (keyed by banner text so a new announcement can show again).

The shared Banner component gains an optional onClose handler and close button styling; WithBanner is now a client component that tracks dismissal, writes to storage on close, and only renders when the banner is active and not dismissed. A small pre-hydration script in the root layout reads the same storage key before paint and sets data-banner-dismissed on <html>, paired with CSS that hides [data-dismissible-banner] to avoid a flash on reload. @node-core/ui-components is bumped to 1.7.2 with a Storybook Dismissible story.

^{Reviewed by Cursor Bugbot for commit dc9e701. Bugbot is set up for automated code reviews on this repo. Configure here.}

codecov · 2026-07-14T19:14:50Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.60%. Comparing base (61cd932) to head (dc9e701).
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #9008      +/-   ##
==========================================
+ Coverage   74.58%   74.60%   +0.02%     
==========================================
  Files         100      100              
  Lines        8801     8801              
  Branches      324      324              
==========================================
+ Hits         6564     6566       +2     
+ Misses       2233     2231       -2     
  Partials        4        4

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

cursor · 2026-07-14T19:18:45Z

+      transition-colors
+      hover:text-white;
+  }
+}


Close button overlaps content

Medium Severity

The close control is absolute with right-4 while the banner still uses symmetric px-8 and centered content. Nothing reserves space for the button, so on narrower viewports or with longer banner text the label, link, or icon can sit under the ×. Elsewhere in this package, absolute controls get matching padding so content stays clear.

Additional Locations (1)

packages/ui-components/src/Common/Banner/index.module.css#L2-L13

^{Reviewed by Cursor Bugbot for commit bd140ea. Configure here.}

github-actions · 2026-07-14T19:27:49Z

📦 Build Size Comparison

Summary

Metric	Value
Old Total First Load JS	10.70 MB
New Total First Load JS	10.64 MB
Delta	-60.08 KB (-0.55%)

Changes

🔄 Modified Routes (4)

Route	Old First Load JS	New First Load JS	Delta
`/[locale]`	2.55 MB	2.54 MB	📉 -15.02 KB (-0.57%)
`/[locale]/[...path]`	2.55 MB	2.54 MB	📉 -15.02 KB (-0.57%)
`/[locale]/blog/[...path]`	2.55 MB	2.54 MB	📉 -15.02 KB (-0.57%)
`/[locale]/download/archive/[version]`	2.55 MB	2.54 MB	📉 -15.02 KB (-0.57%)

bjohansebas

I think that if we ever update the banner and a user previously dismissed it, we should show it to them again

cursor

Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.

There are 3 total unresolved issues (including 1 from previous review).

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit b2dc64f. Configure here.}

cursor · 2026-07-14T19:59:08Z

+      >
+        <span aria-hidden="true">&times;</span>
+      </button>
+    )}


Close label not localizable

Low Severity

The close control hardcodes aria-label="Close banner" on the button, and that string is not overridable via props. Extra HTML attributes only spread onto the section, so consumers such as withBanner cannot pass a translated label despite already using useTranslations for the banner itself.

^{Reviewed by Cursor Bugbot for commit b2dc64f. Configure here.}

AugustinMauroy · 2026-07-14T20:34:24Z

+  // rendered on the server and hidden after hydration if previously dismissed
+  useEffect(() => {
+    if (banner) {
+      setDismissed(localStorage.getItem(STORAGE_KEY) === banner.text);


there are a thing here localstroage is shared with api docs but not with sub domain like undici.nodejs.org

Then I guess a cookie would solve that, right?

I'm not a fan of accessing the cookie API from the client, IMHO localStorage is meant for client side editing in KV, cookies aren't

👍 localStorage seems fine to me, I think it's acceptable (and perhaps desired) that different domains (e.g. undici) will have their own state.

AugustinMauroy · 2026-07-14T20:35:52Z

there are a flash humm not fan of it maybe not display it by default. also what will happen on doc-kit since it's already flash when fetching site.json

trivikr · 2026-07-14T23:31:10Z

To avoid flash, can we dismiss it by default and show only if the value is not set?

Screen recording of flash on reload post dismiss

nodejs-banner-flash-on-reload-post-dismiss.mov

trivikr · 2026-07-15T01:14:15Z

Fixes: #6292

avivkeller · 2026-07-15T06:01:45Z

@trivikr either we flash on the way in, or the way out (fwiw we can make it a fade)

The way I see it, if we fade on the way in, we fade for all website viewers.

If we fade on the way out, we only fade for a small subset of viewers who have opted in to hiding this banner

trivikr · 2026-07-15T07:03:18Z

With assistance from openai:gpt-5.6-sol, I attempted going prehydration in #9009. Would that help fix the flashing issue?

avivkeller · 2026-07-15T07:30:41Z

No, unfortunately. While I appreciate the time and tokens spent trying to help, there's several layers to how we render our components currently, including:

Server-Side Next.js Rendering
Static-Side rolldown Bundling (doc-kit)
Client-Side Hydration (here and doc-kit, seperately)

Attaching a script element is a extremely hacky way (and one we can't accept for a number of reasons) to resolve the issue on one client (here), it wouldn't resolve it on the other clients. Ultimately, we can't predict the value of localStorage, so we need to either fade/flash the banner in or fade/flash the banner out.

In doc-kit, we are forced to fade the banner in no matter what (due to the fetching of banner content) so, the solution is already in place there. For consistency, I suppose, we can adjust this PR to also fade the banner in.

Would that satisfy your concern?

avivkeller · 2026-07-15T07:35:45Z

I've manually re-deployed the previous commit to match Vercel's branch preview with the correct commit.

trivikr · 2026-07-15T07:37:58Z

we can adjust this PR to also fade the banner in.

Would that satisfy your concern?

Sure.

MattIPv4 · 2026-07-15T11:59:22Z

In doc-kit, we are forced to fade the banner in no matter what (due to the fetching of banner content) so, the solution is already in place there. For consistency, I suppose, we can adjust this PR to also fade the banner in.

++ I think I would actually prefer that we do fade in everywhere, for consistency and to clear the way for this potentially being fully dynamic in the future instead of needing a site rebuild every time it is changed.

feat(banner): add x for closing

b70b8ab

Copilot AI review requested due to automatic review settings July 14, 2026 19:13

avivkeller requested a review from a team as a code owner July 14, 2026 19:13

avivkeller deployed to Storybook July 14, 2026 19:13 — with GitHub Actions View deployment

Copilot AI reviewed Jul 14, 2026

vercel Bot deployed to Preview July 14, 2026 19:13 View deployment

avivkeller requested a review from trivikr July 14, 2026 19:14

MattIPv4 reviewed Jul 14, 2026

View reviewed changes

Comment thread packages/ui-components/src/Common/Banner/index.tsx Outdated

Comment thread packages/ui-components/src/Common/Banner/index.tsx

fixup!

bd140ea

avivkeller deployed to Storybook July 14, 2026 19:16 — with GitHub Actions View deployment

vercel Bot deployed to Preview July 14, 2026 19:18 View deployment

cursor Bot reviewed Jul 14, 2026

View reviewed changes

bjohansebas reviewed Jul 14, 2026

View reviewed changes

fixup!

b2dc64f

avivkeller deployed to Storybook July 14, 2026 19:55 — with GitHub Actions View deployment

vercel Bot deployed to Preview July 14, 2026 19:56 View deployment

cursor Bot reviewed Jul 14, 2026

View reviewed changes

AugustinMauroy reviewed Jul 14, 2026

View reviewed changes

avivkeller linked an issue Jul 15, 2026 that may be closed by this pull request

Feat(banner): take place on screen, should we add the possibility of hide #6292

Open

trivikr deployed to Storybook July 15, 2026 06:53 — with GitHub Actions View deployment

vercel Bot deployed to Preview July 15, 2026 06:54 View deployment

trivikr force-pushed the banner-close branch from dc9e701 to b2dc64f Compare July 15, 2026 06:58

trivikr deployed to Storybook July 15, 2026 06:58 — with GitHub Actions View deployment

trivikr mentioned this pull request Jul 15, 2026

feat(banner): add x for closing (with prehydration) #9009

Closed

This comment was marked as outdated.

Sign in to view

Uh oh!

Conversation

avivkeller commented Jul 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vercel Bot commented Jul 14, 2026 • 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

Uh oh!

cursor Bot commented Jul 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

Uh oh!

codecov Bot commented Jul 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cursor Bot Jul 14, 2026

Choose a reason for hiding this comment

Close button overlaps content

Uh oh!

github-actions Bot commented Jul 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📦 Build Size Comparison

Summary

Changes

Uh oh!

bjohansebas left a comment

Choose a reason for hiding this comment

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cursor Bot Jul 14, 2026

Choose a reason for hiding this comment

Close label not localizable

Uh oh!

AugustinMauroy Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

bjohansebas Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

avivkeller Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

MattIPv4 Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

AugustinMauroy commented Jul 14, 2026

Uh oh!

trivikr commented Jul 14, 2026

Uh oh!

trivikr commented Jul 15, 2026

Uh oh!

avivkeller commented Jul 15, 2026

Uh oh!

This comment was marked as outdated.

trivikr commented Jul 15, 2026

Uh oh!

avivkeller commented Jul 15, 2026

Uh oh!

avivkeller commented Jul 15, 2026

Uh oh!

trivikr commented Jul 15, 2026

Uh oh!

MattIPv4 commented Jul 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

avivkeller commented Jul 14, 2026 •

edited

Loading

vercel Bot commented Jul 14, 2026 •

edited

Loading

cursor Bot commented Jul 14, 2026 •

edited

Loading

codecov Bot commented Jul 14, 2026 •

edited

Loading

github-actions Bot commented Jul 14, 2026 •

edited

Loading