Skip to content

feat(prompt-input): add the PromptInput toolbar parts - #118

Merged
TheGreatAxios merged 7 commits into
conversation/02-samplesfrom
conversation/09-prompt-input-toolbar
Oct 28, 2026
Merged

TheGreatAxios merged 7 commits into
conversation/02-samplesfrom
conversation/09-prompt-input-toolbar

Conversation

@pratikbuilds

Copy link
Copy Markdown
Collaborator

Add PromptInputToolbar with an add menu, icon buttons, model and mode pickers, a float above the box, a context chip and a note. The pickers are select-only comboboxes and the add menu is a menu button; both keep or return focus to their trigger and need no optional peer. A toolbar placed last in PromptInput takes its own row.

Stacked on #117: review only this commit. Part of #94.

Notes for review

  • Toolbar parts take their own disabled; PromptInput's context stays private, so a host disabling the prompt passes disabled to the toolbar parts too.
  • No reasoning-level choice in the model picker; onValueChange stays a single id.
  • One class added to PromptInput's root so a trailing toolbar wraps onto its own row (e2e checks with and without a toolbar).
  • Escape in pickers/menu stops bubble-phase listeners (as the suggestions list does); untested inside a dialog.

Verification

  • This commit passes bun run typecheck and bun run lint on its own (the stack is checked commit by commit).
  • At the top of the stack: bun run check, bun run build, bun run dep-guard, bun run contrast-test and bun run test:e2e (Playwright specs and the Storybook axe sweep in light and dark) pass.

Closes #103

Add PromptInputToolbar with an add menu, icon buttons, model and mode
pickers, a float above the box, a context chip and a note. The pickers
are select-only comboboxes and the add menu is a menu button; both keep
or return focus to their trigger and need no optional peer. A toolbar
placed last in PromptInput takes its own row.

@TheGreatAxios TheGreatAxios left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated multi-agent review: independent review + second-agent validation; only confirmed items listed (re-verified against branch head 35dd216).

  • should-fix a11y src/ui/prompt-input-toolbar.tsx:156, 399 (+ use-dismissable-popover.ts:79-84) — popups the host opens don't get the dismissal net: closeOnEscape: false disables the hook's document-level Escape handler, so Escape is handled only on the trigger and inside the popup body, and Tab only inside the popup. A keyboard-only user with focus in the textarea cannot dismiss an open AddMenu/select at all (the only remaining dismissal is an outside pointerdown). Smallest fix: a capture-phase document Escape handler (with stopPropagation when open) plus root-level Tab handling; add the host-opened regression test.
  • should-fix a11y toolbar.tsx:494, 552 — the trigger's aria-label={label} replaces the whole accessible name, so a screen-reader user never hears the chosen model/mode while the popup is closed (sighted users see "Claude Opus", they hear "Model"). Fix: aria-label={chosen ? \${label}: ${chosen.name}` : label}or a visually-hidden span +aria-labelledby` including the value; regression-test that the accessible name contains the chosen option.
  • minor api-design — toolbar parts ignore PromptInput's disabled (the context is module-private and no part reads it; each takes its own disabled). Stories demonstrate the threading but no docstring states the contract — document it, or let parts consume the context.
  • nits confirmed — assignRef in toolbar.tsx:12-15 is character-identical to prompt-input.tsx:209-212 (consider one shared src/lib/ref.ts helper; the same idiom recurs ~9× across the series); Home/End open a collapsed select but the docstring's opener list omits them (:510-513 vs :361-365); PromptInputToolbar/Float/Note have no unit tests (prompt-input-toolbar.test.tsx never renders them); select-only combobox typeahead is absent (APG-optional, fine to defer — just note it).

A toolbar popup the host opened stayed open when Escape was pressed in
the textarea, because the popups opt out of the shared hook's Escape
handling to run their own. Listen for Escape on the document in the
bubble phase while open and close when focus is outside the popup,
leaving focus where it is. Handled Escapes are skipped, so the
suggestion list still closes first.

The toolbar parts each took their own disabled flag and ignored the
one on PromptInput, so a disabled prompt still offered a live add menu,
selects and buttons. Expose usePromptInputDisabled and default each
part's disabled to it; a part's own disabled still wins.

Also say that Home and End open a select on the first or last option,
and cover the toolbar, float and note parts with a test.
The select and add menu rows stacked a bold name over its description
and marked the active row with an accent stripe as well as a fill.
Draw each option as one row, the description inline and muted, mark
the active row with the neutral fill alone, as Menu does, and keep the
check for the chosen option. Rows sit flush with less padding.
@pratikbuilds

pratikbuilds commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator Author

Thanks for the review. Here's what happened to each point.

Fixed

1. Escape in the textarea couldn't close a popup the host had opened (should-fix)

  • The fix: while a popup is open, Escape pressed anywhere outside it closes it, and focus stays where it was. I used the bubble phase rather than capture, and Escapes something else has already handled are skipped. That way, if the @-suggestion list is also open, the first Escape closes the list and the second closes the popup.
  • Tests: three unit tests that failed before the fix, plus a Playwright case.

2. The toolbar ignored PromptInput's disabled (minor)

  • The fix: a new usePromptInputDisabled() makes the add menu, both selects and PromptInputButton follow the prompt's disabled. A part's own disabled still wins.

3. Nits

  • The select's docs now say Home and End open it.
  • PromptInputToolbar, PromptInputFloat and PromptInputNote now have a unit test.

Not changed, and why

  • Putting the chosen value in the trigger's aria-label: in Chromium, the trigger already reads as the name "Model" with the value "Claude Opus", which is how the APG select-only combobox works. Adding the value to the label would make it read twice.
  • A shared assignRef helper: it's copied about 11 times across the stack, so it's better as one clean-up once the stack lands.
  • Typeahead in the select: optional in the APG pattern, so deferred.

Also in this PR

A style(prompt-input) commit tightens the select and add-menu popups:

  • each option is one row, with the description inline
  • the active row has a plain fill instead of a coloured stripe
  • a check marks the chosen option
  • the padding is smaller

Commits: fix(prompt-input): close host-opened popups on escape; follow disabled, style(prompt-input): tighten the toolbar popups

@TheGreatAxios TheGreatAxios left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review · Request changes

Adds the PromptInput toolbar: add menu, model and mode selects, and the icon button.

Findings

  • src/ui/prompt-input-toolbar.tsx:278 — Escape inside the add menu or a select closes a dialog behind it. Both handlers stopPropagation in the bubble phase (prompt-input-toolbar.tsx:569 for the select). Radix DismissableLayer listens for Escape on document in the capture phase, so the dialog dismisses before this handler runs. Stop the key in capture, or preventDefault before the layer sees it, so one Escape closes only the popup.

  • src/ui/prompt-input-toolbar.tsx:56 — A host-opened popup closed from outside (the textarea) does not mark Escape handled. useCloseOnEscapeOutside calls setOpen(false) and leaves defaultPrevented false, and it also listens in the bubble phase, so a dialog's capture listener has already run. The suggestion list stops Escape on the textarea. This path should too.

  • src/ui/prompt-input-toolbar.tsx:313 — A disabled prompt still accepts a choice while the popup is open. disabled is only on the trigger (:252, :544). The menu stays mounted when open is true, and the item onClick checks per-item disabled only. choose (:478) does not check the select's disabled either (:505). defaultOpen, a controlled open, or disabling the prompt while the menu is up still adds a file or changes the model. Ignore the click when the part is disabled, and close the popup when it becomes disabled.

  • src/ui/prompt-input-toolbar.tsx:218 — Dismissing the add menu with a pointer press on a non-focusable node drops focus to document.body. Outside dismiss calls setOpen(false) (use-dismissable-popover.ts:77) and does not focus the trigger; close() would. The note at :172 is right when the press lands on the textarea. When it lands on something that does not take focus, return focus to the trigger.

  • Commit style(prompt-input): tighten the toolbar popups uses style, which CONTRIBUTING.md does not list. refactor matches that change.

@pratikbuilds
pratikbuilds force-pushed the conversation/09-prompt-input-toolbar branch from 3329560 to e9d2552 Compare October 28, 2026 08:41
A dialog dismisses on Escape from a capture listener on the document,
so the popups' bubble-phase stopPropagation never kept it open: one
Escape closed both the popup and the dialog around the prompt. While
open, each popup now prevents Escape in the window's capture phase,
ahead of the dialog. Escape from outside the popup, such as the
textarea beside a popup the host opened, closes it there too, unless
the suggestion list is showing and takes that Escape first.

A disabled part now shows its popup closed, and turning disabled
while open closes it for the owner, so nothing can be chosen from a
popup in a disabled prompt, as the toolbar's docs already promised.

The add menu kept the focus request of an ArrowUp the owner declined,
so the owner's own later open moved focus into the menu. The request
now lasts only for the render that answers it.
@pratikbuilds
pratikbuilds force-pushed the conversation/09-prompt-input-toolbar branch from e9d2552 to d2c36ab Compare October 28, 2026 11:28
@pratikbuilds

Copy link
Copy Markdown
Collaborator Author

Thanks for the review. Four of the five points are fixed. The outside press is by design.

1. Escape inside the add menu or a select also closed a dialog around the prompt

  • The fix: while a popup is open, it claims Escape in the window's capture phase, which runs before the dialog's capture listener on the document. An Escape inside the popup is marked handled (preventDefault), so the dialog leaves it alone, and the popup's own key handler closes the popup as before. An Escape that something else has already handled is skipped.
  • Tests: "Escape inside the add menu closes the menu, not an enclosing dialog" and "Escape on an open select closes the list, not an enclosing dialog". Both failed before the fix. There are also two Playwright cases in a real dialog.

2. Escape from the textarea closed a host-opened popup without marking it handled

  • The fix: the same capture-phase listener handles an Escape from outside the popup. It marks the Escape handled and closes the popup, and focus stays where it is.
  • Tests:
    • "Escape from the textarea closes the host-opened add popup, not an enclosing dialog", and the same for the model popup.
    • "in a dialog, with the popup opened first, Escape closes the list, then the popup, and never the dialog", and the same with the list opened first.
    • All four failed before the fix.

3. A disabled prompt still accepted a choice from an open popup

  • The fix: a disabled part never shows its popup. That holds whether the popup was opened with defaultOpen, through a controlled open, or before the prompt was disabled. With no popup on screen, nothing can be chosen.
    • When a part turns disabled while open, it also asks its owner to close it (onOpenChange(false)). So the popup doesn't come back when the prompt is enabled again.
    • The latest setOpen is kept in a ref, updated in a layout effect. A controlled owner is asked once per change, not on every render.
  • Tests: for both the add menu and the model select, "open from the start in a disabled prompt, is not shown" and "disabling the prompt closes the open popup for good". All of them failed before the fix.

4. Closing the add menu with a press on plain content dropped focus to the body

  • Not changed, by design. In Chromium, a press on content that can't take focus moves focus to the body whether a menu is open or not. That's the browser's handling of the press. The menu's docs say a press outside closes it without taking focus back. NotificationsBell and Radix's non-modal popovers work the same way.
  • Focusing the trigger on pointerdown doesn't work anyway, because the browser's own focus change on mousedown runs after it and still lands on the body. Taking focus back would also break pressing straight into another control.

5. The style commit type isn't in CONTRIBUTING.md

  • The fix: the commit is renamed to refactor(prompt-input): tighten the toolbar popups.

Also in this PR

The add menu had the same stuck ArrowUp flag you found in #121's menu. If a controlled owner declined an ArrowUp open, the owner's own later open moved focus to the last item. The request now lasts only for the render that answers it. The test "an ArrowUp the host declined does not make its later open take focus" failed before the fix.

Commits: refactor(prompt-input): tighten the toolbar popups, fix(prompt-input): keep toolbar escape out of enclosing dialogs

While a toolbar popup was open it took every Escape in the window
whose target lay outside the popup, closing itself and preventing the
event. A dialog opened elsewhere on the page then saw its Escape as
handled and stayed open while the popup behind it closed, and an
Escape inside the add menu also closed a model list the host had
opened beside it.

A popup now closes on an outside Escape only when it comes from the
same PromptInput, such as its textarea, and not from inside another
open toolbar popup, which handles its own. Each popup's root carries
data-state="open" while open, so that check reads the DOM. Escapes
from anywhere else are left alone for the layer they were pressed in.
Only each select option prevented the mousedown that moves focus, so
a press on a group heading, the list's padding or the gap between
rows dropped focus to the page. The trigger, which holds the list's
keys, then got none: the arrows did nothing and Escape closed the
list without returning focus. The add menu had the same gap around
its items.

The listbox and the menu now prevent that mousedown as a whole, so
focus stays on the select's trigger or the focused menu item wherever
the press lands, and clicks still choose.
PromptInputAddMenu and the model and mode selects read ref from their
props, which only React 19 passes to a function component; under
React 18, inside the peer range, the ref was dropped with a warning.
Wrap them, and the select they share, in forwardRef so a ref reaches
each part's root element on both versions.
@pratikbuilds
pratikbuilds force-pushed the conversation/09-prompt-input-toolbar branch from d2c36ab to a06fe49 Compare October 28, 2026 14:47
@pratikbuilds

Copy link
Copy Markdown
Collaborator Author

Round 3 fixes

A fresh review pass over the stack turned up the points below; each is fixed in a new commit at the end of this branch, and the stack is rebased onto it.

  • f2cfdb4: An open toolbar popup claimed every Escape in the window, so an unrelated dialog stayed open while the popup closed, and one Escape could close two popups. A popup now closes on an outside Escape only when it comes from its own PromptInput, and leaves an Escape pressed inside another open toolbar popup to that one (each popup root carries data-state="open" while open; no shared state).
  • 9648dc6: A press on a popup's padding or a group heading sent focus to the body while the popup stayed open. mousedown is cancelled on the listbox and the menu container; an e2e test covers it.
  • a06fe49: React 18 refs: components that read ref from props only receive it on React 19, while the peer range allows ^18.2.0. They are now wrapped in forwardRef, so a host ref reaches the element on both; the peer range is unchanged. Covers PromptInputAddMenu, PromptInputModelSelect and PromptInputModeSelect.
  • The faint fill on the highlighted option: the chosen option already carries a visible check mark, so it is not colour alone.

bun run check and dep-guard pass on every branch of the stack, and the full e2e suite passes at the top of the stack (163/163, including the stories axe sweep). Each behaviour fix comes with a test that fails without it; the forwardRef tests can only guard, since the suite runs on React 19.

@TheGreatAxios
TheGreatAxios removed this pull request from stack #130 October 28, 2026 21:38
@TheGreatAxios
TheGreatAxios added this pull request to stack #139 October 28, 2026 21:38
@TheGreatAxios
TheGreatAxios force-pushed the conversation/09-prompt-input-toolbar branch from a06fe49 to 227dc18 Compare October 28, 2026 21:42
@TheGreatAxios
TheGreatAxios dismissed their stale review October 28, 2026 21:49

Superseded by the completed stacked review; blocking fixes were addressed and the rebased stack is being landed.

@TheGreatAxios
TheGreatAxios merged commit 73a9348 into main Oct 28, 2026
7 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add the PromptInput toolbar

2 participants