Files
Ivan Malison df32e6ddcd Fix the CI failures on main (#2850)
* fix(app): keep an open workspace menu from unmounting its own trigger

The sidebar kebab is revealed by hover, and on a compact layout the menu it
opens is a sheet that slides up over the row it came from. The row un-hovers,
the trailing overlay unmounts the trigger, and the menu's open state goes with
it — nothing is left that can close the surface already presented in the
portal, so a full-screen backdrop swallows every click until reload.

Lift the menu's open state to the component that decides whether to render the
kebab, and keep the kebab rendered while the menu is up.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(app): follow the sidebar and host badge redesigns in the e2e suite

Three specs were still driving surfaces that moved out from under them, so CI
has been red on main since the sidebar polish landed.

Display preferences became one row per decision with the options a page down,
so grouping and the host filter need that page walked first. Five specs had
their own copy of the two-line dance; it lives in the sidebar helper now, and
the host filter has its own opener next to the menu it belongs to.

The host badge is plain secondary text on the meta line and its identity colour
lives on the server icon alone, so the helpers stop asserting a tinted label
and a pill fill that the redesign removed.

Alignment rails are compared across canvas glyph metrics and SVG bounding
boxes, so the residual depends on the host's font; a 1px and a 0.5px tolerance
both landed on the wrong side of it in CI. Two pixels still catches the whole-
pixel offsets these rails encode. The compact sheet spec also read its rails
while the panel was still sliding in.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* chore(app): let a browser e2e run take a private Metro cache

Metro's on-disk cache is shared by every checkout on the machine. Editing one
app source file rebuilds that module against everyone else's cached output and
the Unistyles styles can come back desynced, which surfaces as alignment specs
failing with real-looking pixel numbers that go away when the edit is reverted.

E2E_METRO_CACHE_VERSION namespaces a run's cache without deleting anyone
else's entries, so the cache can be ruled out before a layout failure is
believed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(app): stop the chat scroll helper from hanging on a stalled sampler

`scrollChatAwayFromBottom` waited on a promise that could only ever resolve
from a `requestAnimationFrame` sampler started by a `wheel` listener. rAF does
not fire while the page is occluded, which is reachable on a CI runner, so the
sampler could stall on its first frame and the wait never returned — the test
died on its own 240s timeout with nothing to read. A missed wheel event did the
same.

Settling the scroll before measuring is an optimization; the assertion after it
is what decides the test. Sample on a timer with a deadline so a stall falls
through to that assertion instead of consuming the test.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(app): remove layout geometry e2e coverage

Rail and pixel-alignment assertions depend on host rendering details and should not define the browser E2E contract. Preserve the surrounding behavior coverage while dropping the Metro cache escape hatch that only supported diagnosing those assertions.

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Mohamed Boudra <boudra.moha@gmail.com>
2026-08-04 10:29:52 +02:00

10 KiB

Hover

Read this before writing any hover code. Every hover regression we ship is one of the three failure modes below, and every one of them is solved by the same canonical pattern. The pattern is hardwon — it survived every other shape we tried — so copy it, don't reinvent it.

The pattern

The canonical implementation lives in packages/app/src/components/sidebar-workspace-list.tsx, in the workspace row (around line 1369). When in doubt, open that file and copy the shape.

//
//   ┌─ Plain View. Tracks hover via pointerenter/pointerleave.
//   │
<View
  style={styles.workspaceRowContainer}
  onPointerEnter={handlePointerEnter}
  onPointerLeave={handlePointerLeave}
>
  <Pressable                          // ┐ Separate inner Pressable.
    onPress={handlePress}             // │ Handles press only.
    onPressIn={...}                   // │ Never has onHoverIn/onHoverOut.
    onPressOut={...}                  // ┘
    style={workspaceRowStyle}
  >
    <View style={styles.workspaceRowMain}>
      <View style={styles.workspaceRowLeft}></View>
      <WorkspaceRowRightGroup isHovered={isHovered} />
      {/*                    └─ Reveals content based on hover state. */}
    </View>
  </Pressable>
</View>

Five things make this work. Every one of them matters.

  1. Hover lives on a plain View, not a Pressable. Pressable carries its own internal hover state machine. Nested Pressables fight over it. A plain View just dispatches DOM events — no state machine, no fighting.
  2. Press lives on a separate inner Pressable. Hover and press never share an element. The two state machines never see each other.
  3. onPointerEnter / onPointerLeave are non-bubbling, mouseenter-style by W3C spec. They fire only when crossing the outer View's bounding box. Crossing into descendants — including descendant Pressables (the kebab menu's buttons, a copy button, a tooltip target) — does not fire pointerleave. This is why nesting Pressables inside is safe.
  4. The row has a fixed minHeight. When content swaps in on hover (kebab replacing a diff stat), both occupy the same fixed slot. Zero layout shift, zero geometry flicker.
  5. The outer View has nothing but position: relative. It exists only to be the hover target. All real layout lives on the inner Pressable. The hover-tracker is a sealed envelope around the row; layout changes inside it never leak out and re-enter through the side.

That's the whole pattern. Internalize it.

When you skip the pattern, here is what breaks

Failure mode 1 — Nested Pressables fight over hover state

If you put onHoverIn / onHoverOut on a Pressable that has another Pressable anywhere inside it (a copy button, an icon button, a nested action), the moment the cursor moves onto the inner Pressable, the inner one's hover state machine claims hover and the outer one's onHoverOut fires. Your reveal state flips off. The reveal hides. The cursor is no longer over the hidden reveal, so it ends up back over the trigger area. The outer's onHoverIn fires. Loop.

This is the most common hover bug shipped in this codebase, by a wide margin. It is what the workspace row is structured to avoid. The fix is not "be clever about handlers" — it's "don't put hover on a Pressable that contains other Pressables."

Rule: the hover-tracking element is a plain View with onPointerEnter / onPointerLeave. Any Pressables — including ones you forgot are Pressables, like TurnCopyButton, icon buttons, anything that handles a tap — live inside it.

Failure mode 2 — The hovered state changes the trigger's geometry

Symptom: you hover a button, it changes appearance, then flickers between hovered and not-hovered without the cursor moving.

Cause: the hover state changed the size or position of the trigger. The cursor was on the original element; the new layout shifts or shrinks it out from under the cursor; onHoverOut fires; state reverts; original layout returns; cursor is back over the trigger; onHoverIn fires; loop.

Common variants:

  • Hover state changes the trigger's width, height, padding, or borderWidth.
  • Hover state mounts/unmounts a child that pushes the trigger to a new position.
  • Hover state swaps the trigger for a different element type, remounting it.

Fixes, in preferred order:

  1. Don't change the trigger's outer geometry on hover. Change colors, opacity, borders that don't take layout space (outlineWidth on web, absolutely positioned overlays), or child content that fits inside the same fixed box. Never change width, height, padding, or borderWidth of the hover target itself.
  2. Hide with opacity + pointerEvents, not conditional rendering, when the hidden element lives inside the trigger. Mounting/unmounting on hover reflows the layout under the cursor.
  3. Pin the hit area. Set a fixed minHeight / minWidth on the trigger so internal swaps (icon-A becomes icon-B on hover) leave the bounding box unchanged. The workspace row's minHeight: 36 is what makes the kebab/diff-stat swap stable.

Failure mode 3 — Revealed content lives outside the hover trigger

If hovering element A reveals element B, B must be inside A's hover trigger. If B is a sibling, the moment the cursor moves from A toward B it crosses out of A's bounding box, pointerleave fires, B disappears.

Wrong:

<View>
  <View onPointerEnter={...} onPointerLeave={...}>     {/* hover trigger */}
    <Bubble />
  </View>
  <TrailingRow />                                       {/* OUTSIDE — sibling, not child */}
</View>

Right:

<View onPointerEnter={...} onPointerLeave={...}>      {/* hover trigger */}
  <Bubble />
  <TrailingRow />                                      {/* INSIDE — child */}
</View>

Any gap between A and B (margins between siblings inside the same parent) is part of the parent's bounding box, so the cursor stays inside the hover region while crossing it. No bridge needed.

If A and B genuinely can't share a parent — B portals into a different layer, floats above other content — see Section: real gaps below.

Failure mode 4 — A hover-revealed trigger unmounts its own open menu

A kebab that only exists while its row is hovered opens a menu, and that menu takes the pointer off the row: a sheet slides up over it, a popover covers it. The row un-hovers, the trailing overlay unmounts, and the trigger goes with it — taking the menu's open state, which lived inside the trigger's subtree. The surface itself is already presented in a portal, so nothing unmounts it and nothing is left that can close it. On a bottom sheet that means a full-screen backdrop swallowing every click until reload.

Lift the menu's open state to whatever decides to render the trigger, and keep the trigger rendered while the menu is up. useOpenKebabMenuVisibility (packages/app/src/components/sidebar/use-open-kebab-menu-visibility.ts) is that shape for the sidebar rows: it owns open, hands the menu its controlled props, and ORs open into the row's own showKebab.

Native fallback

Hover doesn't exist on touch devices. Anything you hide behind hover must have a non-hover path on native and compact layouts:

const showControls = isHovered || isNative || isCompact;

isNative and isCompact come from @/constants/platform and @/constants/layout. Don't use Platform.OS === "ios" as a proxy.

onPointerEnter / onPointerLeave are DOM events. They do not fire on native. You do not need to gate them — on native, hover is unreachable anyway and visibility is driven by isNative / isCompact in your show-the-controls expression above. This is why the workspace row's pointer events are not wrapped in if (isWeb).

What about Pressable.onHoverIn / onHoverOut?

It's fine when a Pressable styles itself based on its own hover — for example, an icon button that changes color on hover. That's self-contained. The render-prop <Pressable style={({ hovered }) => ...}> does the same thing more cleanly and is the preferred form.

It is not fine for tracking hover to drive state outside that Pressable (revealing a sibling, opening a tooltip, showing a kebab) when there is any other Pressable inside — because that's Failure Mode 1.

Heuristic: if your hover state is going to be useState'd and read by anything other than the same Pressable's own style, do not use onHoverIn / onHoverOut. Use the canonical pattern.

Real gaps with floating panels

Sometimes the revealed content can't live inside the trigger — a hover card portals into a different layer, a tooltip floats above other content, a popover renders into a Portal. There's a real visual gap the user has to cross with the cursor.

For this case, use useHoverSafeZone (packages/app/src/hooks/use-hover-safe-zone.ts). It computes a rectangular "bridge" between the trigger and the content; while the pointer is inside trigger, content, or the bridge, the card stays open. A short grace timer absorbs jitter at the edges. The canonical caller is packages/app/src/components/workspace-hover-card.tsx.

Don't roll your own. The math is annoying, the edge cases (pointer leaves window, drag in progress, content unmounts) are subtle, and we already paid for the hook.

Pre-PR checklist

Before opening a PR that touches hover:

  • Hover-tracking is on a plain View with onPointerEnter / onPointerLeave, not on a Pressable that wraps anything pressable.
  • Any press behavior lives on a separate inner Pressable that does not have onHoverIn / onHoverOut.
  • The hover trigger's bounding box contains every element the user might mouse into while interacting with the feature.
  • Hovered state does not change the trigger's outer geometry (width, height, padding, borderWidth, mount/unmount of siblings that shift it). Internal swaps fit inside a fixed minHeight / minWidth.
  • Revealed content inside the trigger uses opacity + pointerEvents, not conditional rendering, if mounting it would reflow the trigger.
  • Visibility on native and compact layouts works without hover (isHovered || isNative || isCompact).
  • A menu opened from the revealed trigger keeps the trigger rendered while it is open, so losing hover can't strand it.
  • If the revealed content sits in a separate layer (portal, floating panel), useHoverSafeZone is wired up.
  • You opened the dev server, hovered the trigger, and slowly moved the mouse along every revealed element — including any visible gaps — without losing hover state.