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
37 changes: 37 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,43 @@ Before commit/push/PR: use the **`pre-push-review`** skill (`~/.cursor/skills/pr
- **Permission modes**: `default`, `acceptEdits`, `bypassPermissions`, `plan`
- **Namespaces**: Multi-user isolation via `CLI_API_TOKEN:<namespace>` suffix

## Adding new web features — consider an FUE

When you ship a non-essential feature (the 20% of sessions, not the 80%), consider wrapping its affordance in the generic First-User-Experience primitive so existing users discover it without a giant always-visible UI block.

- **Hook**: `web/src/lib/use-fue.ts` — `useFue(featureId)` returns `{ status, engage, dismiss }`. Storage namespace `hapi.fue.v1.<featureId>` (one localStorage key per feature, isolated from any upstream onboarding flow).
- **Components**: `web/src/components/Fue.tsx` — `<FueDot>` (small pulsing badge for the affordance) and `<FueCallout>` (portal-rendered popover with title/body + "Got it" affirmative-action dismiss).

Pattern (~10 lines around the affordance):

```tsx
const fue = useFue('my-feature')
const buttonRef = useRef<HTMLButtonElement>(null)
return (
<>
<button ref={buttonRef} onClick={() => { fue.engage(); doThing() }}>
<Icon />
{fue.status !== 'acknowledged' ? <FueDot pulsing={fue.status === 'unseen'} /> : null}
</button>
{fue.status === 'engaging' ? (
<FueCallout
title={t('myFeature.fueTitle')}
body={t('myFeature.fueBody')}
onDismiss={fue.dismiss}
anchorRef={buttonRef}
/>
) : null}
</>
)
```

Rules:
- Affirmative action only: there is no auto-timeout — user dismisses by clicking "Got it" (reading speed varies).
- The FUE dot and any feature-specific badge (e.g. an entry counter) should be **mutually exclusive**: onboarding signal beats inventory signal until acknowledged.
- Storage is opt-in per-feature; if upstream ships its own onboarding for a feature, just don't wrap that affordance.

Canonical example: scratchlist toggle in `web/src/components/AssistantChat/ComposerButtons.tsx` (`ScratchlistToggleButton`).

## Critical Thinking

1. Fix root cause (not band-aid).
Expand Down
85 changes: 85 additions & 0 deletions web/src/components/AssistantChat/ComposerButtons.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
import type { ReactElement } from 'react'
import { cleanup, render, screen } from '@testing-library/react'
import { afterEach, describe, expect, it } from 'vitest'
import { I18nProvider } from '@/lib/i18n-context'
import { UnifiedButton } from './ComposerButtons'

function renderInProviders(ui: ReactElement) {
return render(<I18nProvider>{ui}</I18nProvider>)
}

/**
* Regression tests for upstream review on PR #798
* (github-actions[bot] [Major]: "Send button advertises scratchlist
* routing even when the submit will go to chat").
*
* UnifiedButton's visible state (amber + "Send to scratchlist" label
* vs. black + "Send message" label) MUST reflect the actual routing
* decision rather than the raw scratchlist toggle. Callers are
* responsible for computing routesToScratchlist from
* (mode, attachments, schedule); these tests pin the contract that
* routesToScratchlist=false drives the chat-style render.
*/

function getButton(label: RegExp | string): HTMLButtonElement {
return screen.getByRole('button', { name: label }) as HTMLButtonElement
}

describe('UnifiedButton — routesToScratchlist visual state', () => {
const noop = () => {}

afterEach(() => {
cleanup()
})

it('paints amber + announces "Send to scratchlist" when routesToScratchlist=true', () => {
renderInProviders(
<UnifiedButton
canSend
voiceStatus="disconnected"
voiceEnabled={false}
controlsDisabled={false}
onSend={noop}
onVoiceToggle={noop}
routesToScratchlist
/>,
)
const btn = getButton(/scratchlist/i)
expect(btn.className).toContain('bg-amber-500')
})

it('paints chat black + announces "Send" when routesToScratchlist=false even if scratchlist toggle conceptually on', () => {
// Caller computed routesToScratchlist=false because the payload
// would carry attachments or a pending schedule. The button must
// therefore look like a normal chat send.
renderInProviders(
<UnifiedButton
canSend
voiceStatus="disconnected"
voiceEnabled={false}
controlsDisabled={false}
onSend={noop}
onVoiceToggle={noop}
routesToScratchlist={false}
/>,
)
const btn = getButton('Send')
expect(btn.className).not.toContain('bg-amber-500')
expect(btn.className).toContain('bg-black')
})

it('defaults routesToScratchlist to false when omitted', () => {
renderInProviders(
<UnifiedButton
canSend
voiceStatus="disconnected"
voiceEnabled={false}
controlsDisabled={false}
onSend={noop}
onVoiceToggle={noop}
/>,
)
const btn = getButton('Send')
expect(btn.className).not.toContain('bg-amber-500')
})
})
184 changes: 176 additions & 8 deletions web/src/components/AssistantChat/ComposerButtons.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import { useTranslation } from '@/lib/use-translation'
import { ScheduleIcon } from '@/components/icons'
import { ScheduleTimePicker } from './ScheduleTimePicker'
import type { PendingSchedule } from './ScheduleTimePicker'
import { useFue } from '@/lib/use-fue'
import { FueCallout, FueDot } from '@/components/Fue'
import { useRef, useState } from 'react'

function VoiceAssistantIcon() {
Expand Down Expand Up @@ -198,6 +200,101 @@ function SendIcon() {
)
}

function ScratchlistToggleIcon() {
return (
<svg
xmlns="http://www.w3.org/2000/svg"
width="18"
height="18"
viewBox="0 0 16 16"
fill="none"
stroke="currentColor"
strokeWidth="1.5"
strokeLinecap="round"
strokeLinejoin="round"
>
<path d="M3.5 2.5h6L12.5 5.5v8a1 1 0 0 1-1 1h-8a1 1 0 0 1-1-1v-10a1 1 0 0 1 1-1Z" />
<path d="M9.5 2.5v3h3M5 8.5h6M5 11h4" />
</svg>
)
}

/**
* ScratchlistToggleButton — composer affordance for toggling scratchlist mode,
* wrapped in the generic FUE (First-User Experience) primitive so a new
* operator sees a pulsing dot + a one-time explainer popover the first time
* they encounter the feature; once they engage with it, the dot disappears
* for good and the entry counter takes over.
*
* The FUE wiring here is the canonical example for future features: the
* pattern is "wrap the affordance in useFue + FueDot, conditionally render
* FueCallout while engaging". See web/src/lib/use-fue.ts for the contract.
*/
function ScratchlistToggleButton(props: {
scratchlistMode: boolean
scratchlistCount: number
onScratchlistToggle: () => void
controlsDisabled?: boolean
}) {
const { t } = useTranslation()
const fue = useFue('scratchlist-toggle')
const buttonRef = useRef<HTMLButtonElement>(null)

const showFueDot = fue.status !== 'acknowledged'
// Counter and FUE dot are mutually exclusive (see FueDot doc comment).
// Onboarding signal beats inventory signal: the user can't read the
// counter as "you have N items" until they understand the feature.
const showCounter = !showFueDot && props.scratchlistCount > 0

return (
<>
<button
ref={buttonRef}
type="button"
aria-label={t('scratchlist.toggleAriaLabel')}
title={t('scratchlist.toggleTooltip')}
aria-pressed={props.scratchlistMode ? true : false}
disabled={props.controlsDisabled}
onClick={() => {
fue.engage()
props.onScratchlistToggle()
}}
className={`relative flex h-8 w-8 items-center justify-center rounded-full transition-colors disabled:cursor-not-allowed disabled:opacity-50 ${
props.scratchlistMode
? 'bg-amber-500 text-white hover:bg-amber-600'
: 'text-[var(--app-fg)]/60 hover:bg-[var(--app-bg)] hover:text-[var(--app-fg)]'
}`}
>
<ScratchlistToggleIcon />
{showFueDot ? (
<FueDot
pulsing={fue.status === 'unseen'}
ariaLabel={t('fue.newFeatureDot')}
/>
) : null}
{showCounter ? (
<span
aria-hidden="true"
className="absolute -top-0.5 -right-0.5 min-w-[12px] h-3 px-[3px] flex items-center justify-center rounded-full bg-amber-500 text-white text-[8px] font-semibold leading-none tabular-nums shadow-sm"
>
{props.scratchlistCount > 99 ? '99+' : props.scratchlistCount}
</span>
) : null}
</button>
{fue.status === 'engaging' ? (
<FueCallout
title={t('scratchlist.fueTitle')}
body={t('scratchlist.fueBody')}
onDismiss={fue.dismiss}
dismissLabel={t('fue.gotIt')}
closeAriaLabel={t('fue.closeAriaLabel')}
anchorRef={buttonRef}
/>
) : null}
</>
)
}

function StopIcon() {
return (
<svg
Expand Down Expand Up @@ -230,34 +327,47 @@ function LoadingIcon() {
)
}

function UnifiedButton(props: {
export function UnifiedButton(props: {
canSend: boolean
voiceStatus: ConversationStatus
voiceEnabled: boolean
controlsDisabled: boolean
onSend: () => void
onVoiceToggle: () => void
/**
* When true, the send button repaints amber and the aria-label
* announces "Send to scratchlist" instead of "Send message". The
* actual routing happens in SessionChat's wrapped onSend - the
* button itself is content-agnostic.
*
* Caller MUST compute this from the actual routing decision (mode
* AND no-attachments AND no-pending-schedule), not the raw
* scratchlist toggle. If the toggle is on but the submission would
* fall back to chat (because the scratchlist can't represent the
* payload), the button must look like a normal chat send. Per
* upstream review on PR #798: [Major] "Send button advertises
* scratchlist routing even when the submit will go to chat".
*/
routesToScratchlist?: boolean
}) {
const { t } = useTranslation()

// Determine button state
const isConnecting = props.voiceStatus === 'connecting'
const isConnected = props.voiceStatus === 'connected'
const isVoiceActive = isConnecting || isConnected
const hasText = props.canSend
const routesToScratchlist = props.routesToScratchlist ?? false

// Determine button behavior
const handleClick = () => {
if (isVoiceActive) {
props.onVoiceToggle() // Stop voice
} else if (hasText) {
props.onSend() // Send message
} else if (props.voiceEnabled) {
props.onVoiceToggle() // Start voice
props.onSend() // Send message (or scratchlist add — wrapper decides)
} else if (props.voiceEnabled && !routesToScratchlist) {
props.onVoiceToggle() // Start voice (suppressed in scratchlist mode)
}
}

// Determine button style and icon
let icon: React.ReactNode
let className: string
let ariaLabel: string
Expand All @@ -270,6 +380,13 @@ function UnifiedButton(props: {
icon = <StopIcon />
className = 'bg-black text-white'
ariaLabel = t('composer.stop')
} else if (routesToScratchlist) {
// Amber send button - matches the scratchlist drawer accent.
// Single visual signal carries the "this goes to the scratchlist"
// contract; without it, the modal state is invisible to the user.
icon = <SendIcon />
className = 'bg-amber-500 text-white hover:bg-amber-600'
ariaLabel = t('scratchlist.sendToScratchlist')
} else if (hasText) {
icon = <SendIcon />
className = 'bg-black text-white'
Expand All @@ -284,7 +401,16 @@ function UnifiedButton(props: {
ariaLabel = t('composer.send')
}

const isDisabled = props.controlsDisabled || (!hasText && !props.voiceEnabled && !isVoiceActive)
// When the submission routes to scratchlist the send button is the
// only path that does anything useful, so it must be enabled whenever
// there is text - we deliberately do NOT fall back to voice-toggle-on-
// empty-text. (When attachments / schedule force a chat fallback the
// normal chat-send disable rules apply.)
const isDisabled = props.controlsDisabled || (
routesToScratchlist
? !hasText
: !hasText && !props.voiceEnabled && !isVoiceActive
)

return (
<button
Expand Down Expand Up @@ -331,6 +457,13 @@ export function ComposerButtons(props: {
// The composer must surface that constraint at UI time so the user never
// builds a submission the hub will reject — see hub/web/routes/messages.ts.
hasAttachments?: boolean
// Scratchlist drawer toggle. When `onScratchlistToggle` is provided, a
// notepad icon appears next to the schedule-send icon. Click toggles
// composer-send-routing between chat and scratchlist; SessionChat owns
// the actual routing decision via its wrapped onSend.
scratchlistMode?: boolean
scratchlistCount?: number
onScratchlistToggle?: () => void
}) {
const { t } = useTranslation()
const isVoiceConnected = props.voiceStatus === 'connected'
Expand Down Expand Up @@ -420,6 +553,26 @@ export function ComposerButtons(props: {
</button>
) : null}

{/*
* Scratchlist toggle - prototype of the composer-controlled
* drawer (replaces the always-visible orange band). Counter
* shown only when entries exist (>0); empty-state shows just
* the icon to avoid the "you have 0 things" guilt UI.
*
* Clicking enters scratchlist mode: the send button repaints
* amber and SessionChat's wrapped onSend routes the next
* submission to addScratchlistEntry() instead of the chat.
* Mode is sticky - operator clicks the icon again to exit.
*/}
{props.onScratchlistToggle ? (
<ScratchlistToggleButton
scratchlistMode={props.scratchlistMode ?? false}
scratchlistCount={props.scratchlistCount ?? 0}
onScratchlistToggle={props.onScratchlistToggle}
controlsDisabled={props.controlsDisabled}
/>
) : null}

{/* Schedule button — only shown when onSchedule handler is provided */}
{props.onSchedule ? (
<>
Expand Down Expand Up @@ -466,6 +619,21 @@ export function ComposerButtons(props: {
controlsDisabled={props.controlsDisabled}
onSend={props.onSend}
onVoiceToggle={props.onVoiceToggle}
/*
* Derived, NOT raw scratchlistMode. Mirror SessionChat's
* shouldRouteToScratchlist so the visible send-button state
* matches the actual routing decision: amber + "Send to
* scratchlist" only when mode is on AND the payload would
* be a pure-text scratchlist add. Attachments or a pending
* schedule force a chat fallback in onSendForComposer; the
* button must reflect that, otherwise the UI lies about
* where the user's content is going.
*/
routesToScratchlist={
(props.scratchlistMode ?? false)
&& !hasAttachments
&& props.pendingSchedule == null
}
/>
</div>
)
Expand Down
Loading
Loading