docs: add opt-in, participant-targeted recruitment banner to Starlight docs#42595
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
| @@ -0,0 +1,69 @@ | |||
| # Research recruitment banner | |||
There was a problem hiding this comment.
@copilot convert this file to ADR (in adr folder)
There was a problem hiding this comment.
Converted to ADR in ded8f96: docs/adr/46051-recruitment-banner-for-targeted-docs-research.md, and removed docs/RECRUITMENT_BANNER.md.
| </style> | ||
|
|
||
| <script> | ||
| // Client-side eligibility + reveal. Runs on every page load (Starlight is MPA by default). |
There was a problem hiding this comment.
@copilot format JavaScript, check for security issues
There was a problem hiding this comment.
Addressed in ded8f96 and follow-up ddf6f08: formatted the client script and added security hardening (UID sanitization, protocol guard for CTA URL rewriting, stricter path-prefix matching, and rationale note).
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
🤖 PR Triage — §28486872548
Score breakdown: Impact 15/50 · Urgency 5/30 · Quality 12/20 Batch: Rationale: Draft opt-in recruitment banner for docs site (+409/-0, 5 files including tests). Low urgency, no reviews. Defer until marked ready. Labels applied:
|
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in |
There was a problem hiding this comment.
Pull request overview
This PR adds an opt-in, participant-targeted research recruitment banner to the Starlight-based docs site, designed to work in a static GitHub Pages deployment and ship disabled by default.
Changes:
- Adds a Starlight
components.Banneroverride (RecruitmentBanner.astro) that preserves any native frontmatter banner content and conditionally renders a recruitment CTA. - Introduces a typed configuration module (
recruitmentBanner.ts) to control enablement, targeting, and CTA details. - Adds Playwright coverage to ensure the banner does not render when
enabled: false.
Show a summary per file
| File | Description |
|---|---|
| docs/src/components/RecruitmentBanner.astro | Implements the Banner slot override and client-side eligibility/persistence logic. |
| docs/src/config/recruitmentBanner.ts | Defines the operational configuration for enablement, slug, CTA, paths, and frequency. |
| docs/astro.config.mjs | Wires the Banner slot override into Starlight. |
| docs/tests/recruitment-banner.spec.ts | Adds a regression test asserting the banner is absent while disabled. |
| docs/adr/46051-recruitment-banner-for-targeted-docs-research.md | Adds an ADR documenting decision and requirements for the feature. |
| .github/workflows/pr-code-quality-reviewer.lock.yml | Updates generated workflow lock metadata. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 6/6 changed files
- Comments generated: 4
- Review effort level: Low
| /** | ||
| * Optional path scoping. Each entry is matched as a substring of the current pathname | ||
| * (e.g. `/gh-aw/guides/`). Empty array = show on every docs page. | ||
| */ |
There was a problem hiding this comment.
Fixed in 83f22f9: updated the comment from "matched as a substring" to "matched as a path prefix" to match the here.startsWith(p) implementation.
|
|
||
| --- | ||
|
|
||
| *ADR created from the original recruitment banner operator doc and pending final review before status changes from Draft.* |
There was a problem hiding this comment.
Addressed in the latest commit: updated the ADR footer to explicitly state it is the operator guidance document replacing docs/RECRUITMENT_BANNER.md. The PR description also needs updating but the API is restricted in this environment — a maintainer can update it to reference docs/adr/46051-recruitment-banner-for-targeted-docs-research.md.
| } catch (e) { | ||
| /* leave the configured href as-is */ | ||
| } |
There was a problem hiding this comment.
Fixed in 83f22f9: the catch block now returns immediately, keeping the banner hidden rather than revealing it with an unvalidated href.
| if (params.get('recruit') === slug) { | ||
| const incomingUid = sanitizeUid(params.get('uid') || ''); | ||
| set(KEY, incomingUid || '1'); | ||
| } | ||
|
|
||
| // 2) Respect dismissals (unless frequency is "always"). | ||
| if (el.getAttribute('data-frequency') !== 'always' && get(DISMISS_KEY)) return; | ||
|
|
||
| // 3) Eligible only if we captured the recruitment link at some point. | ||
| const stored = get(KEY); | ||
| const uid = sanitizeUid(stored && stored !== '1' ? stored : ''); | ||
| const eligible = Boolean(stored) && (!requireUid || Boolean(uid)); |
There was a problem hiding this comment.
Fixed in 83f22f9: replaced the '1' sentinel with '__present__' (a non-numeric string that can never match the ^[0-9]+$ uid pattern), and updated the corresponding sentinel check on the read path.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Hey A few things stand out as particularly well done:
The PR is well-described, focused, adds no new package dependencies, and includes tests. It looks ready for maintainer review. 🚀
|
|
@copilot run pr-finisher skill |
…l, fail-closed CTA) Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
✅ Branch updated — merged latest from @copilot This PR looks ready to finish! Run the pr-sous-chef | 🤖
|
…BANNER.md Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Done. All review threads addressed, local |
|
🎉 This pull request is included in a new release. Release: |
This introduces a dismissible research recruitment banner for the docs site that works in a static GitHub Pages deployment (no server-side
current_user). Banner visibility is opt-in via recruitment link parameters and ships disabled by default.Banner architecture (Starlight slot override)
docs/src/components/RecruitmentBanner.astroand wired it tocomponents.Bannerindocs/astro.config.mjs.bannercontent first.Targeting + persistence model (static-site safe)
docs/src/config/recruitmentBanner.tsfor operational controls:enabled,slug, copy,ctaUrl,paths,frequency,requireUid.?recruit=<slug>(optionally&uid=<dotcom_id>) and persisted inlocalStorage.utm_source=inproduct_banner,utm_medium=docs,utm_campaign=<slug>, and optionalpid=<uid>.banner_research_ctaas a best-effort browser event on click.Operator documentation
docs/RECRUITMENT_BANNER.mdwith configuration guidance and go-live guardrails (including explicit “ships OFF” behavior and non-commitment of participant IDs).Focused regression coverage
docs/tests/recruitment-banner.spec.tsto assert no recruitment banner renders whileenabled: false, even when recruit query params are present.