mirror of
https://github.com/getpaseo/paseo.git
synced 2026-10-29 04:32:57 +00:00
Mohamed Boudra
20b2dad610
* fix(app): seat the subagent sheet's parent callout like the composer
The sheet tab body padded the bottom safe-area inset, and the composer dock
padded it again, so the parent callout in a subagent sheet floated a full
navigation-bar height above where the composer sits in a tab.
The sheet body (SheetBottomInset) now owns the inset: it pads it once and
tells its content none is left, so the dock stays host-agnostic. Overlay
sheets (adaptive modal sheet, menu sheet, combobox sheet) read the inset on
the far side of their portal, so ones opened from sheet content still clear
the screen edge.
The callout takes the composer's rail and shows the parent's provider icon
through StatusIcon: no icon box, with the running ring, attention dot or
needs-input alert on the icon.
* fix(app): give the attachment lightbox its own safe area
An RN Modal passes its call site's context straight through, so a lightbox
opened from a sheet tab read the bottom inset the sheet body had already
spent (0) and drew over the navigation bar. The modal is its own window:
it now gets its own SafeAreaProvider, which measures that window, and the
image area reads its insets below it.
floating-panels.md records the rule for overlays: read insets where the
overlay draws.
* feat(app): open a subagent in place of the subagents list on compact
On phones, choosing a subagent from the subagents list now replaces the list
inside the same sheet instead of stacking a second sheet over it. Back returns
to the list, Close closes both, Android Back steps subagent -> list -> closed,
and Open parent lands on the pane the list was opened from (for a nested
subagent, its parent's sheet).
- Menu engine: fill pages (`fill: true`) take the full sheet height, Back pops
a page via `goBack`, and a page whose definition disappears falls back to
the list.
- One sheet pane (`screens/workspace/sheet-pane`) renders the header, body,
bottom inset and focus for both containers: the workspace's detached sheet
tabs and the in-list menu page. Only the container differs.
- The header drags the sheet through `SheetDragArea`; compact menu sheets use
the shared `SheetBackground`/`SheetHandle` chrome.
- `selectSheetTab` describes the sheet a reveal would show without opening it.
* fix(app): put the parent callout on one row
The Open parent button already says what the callout is, so the Parent label
goes. The parent's title shrinks to one line before the button does, so Open
parent never wraps or leaves the row.
* refactor(app): register panels once in the workspace screen
Every reader of the panel registry called ensurePanelsRegistered, which
imports every panel module. The panels render those readers (the agent panel's
subagent track and parent callout, the new-tab panel's launcher, the diff
panel's review surface), so each reader closed an import cycle through
register-panels.
The workspace screen, the only route that renders a registry reader, now
registers the panels before anything under it renders, and the readers import
only the leaf panel-registry. The pane content test registers a probe panel for
real instead of mocking the registry.
* test(app): transform the markdown WASM factory when Vite versions its URL
Importing the sheet pane barrel in retained-chat-content.browser.test.tsx made
Vite request md4c.js with a ?v= query, which the CommonJS-to-ESM transform did
not match, so the test file failed to import.
(cherry picked from commit d6d5a266ad)
231 lines
14 KiB
Markdown
231 lines
14 KiB
Markdown
# Menus
|
||
|
||
One engine, in `packages/app/src/components/ui/menu/`. `dropdown-menu.tsx` and `context-menu.tsx`
|
||
are wrappers over it and differ only in what opens them — a press, or a long press / right click.
|
||
Import the `DropdownMenu*` or `ContextMenu*` names as before; reach for `@/components/ui/menu`
|
||
directly only when you are building a third trigger shape.
|
||
|
||
Do not add a third menu implementation. The two that existed were byte-identical from
|
||
`computePosition` down, and the copies drifted: only one of them ever grew a sheet.
|
||
|
||
## Two presentations
|
||
|
||
`MenuRoot` picks one from form factor, never from platform — a tablet in a narrow split view
|
||
sheets the same way a phone does.
|
||
|
||
| Screen | Surface | Submenus |
|
||
| ------------------------------ | ------------------------------- | -------------------------------------------------- |
|
||
| Wide | Popover anchored to the trigger | Flyout overlapping the row, opened by hover intent |
|
||
| Compact, `compactMode="sheet"` | Bottom sheet | The page is replaced in place, with a back header |
|
||
| Compact, default | Popover | Same as wide |
|
||
|
||
`compactMode` defaults to `"popover"`, so adopting the sheet is per-menu. That is deliberate:
|
||
flipping every menu in the app to sheets at once is not a change anyone can review. Opt a menu in
|
||
when you have actually looked at it on a phone.
|
||
|
||
`ContextMenu` is the exception: it defaults to `compactMode="sheet"` and enables native long press.
|
||
Disable mobile triggering explicitly on draggable rows, where long press belongs to drag instead.
|
||
|
||
## When the items differ per form factor
|
||
|
||
A menu whose contents depend on what else is on screen gets one component per surface, not one
|
||
component with `isMobile` branches inside it. The workspace header menu is the example
|
||
(`packages/app/src/screens/workspace/workspace-header-menu.tsx`): compact has no tab strip, so it
|
||
carries new-tab actions, while wide leaves those to the strip's `+` menu and lists workspace
|
||
actions only. Items both surfaces share live in one component that each menu renders, from the same
|
||
callbacks, so the two can't drift.
|
||
|
||
## Selecting an item on iOS
|
||
|
||
An item that closes the menu runs its action after a fixed grace period on iOS, not immediately:
|
||
a native presenter launched while UIKit is still tearing down the surface can hang. Both surfaces
|
||
unmount the moment the menu closes, so the wait is a timer rather than the surface's own dismissal
|
||
callback — a callback fired from inside a subtree that is being removed is racing its own removal,
|
||
and it loses. Selections used to be dropped entirely for exactly that reason.
|
||
|
||
The consequence for callers: an `onSelect` on iOS runs a beat after the press. Don't add a second
|
||
delay on top of it, and don't read state that the same press mutated.
|
||
|
||
This is why a row inside a menu surface goes through `selectItem` rather than calling its handler
|
||
from its own `Pressable`. `MenuItem` is not the only row shape on the engine — `ComposerTrackRow`
|
||
is another, with its own icons and hover-revealed actions — but both hand the press to the engine
|
||
and take `closeOnSelect` to say whether choosing them ends the surface. A row that owns its press
|
||
outright leaves the surface open behind whatever it opened, and skips the iOS wait.
|
||
|
||
A row whose result opens a sheet opens it as a page of the menu's own sheet, never as a second
|
||
sheet over it: the row names a [fill page](#a-page-that-fills-the-sheet) with `opensPage`, the page
|
||
replaces the list in place, Back returns to the list, and Close ends both. A row that opens a tab
|
||
closes, because the tab replaces the pane the menu belongs to. The subagents track asks the layout
|
||
store which sheet tab a row would show (`useSubagentSheetTab`), since the answer changes with the
|
||
layout; a row with none opens its tab.
|
||
|
||
## Pages
|
||
|
||
A submenu is a page, declared as data on the surface and reached by a `MenuSubTrigger` whose `id`
|
||
matches:
|
||
|
||
```tsx
|
||
<MenuSurface pages={pages} sheetTitle="Display">
|
||
<MenuSubTrigger id="grouping" value="Project">
|
||
Grouping
|
||
</MenuSubTrigger>
|
||
</MenuSurface>
|
||
```
|
||
|
||
Pages are data rather than nested children because the popover renders them as _siblings_ of the
|
||
root surface — a flyout nested inside the root's box would be clipped by its `overflow: hidden`.
|
||
Declaring them separately is what lets one model drive both presentations.
|
||
|
||
`menu-navigation.ts` holds that model, and it is pure: the open flyout chain and the mobile push
|
||
stack are the same path, so the popover renders every entry in it and the sheet renders only the
|
||
last. Nothing else differs between the two.
|
||
|
||
Opening a submenu truncates the path to the depth of the trigger that opened it. Without that,
|
||
sliding the pointer across a row of triggers would stack up every flyout it passed instead of
|
||
swapping between them.
|
||
|
||
## A page that takes input
|
||
|
||
A menu page can hold a small form — `MenuTextField` is the field for it, drawn as a row's own
|
||
fill so its text lands on the same rail as the labels above it. Two things have to be true for
|
||
that page, and both are set on the page definition rather than discovered later:
|
||
|
||
- **`hoverIntent: false`.** A branch you skim is opened and closed by the pointer; a form is not.
|
||
Without this the page opens on a pointer that was only passing through, and dismisses itself —
|
||
draft and all — the moment your hands move to the keyboard and the mouse drifts off the flyout.
|
||
While such a page is open, the whole surface stops closing on hover, because the parent flyout
|
||
leads back to the same dismissal.
|
||
- **The compact presentation has to be a sheet.** `MenuTextField` resolves to
|
||
`BottomSheetTextInput` on compact native, which reads the sheet's context and has none in a
|
||
popover.
|
||
|
||
The sheet keeps `keyboardBehavior="interactive"`. `extend` grows a sheet to its largest snap
|
||
point, and with `enableDynamicSizing` that point is the content's own height — a short page does
|
||
not grow, and the keyboard comes up over the field you are typing into.
|
||
|
||
## A page that fills the sheet
|
||
|
||
A page can carry its own content instead of rows: `{ id, fill: true, content }`. Only the sheet
|
||
renders one. It takes the full-sheet snap point (`FULL_SHEET_SNAP_POINTS`) instead of sizing to
|
||
content, and the sheet stops panning on content drags, because the page scrolls on its own and a
|
||
drag would fight that scroll. The page's header drags the sheet instead, through `SheetDragArea`.
|
||
|
||
The content renders on the far side of the sheet's portal, so it brings its own context bridge,
|
||
built where the page is defined. Sheet tabs are the one user: `useSheetPanePage` turns a `SheetTab`
|
||
into a fill page around the same `SheetPane` the workspace's own sheets render. See
|
||
[explorer-sidebar.md](explorer-sidebar.md#sheets).
|
||
|
||
Back pops one page. The header's Back and Android Back both call `goBack`, which answers `false` at
|
||
the root so the sheet closes instead. A page whose definition disappears while it is open, such as
|
||
a subagent archived from elsewhere, is pruned from the path, and the sheet falls back to the page
|
||
that opened it.
|
||
|
||
## Hover intent
|
||
|
||
A flyout **overlaps its parent by 5pt** rather than sitting beside it. With a gap there is a strip
|
||
of backdrop between the two that belongs to neither surface, and every pixel of it is a chance to
|
||
dismiss the menu you are reaching for. Overlapping deletes the strip; don't reintroduce the gap
|
||
and try to cover it with a longer timer.
|
||
|
||
On top of that, a flyout opens after the pointer rests ~90ms, closes ~260ms after it leaves, and
|
||
cancels its own pending close while the pointer is inside it. The grace still matters because the
|
||
pointer crosses sibling rows on the way down into the flyout.
|
||
|
||
Hover lives on a plain `View`, never on a `Pressable` — see [hover.md](hover.md), which owns that
|
||
rule. Hover only fires on web, which is exactly where flyouts exist; everywhere else the page
|
||
opens on press.
|
||
|
||
## Item states
|
||
|
||
`selected` and `active` are different questions and must not be merged.
|
||
|
||
| Prop | Means | Draws |
|
||
| ---------- | -------------------------- | ------------------------- |
|
||
| `selected` | This is the chosen value | A check, and nothing else |
|
||
| `active` | This row's submenu is open | The fill, and no check |
|
||
|
||
`selected` also announces itself as `aria-checked`, so a multi-select page is audible as the list
|
||
of on/off things it is.
|
||
|
||
A selected row does **not** get a background. A check and a fill are two separate claims about the
|
||
same state, and showing both makes a chosen row compete with the row the pointer is actually on.
|
||
`showSelectedCheck` moves the check to a reserved leading column when a group needs to stay
|
||
aligned whether ticked or not; otherwise it sits at the trailing edge and the leading slot is free
|
||
for the option's icon.
|
||
|
||
Give options icons; leave the root rows without them. The root is labels and their current values,
|
||
and a column of icons there is decoration competing with the values you actually came to read.
|
||
|
||
## Surface details
|
||
|
||
- The hover fill is **inset from the surface's edges and rounded** — a chip inside the menu, not a
|
||
band across it. The inset is taken _out of_ the row, never added to it: padding gives up what
|
||
margin takes, so labels sit at the same 13pt they always did. Insetting by growing the row is how
|
||
the menu ends up taller than it started.
|
||
- **A row is as tall as what is driving it** — 28pt for a pointer, 40pt below `md` for a thumb. The
|
||
split is on breakpoint, not on `presentation`: the compact popover that `compactMode` defaults to
|
||
is worked with a thumb just as a sheet is, and sizing off the sheet would leave it at the desktop
|
||
height. `md` is where `useIsCompactFormFactor` divides, so row height and the popover/sheet choice
|
||
turn over together.
|
||
- The desktop height only holds because the label's `lineHeight` is pinned — 18 line + 8 padding +
|
||
2 border is exactly 28. Leave it to the platform and content outgrows `minHeight`, which then does
|
||
nothing. Compact is the other way round: `minHeight` leads and the label centres in it.
|
||
- **A row owns its fill; `MenuPage` owns the spacing between rows.** The vertical inset above the
|
||
first row and below the last, and the gap between rows, are the page's — one knob each,
|
||
`MENU_ROW_GAP` being the one a redesign turns. A row that carried vertical margin would be
|
||
serving both at once, and since margins don't collapse it would land as one unit at the edges and
|
||
two between rows, so shrinking the gap would eat the inset with it. Horizontal inset does stay on
|
||
the row: there is one left edge and one right edge, so nothing is doubling up.
|
||
- `MenuPage` is also what provides a page's depth, so the popover and the sheet cannot disagree
|
||
about what a page is. Every entry point goes through it — root, flyout, pushed sheet page.
|
||
- The gap reaches a page's **direct children**. A group of rows wrapped in a `View` of its own drops
|
||
out of it; at the current gap of zero there is nothing to lose, but anything above zero wants that
|
||
wrapper to carry the page style too.
|
||
- The separator is the one row-level thing that asks for vertical space of its own, and now that
|
||
rows carry none, that is still one number controlling one gap. It takes no horizontal margin and
|
||
the page has no horizontal padding, so the rule runs the full width of the surface.
|
||
- Separators use `borderAccent`, the same colour the surface outlines itself with. `border` sits
|
||
between `surface1` and `surface2`, which put it within a hair of the hover fill and made
|
||
separators disappear against a hovered row.
|
||
|
||
## When a decision earns a submenu
|
||
|
||
Put a decision behind a submenu when its options are not the point — the current _value_ is. The
|
||
root row then reads as the answer (`Grouping Project ›`) and costs one line instead of one line
|
||
per option. A menu whose every option is on screen at once does not survive its third decision.
|
||
|
||
Independent toggles stay on their page as a checkmark list. A pick-one group can share that page
|
||
below a `MenuSeparator`; make selecting the checked row clear it, so "none" doesn't need a row of
|
||
its own.
|
||
|
||
## Gotchas
|
||
|
||
- **Released height.** Reanimated's web entering animation leaves an inline height snapshot on
|
||
the surface. `AnchoredSurface` clears it, and a `revision` prop re-clears it when content
|
||
identity changes — a pushed page taller than the one it replaced is clipped without that.
|
||
- **Animate only once placed.** The same snapshot carries top/left, and Reanimated writes it back
|
||
750ms after mount. `AnchoredSurface` remounts the surface when its position resolves so the
|
||
entering animation never ends at the off-screen measuring position; on a slow machine that
|
||
write moved the open menu back off-screen.
|
||
- **Sheets size to content.** `enableDynamicSizing`, not fixed snap points. A pushed page is
|
||
rarely the height of the page before it. A fill page is the one exception.
|
||
- **The sheet's content is teleported out of the menu's subtree**, so `MenuSheetSurface` rebuilds
|
||
both menu contexts through the sheet's `contextBridge`. Providing them around the modal puts
|
||
them on the wrong side of the portal and every item inside throws. Gotcha 7 in
|
||
[floating-panels.md](floating-panels.md).
|
||
- **One overlay per menu.** Submenus render inside their parent's layer and paint no second
|
||
backdrop, so there is exactly one `Modal` on native no matter how deep the menu goes.
|
||
- **Sheet stacking and backdrop dismissal belong to `IsolatedBottomSheetModal`.** Menus only
|
||
request a backdrop opacity. See [floating-panels.md](floating-panels.md#gotcha-6--bottom-sheet-refs-are-not-lifecycle-truth).
|
||
- **A menu closes when its panel leaves the screen.** `MenuSurface` closes the menu when its
|
||
retained panel goes inactive. Hiding the surface alone leaves the open state behind, and the
|
||
menu would reappear with the panel on the next tab switch.
|
||
- **Retained panels own visibility.** The shared menu surface unmounts when its panel becomes
|
||
inactive. An async action can navigate before its menu closes; a hidden panel must not leave a
|
||
portal backdrop blocking the destination. On web the chat suspends one commit after it goes
|
||
inactive, and that inactive commit is where the surface unmounts.
|
||
- Anchoring, flipping, and edge clamping live in `components/ui/anchor.ts`, shared with the hover
|
||
card, and are unit-tested. Fix positioning bugs there, not at a call site.
|
||
- Everything else about floating surfaces on Android — Portal/Modal escape, lifecycle gates,
|
||
status-bar offset, the open flash — is in [floating-panels.md](floating-panels.md).
|