Skip to content

Commit fc8b927

Browse files
committed
Guard interface icon ownership and exercise both native tab layouts
1 parent 2ec8353 commit fc8b927

4 files changed

Lines changed: 48 additions & 10 deletions

File tree

‎apps/sim/platform/AGENTS.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ Keep routes thin. Colocate each feature component, its CSS Module and private he
2222

2323
Use `design-system/styles/rhythm.css` for the common 2px title/detail gap, 8px row inset and gap, 6px block inset, 16px interface icon and shared button radius. Align leading artwork to the first text line. Keep dividers outside row hovers with breathing room. Align the **ink** of actions with adjacent text, extending their hit areas into the margin. Only composer actions use circular button chrome. Custom calendar, chart and summary rows still use the same typography, geometry and tooltip as reusable controls.
2424

25-
Interface strokes have one optical weight in CSS pixels and use semantic icon ink; branded artwork and chart marks retain their own geometry. Quiet controls gain the hover surface without changing ink. A selectable menu row gains the same surface and promotes its glyph from secondary to primary ink on hover or active keyboard navigation. Selection, disabled and inverse controls have distinct semantics. Never change stroke thickness, spacing or radius on hover. DOM focus alone adds no paint.
25+
Interface strokes have one optical weight in CSS pixels and use semantic icon ink; branded artwork and chart marks retain their own geometry. Quiet controls gain the hover surface without changing ink. A selectable menu row gains the same surface and promotes its glyph from secondary to primary ink on hover or active keyboard navigation. Selection, disabled and inverse controls have distinct semantics. Never change stroke thickness, spacing or radius on hover. Render interface glyphs through `Icon`. A unique data graphic may own accessible, labeled SVG and paint its own marks. DOM focus alone adds no paint.
2626

2727
## Public API
2828

‎apps/sim/platform/checks/desktop/renderer.spec.ts‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,19 @@ test('the actual desktop renderer shares product routes, reserves native chrome
1313
env: { ...process.env, SIM_DESKTOP_ORIGIN: origin, SIM_DESKTOP_USER_DATA: profile },
1414
})
1515
const page = await app.firstWindow()
16+
const window = await app.browserWindow(page)
1617
try {
18+
await window.evaluate((native) => native.setContentSize(960, 720))
1719
await page.waitForURL((url) => url.origin === origin)
1820
await page.goto(`${origin}/chat/weekly-review?theme=dark&state=full`)
1921
await expect(page.locator('html')).toHaveAttribute('data-sim-desktop-title-bar', 'inset')
20-
await expect(page.getByRole('button', { name: 'Open chat panel' })).toBeVisible()
21-
await page.getByRole('button', { name: 'Open chat panel' }).click()
22+
await page.getByRole('button', { name: 'Open chat panel' }).press('Enter')
2223
const panel = page.getByRole('complementary', { name: 'Chat panel' })
24+
await expect(panel.getByRole('button', { name: 'New panel tab' })).toBeFocused()
2325
await panel.getByRole('button', { name: 'Terminal Run commands in this session' }).click()
24-
await expect(panel.getByRole('tab', { selected: true })).toHaveAccessibleName('Terminal')
26+
await panel.getByRole('button', { name: 'Panel tabs' }).click()
27+
await expect(page.getByRole('menuitemcheckbox', { name: 'Terminal' })).toBeChecked()
28+
await page.keyboard.press('Escape')
2529
await expect(panel.getByRole('region', { name: 'Terminal history' })).toContainText(
2630
'customer-operations'
2731
)
@@ -39,6 +43,7 @@ test('the actual desktop renderer shares product routes, reserves native chrome
3943
).toBeCloseTo(1.25, 1)
4044
await page.screenshot({ path: testInfo.outputPath('desktop-chat.png') })
4145
await page.getByRole('button', { name: 'Collapse sidebar' }).click()
46+
await expect(panel.getByRole('tab', { selected: true })).toHaveAccessibleName('Linear')
4247
await panel.getByRole('button', { name: 'Expand panel' }).click()
4348
const navigation = await page.getByRole('button', { name: 'Expand sidebar' }).boundingBox()
4449
const firstTab = panel.getByRole('tab', { name: 'Terminal' })
@@ -48,11 +53,7 @@ test('the actual desktop renderer shares product routes, reserves native chrome
4853
await firstTab.click()
4954
await expect(panel.getByRole('region', { name: 'Terminal history' })).toBeVisible()
5055
await panel.getByRole('button', { name: 'Restore split panel' }).click()
51-
await app.evaluate(({ BrowserWindow }) =>
52-
BrowserWindow.getAllWindows()
53-
.find((w) => w.isVisible())
54-
?.setFullScreen(true)
55-
)
56+
await window.evaluate((native) => native.setFullScreen(true))
5657
await expect(page.locator('html')).toHaveAttribute('data-sim-desktop-title-bar', 'fullscreen')
5758
await panel.getByRole('tab', { name: 'Terminal' }).click()
5859
await expect(panel.getByRole('region', { name: 'Terminal history' })).toBeVisible()

‎scripts/check-platform-design.test.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,11 @@ it('rejects design drift across real JSX, CSS and Paper ownership boundaries', (
219219
'export const Banner = () => <p>Email delivery is disabled in this preview.</p>'
220220
)
221221
const feature = 'platform/app/settings/_components'
222+
write(
223+
`${feature}/icon.tsx`,
224+
'export const Glyph = () => <svg aria-hidden="true"><path d="M0 0h8" /></svg>'
225+
)
226+
write(`${feature}/ink.module.css`, '.row svg path { stroke: currentColor; }')
222227
write(
223228
`${feature}/trigger.tsx`,
224229
`import styles from '@/platform/design-system/components/button/button.module.css'; export const Trigger = () => <button className={styles.button}/>`
@@ -292,6 +297,8 @@ it('rejects design drift across real JSX, CSS and Paper ownership boundaries', (
292297
expect(result.output).toContain(
293298
'platform/design-system/components/menu/chrome.ts [private-recipe]'
294299
)
300+
expect(result.output).toContain(`${feature}/icon.tsx:1 [icon-owner]`)
301+
expect(result.output).toContain(`${feature}/ink.module.css:1 [icon-owner]`)
295302
for (const issue of [
296303
'legacy-import',
297304
'consumer-style',

‎scripts/check-platform-design.ts‎

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,20 @@ function checkCss(file: string, sheet: postcss.Root) {
325325
}
326326
})
327327
}
328-
sheet.walkDecls(({ prop, value, source }) => {
328+
sheet.walkDecls((declaration) => {
329+
const { prop, value, source, parent } = declaration
330+
if (
331+
privateRoute(file) &&
332+
/^(?:stroke(?:-width)?|fill)$/.test(prop) &&
333+
parent?.type === 'rule' &&
334+
parent.selectors.some((selector) => /\bsvg\s|\[data-interface-icon\]/.test(selector))
335+
) {
336+
report(
337+
'icon-owner',
338+
`${file}:${source?.start?.line ?? 1}`,
339+
'Use the shared glyph ink and stroke; put chart paint on its own marks'
340+
)
341+
}
329342
if (prop === 'composes') {
330343
const from = value.match(/\bfrom\s+['"]([^'"]+)['"]/u)?.[1]
331344
if (from) checkImport(file, from)
@@ -508,6 +521,23 @@ function checkJs(file: string, content: string) {
508521
report('prop-spread', location(node), 'Pass named props explicitly')
509522
if (inPlatform && (ts.isJsxOpeningElement(node) || ts.isJsxSelfClosingElement(node))) {
510523
const tag = node.tagName.getText(sf)
524+
if (
525+
tag === 'svg' &&
526+
file !== `${system}components/icon/icon.tsx` &&
527+
!file.startsWith(catalog) &&
528+
!preview &&
529+
!node.attributes.properties.some(
530+
(attr) =>
531+
ts.isJsxAttribute(attr) &&
532+
['aria-label', 'aria-labelledby'].includes(attr.name.getText(sf))
533+
)
534+
) {
535+
report(
536+
'icon-owner',
537+
location(node),
538+
'Use Icon for interface glyphs; give an original data graphic an accessible name'
539+
)
540+
}
511541
if (tag === 'select')
512542
report('native-picker', location(node), 'Use the shared Platform value picker')
513543
const native = /^[a-z][\w-]*$/.test(tag) || (owner && primitives.has(tag.split('.')[0]))

0 commit comments

Comments
 (0)