Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions apps/web/src/comms/PresenceDot.stories.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import type { Meta, StoryObj } from '@storybook/react-vite'
import type { PresenceStatus } from '@xnetjs/comms'
import { PresenceDot } from './PresenceDot'

/**
* Story coverage for the chat building blocks (exploration 0200): the chat
* surface itself only renders behind the parameterized /channel/$channelId
* route, but its primitives render from plain props, so a co-located story
* gives the visual-capture pipeline a stable, seed-free baseline to diff.
*/
const meta = {
title: 'Web/Comms/PresenceDot',
component: PresenceDot,
args: { status: 'active', ring: true }
} satisfies Meta<typeof PresenceDot>

export default meta

type Story = StoryObj<typeof meta>

export const Active: Story = {}

const STATUSES: (PresenceStatus | undefined)[] = ['active', 'idle', 'dnd', undefined]

export const AllStatuses: Story = {
render: () => (
<div className="flex items-center gap-6">
{STATUSES.map((status) => (
<div key={status ?? 'offline'} className="flex flex-col items-center gap-2">
<PresenceDot status={status} />
<span className="text-xs text-ink-3">{status ?? 'offline'}</span>
</div>
))}
</div>
)
}
42 changes: 42 additions & 0 deletions apps/web/src/comms/ReactionBar.stories.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import type { ProfileEntry } from './hooks'
import type { ReactionGroup } from './reactions'
import type { Meta, StoryObj } from '@storybook/react-vite'
import { ReactionBar } from './ReactionBar'

/**
* Story coverage for the chat building blocks (exploration 0200). ReactionBar
* renders from plain props (reaction groups + profiles), so it gives the
* visual-capture pipeline a stable, seed-free baseline for the emoji reaction
* pills that PR #174 shipped — without booting the app or seeding a channel.
*/
const PROFILES: ProfileEntry[] = [
{ did: 'did:key:zAlice', name: 'Alice' },
{ did: 'did:key:zBob', name: 'Bob' },
{ did: 'did:key:zCara', name: 'Cara' }
]

const GROUPS: ReactionGroup[] = [
{ emoji: '👍', count: 3, mine: true, myReactionId: 'r1', reactors: PROFILES.map((p) => p.did) },
{ emoji: '🎉', count: 1, mine: false, reactors: ['did:key:zBob'] },
{ emoji: '🚀', count: 2, mine: false, reactors: ['did:key:zAlice', 'did:key:zCara'] }
]

const meta = {
title: 'Web/Comms/ReactionBar',
component: ReactionBar,
args: { groups: GROUPS, profiles: PROFILES, onToggle: () => {} }
} satisfies Meta<typeof ReactionBar>

export default meta

type Story = StoryObj<typeof meta>

export const Default: Story = {}

export const SingleReaction: Story = {
args: {
groups: [
{ emoji: '❤️', count: 1, mine: true, myReactionId: 'r9', reactors: ['did:key:zAlice'] }
]
}
}
602 changes: 602 additions & 0 deletions docs/explorations/0200_[x]_VISUAL_CAPTURE_SILENT_COVERAGE_GAPS.md

Large diffs are not rendered by default.

15 changes: 14 additions & 1 deletion scripts/visuals/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ the corresponding sticky comment to a tombstone so no broken image survives.
| `lib/static-server.mjs` | Zero-dep static server for the Storybook iframe |
| `flows.mjs` | Interaction-flow runners, keyed to `manifests.json` flow ids |
| `manifests.json` | Maps source globs → app routes and interaction flows |
| `lib/manifest-coverage.test.mjs` | Drift guard: every singleton route must be mapped (or `EXEMPT`) |
| `lib/manifest-coverage.test.mjs` | Drift guard: every singleton route mapped (or `EXEMPT`); every `$`‑route flow‑covered (or `PARAM_EXEMPT`) |

## Tuning

Expand All @@ -66,6 +66,19 @@ the corresponding sticky comment to a tombstone so no broken image survives.
ever captures the top of funnel. `lib/manifest-coverage.test.mjs` **fails** if a
new singleton route is left unmapped (or not explicitly `EXEMPT`), so this isn't
optional. Background: [`docs/explorations/0191`](../../docs/explorations/0191_%5B_%5D_VISUAL_CAPTURE_MISSES_UNMAPPED_AND_INTERACTION_GATED_SURFACES.md).
- **Parameterized routes** (`name.$param.tsx`, e.g. `/channel/$channelId`) can
**never** be captured as a static URL — they need a real id + seed data, so they
are invisible to the route capturer. Each **must** be reachable by a `flows[]`
runner whose globs include the route file, or be listed in `PARAM_EXEMPT` (with
a reason) in `lib/manifest-coverage.test.mjs` — which **fails** otherwise. This
was the chat‑redesign blind spot. Background:
[`docs/explorations/0200`](../../docs/explorations/0200_%5Bx%5D_VISUAL_CAPTURE_SILENT_COVERAGE_GAPS.md).
- **The coverage‑gap warning**: when changed UI files map to *no* story/route/flow,
capture falls back to the `home` shell and `computeCaptureSet` sets
`fallbackUsed`/`unmappedFiles`. The PR comment then shows a `> [!WARNING]` listing
the unmapped files instead of the misleading "No visual differences detected" —
so a coverage gap reads as a TODO, not a no‑op. If you see that warning on your
PR, add a route/flow mapping for the files it lists.
- **Don't broaden `home`**: keep its globs to the shell (`index`/`__root`/`App`/
`workbench`). A broad `apps/web/src/components/**` glob false‑matches every
domain surface onto `/`, hiding the real diff (the 0191 bug); generic UI changes
Expand Down
11 changes: 10 additions & 1 deletion scripts/visuals/capture.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,16 @@ const VIEWPORT = { width: 1280, height: 800 }
const NO_MOTION = `*,*::before,*::after{transition:none!important;animation:none!important;
caret-color:transparent!important;scroll-behavior:auto!important}`

const manifest = { stories: [], routes: [], flows: [] }
// Carry the coverage-gap signal (exploration 0200) from the capture set through
// to the diff/comment stages: if `home` is here only because nothing specific
// matched, the comment flags it instead of reporting "no visual differences".
const manifest = {
stories: [],
routes: [],
flows: [],
fallbackUsed: set.fallbackUsed ?? false,
unmappedFiles: set.unmappedFiles ?? []
}
mkdirSync(outDir, { recursive: true })

async function settle(page) {
Expand Down
4 changes: 3 additions & 1 deletion scripts/visuals/changed-capture-set.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,9 @@ if (all) {
label: r.label,
path: r.path
})),
flows: []
flows: [],
fallbackUsed: false,
unmappedFiles: []
}
} else {
changedFiles = diffFile
Expand Down
28 changes: 28 additions & 0 deletions scripts/visuals/comment.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,34 @@ export function buildBody(manifest, { baseUrl, runUrl } = {}) {

const out = [MARKER, '## 🖼️ UI changes in this PR', '']

// Coverage-gap signal (exploration 0200): the changed UI files mapped to no
// story/route/flow, so only the home shell was captured -- and it diffs clean.
// Say so loudly instead of the misleading "no visual differences", which made
// big UI changes (e.g. PR #174's chat redesign) look like no-ops.
const unmapped = manifest.unmappedFiles ?? []
if (manifest.fallbackUsed && total === 0) {
out.push(
'> [!WARNING]',
`> **${unmapped.length} changed UI file(s) map to no capture target.**`,
'> Only the home shell was captured, so the surface you changed is not shown',
'> here. Add a `routes[]` entry — or a `flows[]` entry + runner if the UI is',
'> behind a tab/inspector/modal/seed data — in `scripts/visuals/manifests.json`',
'> (`scripts/visuals/README.md` → Tuning).'
)
if (unmapped.length) {
out.push(
'',
'<details><summary>Unmapped files</summary>',
'',
...unmapped.map((f) => `- \`${f}\``),
'',
'</details>'
)
}
if (runUrl) out.push('', `<sub>[CI run](${runUrl})</sub>`)
return out.join('\n')
}

if (total === 0) {
out.push('_No visual differences detected in the changed UI._')
if (runUrl) out.push('', `<sub>[CI run](${runUrl})</sub>`)
Expand Down
3 changes: 3 additions & 0 deletions scripts/visuals/diff.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,9 @@ for (const r of manifest.routes ?? []) routes.push(await classifyStill(r))
const result = {
threshold,
baseline: baselineDir || baselineUrl || null,
// Pass the coverage-gap signal through to the comment (exploration 0200).
fallbackUsed: manifest.fallbackUsed ?? false,
unmappedFiles: manifest.unmappedFiles ?? [],
stories,
routes,
flows: manifest.flows ?? [], // videos always pass through
Expand Down
65 changes: 65 additions & 0 deletions scripts/visuals/flows.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -89,5 +89,70 @@ export const FLOWS = {
await tryClick(/Deal details/i)
await wait(page, 1200)
}
},

// The redesigned chat surface (exploration 0198, PR #174) only renders at the
// parameterized /channel/$channelId route, behind a seeded channel + messages,
// so no static route shot can see it -- this flow seeds it. Every step is
// best-effort (a missing control must not abort the recording): open the Chats
// panel from the rail, create a channel, post two messages (to show grouping),
// then hover a row to reveal the action toolbar, react, and open the thread.
chat: {
label: 'Open a channel and post a message',
async run(page) {
const tryClick = async (target) => {
try {
await target.click({ timeout: 5000 })
} catch {
/* best-effort */
}
}
const byLabel = (name) => page.getByRole('button', { name }).first()

// Open the left "Chats" panel from the 44px rail (aria-label="Chats").
await tryClick(byLabel(/^Chats$/))
await wait(page, 500)

// "New channel" (+) -> type a name -> Enter creates the channel.
await tryClick(byLabel('New channel'))
const nameInput = page.getByPlaceholder(/channel name/i)
try {
await nameInput.fill('visual-demo', { timeout: 4000 })
await nameInput.press('Enter')
} catch {
/* the panel may already have a channel to open */
}
await wait(page, 800)

// Open the channel row we just made (falls back to any channel row).
await tryClick(byLabel(/visual-demo/i))
await page.waitForURL(/\/channel\//, { timeout: 15_000 }).catch(() => {})
await wait(page, 600)

// Post two messages so grouping + the feed redesign are visible.
const composer = page.getByPlaceholder(/Message/i).first()
try {
await composer.click({ timeout: 5000 })
await composer.type('Visual capture demo — first message.', { delay: 25 })
await composer.press('Enter')
await composer.type('And a second, to show message grouping.', { delay: 25 })
await composer.press('Enter')
} catch {
/* composer may be gated; the channel shell is still worth recording */
}
await wait(page, 600)

// Hover the latest row to reveal the action toolbar, then react + reply.
try {
const row = page.getByRole('listitem').last()
await row.hover({ timeout: 4000 })
await wait(page, 300)
await tryClick(byLabel(/add reaction|react/i))
await tryClick(byLabel(/reply|thread/i))
} catch {
/* hover/toolbar is cosmetic */
}
await wait(page, 1200)
}
}
}
36 changes: 24 additions & 12 deletions scripts/visuals/lib/capture-set.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,7 @@
/** Strip a leading `./` and normalize separators so git paths and Storybook
* `importPath`s compare equal. */
export function normalizePath(p) {
return String(p)
.replace(/\\/g, '/')
.replace(/^\.\//, '')
return String(p).replace(/\\/g, '/').replace(/^\.\//, '')
}

/**
Expand Down Expand Up @@ -87,24 +85,38 @@ export function computeCaptureSet(input, opts = {}) {
.filter((route) => changed.some((f) => matchesAny(f, route.globs)))
.map((route) => ({ kind: 'route', id: route.id, label: route.label, path: route.path }))

// Fallback: web UI changed but nothing route-specific matched -> capture home
// so the reviewer still sees the shell the change lives in.
const webUiChanged = changed.some((f) => webUiPattern.test(f))
if (webUiChanged && routes.length === 0) {
const home = routeManifest.find((r) => r.id === homeRouteId)
if (home) routes.push({ kind: 'route', id: home.id, label: home.label, path: home.path })
}

// --- Flows: any changed file matches the flow's globs. ---
const flows = flowManifest
.filter((flow) => changed.some((f) => matchesAny(f, flow.globs)))
.map((flow) => ({ kind: 'flow', id: flow.id, label: flow.label }))

// Fallback: web UI changed but NOTHING specific matched (no route, no story,
// no flow) -> capture home so the reviewer still sees the shell the change
// lives in, AND record why. Without this signal the home shot diffs clean
// against the baseline and the comment reports "no visual differences" -- a
// coverage gap made indistinguishable from a no-op (exploration 0200, the
// PR #174 chat-redesign miss). The comment uses `fallbackUsed`/`unmappedFiles`
// to flag the gap instead. Tightened from the old `routes.length === 0`: a
// story or flow match is "something specific", so home is no longer piled on.
const webUiChanged = changed.some((f) => webUiPattern.test(f))
let fallbackUsed = false
let unmappedFiles = []
if (webUiChanged && routes.length === 0 && stories.length === 0 && flows.length === 0) {
const home = routeManifest.find((r) => r.id === homeRouteId)
if (home) {
routes.push({ kind: 'route', id: home.id, label: home.label, path: home.path })
fallbackUsed = true
unmappedFiles = changed.filter((f) => webUiPattern.test(f)).sort()
}
}

const byId = (a, b) => String(a.id).localeCompare(String(b.id))
return {
stories: stories.sort(byId),
routes: routes.sort(byId),
flows: flows.sort(byId)
flows: flows.sort(byId),
fallbackUsed,
unmappedFiles
}
}

Expand Down
59 changes: 58 additions & 1 deletion scripts/visuals/lib/capture-set.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ test('routes and flows match by glob', () => {
)
})

test('web UI change with no specific route falls back to home', () => {
test('web UI change with no specific route falls back to home + flags the gap (0200)', () => {
const set = computeCaptureSet({
changedFiles: ['apps/web/src/components/Widget.tsx'],
storyEntries: [],
Expand All @@ -120,6 +120,63 @@ test('web UI change with no specific route falls back to home', () => {
set.routes.map((r) => r.id),
['home']
)
// The fallback must announce itself so the comment can flag the coverage gap
// instead of silently reporting "no visual differences" (the PR #174 miss).
assert.equal(set.fallbackUsed, true)
assert.deepEqual(set.unmappedFiles, ['apps/web/src/components/Widget.tsx'])
})

test('a matched story suppresses the home fallback — not a coverage gap (0200)', () => {
// packages/ui change WITH a story: the story is "something specific", so we do
// NOT also pile on the home shell, and the gap signal stays off.
const set = computeCaptureSet({
changedFiles: ['packages/ui/src/primitives/Button.tsx'],
storyEntries: STORIES,
routeManifest: [
{ id: 'home', label: 'Home', path: '/', globs: ['apps/web/src/routes/index.tsx'] }
],
flowManifest: FLOWS
})
assert.deepEqual(
set.stories.map((s) => s.id),
['ui-primitives-button--default']
)
assert.deepEqual(
set.routes.map((r) => r.id),
[]
)
assert.equal(set.fallbackUsed, false)
assert.deepEqual(set.unmappedFiles, [])
})

test('a matched flow suppresses the home fallback — not a coverage gap (0200)', () => {
// An editor change matches the create-page flow but no route: the flow is the
// capture, so no home fallback and no gap warning.
const set = computeCaptureSet({
changedFiles: ['packages/editor/src/Editor.tsx'],
storyEntries: [],
routeManifest: [
{ id: 'home', label: 'Home', path: '/', globs: ['apps/web/src/routes/index.tsx'] }
],
flowManifest: FLOWS
})
assert.deepEqual(
set.flows.map((f) => f.id),
['create-page']
)
assert.equal(set.fallbackUsed, false)
assert.deepEqual(set.unmappedFiles, [])
})

test('a non-UI change sets no fallback and no unmapped files (0200)', () => {
const set = computeCaptureSet({
changedFiles: ['packages/core/src/store.ts'],
storyEntries: STORIES,
routeManifest: ROUTES,
flowManifest: FLOWS
})
assert.equal(set.fallbackUsed, false)
assert.deepEqual(set.unmappedFiles, [])
})

test('a non-UI change captures nothing', () => {
Expand Down
Loading
Loading