Make Piggy part of the product rather than a guest in it
Piggy arrived as a chat panel bolted onto a CRM and then grew a workspace around it. The layout was already right — the audit found the approval card to be the best-designed object in the repo, and the account page's empty panels less finished than anything in the workspace. What was wrong was vocabulary: nobody had written the small things down, so both halves kept inventing them. Piggy was drawn with five different marks — a pig in the dock, a sparkle in the sidebar and again on the model picker, a speech bubble on the Ask buttons, and a stock robot glyph on every assistant message, which is the one people look at most. There is now one mark. The composer, which is the first control in the product since sign-in lands on /piggy, was the only un-adapted shadcn field left: 6px radius against a 12px Send button it sat 8px from. A stat tile had been reinvented six times at three numeral scales, and the same uppercase micro-label existed in five variants, two of them one tab apart in the same rail. There were 63 hand-written font sizes: not a scale, sixty-three opinions. Underneath that, the focus ring was invisible. The global rule used ring-accent, which Tailwind deliberately aliases onto the hover tint, so the ring measured 1.01:1 against the light canvas — no visible focus indicator anywhere in the product, for any accent, in either theme. It is ring-brand now and measures 17:1. The warning, positive and info tones were darkened until each clears 4.5:1 on a card, on inset and on its own chip, and the light canvas moved to 98% so a card lifts without leaning on its shadow. The mobile work is the part worth reading. A landscape phone gave the transcript 28% of the viewport and a keyboard-up phone 16%, against a 45% floor — and the fixed tab bar painted over the composer, covering the safety sentence and half the Send button, because two source comments asserted the bar stood down on short viewports and it never had. Both fixed and measured by hit-testing rather than by screenshot. The composer itself was 64px tall for a blank second line nobody typed, because the auto-resize effect sizes to scrollHeight and scrollHeight counts rows — a CSS height could not win against an inline style, so the attribute was the honest lever. Verified across both themes driven through the app's own control: no horizontal overflow on 15 routes at four viewports, 672 stat values that fit, 297 labels at exactly 11px/500, Escape returning focus to its opener rather than the body on every overlay, and a rejected write no longer reporting "Succeeded" with a green check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -3,7 +3,7 @@
|
||||
*
|
||||
* This is the only control in PIG that decides whether a language model may
|
||||
* write to the company's book, so it is written to be read rather than to be
|
||||
* clever. Three things follow from that and are deliberate:
|
||||
* clever. Four things follow from that and are deliberate:
|
||||
*
|
||||
* names — the segments say "Read only", "Ask first" and "Auto", not
|
||||
* `read_only` / `confirm` / `auto`. The enum is the wire's language
|
||||
@@ -12,6 +12,10 @@
|
||||
* selected NOW, and changes as the selection does. A toggle whose
|
||||
* meaning lives in documentation is a toggle people set once and then
|
||||
* misremember.
|
||||
* the keyboard — the arrows move focus and do NOT select. The ARIA pattern
|
||||
* says they should, and for a preference it would be right; for a
|
||||
* permission it meant that arrowing across to read what Auto does
|
||||
* turned Auto on. See `keyboardAt`.
|
||||
* the exceptions — `auto` still stops at a contract, a commitment, an
|
||||
* allocation and anything compliance-shaped. That is `requiresApproval`'s
|
||||
* rule, and if the control does not say so, the first person to choose
|
||||
@@ -32,7 +36,7 @@ import {
|
||||
} from '@pig/core';
|
||||
import { PIGGY_DEFAULT_MODE } from '@/lib/piggy-chat';
|
||||
import { useOptionalIdentity } from '@/lib/identity';
|
||||
import { cn } from '@/components/ui';
|
||||
import { Label, cn } from '@/components/ui';
|
||||
|
||||
// ------------------------------------------------------------------- copy
|
||||
|
||||
@@ -132,6 +136,21 @@ export function PiggyModeControl({
|
||||
const describedBy = useId();
|
||||
const buttons = useRef(new Map<PiggyMode, HTMLButtonElement>());
|
||||
|
||||
/**
|
||||
* Where the keyboard is, which is not the same as what is chosen.
|
||||
*
|
||||
* The ARIA radiogroup pattern normally selects whatever the arrow keys land
|
||||
* on. That is right for a preference and wrong for a permission: arrowing
|
||||
* across to read what Auto does was granting an agent unattended write access
|
||||
* to the book, and the sentence explaining the consequence appeared *because*
|
||||
* the consequence had already been accepted. Here the arrows move focus, the
|
||||
* sentence updates to describe what is under the cursor, and Space or Enter
|
||||
* is what commits. Null means the keyboard is elsewhere and the tabstop
|
||||
* belongs to the selected segment, so tabbing back in returns to the choice
|
||||
* in force rather than to wherever the last arrow press stopped.
|
||||
*/
|
||||
const [keyboardAt, setKeyboardAt] = useState<PiggyMode | null>(null);
|
||||
|
||||
/**
|
||||
* What is drawn as selected. Not necessarily what the parent holds: a stored
|
||||
* `auto` outlives the capability that justified it, so someone whose write
|
||||
@@ -150,40 +169,60 @@ export function PiggyModeControl({
|
||||
|
||||
const choices = MODE_OPTIONS.filter((option) => canWrite || option.value === 'read_only');
|
||||
|
||||
/** Roving tabstop: the keyboard's position if it has one, else the choice. */
|
||||
const roving: PiggyMode = keyboardAt ?? selected;
|
||||
|
||||
const moveTo = useCallback((next: PiggyMode | undefined) => {
|
||||
if (!next) return;
|
||||
setKeyboardAt(next);
|
||||
buttons.current.get(next)?.focus();
|
||||
}, []);
|
||||
|
||||
const step = useCallback(
|
||||
(direction: 1 | -1) => {
|
||||
const index = choices.findIndex((option) => option.value === selected);
|
||||
const next = choices[(index + direction + choices.length) % choices.length];
|
||||
if (!next) return;
|
||||
onChange(next.value);
|
||||
buttons.current.get(next.value)?.focus();
|
||||
const index = choices.findIndex((option) => option.value === roving);
|
||||
moveTo(choices[(index + direction + choices.length) % choices.length]?.value);
|
||||
},
|
||||
[choices, onChange, selected],
|
||||
[choices, moveTo, roving],
|
||||
);
|
||||
|
||||
const active = optionFor(selected);
|
||||
/**
|
||||
* The sentence describes what the keyboard is on, not what is chosen — so
|
||||
* someone arrowing across Auto reads its consequence before deciding, which
|
||||
* is the whole point of no longer selecting on focus.
|
||||
*/
|
||||
const active = optionFor(roving);
|
||||
/** True while the keyboard is reading a mode that has not been chosen. */
|
||||
const previewing = roving !== selected;
|
||||
|
||||
return (
|
||||
<div className={cn('flex min-w-0 flex-col', compact ? 'gap-1.5' : 'gap-2')}>
|
||||
{compact ? null : (
|
||||
<span className="text-xs font-medium uppercase tracking-wide text-muted">
|
||||
What Piggy may do
|
||||
</span>
|
||||
)}
|
||||
{compact ? null : <Label>What Piggy may do</Label>}
|
||||
|
||||
<div
|
||||
role="radiogroup"
|
||||
aria-label="What Piggy may do"
|
||||
aria-describedby={describedBy}
|
||||
className="grid grid-cols-3 gap-1 rounded-xl border border-border bg-surface-2 p-1"
|
||||
className="grid grid-cols-3 gap-1 rounded-lg border border-border bg-surface-2 p-1"
|
||||
onBlur={(event) => {
|
||||
// Leaving the group hands the tabstop back to the chosen segment, so
|
||||
// the next Tab in lands on the mode in force rather than on whichever
|
||||
// one the reader stopped over last time.
|
||||
if (!event.currentTarget.contains(event.relatedTarget)) setKeyboardAt(null);
|
||||
}}
|
||||
onKeyDown={(event) => {
|
||||
if (event.key === 'ArrowRight' || event.key === 'ArrowDown') {
|
||||
event.preventDefault();
|
||||
step(1);
|
||||
}
|
||||
if (event.key === 'ArrowLeft' || event.key === 'ArrowUp') {
|
||||
} else if (event.key === 'ArrowLeft' || event.key === 'ArrowUp') {
|
||||
event.preventDefault();
|
||||
step(-1);
|
||||
} else if (event.key === 'Home') {
|
||||
event.preventDefault();
|
||||
moveTo(choices[0]?.value);
|
||||
} else if (event.key === 'End') {
|
||||
event.preventDefault();
|
||||
moveTo(choices[choices.length - 1]?.value);
|
||||
}
|
||||
}}
|
||||
>
|
||||
@@ -203,18 +242,20 @@ export function PiggyModeControl({
|
||||
aria-checked={isSelected}
|
||||
// Roving tabstop: a radio group is one stop in the tab order, and
|
||||
// the arrow keys move within it.
|
||||
tabIndex={isSelected ? 0 : -1}
|
||||
tabIndex={option.value === roving ? 0 : -1}
|
||||
disabled={disabled}
|
||||
title={disabled ? NO_WRITE_REASON : option.sentence}
|
||||
onClick={() => onChange(option.value)}
|
||||
onClick={() => {
|
||||
setKeyboardAt(option.value);
|
||||
onChange(option.value);
|
||||
}}
|
||||
className={cn(
|
||||
'flex min-h-[44px] min-w-0 items-center justify-center rounded-lg',
|
||||
'font-medium transition-colors touch-manipulation select-none',
|
||||
'focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-accent',
|
||||
'flex min-h-[44px] min-w-0 items-center justify-center rounded-md',
|
||||
'font-medium transition-colors duration-1 ease-enter touch-manipulation select-none',
|
||||
// Tight enough that "Read only" survives whole in a dock
|
||||
// narrower than the 22rem one; the label is what makes this
|
||||
// control legible, so it is the last thing allowed to truncate.
|
||||
compact ? 'gap-1 px-1 text-[11px]' : 'gap-1.5 px-2 text-xs sm:text-sm',
|
||||
compact ? 'gap-1 px-1 text-xs' : 'gap-1.5 px-2 text-xs sm:text-sm',
|
||||
isSelected
|
||||
? 'bg-surface text-fg shadow-sm'
|
||||
: 'text-muted hover:text-fg disabled:hover:text-muted',
|
||||
@@ -238,32 +279,33 @@ export function PiggyModeControl({
|
||||
{/*
|
||||
Announced on change, because the consequence arrives a beat after the
|
||||
press and a screen-reader user gets no colour to tell them the tone of
|
||||
the panel changed.
|
||||
the panel changed. It follows the keyboard rather than the choice, so
|
||||
arrowing across Auto reads its consequence — which is the only way to
|
||||
find out, now that arrowing no longer turns it on.
|
||||
*/}
|
||||
<div id={describedBy} aria-live="polite" className="min-w-0">
|
||||
<div id={describedBy} aria-live="polite" className="min-w-0 text-xs leading-snug">
|
||||
{active.consequential ? (
|
||||
<p
|
||||
className={cn(
|
||||
'flex items-start gap-2 rounded-lg border border-warning/40 bg-warning/10',
|
||||
compact ? 'px-2 py-1.5 text-[11px]' : 'px-2.5 py-2 text-xs',
|
||||
'leading-snug text-fg',
|
||||
'flex items-start gap-2 rounded-lg border border-warning/40 bg-warning/10 text-fg',
|
||||
compact ? 'px-2 py-1.5' : 'px-2.5 py-2',
|
||||
)}
|
||||
>
|
||||
<TriangleAlert aria-hidden className="mt-px h-3.5 w-3.5 shrink-0 text-warning" />
|
||||
<span>
|
||||
{previewing ? <span className="font-medium">{active.label}: </span> : null}
|
||||
{active.sentence} <span className="font-medium">{GUARDED_SENTENCE}</span>
|
||||
{previewing ? ' Press Enter to choose it.' : null}
|
||||
</span>
|
||||
</p>
|
||||
) : (
|
||||
<p className={cn('leading-snug text-muted', compact ? 'text-[11px]' : 'text-xs')}>
|
||||
<p className="text-muted">
|
||||
{previewing ? <span className="font-medium text-fg">{active.label}: </span> : null}
|
||||
{active.sentence}
|
||||
{previewing ? ' Press Enter to choose it.' : null}
|
||||
</p>
|
||||
)}
|
||||
{canWrite ? null : (
|
||||
<p className={cn('mt-1 leading-snug text-muted', compact ? 'text-[11px]' : 'text-xs')}>
|
||||
{NO_WRITE_REASON}
|
||||
</p>
|
||||
)}
|
||||
{canWrite ? null : <p className="mt-1 text-muted">{NO_WRITE_REASON}</p>}
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user