Repository navigation
Polish message hover fills and shorten tooltip delays - #1993
Conversation
Co-authored-by: Codex <[email protected]> Signed-off-by: Arjun Mahanti <[email protected]>
|
🤖 Reviewed fixture captures for Actual updated message components with synthetic, offline data. Light mode shades the bubble; dark mode lightens it. Both show the pointer action state with unchanged text and links. These captures demonstrate appearance, not elapsed tooltip timing.
|
Co-authored-by: Codex <[email protected]> Signed-off-by: Arjun Mahanti <[email protected]>
kalvinnchau
left a comment
kalvinnchau
left a comment
There was a problem hiding this comment.
🤖 Two hover regressions reproduced in Chromium and WebKit; please fix both before merge.
| --surface-message-hover: var(--bubble-hover); | ||
| --surface-message-own-hover: var(--bubble-own-hover); | ||
| --bubble-hover: color-mix(in srgb, var(--surface-message) 95%, white); | ||
| --bubble-own-hover: color-mix(in srgb, var(--surface-message-own) 95%, white); |
There was a problem hiding this comment.
🤖 [P2] Preserve contrast for solid accent bubbles
Select Dark → Buzzin’ → Cobalt or Magenta in Appearance, then hover an own prose message. The 5% white mix leaves its normal 14px white text at 4.165:1 (Cobalt) or 4.301:1 (Magenta), down from 4.505:1 at rest in both Chromium and WebKit. This falls below DESIGN.md’s 4.5:1 body-text requirement. Preserve the accent bubble’s contrast-aware hover direction and update the contrast guard to measure the rendered hover token; it currently checks the retired overlay, so design:check misses this state. Retire the obsolete overlay token/registry/guard entries with that repair.
There was a problem hiding this comment.
🤖 Per Arjun’s explicit direction, we’re retaining the current hover treatment. He considers this a transient interaction rather than a core visual state and has chosen to accept the contrast tradeoff reported here. No contrast-direction or guard changes are included in this follow-up.
| .message[data-actions-revealed] | ||
| > .messageBody | ||
| > .messageContent | ||
| .messageBubble:not([data-emoji-only]) { |
There was a problem hiding this comment.
🤖 [P2] Preserve standalone table and diff exclusions
Hover a table-only message or a signed kind-40008 diff. Received/own tables and own diffs change from transparent to the prose hover fill in both Chromium and WebKit. These selectors override the standalone-renderer transparent reset at lines 3502–3510; the old ::after suppression cannot suppress a background declaration. Exclude [data-diff-only] and :has(> .text > table:only-child) from both hover selectors alongside emoji-only messages, and remove the obsolete pseudo-element suppression.
There was a problem hiding this comment.
🤖 Fixed in 1296bc8. Both hover-fill selectors now exclude standalone diffs and table-only bubbles alongside emoji-only content, and the obsolete ::after suppression is removed. Added a browser regression covering received tables, own tables, and diffs in light and dark mode. The new regression reproduced the issue in Chromium and WebKit before the fix; the full affected browser file now passes in both engines (14 tests). Required pre-push checks also passed.
wesbillman
left a comment
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Carl, an automated reviewer, commenting via Wes's GitHub account.
Verdict: two blockers, both on the new hover fill (see the inline comments). The tooltip timing changes look fine. Everything that uses the timing constants goes through hoverIntent.ts, and the existing tooltip tests use a controlled clock.
Validation gaps: this was a source-level review with static contrast math. We did not check the theme journeys in the running app.
Merge criteria: keep text readable on the hover fill in every high-color accent, include the new hover surfaces in the contrast guard, and keep standalone tables and diffs free of a bubble fill on hover. Then check Dark, high color, with the Magenta, Cobalt and Bee accents.
| --surface-message-hover: var(--bubble-hover); | ||
| --surface-message-own-hover: var(--bubble-own-hover); | ||
| --bubble-hover: color-mix(in srgb, var(--surface-message) 95%, white); | ||
| --bubble-own-hover: color-mix(in srgb, var(--surface-message-own) 95%, white); |
There was a problem hiding this comment.
Blocker: in Dark mode with high color, the own-bubble hover drops below the 4.5:1 text contrast minimum (DESIGN.md Contrast). At high color level, --surface-message-own is --accent-9 and the ink stays --accent-ink, which is white (lines 1954-1958). Mixing that fill 5% toward white lightens it under white text:
- Magenta
#df1e85: 4.50:1 → about 4.29:1 (#e1298b) - Cobalt
#1475e1: 4.50:1 → about 4.17:1
Repro: Dark, high color, Magenta or Cobalt accent. Send a message and hover it until the actions appear.
Fix: change the hover fill or ink so every accent keeps at least 4.5:1, for example by darkening the high-color own bubble on hover. The contrast guard should measure these new hover surfaces; right now it checks the overlay this PR removes.
| background: var(--affordance-message-own-hover); | ||
| pointer-events: none; | ||
| .messageBubble:not([data-emoji-only]) { | ||
| background: var(--surface-message-own-hover); |
There was a problem hiding this comment.
Blocker: the hover fill now paints under standalone tables and diffs. Both new background rules here (2925-2930 and 2931-2936) are more specific than the renderer exemption at lines 3502-3511 (.messageBubble[data-diff-only] and the table-only :has() selector), so they override its background: transparent. The old ::after overlay didn't have this problem because the exemption hides ::after.
The exemption also resets --reaction-surface and --reaction-ink to match the panel. On hover, those reactions now sit on the bright own fill they weren't styled for, which makes their counts hard to read.
Repro: Dark, high color, Bee accent. Send a message that is only a Markdown table, add a reaction, then hover the message.
Fix: exclude [data-diff-only] and table-only bubbles from both hover rules, or restate background: transparent for them at matching specificity.
Co-authored-by: Codex <[email protected]> Signed-off-by: Arjun Mahanti <[email protected]>
loganj
left a comment
loganj
left a comment
There was a problem hiding this comment.
🤖 Larry and Auto: an independent adversarial review of 1296bc8. Source review only: we read the code and computed the hover colors by hand. Auto ran pnpm design:check (passes) and pnpm design:census. We did not run the app or the browser tests.
Verdict: approve. The hover timing change is correct and complete. The table and diff fix in 1296bc8 is correct. We found no engineering blockers. One item needs a decision from Arjun, and the rest are optional cleanup.
Needs Arjun's decision
- With the Mono accent at the high color level, the own-bubble hover is invisible in both modes (inline on
tokens.css). The accepted tradeoff covers contrast. It does not say the hover can disappear, so this needs a separate yes or a fix.
Optional cleanup
--affordance-message-own-hoverand its feeds are now dead, but the registry still documents them andcheck-contrast.mjs:214and:350still check them (inline onregistry.ts).- The contrast guard does not measure the new fills, because
resolve()cannot readcolor-mix(). The accepted dark high-color drops (Magenta 4.50 → 4.29, Cobalt 4.50 → 4.16, Flame 4.60 → 4.33, Jade 4.66 → 4.28) are therefore unlisted. The guard says "accepted exceptions are listed explicitly", so these pairs belong inEXCEPTIONS. This is the same edit as moving lines 214 and 350 off the dead pair. - Simpler tokens and a single exclusion point for transparent renderers (inline).
- Docs and a positive hover test (inline).
Checked and correct
- Every hover consumer gets its timing from
hoverIntent.ts:Tooltip.tsx:102,PreviewCard.tsx:242,Popover.tsx:25,Menu.tsx:42. We found no hard-coded hover delays elsewhere insrc. - Unit tests import the constants, so the new values carry through. Browser tests ban fixed sleeps.
- The shorter 75 ms and 50 ms close delays are safe: the Base UI Popover, PreviewCard, Tooltip and Menu triggers use
safePolygon. - The
.light, :rootand.darkblocks both declare the hover tokens, so themes scoped to a subtree resolve against their own bubble color. - Performance:
color-mix()resolves once per theme scope and is inherited. There is no new transition, and the:has()already existed.
| --surface-message-hover: var(--bubble-hover); | ||
| --surface-message-own-hover: var(--bubble-own-hover); | ||
| --bubble-hover: color-mix(in srgb, var(--surface-message) 95%, white); | ||
| --bubble-own-hover: color-mix(in srgb, var(--surface-message-own) 95%, white); |
There was a problem hiding this comment.
🤖 error — needs Arjun's decision: with Mono at the high color level, the own-bubble hover is invisible in both modes. This rule (and :1097 for light mode) picks the mix direction from the theme mode, not from the bubble. With Mono at the high color level, --surface-message-own is --accent-9, which is already near the extreme the mix moves toward:
- light:
#1a1a1a→#191919 - dark:
#e6e6e6→#e7e7e7
That is a contrast change of about 1.01:1, compared with about 1.11–1.17 for the neutral bubbles. Before this PR, each accent chose its own direction (tokens.css:1891 gives Mono light a tint), as the reaction chips still do.
The accepted tradeoff covers contrast, not a hover that does nothing. If the hover should show here, a fix that also brings back each accent's own direction (and the contrast for Cobalt and Magenta) is to keep an overlay token and paint it as a background layer instead of ::after:
background: linear-gradient(var(--affordance-message-own-hover) 0 0), var(--surface-message-own);A 5% black layer over a flat color gives the same pixel as color-mix(… 95%, black), so neutral bubbles look the same. Text and media stay uncovered, and SITS_ON in the contrast guard already measures overlays. This needs a 5% shade/tint step and a token for received bubbles.
| --surface-message-own: var(--bubble-neutral); | ||
| --surface-message-hover: var(--bubble-hover); | ||
| --surface-message-own-hover: var(--bubble-own-hover); | ||
| --bubble-hover: color-mix(in srgb, var(--surface-message) 95%, black); |
There was a problem hiding this comment.
🤖 preference (optional): one layer too many. --bubble-hover and --bubble-own-hover are one-off, unregistered names. They use the --bubble-* names that elsewhere hold raw palette values (:777-778), but here they are derived from semantic roles. Setting --surface-message-hover: color-mix(in srgb, var(--surface-message) 95%, black) directly (and the same for own, and in .dark at :1228-1231) removes 4 declarations.
| status: "core", | ||
| }, | ||
| { | ||
| token: "--affordance-message-own-hover", |
There was a problem hiding this comment.
🤖 convention (optional): the old overlay is now dead but still documented and checked. Nothing paints --affordance-message-own-hover now (census: "Read only by the docs"), but this entry still calls it the overlay across your own bubble while its actions show. Its feeds are unused too: --shade-quiet-fill and --tint-quiet-fill (tokens.css:821-822, plus the comment at :817-818), the 16 --accent-overlay-quiet* declarations, and the overrides at tokens.css:1934 and :1951. check-contrast.mjs:214 and :350 still check a pair that is never painted. Retire them, or reuse them if you take the overlay suggestion on tokens.css:1231.
| } | ||
| /* Own bubbles retain their toolbar hover treatment; received bubbles stay | ||
| on the same quiet surface as their reaction strip. */ | ||
| @media (hover: hover) and (pointer: fine) { |
There was a problem hiding this comment.
🤖 preference (optional): one place to exempt transparent renderers. The :not([data-emoji-only], [data-diff-only]):not(:has(> .text > table:only-child)) chain is written twice here, and its table check is a third copy of the one at :3507. A future standalone renderer has to remember all three. This file already uses custom properties for this case (--reaction-surface: initial at :3514). For example:
- set
--message-hover-fillon.messageBubbleand on.messageBubble[data-own]; - set it to
transparentin the emoji-only rule (:3070) and the standalone-renderer rule (:3506); - keep one hover rule with no
:not:background: var(--message-hover-fill).
Then the exemption sits with the rule that makes the bubble transparent. The old comment on this rule was also deleted without a replacement.
| When message actions show, both received and own bubbles use their | ||
| `surface-message-hover` or `surface-message-own-hover` fill: 5% toward black in | ||
| light mode and 5% toward white in dark mode. This changes only the background, | ||
| without a stroke or an overlay on text and media. Emoji-only messages stay clear. |
There was a problem hiding this comment.
🤖 convention (optional): docs. This line says emoji-only messages stay clear, but standalone tables and diffs stay clear too after 1296bc8. The removed text also carried the rule that a bright bubble never takes a white wash in dark mode. The guide still asks for 4.5:1 on text pairs, so one sentence here recording the accepted hover exception would stop the next reader from "fixing" it.
| await bubble.hover(); | ||
| await expect(example.locator("[data-actions-revealed]")).toBeVisible(); | ||
| await expect(bubble).toHaveCSS("background-color", "rgba(0, 0, 0, 0)"); | ||
| } |
There was a problem hiding this comment.
🤖 advice (optional): add a positive case. This test only checks that tables and diffs stay transparent. Nothing checks that received and own bubbles actually change on hover. The ACCENTS × level × theme loop at :55 could compare the hover background with the resting one, which would catch the Mono case on tokens.css:1231.
| /** Hints naming an icon whose meaning is already obvious. */ | ||
| export const FAMILIAR_HINT_REST_MS = 1500; | ||
| export const FAMILIAR_HINT_REST_MS = 500; | ||
| /** Rich previews cover more of the page, so they wait longer than a hint. */ |
There was a problem hiding this comment.
🤖 nit (optional): "Rich previews … wait longer than a hint" was already false for familiar hints, and it still is: previews open at 300 ms, familiar hints at 500 ms. Maybe "longer than a helpful hint".
Preserve upstream reaction spacing and standalone renderer exclusions. Update the reaction preview timing assertion and parse CSS colors through canvas for WebKit. Co-authored-by: Codex <[email protected]> Signed-off-by: Arjun Mahanti <[email protected]>
Co-authored-by: Codex <[email protected]> Signed-off-by: Arjun Mahanti <[email protected]>
Co-authored-by: Codex <[email protected]> Signed-off-by: Arjun Mahanti <[email protected]>
johnmatthewtennant
left a comment
johnmatthewtennant
left a comment
There was a problem hiding this comment.
🤖 Reviewed exact head 984d9d2 (base 0400229) with the full adaptive review fanout: Builderbot plus nine routed lenses, every verdict collected, no correctness blockers found. Existing threads already cover the Mono-high own hover and the accepted dark-accent contrast tradeoff. Source review only; rendering and native behavior were not exercised beyond CI.
…ts own Addresses the remaining optional finding of the review of 3c2719440. A grouped emoji's list could still go away without the row hearing it. Only MenuRoot's onOpenChange called onListClose, so two closes skipped it: the remainder emptying while the list is open, where the emoji clears listOpen during render, and the emoji unmounting with its list open. React then kept pointerInRow and the closed-preview marker, and every pill in the row opened its preview at once until the pointer left and re-entered the row. A ReactionEmoji effect keyed on listOpen now owns the report: its cleanup runs onListClose whenever the list stops being open, by onOpenChange, by the render-time close, or by unmount. onOpenChange only sets listOpen. leaveRow is idempotent and reads nothing but setters, so the captured callback is safe. Tests: - The Vitest split case now also covers the hover state. The pointer enters the row, crosses to the preview and into the list, and never leaves. After the group shrinks to one emoji, focus and the next frame's focus return settle, the pill is blurred, and hovering it waits the full rest: no tooltip at PREVIEW_REST_MS - 1, a tooltip at PREVIEW_REST_MS. Before the fix the tooltip opened at once. Validation: typecheck, lint, the messages Vitest suites (86 files, 2,296 tests; MessageMarkdown's long-body case timed out once under concurrent Playwright load and passed alone), and reactions.spec.mjs plus reactions-polish.spec.mjs in Chromium and WebKit. Three reactions-polish cases fail on this branch before and after this change because #1993 shortened the preview rest to 300 ms and its close grace to 75 ms after their 599 ms and 149 ms assertions were written; with those numbers corrected locally all 16 pass in each engine. Those assertions are left for a separate change. Co-Authored-By: Claude Fable 5.1 <[email protected]> Signed-off-by: Matt Toohey <[email protected]>
… buckets PR #1993 (dc0a681) shortened the preview rest from 600 ms to 300 ms and its close grace from 150 ms to 75 ms. The browser assertions this branch added in reactions-polish.spec.mjs still advanced the paused clock by 149 ms before expecting the preview to survive leaving the row, and by 599 ms before expecting no tooltip after the list closed. Both now advance 74 ms and 299 ms, one short of the current boundaries, and the following 1 ms step crosses each. The browser specs import nothing from src, so the literals stay; the Vitest cases already read PREVIEW_REST_MS. Validation: MessageReactions.test.tsx (26 passed) and reactions.spec.mjs plus reactions-polish.spec.mjs in Chromium and WebKit (32 passed). Co-Authored-By: Claude Fable 5.1 <[email protected]> Signed-off-by: Matt Toohey <[email protected]>
…ts own Addresses the remaining optional finding of the review of 3c2719440. A grouped emoji's list could still go away without the row hearing it. Only MenuRoot's onOpenChange called onListClose, so two closes skipped it: the remainder emptying while the list is open, where the emoji clears listOpen during render, and the emoji unmounting with its list open. React then kept pointerInRow and the closed-preview marker, and every pill in the row opened its preview at once until the pointer left and re-entered the row. A ReactionEmoji effect keyed on listOpen now owns the report: its cleanup runs onListClose whenever the list stops being open, by onOpenChange, by the render-time close, or by unmount. onOpenChange only sets listOpen. leaveRow is idempotent and reads nothing but setters, so the captured callback is safe. Tests: - The Vitest split case now also covers the hover state. The pointer enters the row, crosses to the preview and into the list, and never leaves. After the group shrinks to one emoji, focus and the next frame's focus return settle, the pill is blurred, and hovering it waits the full rest: no tooltip at PREVIEW_REST_MS - 1, a tooltip at PREVIEW_REST_MS. Before the fix the tooltip opened at once. Validation: typecheck, lint, the messages Vitest suites (86 files, 2,296 tests; MessageMarkdown's long-body case timed out once under concurrent Playwright load and passed alone), and reactions.spec.mjs plus reactions-polish.spec.mjs in Chromium and WebKit. Three reactions-polish cases fail on this branch before and after this change because #1993 shortened the preview rest to 300 ms and its close grace to 75 ms after their 599 ms and 149 ms assertions were written; with those numbers corrected locally all 16 pass in each engine. Those assertions are left for a separate change. Co-Authored-By: Claude Fable 5.1 <[email protected]> Signed-off-by: Matt Toohey <[email protected]>
… buckets PR #1993 (dc0a681) shortened the preview rest from 600 ms to 300 ms and its close grace from 150 ms to 75 ms. The browser assertions this branch added in reactions-polish.spec.mjs still advanced the paused clock by 149 ms before expecting the preview to survive leaving the row, and by 599 ms before expecting no tooltip after the list closed. Both now advance 74 ms and 299 ms, one short of the current boundaries, and the following 1 ms step crosses each. The browser specs import nothing from src, so the literals stay; the Vitest cases already read PREVIEW_REST_MS. Validation: MessageReactions.test.tsx (26 passed) and reactions.spec.mjs plus reactions-polish.spec.mjs in Chromium and WebKit (32 passed). Co-Authored-By: Claude Fable 5.1 <[email protected]> Signed-off-by: Matt Toohey <[email protected]>
…ts own Addresses the remaining optional finding of the review of 3c2719440. A grouped emoji's list could still go away without the row hearing it. Only MenuRoot's onOpenChange called onListClose, so two closes skipped it: the remainder emptying while the list is open, where the emoji clears listOpen during render, and the emoji unmounting with its list open. React then kept pointerInRow and the closed-preview marker, and every pill in the row opened its preview at once until the pointer left and re-entered the row. A ReactionEmoji effect keyed on listOpen now owns the report: its cleanup runs onListClose whenever the list stops being open, by onOpenChange, by the render-time close, or by unmount. onOpenChange only sets listOpen. leaveRow is idempotent and reads nothing but setters, so the captured callback is safe. Tests: - The Vitest split case now also covers the hover state. The pointer enters the row, crosses to the preview and into the list, and never leaves. After the group shrinks to one emoji, focus and the next frame's focus return settle, the pill is blurred, and hovering it waits the full rest: no tooltip at PREVIEW_REST_MS - 1, a tooltip at PREVIEW_REST_MS. Before the fix the tooltip opened at once. Validation: typecheck, lint, the messages Vitest suites (86 files, 2,296 tests; MessageMarkdown's long-body case timed out once under concurrent Playwright load and passed alone), and reactions.spec.mjs plus reactions-polish.spec.mjs in Chromium and WebKit. Three reactions-polish cases fail on this branch before and after this change because #1993 shortened the preview rest to 300 ms and its close grace to 75 ms after their 599 ms and 149 ms assertions were written; with those numbers corrected locally all 16 pass in each engine. Those assertions are left for a separate change. Co-Authored-By: Claude Fable 5.1 <[email protected]> Signed-off-by: Matt Toohey <[email protected]>
… buckets PR #1993 (dc0a681) shortened the preview rest from 600 ms to 300 ms and its close grace from 150 ms to 75 ms. The browser assertions this branch added in reactions-polish.spec.mjs still advanced the paused clock by 149 ms before expecting the preview to survive leaving the row, and by 599 ms before expecting no tooltip after the list closed. Both now advance 74 ms and 299 ms, one short of the current boundaries, and the following 1 ms step crosses each. The browser specs import nothing from src, so the literals stay; the Vitest cases already read PREVIEW_REST_MS. Validation: MessageReactions.test.tsx (26 passed) and reactions.spec.mjs plus reactions-polish.spec.mjs in Chromium and WebKit (32 passed). Co-Authored-By: Claude Fable 5.1 <[email protected]> Signed-off-by: Matt Toohey <[email protected]>
…ts own Addresses the remaining optional finding of the review of 3c2719440. A grouped emoji's list could still go away without the row hearing it. Only MenuRoot's onOpenChange called onListClose, so two closes skipped it: the remainder emptying while the list is open, where the emoji clears listOpen during render, and the emoji unmounting with its list open. React then kept pointerInRow and the closed-preview marker, and every pill in the row opened its preview at once until the pointer left and re-entered the row. A ReactionEmoji effect keyed on listOpen now owns the report: its cleanup runs onListClose whenever the list stops being open, by onOpenChange, by the render-time close, or by unmount. onOpenChange only sets listOpen. leaveRow is idempotent and reads nothing but setters, so the captured callback is safe. Tests: - The Vitest split case now also covers the hover state. The pointer enters the row, crosses to the preview and into the list, and never leaves. After the group shrinks to one emoji, focus and the next frame's focus return settle, the pill is blurred, and hovering it waits the full rest: no tooltip at PREVIEW_REST_MS - 1, a tooltip at PREVIEW_REST_MS. Before the fix the tooltip opened at once. Validation: typecheck, lint, the messages Vitest suites (86 files, 2,296 tests; MessageMarkdown's long-body case timed out once under concurrent Playwright load and passed alone), and reactions.spec.mjs plus reactions-polish.spec.mjs in Chromium and WebKit. Three reactions-polish cases fail on this branch before and after this change because #1993 shortened the preview rest to 300 ms and its close grace to 75 ms after their 599 ms and 149 ms assertions were written; with those numbers corrected locally all 16 pass in each engine. Those assertions are left for a separate change. Co-Authored-By: Claude Fable 5.1 <[email protected]> Signed-off-by: Matt Toohey <[email protected]>
… buckets PR #1993 (dc0a681) shortened the preview rest from 600 ms to 300 ms and its close grace from 150 ms to 75 ms. The browser assertions this branch added in reactions-polish.spec.mjs still advanced the paused clock by 149 ms before expecting the preview to survive leaving the row, and by 599 ms before expecting no tooltip after the list closed. Both now advance 74 ms and 299 ms, one short of the current boundaries, and the following 1 ms step crosses each. The browser specs import nothing from src, so the literals stay; the Vitest cases already read PREVIEW_REST_MS. Validation: MessageReactions.test.tsx (26 passed) and reactions.spec.mjs plus reactions-polish.spec.mjs in Chromium and WebKit (32 passed). Co-Authored-By: Claude Fable 5.1 <[email protected]> Signed-off-by: Matt Toohey <[email protected]>
…ts own Addresses the remaining optional finding of the review of 3c2719440. A grouped emoji's list could still go away without the row hearing it. Only MenuRoot's onOpenChange called onListClose, so two closes skipped it: the remainder emptying while the list is open, where the emoji clears listOpen during render, and the emoji unmounting with its list open. React then kept pointerInRow and the closed-preview marker, and every pill in the row opened its preview at once until the pointer left and re-entered the row. A ReactionEmoji effect keyed on listOpen now owns the report: its cleanup runs onListClose whenever the list stops being open, by onOpenChange, by the render-time close, or by unmount. onOpenChange only sets listOpen. leaveRow is idempotent and reads nothing but setters, so the captured callback is safe. Tests: - The Vitest split case now also covers the hover state. The pointer enters the row, crosses to the preview and into the list, and never leaves. After the group shrinks to one emoji, focus and the next frame's focus return settle, the pill is blurred, and hovering it waits the full rest: no tooltip at PREVIEW_REST_MS - 1, a tooltip at PREVIEW_REST_MS. Before the fix the tooltip opened at once. Validation: typecheck, lint, the messages Vitest suites (86 files, 2,296 tests; MessageMarkdown's long-body case timed out once under concurrent Playwright load and passed alone), and reactions.spec.mjs plus reactions-polish.spec.mjs in Chromium and WebKit. Three reactions-polish cases fail on this branch before and after this change because #1993 shortened the preview rest to 300 ms and its close grace to 75 ms after their 599 ms and 149 ms assertions were written; with those numbers corrected locally all 16 pass in each engine. Those assertions are left for a separate change. Co-Authored-By: Claude Fable 5.1 <[email protected]> Signed-off-by: Matt Toohey <[email protected]>
… buckets PR #1993 (dc0a681) shortened the preview rest from 600 ms to 300 ms and its close grace from 150 ms to 75 ms. The browser assertions this branch added in reactions-polish.spec.mjs still advanced the paused clock by 149 ms before expecting the preview to survive leaving the row, and by 599 ms before expecting no tooltip after the list closed. Both now advance 74 ms and 299 ms, one short of the current boundaries, and the following 1 ms step crosses each. The browser specs import nothing from src, so the literals stay; the Vitest cases already read PREVIEW_REST_MS. Validation: MessageReactions.test.tsx (26 passed) and reactions.spec.mjs plus reactions-polish.spec.mjs in Chromium and WebKit (32 passed). Co-Authored-By: Claude Fable 5.1 <[email protected]> Signed-off-by: Matt Toohey <[email protected]>
…ts own Addresses the remaining optional finding of the review of 3c2719440. A grouped emoji's list could still go away without the row hearing it. Only MenuRoot's onOpenChange called onListClose, so two closes skipped it: the remainder emptying while the list is open, where the emoji clears listOpen during render, and the emoji unmounting with its list open. React then kept pointerInRow and the closed-preview marker, and every pill in the row opened its preview at once until the pointer left and re-entered the row. A ReactionEmoji effect keyed on listOpen now owns the report: its cleanup runs onListClose whenever the list stops being open, by onOpenChange, by the render-time close, or by unmount. onOpenChange only sets listOpen. leaveRow is idempotent and reads nothing but setters, so the captured callback is safe. Tests: - The Vitest split case now also covers the hover state. The pointer enters the row, crosses to the preview and into the list, and never leaves. After the group shrinks to one emoji, focus and the next frame's focus return settle, the pill is blurred, and hovering it waits the full rest: no tooltip at PREVIEW_REST_MS - 1, a tooltip at PREVIEW_REST_MS. Before the fix the tooltip opened at once. Validation: typecheck, lint, the messages Vitest suites (86 files, 2,296 tests; MessageMarkdown's long-body case timed out once under concurrent Playwright load and passed alone), and reactions.spec.mjs plus reactions-polish.spec.mjs in Chromium and WebKit. Three reactions-polish cases fail on this branch before and after this change because #1993 shortened the preview rest to 300 ms and its close grace to 75 ms after their 599 ms and 149 ms assertions were written; with those numbers corrected locally all 16 pass in each engine. Those assertions are left for a separate change. Co-Authored-By: Claude Fable 5.1 <[email protected]> Signed-off-by: Matt Toohey <[email protected]>
… buckets PR #1993 (dc0a681) shortened the preview rest from 600 ms to 300 ms and its close grace from 150 ms to 75 ms. The browser assertions this branch added in reactions-polish.spec.mjs still advanced the paused clock by 149 ms before expecting the preview to survive leaving the row, and by 599 ms before expecting no tooltip after the list closed. Both now advance 74 ms and 299 ms, one short of the current boundaries, and the following 1 ms step crosses each. The browser specs import nothing from src, so the literals stay; the Vitest cases already read PREVIEW_REST_MS. Validation: MessageReactions.test.tsx (26 passed) and reactions.spec.mjs plus reactions-polish.spec.mjs in Chromium and WebKit (32 passed). Co-Authored-By: Claude Fable 5.1 <[email protected]> Signed-off-by: Matt Toohey <[email protected]>
…ts own Addresses the remaining optional finding of the review of 3c2719440. A grouped emoji's list could still go away without the row hearing it. Only MenuRoot's onOpenChange called onListClose, so two closes skipped it: the remainder emptying while the list is open, where the emoji clears listOpen during render, and the emoji unmounting with its list open. React then kept pointerInRow and the closed-preview marker, and every pill in the row opened its preview at once until the pointer left and re-entered the row. A ReactionEmoji effect keyed on listOpen now owns the report: its cleanup runs onListClose whenever the list stops being open, by onOpenChange, by the render-time close, or by unmount. onOpenChange only sets listOpen. leaveRow is idempotent and reads nothing but setters, so the captured callback is safe. Tests: - The Vitest split case now also covers the hover state. The pointer enters the row, crosses to the preview and into the list, and never leaves. After the group shrinks to one emoji, focus and the next frame's focus return settle, the pill is blurred, and hovering it waits the full rest: no tooltip at PREVIEW_REST_MS - 1, a tooltip at PREVIEW_REST_MS. Before the fix the tooltip opened at once. Validation: typecheck, lint, the messages Vitest suites (86 files, 2,296 tests; MessageMarkdown's long-body case timed out once under concurrent Playwright load and passed alone), and reactions.spec.mjs plus reactions-polish.spec.mjs in Chromium and WebKit. Three reactions-polish cases fail on this branch before and after this change because #1993 shortened the preview rest to 300 ms and its close grace to 75 ms after their 599 ms and 149 ms assertions were written; with those numbers corrected locally all 16 pass in each engine. Those assertions are left for a separate change. Co-Authored-By: Claude Fable 5.1 <[email protected]> Signed-off-by: Matt Toohey <[email protected]>
… buckets PR #1993 (dc0a681) shortened the preview rest from 600 ms to 300 ms and its close grace from 150 ms to 75 ms. The browser assertions this branch added in reactions-polish.spec.mjs still advanced the paused clock by 149 ms before expecting the preview to survive leaving the row, and by 599 ms before expecting no tooltip after the list closed. Both now advance 74 ms and 299 ms, one short of the current boundaries, and the following 1 ms step crosses each. The browser specs import nothing from src, so the literals stay; the Vitest cases already read PREVIEW_REST_MS. Validation: MessageReactions.test.tsx (26 passed) and reactions.spec.mjs plus reactions-polish.spec.mjs in Chromium and WebKit (32 passed). Co-Authored-By: Claude Fable 5.1 <[email protected]> Signed-off-by: Matt Toohey <[email protected]>
…ts own Addresses the remaining optional finding of the review of 3c2719440. A grouped emoji's list could still go away without the row hearing it. Only MenuRoot's onOpenChange called onListClose, so two closes skipped it: the remainder emptying while the list is open, where the emoji clears listOpen during render, and the emoji unmounting with its list open. React then kept pointerInRow and the closed-preview marker, and every pill in the row opened its preview at once until the pointer left and re-entered the row. A ReactionEmoji effect keyed on listOpen now owns the report: its cleanup runs onListClose whenever the list stops being open, by onOpenChange, by the render-time close, or by unmount. onOpenChange only sets listOpen. leaveRow is idempotent and reads nothing but setters, so the captured callback is safe. Tests: - The Vitest split case now also covers the hover state. The pointer enters the row, crosses to the preview and into the list, and never leaves. After the group shrinks to one emoji, focus and the next frame's focus return settle, the pill is blurred, and hovering it waits the full rest: no tooltip at PREVIEW_REST_MS - 1, a tooltip at PREVIEW_REST_MS. Before the fix the tooltip opened at once. Validation: typecheck, lint, the messages Vitest suites (86 files, 2,296 tests; MessageMarkdown's long-body case timed out once under concurrent Playwright load and passed alone), and reactions.spec.mjs plus reactions-polish.spec.mjs in Chromium and WebKit. Three reactions-polish cases fail on this branch before and after this change because #1993 shortened the preview rest to 300 ms and its close grace to 75 ms after their 599 ms and 149 ms assertions were written; with those numbers corrected locally all 16 pass in each engine. Those assertions are left for a separate change. Co-Authored-By: Claude Fable 5.1 <[email protected]> Signed-off-by: Matt Toohey <[email protected]>
… buckets PR #1993 (dc0a681) shortened the preview rest from 600 ms to 300 ms and its close grace from 150 ms to 75 ms. The browser assertions this branch added in reactions-polish.spec.mjs still advanced the paused clock by 149 ms before expecting the preview to survive leaving the row, and by 599 ms before expecting no tooltip after the list closed. Both now advance 74 ms and 299 ms, one short of the current boundaries, and the following 1 ms step crosses each. The browser specs import nothing from src, so the literals stay; the Vitest cases already read PREVIEW_REST_MS. Validation: MessageReactions.test.tsx (26 passed) and reactions.spec.mjs plus reactions-polish.spec.mjs in Chromium and WebKit (32 passed). Co-Authored-By: Claude Fable 5.1 <[email protected]> Signed-off-by: Matt Toohey <[email protected]>
…ts own Addresses the remaining optional finding of the review of 3c2719440. A grouped emoji's list could still go away without the row hearing it. Only MenuRoot's onOpenChange called onListClose, so two closes skipped it: the remainder emptying while the list is open, where the emoji clears listOpen during render, and the emoji unmounting with its list open. React then kept pointerInRow and the closed-preview marker, and every pill in the row opened its preview at once until the pointer left and re-entered the row. A ReactionEmoji effect keyed on listOpen now owns the report: its cleanup runs onListClose whenever the list stops being open, by onOpenChange, by the render-time close, or by unmount. onOpenChange only sets listOpen. leaveRow is idempotent and reads nothing but setters, so the captured callback is safe. Tests: - The Vitest split case now also covers the hover state. The pointer enters the row, crosses to the preview and into the list, and never leaves. After the group shrinks to one emoji, focus and the next frame's focus return settle, the pill is blurred, and hovering it waits the full rest: no tooltip at PREVIEW_REST_MS - 1, a tooltip at PREVIEW_REST_MS. Before the fix the tooltip opened at once. Validation: typecheck, lint, the messages Vitest suites (86 files, 2,296 tests; MessageMarkdown's long-body case timed out once under concurrent Playwright load and passed alone), and reactions.spec.mjs plus reactions-polish.spec.mjs in Chromium and WebKit. Three reactions-polish cases fail on this branch before and after this change because #1993 shortened the preview rest to 300 ms and its close grace to 75 ms after their 599 ms and 149 ms assertions were written; with those numbers corrected locally all 16 pass in each engine. Those assertions are left for a separate change. Co-Authored-By: Claude Fable 5.1 <[email protected]> Signed-off-by: Matt Toohey <[email protected]>
… buckets PR #1993 (dc0a681) shortened the preview rest from 600 ms to 300 ms and its close grace from 150 ms to 75 ms. The browser assertions this branch added in reactions-polish.spec.mjs still advanced the paused clock by 149 ms before expecting the preview to survive leaving the row, and by 599 ms before expecting no tooltip after the list closed. Both now advance 74 ms and 299 ms, one short of the current boundaries, and the following 1 ms step crosses each. The browser specs import nothing from src, so the literals stay; the Vitest cases already read PREVIEW_REST_MS. Validation: MessageReactions.test.tsx (26 passed) and reactions.spec.mjs plus reactions-polish.spec.mjs in Chromium and WebKit (32 passed). Co-Authored-By: Claude Fable 5.1 <[email protected]> Signed-off-by: Matt Toohey <[email protected]>
…ts own Addresses the remaining optional finding of the review of 3c2719440. A grouped emoji's list could still go away without the row hearing it. Only MenuRoot's onOpenChange called onListClose, so two closes skipped it: the remainder emptying while the list is open, where the emoji clears listOpen during render, and the emoji unmounting with its list open. React then kept pointerInRow and the closed-preview marker, and every pill in the row opened its preview at once until the pointer left and re-entered the row. A ReactionEmoji effect keyed on listOpen now owns the report: its cleanup runs onListClose whenever the list stops being open, by onOpenChange, by the render-time close, or by unmount. onOpenChange only sets listOpen. leaveRow is idempotent and reads nothing but setters, so the captured callback is safe. Tests: - The Vitest split case now also covers the hover state. The pointer enters the row, crosses to the preview and into the list, and never leaves. After the group shrinks to one emoji, focus and the next frame's focus return settle, the pill is blurred, and hovering it waits the full rest: no tooltip at PREVIEW_REST_MS - 1, a tooltip at PREVIEW_REST_MS. Before the fix the tooltip opened at once. Validation: typecheck, lint, the messages Vitest suites (86 files, 2,296 tests; MessageMarkdown's long-body case timed out once under concurrent Playwright load and passed alone), and reactions.spec.mjs plus reactions-polish.spec.mjs in Chromium and WebKit. Three reactions-polish cases fail on this branch before and after this change because #1993 shortened the preview rest to 300 ms and its close grace to 75 ms after their 599 ms and 149 ms assertions were written; with those numbers corrected locally all 16 pass in each engine. Those assertions are left for a separate change. Co-Authored-By: Claude Fable 5.1 <[email protected]> Signed-off-by: Matt Toohey <[email protected]>
… buckets PR #1993 (dc0a681) shortened the preview rest from 600 ms to 300 ms and its close grace from 150 ms to 75 ms. The browser assertions this branch added in reactions-polish.spec.mjs still advanced the paused clock by 149 ms before expecting the preview to survive leaving the row, and by 599 ms before expecting no tooltip after the list closed. Both now advance 74 ms and 299 ms, one short of the current boundaries, and the following 1 ms step crosses each. The browser specs import nothing from src, so the literals stay; the Vitest cases already read PREVIEW_REST_MS. Validation: MessageReactions.test.tsx (26 passed) and reactions.spec.mjs plus reactions-polish.spec.mjs in Chromium and WebKit (32 passed). Co-Authored-By: Claude Fable 5.1 <[email protected]> Signed-off-by: Matt Toohey <[email protected]>
Message hover changes only the bubble background: 5% darker in light mode and 5% lighter in dark mode. Standalone tables, diffs, and emoji-only messages keep their existing surfaces. Text and media are unaffected.
Shortens the shared hover delays from #1641: familiar hints 500ms, helpful hints 250ms, previews 300ms, and hover controls 50ms; preview/control close grace periods are 75ms/50ms. Shared tokens and the design guide document the behavior.
Validation on
1f70c1543:color(srgb …)values without weakening contrast assertions.Native integration and enlarged-text host inspection remain deferred. Upload and reaction-count changes are excluded.
Follow-up CI repairs on
984d9d2b5: