Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9250 +/- ##
==========================================
+ Coverage 86.14% 87.01% +0.86%
==========================================
Files 87 89 +2
Lines 6164 6459 +295
Branches 369 391 +22
==========================================
+ Hits 5310 5620 +310
+ Misses 850 834 -16
- Partials 4 5 +1 ☔ View full report in Codecov by Harness. |
|
Oh boi... here we go. @mcollina could you add screenshots of how it looks when there are ads?
|
|
These are the same ads mdn uses. I'll leave @rginn handle the public comms |
📦 Build Size ComparisonSummary
Changes🔄 Modified Routes (5)
|
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
personally, I'm opposed to:
Important Per content vs code, this is only my opinon. |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
We've taken aims to make the three deployments look like a singular site, so I imagine we will have to extend this to doc-kit for at least learn, and perhaps API docs too. doc-kit consumers from other projects may want this functionality anyway |
|
👀 Two thoughts:
|
Right, I'm talking to @avivkeller in person and we should make a @node-core/analytics package to put all analytics related things in there... Or maybe a component inside @node-core/ui-components like a Provider and a Component for Carbon and then the client/API keys etc get injected on each App. |
ovflowd
left a comment
ovflowd
left a comment
There was a problem hiding this comment.
Adding a formal block for now due to the technical/design open questions we need to figure out first.
To be clear, we are not against the addition and we will comply to it, but we oughta get this aligned as anything else here first :)
Thank you for submitting this, tho, @mcollina
|
Can someone with better design skills take over on where to place those? @bmuenzenmeyer @ovflowd wdyt about proposing something? |
Yes, I think we can skip for this
I'm not the best person to lead a long collaborative UI design discussion, and it would be great if somebody else take over. |
Absolutely, don't worry, we'll take care of it :) |
addressed via 7b4b748 |
|
myself, @ovflowd and @avivkeller did an impromptu session 
with potential security releases 
as opposed to a potential future where we get this 
cc @mcollina |
Very much a fan of this 👍 |
|
Pushed the new topbar strategy to all in-scope layouts for further review. |
ad77c54 to
66e57c3
Compare
Co-authored-by: Claudio Wunder <cwunder@gnome.org> Co-authored-by: Brian Muenzenmeyer <brian.muenzenmeyer@gmail.com> Co-authored-by: Matteo Collina <hello@matteocollina.com>
de67bc8 to
eae362a
Compare
|
@avivkeller what happened to the docs? did yours or mine force push remove it? |
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved ad validation, tracking, and placement defects can cause rendering crashes, duplicate impressions, hidden-tab beacons, missing required ads, and layout shifts.
6 open issues
What changed in this PR
Adds Carbon Ads to the website through browser-side JSON requests, with Top Bar and Sidebar components and a reusable footer slot in the shared MetaBar.
Changes:
- Adds Top Bar placements to about, article and blog layouts, plus Sidebar ads through MetaBar.
- Introduces ad fetching, view-beacon timing, responsive styling and a build-time production-counting flag.
- Adds unit and end-to-end coverage, deployment documentation and a UI-components changeset.
| File | Description |
|---|---|
packages/ui-components/src/Containers/MetaBar/index.tsx |
Adds an optional footer slot. |
packages/ui-components/src/Containers/MetaBar/__tests__/index.test.jsx |
Tests footer rendering and default behavior. |
docs/carbon-ads.md |
Documents placements, configuration and launch steps. |
apps/site/turbo.json |
Includes the production-counting flag in task environments. |
apps/site/tests/e2e/carbon-ads.spec.ts |
Tests homepage exclusion, navigation and narrow screens. |
apps/site/next.constants.mjs |
Defines the API endpoint and production flag. |
apps/site/layouts/Post.tsx |
Adds the blog-post Top Bar. |
apps/site/layouts/Blog.tsx |
Adds the blog-listing Top Bar. |
apps/site/layouts/ArticlePage.tsx |
Adds the article Top Bar. |
apps/site/layouts/About.tsx |
Adds the about-page Top Bar. |
apps/site/hooks/useCarbonAd.ts |
Implements ad fetching and view-beacon timing. |
apps/site/hooks/__tests__/useCarbonAd.test.jsx |
Tests staging requests and beacon timing. |
apps/site/components/MetaBar.tsx |
Renders Sidebar ads through the footer slot. |
apps/site/components/CarbonAds/TopBar.tsx |
Renders the Top Bar creative. |
apps/site/components/CarbonAds/Sidebar.tsx |
Renders the width-gated Sidebar creative. |
apps/site/components/CarbonAds/index.module.css |
Styles both formats and their theme colours. |
.changeset/smart-lilies-invent.md |
Records the MetaBar footer API addition. |
Review effort: Balanced
| import styles from './index.module.css'; | ||
|
|
||
| const CarbonSidebar: FC = () => { | ||
| const { ad, ref } = useCarbonAd('CWBI6KJU', '(min-width: 1280px)'); |
| if (!ad) { | ||
| return null; |
| url.searchParams.set('ignore', 'yes'); | ||
| } | ||
|
|
||
| fetch(url, { signal: controller.signal }) |
|
|
||
| fetch(url, { signal: controller.signal }) | ||
| .then(response => (response.ok ? response.json() : null)) | ||
| .then(data => data?.ads?.[0]?.statlink && setAd(data.ads[0])) |
| let timer: ReturnType<typeof setTimeout>; | ||
|
|
||
| const observer = new IntersectionObserver( | ||
| entries => { | ||
| clearTimeout(timer); | ||
|
|
||
| if (entries.at(-1)!.intersectionRatio >= 0.5) { | ||
| timer = setTimeout(() => { | ||
| new Image().src = ad.statview; | ||
| observer.disconnect(); | ||
| }, 1000); | ||
| } | ||
| }, | ||
| { threshold: 0.5 } | ||
| ); | ||
|
|
||
| observer.observe(ref.current); | ||
|
|
||
| return () => { | ||
| clearTimeout(timer); | ||
| observer.disconnect(); | ||
| }; |
| | `download` | **No** | **No** | | ||
| | `download-archive` | **No** | **No** | |
Description
Adds Carbon Ads, served through the OpenJS Foundation's Carbon project for Node.js, using two formats:
CWBI6KJY): full width, below the navigation, on theabout,article,blog-post,blog-category,downloadanddownload-archivelayouts.CWBI6KJU): below the table of contents. It appears onabout/articlefromxl(1280px) and onblog-postfromml(896px). Below those widths it is neither shown nor requested, as the format requires.The homepage and the error/404 pages never render ad markup, reserve no space and make no ad requests. Layouts opt in explicitly (
<WithNavBar ads />,<WithMetaBar ads="article" | "post" />), and ads are off by default. The fallbackpagelayout has no ads.Brian Edit Start
with Banner
Brian Edit End
Ads are rendered from Carbon's JSON API directly in the browser, without third-party scripts:
apps/site/util/carbonAds.ts: requests ads, validates the untrusted response (required fields, hex colours, HTTPS URLs; anything invalid means no ad), and measures viewability. The view beacon fires once, after at least 50% of the ad has been visible for one continuous second while the tab is visible.apps/site/providers/carbonAdsProvider.tsx: tracks page views by locale and pathname. Each format is requested at most once per page view, so StrictMode, remounts, rerenders, theme changes and hash links never record extra impressions, and a revisited page gets a fresh ad.apps/site/components/CarbonAds: renders both formats. While loading, the slot reserves space; when there is no ad (an empty response, an error or an ad blocker), the slot collapses.MetaBar(ui-components) gains a generic optionalfooterslot, opt-in and with no ad-specific code.Staging vs production
Every request includes
ignore=yes, so impressions are not counted, unless the build setsNEXT_PUBLIC_CARBON_ADS_PRODUCTION=true. Local development and preview deployments are therefore non-counting by default. The variable is embedded at build time and must only be set for the production deployment. Seedocs/carbon-ads.mdfor the configuration and Carbon's launch sequence:Learn and the API docs are separate applications and are not affected.
Validation
pnpm lint,lint:types(site and ui-components), Prettier andpnpm buildpass.MetaBarfooter slottests/e2e/carbon-ads.spec.ts, uses mocked Carbon responses to check:next start, with fixture responses using maximum-length creatives, passed all 238 checks:Note: the e2e spec caught a React Compiler issue that unit tests (which run without the compiler) could not catch. The compiler hoisted the page-view
Map/Setout of theuseMemo, which shared them across navigations. The objects are now created through a function of the page-view key.Reviewers might want to check:
Related Issues
None.
Check List
pnpm formatto ensure the code follows the style guide.pnpm testto check if all tests are passing.pnpm buildto check if the website builds without errors.🤖 Generated with Claude Code
https://claude.ai/code/session_01Sfj4nGFRyF6F4tLo3KmVjH