Skip to content

Commit 58c5e66

Browse files
authored
Merge branch 'main' into rezrah/add-native-actionmenu-hero
2 parents d18ec44 + 836689d commit 58c5e66

30 files changed

Lines changed: 521 additions & 66 deletions

.changeset/clear-pages-compose.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,3 +7,4 @@ Improves guidance and examples delivered through the Primer Brand MCP tools:
77
- Improves guidance delivered through the `primer_brand_review` tool.
88
- Cleaner output from the `primer_brand_examples` tool.
99
- Add new page design guidance advising that `River` descriptions are limited to 160 characters and the default `50:50` image-to-text ratio is preferred.
10+
- Improved guidance on imagery used in `Hero` and `River` components
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@primer/react-brand': patch
3+
---
4+
5+
Fixed missing styles for ESM form controls, where the dependent design tokens were missing . Affects `TextInput`, `Textarea`, `Select`, `Checkbox`, `Radio`, and `FormControl`.

.changeset/fuzzy-years-smile.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@primer/react-brand': patch
3+
---
4+
5+
Fixed the `Hero` image border radius when rendering a `picture` element.

.changeset/neat-lists-align.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@primer/react-brand': patch
3+
---
4+
5+
Fixed `NavList` indentation across grouped nested disclosure levels.

packages/mcp/content/page-design.md

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,17 +65,39 @@ Resolve exact sizes and weights with `primer_brand_tokens`.
6565

6666
## Component & element patterns
6767

68+
### Generated dither backgrounds
69+
70+
**Always pair dither with product UI.** Dither is a supporting background texture, never standalone media or content.
71+
72+
- Use two separate layers for every product UI visual: a dither background and a contained product-shot foreground. Keep them as distinct layers, never one flattened image.
73+
- Treat a solid subtle/gray surround as placeholder-only. Replace the entire surrounding surface with dither.
74+
- When no screenshot is provided, proactively create one lightweight, representative product UI mockup to show that product imagery matters and that the user should replace it with their own screenshot.
75+
- When product UI does not fit the brief, use a relevant non-dither wallpaper, approved illustration, or video.
76+
- Apply dither only as the background of an outer media wrapper.
77+
- Never pass dither to `Hero.Image`, `Image`, `<img>`, or `<picture>`, or make it the direct image child of `River.Visual`.
78+
- Layer the product shot above the dither, leaving dither visible around it.
79+
- In Hero media, make the dither wrapper span the full viewport width.
80+
- In River media, make the dither wrapper fill the `River.Visual` cell.
81+
6882
### Hero
6983

7084
**Do**
7185

72-
- Include relevant media and a label. Prefer a real product shot via `Hero.Image` / `Hero.Video`; use Asset Generator `create_product_landscape` when generating one, or `create_wallpaper` when a product shot does not fit.
86+
- Every Hero must include relevant visual media and a label unless the brief explicitly requires a text-only Hero.
87+
- A missing input screenshot is not a reason to omit Hero media.
88+
- For one non-product media asset, use `Hero.Image` / `Hero.Video`.
89+
- For product UI, use the two-layer treatment in **Generated dither backgrounds**. Do not render product UI directly through `Hero.Image`.
90+
- Place a product UI media band immediately after the Hero copy.
91+
- Constrain only the product-shot foreground to the shared content-grid width; the full-width media band is not permission to make the product shot itself bleed full-width.
92+
- Use a single flattened, precomposed landscape image only when the brief explicitly requires one.
7393
- Keep decorative or illustrative media inside the shared grid with a stable aspect ratio and a token-backed maximum height so it cannot dominate the page or bleed full-width.
7494
- If needed, place custom media after `Hero`; use `trailingComponent` only when it must live inside the Hero composition.
7595

7696
**Don't**
7797

78-
- Add irrelevant hero media or use social templates such as `create_social_square` or `create_landscape` for hero media.
98+
- Finish a Hero as text and actions only because the brief did not provide an image.
99+
- Add irrelevant hero media, or use social or open-graph templates for hero media.
100+
- Put dither behind the Hero copy, inside `Hero.Image`, inside the content grid, or in a flattened product shot. The dither is the full-bleed media-band background; the product shot is the contained foreground.
79101
- Let decorative or illustrative Hero media grow arbitrarily tall or bleed outside the shared grid.
80102

81103
### River
@@ -85,10 +107,15 @@ Resolve exact sizes and weights with `primer_brand_tokens`.
85107
- Use `<River variant="gridline" align="start">` throughout a page; omitting `align` also means start.
86108
- Keep River descriptions to a maximum of 160 characters.
87109
- Prefer the default `50:50` image-to-text ratio where possible; set `imageTextRatio="60:40"` only when the visual needs more space or emphasis.
110+
- When a River explains a product feature, prefer real product UI over a standalone wallpaper.
111+
- For River product UI, use the two-layer treatment in **Generated dither backgrounds** inside `River.Visual`.
112+
- Use a standalone non-dither wallpaper only when no meaningful product UI can demonstrate the feature.
88113

89114
**Don't**
90115

91116
- Zigzag gridline Rivers with `align="end"`.
117+
- Render dither as the direct image child of `River.Visual`; it must cover the full visual cell as the background behind a separate product shot.
118+
- Use a wallpaper as the only River visual when a product shot would communicate the feature; wallpaper-only product Rivers read as placeholders.
92119

93120
### Repeated panels
94121

packages/mcp/src/review/rules.test.ts

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,102 @@ describe('primer_brand_review rules', () => {
7171
expect(ruleIds(findings)).not.toContain('balanced-cta-image')
7272
})
7373

74+
it('flags a Hero with no visual media', () => {
75+
expect(ruleIds(review('<Hero><Hero.Heading>Build</Hero.Heading></Hero>'))).toContain('hero-requires-media')
76+
})
77+
78+
it.each([
79+
'<Hero><Hero.Heading>Build</Hero.Heading><Hero.Image src={image} alt="Product UI" /></Hero>',
80+
'<Hero><Hero.Heading>Build</Hero.Heading><Hero.Video src={video} /></Hero>',
81+
"import heroDither from './dither.png'\nimport heroShot from './shot-checks.png'\n<Hero><Hero.Heading>Build</Hero.Heading></Hero><div style={{backgroundImage: `url(${heroDither})`}}><img src={heroShot} alt=\"Product UI\" /></div>",
82+
])('allows a Hero with visual media: %s', code => {
83+
expect(ruleIds(review(code))).not.toContain('hero-requires-media')
84+
})
85+
86+
it('flags product UI rendered directly in Hero.Image without dither', () => {
87+
const code =
88+
'import securityOverview from "./security-overview.svg"\n<Hero><Hero.Image src={securityOverview} alt="Security dashboard" /></Hero>'
89+
expect(ruleIds(review(code))).toContain('product-shot-needs-dither')
90+
})
91+
92+
it('flags product UI rendered directly in River.Visual without dither', () => {
93+
const code =
94+
'import securityOverview from "./security-overview.svg"\n<River.Visual><img src={securityOverview} alt="Security dashboard" /></River.Visual>'
95+
expect(ruleIds(review(code))).toContain('product-shot-needs-dither')
96+
})
97+
98+
it('allows a product shot layered over a dither background', () => {
99+
const code =
100+
'import securityOverview from "./security-overview.svg"\nimport heroDither from "./hero-dither.png"\n<Hero><Hero.Heading>Security</Hero.Heading></Hero><div style={{backgroundImage: `url(${heroDither})`}}><img src={securityOverview} alt="Security dashboard" /></div>'
101+
expect(ruleIds(review(code))).not.toContain('product-shot-needs-dither')
102+
})
103+
104+
it('does not flag decorative media when only an unrelated attribute contains a product word', () => {
105+
const code =
106+
'import abstractArt from "./abstract.svg"\n<Hero><Hero.Image src={abstractArt} alt="Abstract illustration" className="product-grid" /></Hero>'
107+
expect(ruleIds(review(code))).not.toContain('product-shot-needs-dither')
108+
})
109+
110+
it('does not flag a decorative illustration whose filename contains "overview"', () => {
111+
const code =
112+
'import overviewGraphic from "./overview-illustration.svg"\n<River.Visual><img src={overviewGraphic} alt="Section illustration" /></River.Visual>'
113+
expect(ruleIds(review(code))).not.toContain('product-shot-needs-dither')
114+
})
115+
116+
it('flags product UI detected from the src binding name', () => {
117+
const code = 'import productShot from "./hero-2.png"\n<Hero><Hero.Image src={productShot} alt="" /></Hero>'
118+
expect(ruleIds(review(code))).toContain('product-shot-needs-dither')
119+
})
120+
121+
it('flags the archived Advanced Security Hero product shot (exact source)', () => {
122+
const code =
123+
'import securityOverview from "./security-overview.svg"\n<Hero.Image src={securityOverview} alt="GitHub security overview showing code scanning alerts, blocked secrets, and autofix acceptance" />'
124+
expect(ruleIds(review(code))).toContain('product-shot-needs-dither')
125+
})
126+
127+
it('flags a bare River product shot even when another Hero shot is correctly dithered', () => {
128+
const code = [
129+
'import heroDither from "./hero-dither.png"',
130+
'import securityOverview from "./security-overview.svg"',
131+
'import consoleShot from "./console.svg"',
132+
'<Hero>',
133+
' <Hero.Heading>Security</Hero.Heading>',
134+
' <div style={{backgroundImage: `url(${heroDither})`}}>',
135+
' <img src={securityOverview} alt="Security overview with code scanning alerts" />',
136+
' </div>',
137+
'</Hero>',
138+
'<River>',
139+
' <River.Visual>',
140+
' <img src={consoleShot} alt="Console dashboard showing secret scanning" />',
141+
' </River.Visual>',
142+
'</River>',
143+
].join('\n')
144+
const productShotFindings = review(code).filter(finding => finding.rule === 'product-shot-needs-dither')
145+
expect(productShotFindings).toHaveLength(1)
146+
expect(productShotFindings[0]?.evidence).toContain('consoleShot')
147+
})
148+
149+
it('treats a CSS Module class that paints dither as a covered background', () => {
150+
const code =
151+
'import styles from "./Hero.module.css"\nimport productShot from "./dashboard.png"\n<div className={styles.mediaBand}><img src={productShot} alt="Product dashboard" /></div>\n.mediaBand { background-image: url("./hero-dither.png"); }'
152+
expect(ruleIds(review(code))).not.toContain('product-shot-needs-dither')
153+
})
154+
155+
it.each([
156+
'import heroDither from \'./assets/hero-dither.png\'\n<Hero.Image src={heroDither} alt="" />',
157+
'const riverDither = asset\n<River.Visual><img src={riverDither} alt="" /></River.Visual>',
158+
])('flags dither rendered as image content: %s', code => {
159+
const findings = review(code)
160+
expect(ruleIds(findings)).toContain('dither-as-image')
161+
expect(findings.filter(finding => finding.rule === 'dither-as-image')).toHaveLength(1)
162+
})
163+
164+
it('allows dither as a background behind a separate product shot', () => {
165+
const code =
166+
'import heroDither from \'./assets/hero-dither.png\'\n<div style={{backgroundImage: `url(${heroDither})`}}><img src={productShot} alt="Product UI" /></div>'
167+
expect(ruleIds(review(code))).not.toContain('dither-as-image')
168+
})
169+
74170
it('accepts valid, on-brand usage with no errors', () => {
75171
const findings = review('<Hero align="center"><Hero.Heading>Build</Hero.Heading></Hero>')
76172
expect(errorsOf(findings)).toHaveLength(0)

packages/mcp/src/review/rules.ts

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,141 @@ const balancedCtaRequiresImage: Rule = {
148148
},
149149
}
150150

151+
/** Heroes require visual media unless the brief explicitly says otherwise. */
152+
const heroRequiresMedia: Rule = {
153+
id: 'hero-requires-media',
154+
requiresAssetGenerator: true,
155+
run(code) {
156+
const hasLayeredProductMedia =
157+
/dither/i.test(code) &&
158+
/(?:product[-_ ]?(?:shot|image)|(?:hero|product)Shot|shot[-_])/i.test(code) &&
159+
/background(?:-image|Image)/i.test(code)
160+
const findings: Finding[] = []
161+
for (const match of code.matchAll(/<Hero\b[^>]*>([\s\S]*?)<\/Hero>/g)) {
162+
if (/<Hero\.(?:Image|Video)\b/.test(match[1] ?? '') || hasLayeredProductMedia) continue
163+
findings.push({
164+
severity: 'warning',
165+
rule: this.id,
166+
message:
167+
'This Hero has no visual media. Heroes require `Hero.Image`, `Hero.Video`, or a deliberate full-width dither background with a separate product-shot foreground unless the brief explicitly asks for text only.',
168+
evidence: evidence(match[0]),
169+
})
170+
}
171+
return findings
172+
},
173+
}
174+
175+
/** Product-looking media must use the required dither background treatment. */
176+
const productShotNeedsDither: Rule = {
177+
id: 'product-shot-needs-dither',
178+
run(code) {
179+
// Words that mean "this is real product UI" — layout bits plus GitHub feature names.
180+
const productToken =
181+
/product[-_ ]?(?:shot|ui|screenshot)|screenshot|dashboard|console|panel|editor|settings|mockup|ui[-_]?shot|app[-_ ]?(?:ui|screenshot)|security|code[-_ ]?scanning|scanning|secrets?|\balerts?\b|autofix|dependabot|codeql|vulnerabilit|pull[-_ ]?request/i
182+
183+
// ...and the giveaways that it's just decoration. This is what keeps security-overview.svg
184+
// (product) apart from overview-illustration.svg (decorative).
185+
const decorativeToken =
186+
/illustration|illustrative|abstract|wallpaper|pattern|decorative|texture|gradient|\blogo\b|logomark|\bicon\b|avatar|headshot/i
187+
188+
// Remember each import's path so we can peek at the filename later.
189+
const importPath = new Map<string, string>()
190+
for (const match of code.matchAll(/import\s+([A-Za-z_$][\w$]*)\s+from\s+['"]([^'"]+)['"]/gi)) {
191+
const [, binding, path] = match
192+
if (binding && path !== undefined) importPath.set(binding, path)
193+
}
194+
195+
// CSS classes that paint dither, so a CSS Module wrapper counts too — not just inline styles.
196+
const ditherClasses = new Set<string>()
197+
for (const match of code.matchAll(/\.(-?[A-Za-z_][\w-]*)\s*\{([^}]*)\}/g)) {
198+
const [, cls, body] = match
199+
if (cls && body && /background/i.test(body) && /dither/i.test(body)) ditherClasses.add(cls)
200+
}
201+
202+
const openTagPaintsDither = (openTag: string): boolean => {
203+
if (/background(?:-image|Image)/.test(openTag) && /dither/i.test(openTag)) return true
204+
const classMatch = openTag.match(/class(?:Name)?\s*=\s*(?:"([^"]*)"|'([^']*)'|\{([^}]*)\})/)
205+
const classValue = classMatch?.[1] ?? classMatch?.[2] ?? classMatch?.[3] ?? ''
206+
if (!classValue) return false
207+
if (/dither/i.test(classValue)) return true
208+
return [...ditherClasses].some(cls => new RegExp(`\\b${escapeRegExp(cls)}\\b`).test(classValue))
209+
}
210+
211+
// Walk the tags once and jot down where each dither wrapper starts and ends.
212+
const voidElement = /^(?:img|source|br|input|hr|area|col|embed|track|wbr|meta|link)$/i
213+
const ditherRanges: Array<[number, number]> = []
214+
const openWrappers: Array<{dither: boolean; start: number}> = []
215+
for (const match of code.matchAll(/<(\/?)([A-Za-z][\w.]*)\b[^>]*?(\/?)>/g)) {
216+
const [, closing, name, selfClose] = match
217+
if (selfClose === '/' || (name !== undefined && voidElement.test(name))) continue
218+
if (closing === '/') {
219+
const opened = openWrappers.pop()
220+
if (opened?.dither) ditherRanges.push([opened.start, match.index + match[0].length])
221+
} else {
222+
openWrappers.push({dither: openTagPaintsDither(match[0]), start: match.index})
223+
}
224+
}
225+
// Never closed? Treat it as covering everything after it.
226+
for (const opened of openWrappers) {
227+
if (opened.dither) ditherRanges.push([opened.start, code.length])
228+
}
229+
const insideDitherWrapper = (index: number): boolean =>
230+
ditherRanges.some(([start, end]) => index > start && index < end)
231+
232+
const altText = (tag: string): string => tag.match(/\balt\s*=\s*["']([^"']*)["']/i)?.[1] ?? ''
233+
const srcBinding = (tag: string): string => tag.match(/\bsrc\s*=\s*\{([A-Za-z_$][\w$]*)\}/)?.[1] ?? ''
234+
const srcLiteral = (tag: string): string => tag.match(/\bsrc\s*=\s*["']([^"']*)["']/i)?.[1] ?? ''
235+
236+
const findings: Finding[] = []
237+
for (const match of code.matchAll(/<(?:Hero\.Image|Image|img|picture)\b[^>]*>/g)) {
238+
const tag = match[0]
239+
const binding = srcBinding(tag)
240+
// Go on what describes the image — alt, src binding, filename — not stray attrs like className.
241+
const signals = [altText(tag), binding, importPath.get(binding) ?? '', srcLiteral(tag)]
242+
const looksLikeProduct = signals.some(signal => productToken.test(signal))
243+
const isDecorative = signals.some(signal => decorativeToken.test(signal))
244+
if (!looksLikeProduct || isDecorative) continue
245+
if (insideDitherWrapper(match.index)) continue
246+
findings.push({
247+
severity: 'warning',
248+
rule: this.id,
249+
message:
250+
'Product UI must not use a default/subtle gray media surface. Put the product shot in a contained foreground layer and replace the entire surrounding media background with full-width dither.',
251+
evidence: evidence(tag),
252+
})
253+
}
254+
return findings
255+
},
256+
}
257+
258+
/** Dither is supporting background texture, never the image content itself. */
259+
const ditherAsImage: Rule = {
260+
id: 'dither-as-image',
261+
run(code) {
262+
const ditherBindings = new Set<string>()
263+
for (const match of code.matchAll(/import\s+([A-Za-z_$][\w$]*)\s+from\s+['"][^'"]*dither[^'"]*['"]/gi)) {
264+
if (match[1]) ditherBindings.add(match[1])
265+
}
266+
267+
const findings: Finding[] = []
268+
for (const match of code.matchAll(/<(?:Hero\.Image|Image|img|source)\b[^>]*>/gi)) {
269+
const tag = match[0]
270+
const usesDitherBinding = [...ditherBindings].some(binding =>
271+
new RegExp(`\\b(?:src|srcSet)\\s*=\\s*\\{[^}]*\\b${escapeRegExp(binding)}\\b`).test(tag),
272+
)
273+
if (!/dither/i.test(tag) && !usesDitherBinding) continue
274+
findings.push({
275+
severity: 'error',
276+
rule: this.id,
277+
message:
278+
'Dither must never be rendered as image content. Use it only as an edge-to-edge background behind a separate, contained product shot; if no product screenshot is available, omit the dither.',
279+
evidence: evidence(tag),
280+
})
281+
}
282+
return findings
283+
},
284+
}
285+
151286
type RawPattern = {
152287
id: string
153288
test: RegExp
@@ -325,6 +460,9 @@ export const allRules: Rule[] = [
325460
invalidPropValue,
326461
invalidPropCombination,
327462
balancedCtaRequiresImage,
463+
heroRequiresMedia,
464+
productShotNeedsDither,
465+
ditherAsImage,
328466
rawHtml,
329467
hardcodedValues,
330468
offBrandTells,

packages/mcp/src/review/types.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ export type Finding = {
1212

1313
export type Rule = {
1414
id: string
15+
/** When set, the rule only runs if the GitHub Asset Generator MCP tools are available (internal GitHub use). */
16+
requiresAssetGenerator?: boolean
1517
run(code: string, catalog: Catalog): Finding[]
1618
}
1719

packages/mcp/src/server.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,17 @@ Use these tools whenever a project builds marketing/landing pages, or depends on
1919
This is NOT @primer/react (GitHub's product UI library); if you are building application/product UI,
2020
use the Primer (product) tools instead.
2121
22-
Typical flow: primer_brand_setup once at the start of a new page or project, primer_brand_examples to start
23-
from correct patterns, primer_brand_component for exact props, primer_brand_tokens / primer_brand_asset for tokens and
24-
icons, primer_brand_docs for guidance, and primer_brand_review as the final gate over your complete output (JSX and
22+
Typical flow: \`primer_brand_setup\` once at the start of a new page or project, \`primer_brand_examples\` to start
23+
from correct patterns, \`primer_brand_component\` for exact props, \`primer_brand_tokens\` / \`primer_brand_asset\` for tokens and
24+
icons, \`primer_brand_docs\` for guidance, and \`primer_brand_review\` as the final gate over your complete output (JSX and
2525
CSS together) before you finish.
2626
2727
${
2828
assetGenerator.available
29-
? `Use the GitHub Asset Generator MCP tools (\`asset-generator\`) instead of stock photos or AI art. Use Asset Generator
30-
for wallpapers, hero or river background images, and product shots.`
29+
? `Use the GitHub Asset Generator MCP tools (\`asset-generator\`) instead of stock photos or AI art. For product UI,
30+
call \`create_dither_patterns\` and \`create_full_screen_product_shot\` as a pair, keep the files separate, and render full-width
31+
dither behind the contained product shot. Never use dither alone. If no screenshot exists, proactively create one lightweight,
32+
representative mockup. Use non-dither media when product UI does not fit. Read \`primer_brand_page_design\` for placement.`
3133
: ''
3234
}
3335
`

0 commit comments

Comments
 (0)