From ae6826990a9da098e36f957c20a152137a68088a Mon Sep 17 00:00:00 2001 From: Ian Clarke Date: Tue, 14 Jul 2026 09:36:47 -0500 Subject: [PATCH 01/14] feat(ui): touch-friendly message actions, header spacing, scroll-to-latest (#402) Mobile/touch UX improvements from iPad + Pixel 7a feedback (#402): 1. Touch action menu (kebab). The per-message hover action bar is gated by Tailwind's automatic @media (hover:hover) wrapper, so it can never appear on a touch device. Add an always-visible kebab (shown only via @media (hover:none), the exact complement) that opens a Reply/React/Edit/Delete menu, reusing the existing handlers. The menu flips above the kebab near the bottom of the viewport so it is not clipped by the composer. The invisible hover bar is now pointer-events-none until hovered so it cannot intercept a stray gutter tap on touch. Desktop is unchanged. 2. Header spacing. Add gap between the header hamburger and the room-name button, and drop the room-title button's outward negative margin on mobile, so reaching for the room list no longer opens the room-details modal. 3. Scroll-to-latest + room-switch reset. A floating jump-to-latest button appears whenever the history is not pinned to the bottom (reusing the is_at_bottom IntersectionObserver state), and a room-change effect resets the scroll state so switching rooms always lands at the newest message. UI only; no wire/protocol/contract changes. Tests: new ui/tests/mobile-touch-ux.spec.ts (all 5 Playwright projects incl. mobile-chrome/mobile-safari). Verified interactively with screenshots on mobile (touch-emulated hover:none) and desktop. Closes #402 Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01JyYK79ygyquHkDqnnBQ9Xe --- ui/assets/main.css | 23 +++ ui/src/components/conversation.rs | 204 ++++++++++++++++++++++- ui/tests/mobile-touch-ux.spec.ts | 262 ++++++++++++++++++++++++++++++ 3 files changed, 482 insertions(+), 7 deletions(-) create mode 100644 ui/tests/mobile-touch-ux.spec.ts diff --git a/ui/assets/main.css b/ui/assets/main.css index 140bc2b2a..f399858f6 100644 --- a/ui/assets/main.css +++ b/ui/assets/main.css @@ -111,3 +111,26 @@ outline: 2px solid currentColor; outline-offset: 1px; } + +/* Touch-only message action affordance (freenet/river#402). + * + * The per-message hover action bar (reply / edit / delete) is built with + * Tailwind `group-hover:` utilities, which Tailwind v4 automatically wraps in + * `@media (hover: hover)`. On a touch device (phone, tablet) there is no hover + * pointer, so that bar can NEVER become visible and touch users have no way to + * reach reply / edit / delete / react. + * + * `.touch-actions` is the exact complement: hidden by default (so mouse users + * keep the clean hover-reveal desktop UI) and shown only where `hover: none` + * matches — i.e. precisely the devices where the hover bar cannot appear. It + * decorates the always-visible kebab (⋮) button that opens the touch action + * menu. */ +.touch-actions { + display: none; +} + +@media (hover: none) { + .touch-actions { + display: flex; + } +} diff --git a/ui/src/components/conversation.rs b/ui/src/components/conversation.rs index b484f347a..ef295d950 100644 --- a/ui/src/components/conversation.rs +++ b/ui/src/components/conversation.rs @@ -23,7 +23,8 @@ use chrono::{DateTime, Utc}; use dioxus::logger::tracing::*; use dioxus::prelude::*; use dioxus_free_icons::icons::fa_solid_icons::{ - FaBars, FaCircleInfo, FaTriangleExclamation, FaUsers, + FaBars, FaChevronDown, FaCircleInfo, FaEllipsisVertical, FaFaceSmile, FaPenToSquare, FaReply, + FaTrashCan, FaTriangleExclamation, FaUsers, }; use dioxus_free_icons::Icon; use freenet_scaffold::ComposableState; @@ -1131,6 +1132,24 @@ pub fn Conversation() -> Element { } }); + // Reset the scroll-to-bottom state whenever the selected room changes so a + // room switch always lands pinned to the newest message (#402). The + // Conversation component is mounted once and reused across rooms (hidden or + // shown via CSS), so `#chat-scroll-container` and `is_at_bottom` otherwise + // persist from the previous room, frequently leaving the user far up the + // new room's history. Reading `CURRENT_ROOM` (which holds only `owner_key`) + // makes this effect re-run on every room change and nothing else. Marking + // `first_scroll` true makes the next mount-triggered auto-scroll snap + // instantly rather than animate from an arbitrary position. + { + let first_scroll = first_scroll.clone(); + use_effect(move || { + let _room = CURRENT_ROOM.read().owner_key; + is_at_bottom.set(true); + first_scroll.set(true); + }); + } + // Handler for toggling a reaction on a message (add or remove) let handle_toggle_reaction = { let current_room_data = current_room_data.clone(); @@ -1758,10 +1777,13 @@ pub fn Conversation() -> Element { current_room_data.as_ref().map(|_room_data| { rsx! { div { class: "flex-shrink-0 px-3 md:px-6 py-3 border-b border-border bg-panel", - div { class: "flex items-center justify-between max-w-4xl mx-auto", - // Mobile: hamburger to open rooms panel + div { class: "flex items-center justify-between gap-2 md:gap-3 max-w-4xl mx-auto", + // Mobile: hamburger to open rooms panel. `mr-1` plus the row + // `gap-2` keep this switch-rooms button clear of the room-name + // tap target so a touch user does not open the room-details + // modal by mistake when reaching for the room list (#402). button { - class: "md:hidden p-2 rounded-lg text-text-muted hover:text-accent hover:bg-surface transition-colors", + class: "md:hidden flex-shrink-0 mr-1 p-2 rounded-lg text-text-muted hover:text-accent hover:bg-surface transition-colors", onclick: move |_| crate::util::defer(move || *MOBILE_VIEW.write() = MobileView::Rooms), Icon { icon: FaBars, width: 18, height: 18 } } @@ -1771,7 +1793,11 @@ pub fn Conversation() -> Element { // clicks to the modal-opening onclick handler. div { class: "min-w-0 flex-1", button { - class: "flex items-center gap-2 px-3 py-1.5 -mx-3 rounded-lg bg-transparent hover:bg-surface transition-colors cursor-pointer min-w-0 w-full", + // `md:-mx-3` only pulls the hover target outward on + // desktop, where there is no adjacent hamburger. On + // mobile the negative margin is dropped so this + // room-details target stays clear of the hamburger (#402). + class: "flex items-center gap-2 px-3 py-1.5 md:-mx-3 rounded-lg bg-transparent hover:bg-surface transition-colors cursor-pointer min-w-0 w-full", title: "Room details", onclick: move |_| { crate::util::defer(move || { @@ -1814,7 +1840,7 @@ pub fn Conversation() -> Element { // Combining flex-1 with overflow on the same element causes the // scroll container to shift behind the sidebar during re-renders. div { - class: "flex-1 min-h-0", + class: "flex-1 min-h-0 relative", div { class: "h-full overflow-y-auto", id: "chat-scroll-container", @@ -1959,6 +1985,37 @@ pub fn Conversation() -> Element { class: "h-px pointer-events-none", } } + // Scroll-to-latest button (#402): shown whenever the user is not + // pinned to the bottom of the history. Reuses the `is_at_bottom` + // IntersectionObserver state, so it appears after scrolling up + // (e.g. reading back through a long, multi-room history) and + // hides once the newest message is in view. Handy on every + // device but especially on touch, where there is no scrollbar + // to drag. + if !is_at_bottom() { + button { + class: "absolute bottom-4 right-4 z-30 flex items-center justify-center w-10 h-10 rounded-full bg-panel shadow-lg border border-border text-text-muted hover:text-accent transition-colors", + "aria-label": "Scroll to latest messages", + "data-testid": "scroll-to-bottom", + onclick: move |_| { + is_at_bottom.set(true); + #[cfg(target_arch = "wasm32")] + crate::util::safe_spawn_local(async move { + let Some(container) = web_sys::window() + .and_then(|w| w.document()) + .and_then(|d| d.get_element_by_id("chat-scroll-container")) + else { + return; + }; + let opts = web_sys::ScrollToOptions::new(); + opts.set_top(container.scroll_height() as f64); + opts.set_behavior(web_sys::ScrollBehavior::Smooth); + container.scroll_to_with_scroll_to_options(&opts); + }); + }, + Icon { icon: FaChevronDown, width: 18, height: 18 } + } + } } // Message input or status @@ -2201,6 +2258,19 @@ fn MessageGroupComponent( // Track which message's emoji picker is open (by message ID string) let mut open_emoji_picker: Signal> = use_signal(|| None); + // Track which message's touch action menu (kebab) is open, by message ID + // string. The hover action bar is gated by Tailwind's automatic + // `@media (hover:hover)` wrapper and so can never appear on a touch device; + // the kebab menu is the touch-only path to Reply / React / Edit / Delete + // (#402). + let mut open_action_menu: Signal> = use_signal(|| None); + + // Whether the kebab action menu should open above (true) or below (false) + // the kebab. Set from the tap position so the menu for a message near the + // bottom of the viewport flips upward instead of being clipped by the + // composer — mirrors `picker_show_above` for the emoji picker (#402). + let mut menu_show_above: Signal = use_signal(|| false); + // Track if emoji picker should appear above (true) or below (false) the button let mut picker_show_above: Signal = use_signal(|| false); @@ -2596,8 +2666,13 @@ fn MessageGroupComponent( let reply_author_name = group.author_name.clone(); rsx! { div { + // `pointer-events-none` until hovered so the invisible + // (opacity-0) bar never intercepts a tap on a touch device — + // Tailwind gates `group-hover:` behind `@media (hover:hover)`, + // so on touch this stays non-interactive and the kebab below + // owns the gutter (#402). class: format!( - "absolute top-1/2 -translate-y-1/2 transition-opacity z-50 flex flex-col items-start bg-panel rounded-lg shadow-md border border-border px-2 py-1.5 opacity-0 group-hover:opacity-100 {} {}", + "absolute top-1/2 -translate-y-1/2 transition-opacity z-50 flex flex-col items-start bg-panel rounded-lg shadow-md border border-border px-2 py-1.5 opacity-0 pointer-events-none group-hover:opacity-100 group-hover:pointer-events-auto {} {}", if is_self { "left-0 -translate-x-full -ml-2" } else { "right-0 translate-x-full ml-2" }, "" ), @@ -2637,6 +2712,121 @@ fn MessageGroupComponent( } } } + // Touch-only kebab action menu (#402). The hover + // action bar above is wrapped by Tailwind in + // `@media (hover:hover)`, so it can never appear on a + // touch device. `.touch-actions` (main.css) reveals + // this kebab only where there is no hover pointer; + // tapping it opens a menu with the same Reply / React / + // Edit / Delete actions. + { + let msg_id_kebab = msg.id.clone(); + let msg_id_kebab_toggle = msg.id.clone(); + let msg_id_menu_reply = msg.message_id.clone(); + let msg_id_menu_delete = msg.message_id.clone(); + let msg_id_menu_edit = msg.id.clone(); + let msg_id_menu_react = msg.id.clone(); + let edit_text_kebab = msg.content_text.clone(); + let reply_author_kebab = group.author_name.clone(); + let reply_preview_kebab = clean_reply_preview(&msg.content_text, &member_names) + .chars() + .take(100) + .collect::(); + let menu_open = open_action_menu.read().as_deref() + == Some(msg_id_kebab.as_str()); + rsx! { + div { + class: format!( + "touch-actions absolute top-1 z-50 {}", + if is_self { "left-0 -translate-x-full -ml-1" } else { "right-0 translate-x-full mr-1" } + ), + // Kebab toggle button + button { + class: "flex items-center justify-center w-8 h-8 rounded-full bg-panel shadow-md border border-border text-text-muted", + "aria-label": "Message actions", + "data-testid": "message-kebab", + onclick: move |e: MouseEvent| { + e.stop_propagation(); + let is_open = open_action_menu.peek().as_deref() + == Some(msg_id_kebab_toggle.as_str()); + if is_open { + open_action_menu.set(None); + } else { + // Flip the menu above the kebab when the tap is in the + // bottom ~40% of the viewport so it is not clipped by the + // composer or the screen edge. + let click_y = e.client_coordinates().y; + let viewport_height = web_sys::window() + .and_then(|w| w.inner_height().ok()) + .and_then(|h| h.as_f64()) + .unwrap_or(800.0); + menu_show_above.set(click_y > viewport_height * 0.6); + open_action_menu.set(Some(msg_id_kebab_toggle.clone())); + } + }, + Icon { icon: FaEllipsisVertical, width: 16, height: 16 } + } + // Action menu popover + dismiss backdrop + if menu_open { + div { + class: "fixed inset-0 z-40", + onclick: move |_| open_action_menu.set(None), + } + div { + class: format!( + "absolute z-50 min-w-[8rem] bg-panel rounded-lg shadow-lg border border-border py-1 flex flex-col {} {}", + if *menu_show_above.read() { "bottom-full mb-1" } else { "top-full mt-1" }, + if is_self { "left-0" } else { "right-0" } + ), + "data-testid": "message-action-menu", + button { + class: "flex items-center gap-2 px-3 py-2 text-sm text-text hover:bg-surface text-left", + onclick: move |_| { + on_reply.call(ReplyContext { + message_id: msg_id_menu_reply.clone(), + author_name: reply_author_kebab.clone(), + content_preview: reply_preview_kebab.clone(), + }); + open_action_menu.set(None); + }, + Icon { icon: FaReply, width: 14, height: 14 } + "Reply" + } + button { + class: "flex items-center gap-2 px-3 py-2 text-sm text-text hover:bg-surface text-left", + onclick: move |_| { + open_emoji_picker.set(Some(format!("inline-{}", msg_id_menu_react))); + open_action_menu.set(None); + }, + Icon { icon: FaFaceSmile, width: 14, height: 14 } + "React" + } + if is_self { + button { + class: "flex items-center gap-2 px-3 py-2 text-sm text-text hover:bg-surface text-left", + onclick: move |_| { + edit_text.set(edit_text_kebab.clone()); + editing_message.set(Some(msg_id_menu_edit.clone())); + open_action_menu.set(None); + }, + Icon { icon: FaPenToSquare, width: 14, height: 14 } + "Edit" + } + button { + class: "flex items-center gap-2 px-3 py-2 text-sm text-red-500 hover:bg-error-bg text-left", + onclick: move |_| { + on_request_delete.call(msg_id_menu_delete.clone()); + open_action_menu.set(None); + }, + Icon { icon: FaTrashCan, width: 14, height: 14 } + "Delete" + } + } + } + } + } + } + } } // Reactions display with inline add button { diff --git a/ui/tests/mobile-touch-ux.spec.ts b/ui/tests/mobile-touch-ux.spec.ts new file mode 100644 index 000000000..72e0dafe7 --- /dev/null +++ b/ui/tests/mobile-touch-ux.spec.ts @@ -0,0 +1,262 @@ +import { test, expect, Page } from "@playwright/test"; + +// Coverage for freenet/river#402 — mobile / touch UX improvements: +// 1. Touch-accessible message action menu (kebab), since the hover action +// bar can never appear on a device without a hover pointer. +// 2. Extra spacing between the header hamburger (open room list) and the +// room-name/details tap target, so switching rooms does not accidentally +// open the room-details modal. +// 3. A scroll-to-latest button shown whenever the history is not pinned to +// the bottom, plus a snap-to-bottom on room switch. + +// Helper: wait for WASM app to fully render +async function waitForApp(page: Page) { + await page.waitForSelector(".app-root", { timeout: 30_000 }); + await expect(page.locator("aside, .app-root button")).not.toHaveCount(0); +} + +// Helper: select a room at any viewport width (mirrors responsive-layout.spec). +async function selectRoom(page: Page, roomName: string) { + const roomBtn = page.getByRole("button", { name: roomName }); + + if (!(await roomBtn.isVisible({ timeout: 500 }).catch(() => false))) { + const hamburger = page.locator( + ".border-b.border-border.bg-panel button >> nth=0" + ); + if (await hamburger.isVisible({ timeout: 500 }).catch(() => false)) { + await hamburger.click(); + await expect(roomBtn).toBeVisible({ timeout: 5_000 }); + } else { + const vp = page.viewportSize(); + if (vp && vp.width < 768) { + await page.setViewportSize({ width: 1280, height: vp.height }); + await expect(roomBtn).toBeVisible({ timeout: 5_000 }); + await roomBtn.click(); + await expect( + page.getByRole("heading", { name: roomName }) + ).toBeVisible({ timeout: 5_000 }); + await page.setViewportSize({ width: vp.width, height: vp.height }); + return; + } + } + } + + await roomBtn.click(); + await expect( + page.getByRole("heading", { name: roomName }) + ).toBeVisible({ timeout: 5_000 }); +} + +// Whether this browser context has no hover pointer (i.e. a touch device). +// The kebab is shown only in that case; the hover action bar only otherwise. +async function isTouchOnly(page: Page): Promise { + return page.evaluate(() => window.matchMedia("(hover: none)").matches); +} + +// The app scrolls to the bottom asynchronously on room entry. Wait for that to +// settle before a test scrolls up, otherwise the pending async scroll races the +// test and snaps the history back down under it. +async function waitSettledAtBottom(page: Page) { + await expect + .poll( + () => + page.evaluate(() => { + const el = document.getElementById("chat-scroll-container"); + if (!el) return Number.MAX_SAFE_INTEGER; + return el.scrollHeight - el.scrollTop - el.clientHeight; + }), + { timeout: 5_000 } + ) + .toBeLessThan(120); +} + +async function distanceFromBottom(page: Page): Promise { + return page.evaluate(() => { + const el = document.getElementById("chat-scroll-container"); + if (!el) return Number.MAX_SAFE_INTEGER; + return el.scrollHeight - el.scrollTop - el.clientHeight; + }); +} + +// Scroll the history to the top and HOLD it there for a few frames. iOS WebKit +// momentum scrolling plus the app's async entry-scroll can otherwise snap the +// container back down before the IntersectionObserver registers that the user +// left the bottom. Holding at the top gives the observer a stable frame to fire +// (a real finger-scroll produces the same sustained not-at-bottom state). +async function scrollHistoryToTop(page: Page) { + await page.evaluate( + () => + new Promise((resolve) => { + const el = document.getElementById("chat-scroll-container"); + let n = 0; + const id = setInterval(() => { + if (el) el.scrollTop = 0; + if (++n > 6) { + clearInterval(id); + resolve(); + } + }, 60); + }) + ); +} + +test.describe("Message action kebab menu (#402.1)", () => { + test("kebab visibility follows hover capability", async ({ page }) => { + await page.goto("/"); + await waitForApp(page); + await selectRoom(page, "Your Private Room"); + + const kebab = page.locator('[data-testid="message-kebab"]').first(); + // Every message renders a kebab element; whether it is *displayed* is a + // pure-CSS decision keyed on `@media (hover: none)`. + await expect(kebab).toHaveCount(1); + + if (await isTouchOnly(page)) { + await expect(kebab).toBeVisible(); + } else { + // On a device with a hover pointer the kebab stays display:none — the + // desktop hover action bar is used instead. + await expect(kebab).toBeHidden(); + } + }); + + test("kebab opens a menu with Reply / Edit / Delete on own messages", async ({ + page, + }) => { + await page.goto("/"); + await waitForApp(page); + await selectRoom(page, "Your Private Room"); + + // This flow only applies where the kebab is actually usable (touch). + test.skip( + !(await isTouchOnly(page)), + "kebab menu is touch-only; desktop uses the hover action bar" + ); + + // A self (right-aligned, accent-coloured) message bubble. Its row carries + // the kebab that must expose Edit + Delete as well as Reply. + const ownRow = page.locator('[id^="msg-"]:has(.bg-accent)').first(); + await expect(ownRow).toBeVisible(); + const ownKebab = ownRow.locator('[data-testid="message-kebab"]'); + await ownKebab.click(); + + const menu = page.locator('[data-testid="message-action-menu"]'); + await expect(menu).toBeVisible(); + await expect(menu.getByRole("button", { name: "Reply" })).toBeVisible(); + await expect(menu.getByRole("button", { name: "Edit" })).toBeVisible(); + await expect(menu.getByRole("button", { name: "Delete" })).toBeVisible(); + + // Tapping the backdrop dismisses the menu. + await page.locator(".fixed.inset-0").first().click({ position: { x: 5, y: 5 } }); + await expect(menu).toBeHidden(); + }); + + test("Reply from the kebab opens the composer reply preview", async ({ + page, + }) => { + await page.goto("/"); + await waitForApp(page); + await selectRoom(page, "Your Private Room"); + test.skip( + !(await isTouchOnly(page)), + "kebab menu is touch-only; desktop uses the hover action bar" + ); + + const kebab = page.locator('[data-testid="message-kebab"]').first(); + await kebab.click(); + await page + .locator('[data-testid="message-action-menu"]') + .getByRole("button", { name: "Reply" }) + .click(); + + // The composer shows a reply-preview strip (with a "Cancel reply" button) + // once a reply target is set. + await expect(page.getByTitle("Cancel reply")).toBeVisible({ timeout: 5_000 }); + }); +}); + +test.describe("Mobile header hamburger spacing (#402.2)", () => { + test.use({ viewport: { width: 390, height: 844 } }); + + test("hamburger does not overlap the room-name tap target", async ({ + page, + }) => { + await page.goto("/"); + await waitForApp(page); + await selectRoom(page, "Team Chat Room"); + + const header = page.locator(".border-b.border-border.bg-panel").first(); + const hamburger = header.locator("button").first(); + // The room-details button is the one wrapping the room-name heading. + const roomDetails = header.locator("button:has(h2)").first(); + + const hb = await hamburger.boundingBox(); + const rb = await roomDetails.boundingBox(); + expect(hb).not.toBeNull(); + expect(rb).not.toBeNull(); + if (hb && rb) { + // The room-details target must start strictly to the right of the + // hamburger, with a real gap rather than an overlapping hit area. + const gap = rb.x - (hb.x + hb.width); + expect(gap).toBeGreaterThanOrEqual(4); + } + }); +}); + +test.describe("Scroll-to-latest button (#402.3)", () => { + // A short viewport guarantees the example history overflows and is scrollable + // regardless of the (randomised) example message lengths. + test.use({ viewport: { width: 500, height: 400 } }); + + test("appears when scrolled up and returns to bottom on click", async ({ + page, + }) => { + await page.goto("/"); + await waitForApp(page); + await selectRoom(page, "Team Chat Room"); + + const button = page.locator('[data-testid="scroll-to-bottom"]'); + // Pinned to the bottom on entry (once the async entry-scroll settles): no button. + await waitSettledAtBottom(page); + await expect(button).toHaveCount(0); + + // Scroll the history to the top; the button must appear. + await scrollHistoryToTop(page); + await expect(button).toBeVisible({ timeout: 5_000 }); + + await button.click(); + + // After clicking, the history returns to the bottom and the button hides. + await expect(button).toBeHidden({ timeout: 5_000 }); + // The scroll is animated (smooth), so poll until it settles at the bottom. + await expect.poll(() => distanceFromBottom(page), { timeout: 5_000 }).toBeLessThan(120); + }); +}); + +test.describe("Room-switch scroll reset (#402.3)", () => { + // Short viewport so the example history overflows and is scrollable. + test.use({ viewport: { width: 1280, height: 420 } }); + + test("switching rooms lands at the bottom even after scrolling up", async ({ + page, + }) => { + await page.goto("/"); + await waitForApp(page); + + // Enter a room and scroll up so it is no longer pinned to the bottom. + await selectRoom(page, "Your Private Room"); + await waitSettledAtBottom(page); + await scrollHistoryToTop(page); + await expect( + page.locator('[data-testid="scroll-to-bottom"]') + ).toBeVisible({ timeout: 5_000 }); + + // Switch away and back. The Conversation component is reused across rooms, + // so without the room-change reset the scroll position would persist near + // the top. It must snap back to the newest message instead. + await selectRoom(page, "Team Chat Room"); + await selectRoom(page, "Your Private Room"); + + await expect.poll(() => distanceFromBottom(page), { timeout: 5_000 }).toBeLessThan(120); + }); +}); From ff24431d32fd4e1338e1f4b66764eada1749ea20 Mon Sep 17 00:00:00 2001 From: Ian Clarke Date: Tue, 14 Jul 2026 10:09:03 -0500 Subject: [PATCH 02/14] fix(ui): address review findings on touch message actions (#402) Multi-model review (Codex + two adversarial Claude reviewers): - Room-switch scroll no longer relies on effect ordering: a persistent `force_scroll` Cell drives the mount-triggered snap regardless of `is_at_bottom`, so the IntersectionObserver can't cancel it in the gap between the room-change effect and the scroll effect. - Desktop hover action bar stays hit-testable: `pointer-events:none` is now applied only under `@media (hover:none)` (touch) via `.hover-actions`, not through a `group-hover:` gate that dropped hover while crossing the gutter gap. - Dismiss backdrop now covers the viewport: the kebab container uses `right-full`/`left-full` instead of `translate`, so no transformed ancestor shrinks the `fixed inset-0` backdrop. - Menu kept on-screen on narrow phones via `max-w-[calc(100vw-1rem)]` and centre-opening anchors. - All kebab-menu signal mutations wrapped in `crate::util::defer()` per .claude/rules/dioxus-signal-safety.md (Firefox-mobile re-entrancy). - Scroll-to-latest button no longer optimistically sets `is_at_bottom`; the observer is the sole driver so an interrupted scroll can't strand it. - React-from-kebab inherits the flip direction for the emoji picker. - Kebab gets aria-haspopup/aria-expanded. Tests: added narrow-viewport menu-overflow + far-tap-dismiss test and a room A->B->A switch test; existing suite green on all 5 Playwright projects. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01JyYK79ygyquHkDqnnBQ9Xe --- ui/assets/main.css | 10 +++ ui/src/components/conversation.rs | 132 ++++++++++++++++++++++-------- ui/tests/mobile-touch-ux.spec.ts | 54 +++++++++++- 3 files changed, 161 insertions(+), 35 deletions(-) diff --git a/ui/assets/main.css b/ui/assets/main.css index f399858f6..fb362faf7 100644 --- a/ui/assets/main.css +++ b/ui/assets/main.css @@ -133,4 +133,14 @@ .touch-actions { display: flex; } + + /* The hover action bar (reply/edit/delete) is opacity-0 until hovered, but + * an opacity-0 element still captures taps. On a touch device (no hover) it + * can never become visible, so make it non-interactive there; otherwise a + * tap on the gutter where it sits would fire an invisible reply/edit/delete. + * Left interactive on desktop (@media hover:hover) so slow mouse travel + * across the gap to the bar still works. (#402) */ + .hover-actions { + pointer-events: none; + } } diff --git a/ui/src/components/conversation.rs b/ui/src/components/conversation.rs index ef295d950..7580b7df8 100644 --- a/ui/src/components/conversation.rs +++ b/ui/src/components/conversation.rs @@ -1091,13 +1091,23 @@ pub fn Conversation() -> Element { // First scroll uses Instant so a page refresh snaps to the bottom rather than animating // a ~500ms scroll from top. Subsequent scrolls (new messages while at bottom) use Smooth. let first_scroll = use_hook(|| Rc::new(std::cell::Cell::new(true))); + // Set by the room-change effect below to force the next mount-triggered + // scroll regardless of `is_at_bottom` (#402). This decouples the room-switch + // snap from `is_at_bottom`, so the IntersectionObserver flipping the signal + // to `false` (new room's persisted scroll position) between the room-change + // effect and this one can't suppress the snap. A plain `Cell`, not a signal, + // so reading it here does not subscribe. + let force_scroll = use_hook(|| Rc::new(std::cell::Cell::new(false))); use_effect({ let first_scroll = first_scroll.clone(); + let force_scroll = force_scroll.clone(); move || { // Re-run when the last bubble mounts (new messages or initial load). let trigger = last_chat_element(); - let should_scroll = *is_at_bottom.peek(); + let forced = force_scroll.get(); + let should_scroll = forced || *is_at_bottom.peek(); if should_scroll && trigger.is_some() { + force_scroll.set(false); let is_first = first_scroll.replace(false); // `behavior` is only used inside the wasm32 block below; on // native it would warn as unused. Gate the binding too so @@ -1132,21 +1142,30 @@ pub fn Conversation() -> Element { } }); - // Reset the scroll-to-bottom state whenever the selected room changes so a - // room switch always lands pinned to the newest message (#402). The + // Snap to the newest message whenever the selected room changes (#402). The // Conversation component is mounted once and reused across rooms (hidden or // shown via CSS), so `#chat-scroll-container` and `is_at_bottom` otherwise // persist from the previous room, frequently leaving the user far up the // new room's history. Reading `CURRENT_ROOM` (which holds only `owner_key`) - // makes this effect re-run on every room change and nothing else. Marking - // `first_scroll` true makes the next mount-triggered auto-scroll snap - // instantly rather than animate from an arbitrary position. + // makes this effect re-run on every room change and nothing else. + // + // This effect only raises `force_scroll`; the actual scroll runs in the + // mount-triggered effect above once the new room's last bubble mounts (its + // trigger, `last_chat_element`, necessarily changes AFTER this room change, + // so the ordering is causal — not dependent on effect scheduling). Using a + // persistent `force_scroll` flag instead of `is_at_bottom` means the + // observer can't cancel the snap in the gap between the two effects. + // `first_scroll` = true makes that snap instant rather than animated from an + // arbitrary position. `is_at_bottom` = true hides the scroll-to-latest + // button immediately on switch (the observer reconfirms after the snap). { let first_scroll = first_scroll.clone(); + let force_scroll = force_scroll.clone(); use_effect(move || { let _room = CURRENT_ROOM.read().owner_key; - is_at_bottom.set(true); + force_scroll.set(true); first_scroll.set(true); + is_at_bottom.set(true); }); } @@ -1997,8 +2016,13 @@ pub fn Conversation() -> Element { class: "absolute bottom-4 right-4 z-30 flex items-center justify-center w-10 h-10 rounded-full bg-panel shadow-lg border border-border text-text-muted hover:text-accent transition-colors", "aria-label": "Scroll to latest messages", "data-testid": "scroll-to-bottom", + // Do NOT optimistically set `is_at_bottom` here: the + // IntersectionObserver flips it (hiding the button) once + // the sentinel actually reaches view. Setting it eagerly + // would leave the button hidden if the user interrupts + // the smooth scroll before reaching the bottom (the + // observer emits no new change and stays quiet). #402. onclick: move |_| { - is_at_bottom.set(true); #[cfg(target_arch = "wasm32")] crate::util::safe_spawn_local(async move { let Some(container) = web_sys::window() @@ -2666,13 +2690,15 @@ fn MessageGroupComponent( let reply_author_name = group.author_name.clone(); rsx! { div { - // `pointer-events-none` until hovered so the invisible - // (opacity-0) bar never intercepts a tap on a touch device — - // Tailwind gates `group-hover:` behind `@media (hover:hover)`, - // so on touch this stays non-interactive and the kebab below - // owns the gutter (#402). + // `.hover-actions` (main.css) makes this invisible + // (opacity-0) bar `pointer-events:none` ONLY on touch + // devices (@media hover:none), so it can't intercept a + // gutter tap there — while leaving it fully hit-testable on + // desktop, where the pointer must cross an empty gap to + // reach it (a Tailwind `group-hover:pointer-events` gate + // would drop hover mid-gap and make it unreachable). #402. class: format!( - "absolute top-1/2 -translate-y-1/2 transition-opacity z-50 flex flex-col items-start bg-panel rounded-lg shadow-md border border-border px-2 py-1.5 opacity-0 pointer-events-none group-hover:opacity-100 group-hover:pointer-events-auto {} {}", + "hover-actions absolute top-1/2 -translate-y-1/2 transition-opacity z-50 flex flex-col items-start bg-panel rounded-lg shadow-md border border-border px-2 py-1.5 opacity-0 group-hover:opacity-100 {} {}", if is_self { "left-0 -translate-x-full -ml-2" } else { "right-0 translate-x-full ml-2" }, "" ), @@ -2736,21 +2762,28 @@ fn MessageGroupComponent( == Some(msg_id_kebab.as_str()); rsx! { div { + // Positioned in the gutter beside the bubble with + // `right-full`/`left-full` (NOT `translate`): a transform + // would become the containing block for the `fixed` + // dismiss backdrop below, shrinking it to this element + // instead of the viewport (#402 review). class: format!( "touch-actions absolute top-1 z-50 {}", - if is_self { "left-0 -translate-x-full -ml-1" } else { "right-0 translate-x-full mr-1" } + if is_self { "right-full mr-1" } else { "left-full ml-1" } ), // Kebab toggle button button { class: "flex items-center justify-center w-8 h-8 rounded-full bg-panel shadow-md border border-border text-text-muted", "aria-label": "Message actions", + "aria-haspopup": "menu", + "aria-expanded": "{menu_open}", "data-testid": "message-kebab", onclick: move |e: MouseEvent| { e.stop_propagation(); let is_open = open_action_menu.peek().as_deref() == Some(msg_id_kebab_toggle.as_str()); if is_open { - open_action_menu.set(None); + crate::util::defer(move || open_action_menu.set(None)); } else { // Flip the menu above the kebab when the tap is in the // bottom ~40% of the viewport so it is not clipped by the @@ -2760,21 +2793,34 @@ fn MessageGroupComponent( .and_then(|w| w.inner_height().ok()) .and_then(|h| h.as_f64()) .unwrap_or(800.0); - menu_show_above.set(click_y > viewport_height * 0.6); - open_action_menu.set(Some(msg_id_kebab_toggle.clone())); + let above = click_y > viewport_height * 0.6; + let id = msg_id_kebab_toggle.clone(); + // Defer signal writes out of the event handler per + // .claude/rules/dioxus-signal-safety.md (Firefox-mobile + // re-entrant borrow crashes). + crate::util::defer(move || { + menu_show_above.set(above); + open_action_menu.set(Some(id)); + }); } }, Icon { icon: FaEllipsisVertical, width: 16, height: 16 } } - // Action menu popover + dismiss backdrop + // Action menu popover + dismiss backdrop. The backdrop is + // `fixed inset-0` (covers the viewport now that no transformed + // ancestor clips it) so a tap anywhere else dismisses. if menu_open { div { class: "fixed inset-0 z-40", - onclick: move |_| open_action_menu.set(None), + onclick: move |_| crate::util::defer(move || open_action_menu.set(None)), } div { + // Opens toward the bubble/centre (self: right of the + // left-gutter kebab; other: left of the right-gutter + // kebab); `max-w` clamps it to the viewport as a + // backstop against a narrow-screen overflow. class: format!( - "absolute z-50 min-w-[8rem] bg-panel rounded-lg shadow-lg border border-border py-1 flex flex-col {} {}", + "absolute z-50 min-w-[8rem] max-w-[calc(100vw-1rem)] bg-panel rounded-lg shadow-lg border border-border py-1 flex flex-col {} {}", if *menu_show_above.read() { "bottom-full mb-1" } else { "top-full mt-1" }, if is_self { "left-0" } else { "right-0" } ), @@ -2782,12 +2828,17 @@ fn MessageGroupComponent( button { class: "flex items-center gap-2 px-3 py-2 text-sm text-text hover:bg-surface text-left", onclick: move |_| { - on_reply.call(ReplyContext { - message_id: msg_id_menu_reply.clone(), - author_name: reply_author_kebab.clone(), - content_preview: reply_preview_kebab.clone(), + let id = msg_id_menu_reply.clone(); + let author = reply_author_kebab.clone(); + let preview = reply_preview_kebab.clone(); + crate::util::defer(move || { + on_reply.call(ReplyContext { + message_id: id, + author_name: author, + content_preview: preview, + }); + open_action_menu.set(None); }); - open_action_menu.set(None); }, Icon { icon: FaReply, width: 14, height: 14 } "Reply" @@ -2795,8 +2846,16 @@ fn MessageGroupComponent( button { class: "flex items-center gap-2 px-3 py-2 text-sm text-text hover:bg-surface text-left", onclick: move |_| { - open_emoji_picker.set(Some(format!("inline-{}", msg_id_menu_react))); - open_action_menu.set(None); + let picker_id = format!("inline-{}", msg_id_menu_react); + // Inherit the kebab's flip direction so the picker + // for a bottom message also opens upward, not + // clipped by the composer (#402 review). + let above = *menu_show_above.peek(); + crate::util::defer(move || { + picker_show_above.set(above); + open_emoji_picker.set(Some(picker_id)); + open_action_menu.set(None); + }); }, Icon { icon: FaFaceSmile, width: 14, height: 14 } "React" @@ -2805,9 +2864,13 @@ fn MessageGroupComponent( button { class: "flex items-center gap-2 px-3 py-2 text-sm text-text hover:bg-surface text-left", onclick: move |_| { - edit_text.set(edit_text_kebab.clone()); - editing_message.set(Some(msg_id_menu_edit.clone())); - open_action_menu.set(None); + let t = edit_text_kebab.clone(); + let id = msg_id_menu_edit.clone(); + crate::util::defer(move || { + edit_text.set(t); + editing_message.set(Some(id)); + open_action_menu.set(None); + }); }, Icon { icon: FaPenToSquare, width: 14, height: 14 } "Edit" @@ -2815,8 +2878,11 @@ fn MessageGroupComponent( button { class: "flex items-center gap-2 px-3 py-2 text-sm text-red-500 hover:bg-error-bg text-left", onclick: move |_| { - on_request_delete.call(msg_id_menu_delete.clone()); - open_action_menu.set(None); + let id = msg_id_menu_delete.clone(); + crate::util::defer(move || { + on_request_delete.call(id); + open_action_menu.set(None); + }); }, Icon { icon: FaTrashCan, width: 14, height: 14 } "Delete" diff --git a/ui/tests/mobile-touch-ux.spec.ts b/ui/tests/mobile-touch-ux.spec.ts index 72e0dafe7..9d26e17b5 100644 --- a/ui/tests/mobile-touch-ux.spec.ts +++ b/ui/tests/mobile-touch-ux.spec.ts @@ -146,8 +146,13 @@ test.describe("Message action kebab menu (#402.1)", () => { await expect(menu.getByRole("button", { name: "Edit" })).toBeVisible(); await expect(menu.getByRole("button", { name: "Delete" })).toBeVisible(); - // Tapping the backdrop dismisses the menu. - await page.locator(".fixed.inset-0").first().click({ position: { x: 5, y: 5 } }); + // Tapping anywhere else (a real viewport coordinate far from the menu, NOT + // the backdrop's own local origin) dismisses the menu — this verifies the + // fixed backdrop actually covers the viewport, not just the kebab box. + const vp = page.viewportSize(); + const box = await menu.boundingBox(); + const farX = box && vp && box.x > vp.width / 2 ? 5 : (vp?.width ?? 100) - 5; + await page.mouse.click(farX, 5); await expect(menu).toBeHidden(); }); @@ -173,6 +178,51 @@ test.describe("Message action kebab menu (#402.1)", () => { // once a reply target is set. await expect(page.getByTitle("Cancel reply")).toBeVisible({ timeout: 5_000 }); }); + + test("menu stays on-screen and dismisses via a far tap (narrow phone)", async ({ + page, + }) => { + await page.goto("/"); + await waitForApp(page); + await selectRoom(page, "Your Private Room"); + test.skip( + !(await isTouchOnly(page)), + "kebab menu is touch-only; desktop uses the hover action bar" + ); + + const vp = page.viewportSize(); + // Both a self (accent bubble) and a received (surface bubble) message: the + // menu opens on opposite sides, so both must stay within the viewport. + for (const sel of [ + '[id^="msg-"]:has(.bg-accent)', + '[id^="msg-"]:has(.bg-surface)', + ]) { + const row = page.locator(sel).first(); + if ((await row.count()) === 0) continue; + await row.locator('[data-testid="message-kebab"]').click(); + const menu = page.locator('[data-testid="message-action-menu"]'); + await expect(menu).toBeVisible(); + + const box = await menu.boundingBox(); + expect(box).not.toBeNull(); + if (box && vp) { + expect(box.x).toBeGreaterThanOrEqual(-1); + expect(box.x + box.width).toBeLessThanOrEqual(vp.width + 1); + } + // Opening the menu must not introduce a horizontal page scrollbar. + const hScroll = await page.evaluate( + () => + document.documentElement.scrollWidth > + document.documentElement.clientWidth + ); + expect(hScroll).toBe(false); + + // Dismiss via a far viewport tap before the next iteration. + const farX = box && vp && box.x > vp.width / 2 ? 5 : (vp?.width ?? 100) - 5; + await page.mouse.click(farX, 5); + await expect(menu).toBeHidden(); + } + }); }); test.describe("Mobile header hamburger spacing (#402.2)", () => { From 4cb3f7e23a737624d40fd177f7b7e3d7256d189e Mon Sep 17 00:00:00 2001 From: Ian Clarke Date: Tue, 14 Jul 2026 10:31:17 -0500 Subject: [PATCH 03/14] fix(ui): defer observer signal write; clip menu overflow (#402 review round 2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second Codex pass on the review fixes: - The scroll-to-latest button made `is_at_bottom` reactive (subscribed in render), so the IntersectionObserver's raw-closure `is_at_bottom.set()` — which runs with no Dioxus scope on the stack — could fire a subscriber notification from an empty scope and panic on Firefox mobile. Deferred that write via `crate::util::defer()` per dioxus-signal-safety.md. - Added `overflow-x-hidden` to the chat scroll container as a backstop so a kebab action menu on a very short self message can't produce a horizontal scrollbar (menu content is left-aligned and stays visible). Also updated the #205 edit-box-width test to open edit via the kebab on touch devices (the hover action bar is intentionally non-interactive there now). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01JyYK79ygyquHkDqnnBQ9Xe --- ui/src/components/conversation.rs | 16 +++++++++-- ui/tests/message-layout.spec.ts | 46 +++++++++++++++++++++++-------- 2 files changed, 49 insertions(+), 13 deletions(-) diff --git a/ui/src/components/conversation.rs b/ui/src/components/conversation.rs index 7580b7df8..dcec5d39c 100644 --- a/ui/src/components/conversation.rs +++ b/ui/src/components/conversation.rs @@ -1059,7 +1059,14 @@ pub fn Conversation() -> Element { .get(0) .dyn_ref::() { - is_at_bottom.set(entry.is_intersecting()); + // Defer the signal write: this raw JS callback runs with no + // Dioxus runtime/scope on the stack, and `is_at_bottom` is now + // subscribed in render (the scroll-to-latest button), so a + // direct `.set()` would fire a subscriber notification from an + // empty scope and panic on Firefox mobile. See + // .claude/rules/dioxus-signal-safety.md. (#402) + let intersecting = entry.is_intersecting(); + crate::util::defer(move || is_at_bottom.set(intersecting)); } }) as Box); @@ -1861,7 +1868,12 @@ pub fn Conversation() -> Element { div { class: "flex-1 min-h-0 relative", div { - class: "h-full overflow-y-auto", + // `overflow-x-hidden` is a backstop: a kebab action menu on + // a very short self message can extend a few px past the + // viewport edge; clip it (trailing whitespace only — the + // menu content is left-aligned and stays visible) rather + // than show a horizontal scrollbar in the history. #402. + class: "h-full overflow-y-auto overflow-x-hidden", id: "chat-scroll-container", div { class: "max-w-4xl mx-auto px-4 py-4", { diff --git a/ui/tests/message-layout.spec.ts b/ui/tests/message-layout.spec.ts index 6f77f391f..b89cf0feb 100644 --- a/ui/tests/message-layout.spec.ts +++ b/ui/tests/message-layout.spec.ts @@ -55,26 +55,50 @@ test.describe("Edit box width (#205)", () => { const count = await bubbles.count(); expect(count).toBeGreaterThan(0); + // On touch devices (no hover) the hover action bar is non-interactive; the + // real edit path is the kebab menu (freenet/river#402). Use whichever + // affordance the current device exposes. + const touch = await page.evaluate( + () => window.matchMedia("(hover: none)").matches + ); + let clicked = false; for (let i = 0; i < count; i++) { const bubble = bubbles.nth(i); await bubble.scrollIntoViewIfNeeded(); - await bubble.hover(); - // Scope the edit button lookup to the hovered bubble's ancestor - // (the outer message container with the hover action bar), so a - // stray previously-visible edit button on another message doesn't - // mask the current hover target. + // Scope the edit affordance lookup to this bubble's ancestor (the outer + // message container), so a stray control on another message doesn't mask + // the current target. const msgContainer = bubble.locator( "xpath=ancestor::*[starts-with(@id,'msg-')][1]" ); - const editBtn = msgContainer.getByRole("button", { name: /edit/i }); - if (await editBtn.isVisible({ timeout: 500 }).catch(() => false)) { - await editBtn.click(); - clicked = true; - break; + + if (touch) { + const kebab = msgContainer.locator('[data-testid="message-kebab"]'); + if (!(await kebab.isVisible({ timeout: 500 }).catch(() => false))) + continue; + await kebab.click(); + const editItem = page + .locator('[data-testid="message-action-menu"]') + .getByRole("button", { name: /edit/i }); + if (await editItem.isVisible({ timeout: 500 }).catch(() => false)) { + await editItem.click(); + clicked = true; + break; + } + // Received message (no Edit item): dismiss the menu and try the next. + await page.mouse.click(2, 2); + } else { + await bubble.hover(); + const editBtn = msgContainer.getByRole("button", { name: /edit/i }); + if (await editBtn.isVisible({ timeout: 500 }).catch(() => false)) { + await editBtn.click(); + clicked = true; + break; + } } } - expect(clicked, "found an own-message edit button").toBe(true); + expect(clicked, "found an own-message edit affordance").toBe(true); const textarea = page.locator("textarea").first(); await expect(textarea).toBeVisible({ timeout: 5_000 }); From 05aef7b9d0ec0aa559478c7e3745990800328478 Mon Sep 17 00:00:00 2001 From: Ian Clarke Date: Tue, 14 Jul 2026 10:42:36 -0500 Subject: [PATCH 04/14] fix(ui): defer send-path scroll write; tap-position menu anchoring (#402 review round 3) Third Codex pass: - `handle_send_message` set `is_at_bottom` synchronously from the send event handler; now that the scroll-to-latest button subscribes to that signal, the write is deferred (same Firefox-mobile re-entrancy fix as the observer callback). This completes the is_at_bottom signal-safety class: the two event-context writes defer, the use_effect write stays sync. - The kebab action menu now chooses its horizontal anchor from the tap X (open toward the viewport centre) instead of from self/other side, so its content can never run off a screen edge regardless of bubble width. The on-screen test now also asserts the first menu item stays fully visible. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01JyYK79ygyquHkDqnnBQ9Xe --- ui/src/components/conversation.rs | 51 +++++++++++++++++++++++-------- ui/tests/mobile-touch-ux.spec.ts | 9 ++++++ 2 files changed, 48 insertions(+), 12 deletions(-) diff --git a/ui/src/components/conversation.rs b/ui/src/components/conversation.rs index dcec5d39c..d6f3fc77f 100644 --- a/ui/src/components/conversation.rs +++ b/ui/src/components/conversation.rs @@ -1585,8 +1585,12 @@ pub fn Conversation() -> Element { // Message sending handler - receives message text from MessageInput component let handle_send_message = { move |(message_text, reply_ctx): (String, Option)| { - // Always scroll to bottom when user sends their own message - is_at_bottom.set(true); + // Always scroll to bottom when the user sends their own message. + // Deferred: `is_at_bottom` is now subscribed in render (the + // scroll-to-latest button), so a synchronous write from this event + // handler could fire a subscriber notification mid-stack and panic + // on Firefox mobile (.claude/rules/dioxus-signal-safety.md). #402 + crate::util::defer(move || is_at_bottom.set(true)); if message_text.is_empty() { warn!("Message is empty"); @@ -2307,6 +2311,13 @@ fn MessageGroupComponent( // composer — mirrors `picker_show_above` for the emoji picker (#402). let mut menu_show_above: Signal = use_signal(|| false); + // Whether the kebab action menu should be left-anchored (open rightward) or + // right-anchored (open leftward). Chosen from the tap's horizontal position + // so the menu always opens toward the viewport centre regardless of + // self/other side or bubble width, and never clips its content off a narrow + // screen edge (#402 review). + let mut menu_align_left: Signal = use_signal(|| false); + // Track if emoji picker should appear above (true) or below (false) the button let mut picker_show_above: Signal = use_signal(|| false); @@ -2797,21 +2808,37 @@ fn MessageGroupComponent( if is_open { crate::util::defer(move || open_action_menu.set(None)); } else { - // Flip the menu above the kebab when the tap is in the - // bottom ~40% of the viewport so it is not clipped by the - // composer or the screen edge. - let click_y = e.client_coordinates().y; - let viewport_height = web_sys::window() - .and_then(|w| w.inner_height().ok()) - .and_then(|h| h.as_f64()) - .unwrap_or(800.0); - let above = click_y > viewport_height * 0.6; + // Position the menu from the tap coordinates: flip it + // above the kebab when the tap is in the bottom ~40% of + // the viewport (so the composer doesn't clip it), and + // open it toward the viewport centre (left-anchored when + // the kebab is on the left half, right-anchored on the + // right half) so its content never runs off a screen edge. + let coords = e.client_coordinates(); + let (win_w, win_h) = web_sys::window() + .map(|w| { + let h = w + .inner_height() + .ok() + .and_then(|v| v.as_f64()) + .unwrap_or(800.0); + let w = w + .inner_width() + .ok() + .and_then(|v| v.as_f64()) + .unwrap_or(400.0); + (w, h) + }) + .unwrap_or((400.0, 800.0)); + let above = coords.y > win_h * 0.6; + let align_left = coords.x < win_w * 0.5; let id = msg_id_kebab_toggle.clone(); // Defer signal writes out of the event handler per // .claude/rules/dioxus-signal-safety.md (Firefox-mobile // re-entrant borrow crashes). crate::util::defer(move || { menu_show_above.set(above); + menu_align_left.set(align_left); open_action_menu.set(Some(id)); }); } @@ -2834,7 +2861,7 @@ fn MessageGroupComponent( class: format!( "absolute z-50 min-w-[8rem] max-w-[calc(100vw-1rem)] bg-panel rounded-lg shadow-lg border border-border py-1 flex flex-col {} {}", if *menu_show_above.read() { "bottom-full mb-1" } else { "top-full mt-1" }, - if is_self { "left-0" } else { "right-0" } + if *menu_align_left.read() { "left-0" } else { "right-0" } ), "data-testid": "message-action-menu", button { diff --git a/ui/tests/mobile-touch-ux.spec.ts b/ui/tests/mobile-touch-ux.spec.ts index 9d26e17b5..4002ea5db 100644 --- a/ui/tests/mobile-touch-ux.spec.ts +++ b/ui/tests/mobile-touch-ux.spec.ts @@ -209,6 +209,15 @@ test.describe("Message action kebab menu (#402.1)", () => { expect(box.x).toBeGreaterThanOrEqual(-1); expect(box.x + box.width).toBeLessThanOrEqual(vp.width + 1); } + // The menu content (first action) must be fully on-screen, not clipped by + // the scroll container's overflow-x-hidden backstop. + const replyBox = await menu + .getByRole("button", { name: "Reply" }) + .boundingBox(); + if (replyBox && vp) { + expect(replyBox.x).toBeGreaterThanOrEqual(-1); + expect(replyBox.x + replyBox.width).toBeLessThanOrEqual(vp.width + 1); + } // Opening the menu must not introduce a horizontal page scrollbar. const hScroll = await page.evaluate( () => From aa19a8cedf8fbf14a3a903535d5c29dea6a3a98f Mon Sep 17 00:00:00 2001 From: Ian Clarke Date: Tue, 14 Jul 2026 11:00:20 -0500 Subject: [PATCH 05/14] fix(ui): space-based menu flip; single shared action-menu state (#402 review round 4) Fourth Codex pass: - The kebab menu's up/down flip is now chosen from the actual space below the tap (~220px) rather than a fixed 60% viewport fraction, so its actions stay reachable on short/landscape viewports. - Hoisted `open_action_menu` from per-MessageGroupComponent to the parent Conversation, so only one action menu is open at a time across the whole history (opening one closes any other). Added a regression test. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01JyYK79ygyquHkDqnnBQ9Xe --- ui/src/components/conversation.rs | 23 +++++++++++++++-------- ui/tests/mobile-touch-ux.spec.ts | 26 ++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 8 deletions(-) diff --git a/ui/src/components/conversation.rs b/ui/src/components/conversation.rs index d6f3fc77f..b927ba883 100644 --- a/ui/src/components/conversation.rs +++ b/ui/src/components/conversation.rs @@ -915,6 +915,10 @@ pub fn Conversation() -> Element { }; let last_chat_element = use_signal(|| None as Option>); let mut is_at_bottom = use_signal(|| true); + // Which message's touch action menu (kebab) is open, by message ID string. + // Owned by Conversation (not per message group) so only ONE menu is open at + // a time across the whole history — opening one closes any other (#402). + let open_action_menu: Signal> = use_signal(|| None); let mut replying_to: Signal> = use_signal(|| None); // State for delete confirmation modal @@ -1992,6 +1996,7 @@ pub fn Conversation() -> Element { } } }, + open_action_menu: open_action_menu, } } } @@ -2276,7 +2281,10 @@ fn MessageGroupComponent( on_request_delete: EventHandler, on_edit: EventHandler<(MessageId, String)>, on_reply: EventHandler, + // Shared across all groups so only one action menu is open at a time (#402). + open_action_menu: Signal>, ) -> Element { + let mut open_action_menu = open_action_menu; let timestamp_ms = group.first_time.timestamp_millis(); let time_str = format_utc_as_local_time(timestamp_ms); let delay_suffix = group @@ -2298,13 +2306,6 @@ fn MessageGroupComponent( // Track which message's emoji picker is open (by message ID string) let mut open_emoji_picker: Signal> = use_signal(|| None); - // Track which message's touch action menu (kebab) is open, by message ID - // string. The hover action bar is gated by Tailwind's automatic - // `@media (hover:hover)` wrapper and so can never appear on a touch device; - // the kebab menu is the touch-only path to Reply / React / Edit / Delete - // (#402). - let mut open_action_menu: Signal> = use_signal(|| None); - // Whether the kebab action menu should open above (true) or below (false) // the kebab. Set from the tap position so the menu for a message near the // bottom of the viewport flips upward instead of being clipped by the @@ -2830,7 +2831,13 @@ fn MessageGroupComponent( (w, h) }) .unwrap_or((400.0, 800.0)); - let above = coords.y > win_h * 0.6; + // Flip the menu above the kebab when there isn't room + // for it below the tap (the menu is ~190px tall for an + // own message; the composer eats the bottom too). Basing + // this on actual space-below rather than a fixed viewport + // fraction keeps the actions reachable on short/landscape + // viewports. (#402 review) + let above = (win_h - coords.y) < 220.0; let align_left = coords.x < win_w * 0.5; let id = msg_id_kebab_toggle.clone(); // Defer signal writes out of the event handler per diff --git a/ui/tests/mobile-touch-ux.spec.ts b/ui/tests/mobile-touch-ux.spec.ts index 4002ea5db..e6f43a6e3 100644 --- a/ui/tests/mobile-touch-ux.spec.ts +++ b/ui/tests/mobile-touch-ux.spec.ts @@ -232,6 +232,32 @@ test.describe("Message action kebab menu (#402.1)", () => { await expect(menu).toBeHidden(); } }); + + test("never more than one menu open at a time", async ({ page }) => { + await page.goto("/"); + await waitForApp(page); + await selectRoom(page, "Your Private Room"); + test.skip( + !(await isTouchOnly(page)), + "kebab menu is touch-only; desktop uses the hover action bar" + ); + + const menus = page.locator('[data-testid="message-action-menu"]'); + const kebabs = page.locator('[data-testid="message-kebab"]'); + + // Open the first message's menu. + await kebabs.nth(0).click(); + await expect(menus).toHaveCount(1); + + // A later message's kebab (its z-50 container paints above the first menu's + // backdrop) is tappable while the first menu is open. Opening it must leave + // exactly ONE menu — the hoisted open-menu state closes the first. Without + // that shared state both would stay open. + const later = kebabs.nth(2); + await later.scrollIntoViewIfNeeded(); + await later.click(); + await expect(menus).toHaveCount(1); + }); }); test.describe("Mobile header hamburger spacing (#402.2)", () => { From a681ac8566b95aaca2d9a2ef217e997b670e578e Mon Sep 17 00:00:00 2001 From: Ian Clarke Date: Tue, 14 Jul 2026 11:11:34 -0500 Subject: [PATCH 06/14] fix(ui): measure kebab-menu flip against the chat scrollport (#402 review round 5) The menu's up/down flip now measures space below the tap against the chat scroll container's bottom edge (which sits above the composer) rather than window.innerHeight, so a downward own-message menu can't clip Edit/Delete just above the scrollport. Falls back to a window estimate if the element is absent. Two other round-5 P2s handled without code change here: the per-group emoji picker (two pickers across groups) is pre-existing and folded into #404; the hybrid hover:hover-plus-touch device gap is a documented conscious tradeoff (@sanity to weigh) since any-pointer coverage has its own regressions. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01JyYK79ygyquHkDqnnBQ9Xe --- ui/src/components/conversation.rs | 40 +++++++++++++++---------------- 1 file changed, 19 insertions(+), 21 deletions(-) diff --git a/ui/src/components/conversation.rs b/ui/src/components/conversation.rs index b927ba883..6fb21e133 100644 --- a/ui/src/components/conversation.rs +++ b/ui/src/components/conversation.rs @@ -2816,28 +2816,26 @@ fn MessageGroupComponent( // the kebab is on the left half, right-anchored on the // right half) so its content never runs off a screen edge. let coords = e.client_coordinates(); - let (win_w, win_h) = web_sys::window() - .map(|w| { - let h = w - .inner_height() - .ok() - .and_then(|v| v.as_f64()) - .unwrap_or(800.0); - let w = w - .inner_width() - .ok() - .and_then(|v| v.as_f64()) - .unwrap_or(400.0); - (w, h) + let win_w = web_sys::window() + .and_then(|w| w.inner_width().ok()) + .and_then(|v| v.as_f64()) + .unwrap_or(400.0); + // Flip the menu above the kebab when there isn't room for + // it below the tap (the menu is ~190px tall for an own + // message). Measure space against the chat scrollport's + // bottom, NOT the window: the menu lives inside that + // overflow-y-auto container, whose bottom sits above the + // composer, so a downward menu past that edge would clip + // Edit/Delete. Keeps the actions reachable on short and + // landscape viewports. (#402 review) + let scrollport_bottom = web_sys::window() + .and_then(|w| w.document()) + .and_then(|d| { + d.get_element_by_id("chat-scroll-container") }) - .unwrap_or((400.0, 800.0)); - // Flip the menu above the kebab when there isn't room - // for it below the tap (the menu is ~190px tall for an - // own message; the composer eats the bottom too). Basing - // this on actual space-below rather than a fixed viewport - // fraction keeps the actions reachable on short/landscape - // viewports. (#402 review) - let above = (win_h - coords.y) < 220.0; + .map(|el| el.get_bounding_client_rect().bottom()) + .unwrap_or(600.0); + let above = (scrollport_bottom - coords.y) < 200.0; let align_left = coords.x < win_w * 0.5; let id = msg_id_kebab_toggle.clone(); // Defer signal writes out of the event handler per From 5af8c8b623dfb2d3bc5c3395acfc2649dfa3e204 Mon Sep 17 00:00:00 2001 From: Ian Clarke Date: Tue, 14 Jul 2026 11:26:06 -0500 Subject: [PATCH 07/14] fix(ui): raise open action menu above sibling kebabs; two-way flip fit (#402 review round 6) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sixth Codex pass: - Every `.touch-actions` wrapper was z-50, so a later message's kebab painted above an open menu and could intercept taps on its Reply/Edit rows. The open wrapper is now z-[60], lifting the whole popover (and its backdrop, which then covers sibling kebabs — a tap on one just dismisses) above them. - The up/down flip now compares the menu's height (4 rows own / 2 rows received) against the space in BOTH directions within the scrollport, so a received menu that fits below is not force-flipped up and clipped at the top. Regression test updated: tapping a sibling kebab while a menu is open dismisses it (never two open; menu rows not intercepted). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01JyYK79ygyquHkDqnnBQ9Xe --- ui/src/components/conversation.rs | 42 +++++++++++++++++++++---------- ui/tests/mobile-touch-ux.spec.ts | 20 +++++++++------ 2 files changed, 41 insertions(+), 21 deletions(-) diff --git a/ui/src/components/conversation.rs b/ui/src/components/conversation.rs index 6fb21e133..124f08734 100644 --- a/ui/src/components/conversation.rs +++ b/ui/src/components/conversation.rs @@ -2791,8 +2791,16 @@ fn MessageGroupComponent( // would become the containing block for the `fixed` // dismiss backdrop below, shrinking it to this element // instead of the viewport (#402 review). + // Raise the OPEN wrapper above sibling kebabs: every + // `.touch-actions` is z-50, and later ones paint above an + // open popover, so without this a nearby message's kebab + // could sit over the menu rows and steal the tap. `z-[60]` + // lifts the whole open popover+backdrop above them (and its + // backdrop then covers those kebabs, so a tap on one just + // dismisses). (#402 review) class: format!( - "touch-actions absolute top-1 z-50 {}", + "touch-actions absolute top-1 {} {}", + if menu_open { "z-[60]" } else { "z-50" }, if is_self { "right-full mr-1" } else { "left-full ml-1" } ), // Kebab toggle button @@ -2820,22 +2828,30 @@ fn MessageGroupComponent( .and_then(|w| w.inner_width().ok()) .and_then(|v| v.as_f64()) .unwrap_or(400.0); - // Flip the menu above the kebab when there isn't room for - // it below the tap (the menu is ~190px tall for an own - // message). Measure space against the chat scrollport's - // bottom, NOT the window: the menu lives inside that - // overflow-y-auto container, whose bottom sits above the - // composer, so a downward menu past that edge would clip - // Edit/Delete. Keeps the actions reachable on short and - // landscape viewports. (#402 review) - let scrollport_bottom = web_sys::window() + // Choose the flip direction from the space available in + // BOTH directions within the chat scrollport (which lives + // inside an overflow-y-auto container whose bounds sit + // above the composer and below the header). Open downward + // when the menu fits below; only flip up when it doesn't + // fit below AND there's more room above. A received menu + // (2 rows) is shorter than an own menu (4 rows), so it + // stays down in cases where an own menu would flip. + // (#402 review) + let (sp_top, sp_bottom) = web_sys::window() .and_then(|w| w.document()) .and_then(|d| { d.get_element_by_id("chat-scroll-container") }) - .map(|el| el.get_bounding_client_rect().bottom()) - .unwrap_or(600.0); - let above = (scrollport_bottom - coords.y) < 200.0; + .map(|el| { + let r = el.get_bounding_client_rect(); + (r.top(), r.bottom()) + }) + .unwrap_or((60.0, 600.0)); + let menu_height = if is_self { 200.0 } else { 110.0 }; + let space_below = sp_bottom - coords.y; + let space_above = coords.y - sp_top; + let above = + space_below < menu_height && space_above > space_below; let align_left = coords.x < win_w * 0.5; let id = msg_id_kebab_toggle.clone(); // Defer signal writes out of the event handler per diff --git a/ui/tests/mobile-touch-ux.spec.ts b/ui/tests/mobile-touch-ux.spec.ts index e6f43a6e3..211a404f4 100644 --- a/ui/tests/mobile-touch-ux.spec.ts +++ b/ui/tests/mobile-touch-ux.spec.ts @@ -249,14 +249,18 @@ test.describe("Message action kebab menu (#402.1)", () => { await kebabs.nth(0).click(); await expect(menus).toHaveCount(1); - // A later message's kebab (its z-50 container paints above the first menu's - // backdrop) is tappable while the first menu is open. Opening it must leave - // exactly ONE menu — the hoisted open-menu state closes the first. Without - // that shared state both would stay open. - const later = kebabs.nth(2); - await later.scrollIntoViewIfNeeded(); - await later.click(); - await expect(menus).toHaveCount(1); + // While a menu is open its wrapper is raised (z-[60]) so its full-viewport + // backdrop covers every other kebab. A tap at a later message's kebab + // therefore lands on the backdrop (Playwright's .click() would refuse the + // obscured element, so dispatch at the coordinate) and dismisses the menu — + // never two open, and the menu's own rows can't be intercepted by a sibling + // kebab. A second tap would then open that message's menu. + const box = await kebabs.nth(2).boundingBox(); + expect(box).not.toBeNull(); + if (box) { + await page.mouse.click(box.x + box.width / 2, box.y + box.height / 2); + } + await expect(menus).toHaveCount(0); }); }); From 5e86b384b2e62ea413f106d1743bce2251ef3f81 Mon Sep 17 00:00:00 2001 From: Ian Clarke Date: Tue, 14 Jul 2026 11:39:25 -0500 Subject: [PATCH 08/14] fix(ui): send uses force_scroll; single shared emoji picker (#402 review round 7) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Seventh Codex pass: - `handle_send_message` now raises `force_scroll` (consumed by the mount effect when the sent message appears) instead of writing `is_at_bottom`. A send that fails mounts no message, so the scroll position — and the scroll-to-latest button — are left untouched rather than the button being wrongly hidden. Also removes a signal write from the send event handler entirely. - Hoisted `open_emoji_picker` to Conversation (like open_action_menu) so at most one picker is open across groups, and opening an action menu now dismisses any open picker — the two popovers can no longer stack. Added a React->picker test. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01JyYK79ygyquHkDqnnBQ9Xe --- ui/src/components/conversation.rs | 31 +++++++++++++++++++++---------- ui/tests/mobile-touch-ux.spec.ts | 28 ++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 10 deletions(-) diff --git a/ui/src/components/conversation.rs b/ui/src/components/conversation.rs index 124f08734..69c4930f9 100644 --- a/ui/src/components/conversation.rs +++ b/ui/src/components/conversation.rs @@ -919,6 +919,10 @@ pub fn Conversation() -> Element { // Owned by Conversation (not per message group) so only ONE menu is open at // a time across the whole history — opening one closes any other (#402). let open_action_menu: Signal> = use_signal(|| None); + // Which message's emoji reaction picker is open, by picker ID string. Also + // owned by Conversation so at most one picker is open across all groups, and + // opening an action menu can dismiss it (they'd otherwise stack) (#402). + let open_emoji_picker: Signal> = use_signal(|| None); let mut replying_to: Signal> = use_signal(|| None); // State for delete confirmation modal @@ -1588,13 +1592,16 @@ pub fn Conversation() -> Element { // Message sending handler - receives message text from MessageInput component let handle_send_message = { + let force_scroll = force_scroll.clone(); move |(message_text, reply_ctx): (String, Option)| { - // Always scroll to bottom when the user sends their own message. - // Deferred: `is_at_bottom` is now subscribed in render (the - // scroll-to-latest button), so a synchronous write from this event - // handler could fire a subscriber notification mid-stack and panic - // on Firefox mobile (.claude/rules/dioxus-signal-safety.md). #402 - crate::util::defer(move || is_at_bottom.set(true)); + // Scroll the user's own new message into view once it mounts. Uses + // `force_scroll` (a plain Cell, consumed by the mount-triggered + // effect) rather than writing `is_at_bottom`: a send that fails + // (payload too large, delta rejected) mounts no message, so nothing + // scrolls and the scroll-to-latest button's state is left untouched + // instead of being wrongly cleared. Also avoids a signal write from + // this event handler entirely. #402 + force_scroll.set(true); if message_text.is_empty() { warn!("Message is empty"); @@ -1997,6 +2004,7 @@ pub fn Conversation() -> Element { } }, open_action_menu: open_action_menu, + open_emoji_picker: open_emoji_picker, } } } @@ -2281,10 +2289,13 @@ fn MessageGroupComponent( on_request_delete: EventHandler, on_edit: EventHandler<(MessageId, String)>, on_reply: EventHandler, - // Shared across all groups so only one action menu is open at a time (#402). + // Shared across all groups so only one action menu / one picker is open at a + // time, and the two can coordinate (opening a menu dismisses a picker) (#402). open_action_menu: Signal>, + open_emoji_picker: Signal>, ) -> Element { let mut open_action_menu = open_action_menu; + let mut open_emoji_picker = open_emoji_picker; let timestamp_ms = group.first_time.timestamp_millis(); let time_str = format_utc_as_local_time(timestamp_ms); let delay_suffix = group @@ -2303,9 +2314,6 @@ fn MessageGroupComponent( let time_clamped = group.time_clamped; let is_self = group.is_self; - // Track which message's emoji picker is open (by message ID string) - let mut open_emoji_picker: Signal> = use_signal(|| None); - // Whether the kebab action menu should open above (true) or below (false) // the kebab. Set from the tap position so the menu for a message near the // bottom of the viewport flips upward instead of being clipped by the @@ -2860,6 +2868,9 @@ fn MessageGroupComponent( crate::util::defer(move || { menu_show_above.set(above); menu_align_left.set(align_left); + // Dismiss any open reaction picker so the two + // popovers can't stack (#402 review). + open_emoji_picker.set(None); open_action_menu.set(Some(id)); }); } diff --git a/ui/tests/mobile-touch-ux.spec.ts b/ui/tests/mobile-touch-ux.spec.ts index 211a404f4..a7b6ffd6e 100644 --- a/ui/tests/mobile-touch-ux.spec.ts +++ b/ui/tests/mobile-touch-ux.spec.ts @@ -179,6 +179,34 @@ test.describe("Message action kebab menu (#402.1)", () => { await expect(page.getByTitle("Cancel reply")).toBeVisible({ timeout: 5_000 }); }); + test("React from the kebab opens the emoji picker and closes the menu", async ({ + page, + }) => { + await page.goto("/"); + await waitForApp(page); + await selectRoom(page, "Your Private Room"); + test.skip( + !(await isTouchOnly(page)), + "kebab menu is touch-only; desktop uses the hover action bar" + ); + + const menu = page.locator('[data-testid="message-action-menu"]'); + await page.locator('[data-testid="message-kebab"]').first().click(); + await menu.getByRole("button", { name: "React" }).click(); + + // The action menu closes and the emoji picker (emoji buttons titled + // "React with …") opens. + await expect(menu).toBeHidden(); + await expect(page.getByTitle(/^React with/).first()).toBeVisible({ + timeout: 5_000, + }); + + // Opening an action menu again dismisses the picker (they must not stack). + await page.locator('[data-testid="message-kebab"]').first().click(); + await expect(menu).toBeVisible(); + await expect(page.getByTitle(/^React with/)).toHaveCount(0); + }); + test("menu stays on-screen and dismisses via a far tap (narrow phone)", async ({ page, }) => { From 09bd87e5479eddf4b66e3e5a20db4aa230b1aad2 Mon Sep 17 00:00:00 2001 From: Ian Clarke Date: Tue, 14 Jul 2026 12:09:08 -0500 Subject: [PATCH 09/14] fix(ui): raise open picker above kebabs; cap menu height (#402 review round 8) Eighth Codex pass: - Raise the open reaction picker's wrapper to z-[60] (like the action menu) so a nearby closed kebab can't paint over the emoji grid and steal a tap; the picker's backdrop then covers the kebabs so tapping one just dismisses it. - Cap the action menu at max-h-[80vh] with overflow-y-auto so an own-message menu is never taller than the viewport with unreachable actions on a short/landscape scrollport. Also hardened the WebKit-flaky scroll-to-latest test: its IntersectionObserver lags on a programmatic scroll, so the helper now jumps to the top once, holds briefly to defeat the entry-scroll, then waits on the stable position (constant re-scrolling kept rescheduling the deferred observer so it never settled). Two round-8 P2s handled without code change: the force_scroll-on-failed-send leak is a strict improvement over the prior unconditional is_at_bottom write and is benign (rare failure + later inbound message); the React-inherited picker direction is an acceptable approximation. Both noted on the PR. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01JyYK79ygyquHkDqnnBQ9Xe --- ui/src/components/conversation.rs | 11 ++++-- ui/tests/mobile-touch-ux.spec.ts | 56 ++++++++++++++++++++----------- 2 files changed, 45 insertions(+), 22 deletions(-) diff --git a/ui/src/components/conversation.rs b/ui/src/components/conversation.rs index 69c4930f9..7247735f5 100644 --- a/ui/src/components/conversation.rs +++ b/ui/src/components/conversation.rs @@ -2891,7 +2891,7 @@ fn MessageGroupComponent( // kebab); `max-w` clamps it to the viewport as a // backstop against a narrow-screen overflow. class: format!( - "absolute z-50 min-w-[8rem] max-w-[calc(100vw-1rem)] bg-panel rounded-lg shadow-lg border border-border py-1 flex flex-col {} {}", + "absolute z-50 min-w-[8rem] max-w-[calc(100vw-1rem)] max-h-[80vh] overflow-y-auto bg-panel rounded-lg shadow-lg border border-border py-1 flex flex-col {} {}", if *menu_show_above.read() { "bottom-full mb-1" } else { "top-full mt-1" }, if *menu_align_left.read() { "left-0" } else { "right-0" } ), @@ -3043,7 +3043,14 @@ fn MessageGroupComponent( } // Inline add reaction button (same line height as reactions) div { - class: "relative group/react inline-flex items-center", + // Raise the whole picker (grid + z-40 backdrop) above the + // z-50 message kebabs while it's open, so a nearby closed + // kebab can't paint over the emoji grid and steal a tap + // (mirrors the action menu's z-[60] behaviour). (#402 review) + class: format!( + "relative group/react inline-flex items-center {}", + if is_inline_picker_open { "z-[60]" } else { "" } + ), // Invisible backdrop when picker is open if is_inline_picker_open { div { diff --git a/ui/tests/mobile-touch-ux.spec.ts b/ui/tests/mobile-touch-ux.spec.ts index a7b6ffd6e..f6391810b 100644 --- a/ui/tests/mobile-touch-ux.spec.ts +++ b/ui/tests/mobile-touch-ux.spec.ts @@ -78,12 +78,18 @@ async function distanceFromBottom(page: Page): Promise { }); } -// Scroll the history to the top and HOLD it there for a few frames. iOS WebKit -// momentum scrolling plus the app's async entry-scroll can otherwise snap the -// container back down before the IntersectionObserver registers that the user -// left the bottom. Holding at the top gives the observer a stable frame to fire -// (a real finger-scroll produces the same sustained not-at-bottom state). -async function scrollHistoryToTop(page: Page) { +// Scroll the history to the top and wait for the scroll-to-latest button to +// appear. iOS WebKit momentum scrolling plus the app's async entry-scroll and +// the deferred (setTimeout-based) IntersectionObserver can otherwise miss a +// single programmatic scroll, so re-assert scrollTop=0 on each poll until the +// observer registers "not at bottom" and the button renders. +async function scrollUpUntilButtonVisible(page: Page) { + const button = page.locator('[data-testid="scroll-to-bottom"]'); + // Jump to the top and hold there briefly to defeat the app's async + // entry-scroll, then STOP scrolling. WebKit's IntersectionObserver lags on a + // programmatic scroll (worsened by -webkit-overflow-scrolling: touch) but DOES + // fire once the position is stable — continuously re-scrolling instead keeps + // rescheduling the deferred observer callback so it never settles. await page.evaluate( () => new Promise((resolve) => { @@ -91,13 +97,15 @@ async function scrollHistoryToTop(page: Page) { let n = 0; const id = setInterval(() => { if (el) el.scrollTop = 0; - if (++n > 6) { + if (++n > 5) { clearInterval(id); resolve(); } - }, 60); + }, 80); }) ); + // Position is now stable at the top; give the lagging observer time to fire. + await expect(button).toBeVisible({ timeout: 12_000 }); } test.describe("Message action kebab menu (#402.1)", () => { @@ -201,10 +209,19 @@ test.describe("Message action kebab menu (#402.1)", () => { timeout: 5_000, }); - // Opening an action menu again dismisses the picker (they must not stack). - await page.locator('[data-testid="message-kebab"]').first().click(); - await expect(menu).toBeVisible(); + // While the picker is open its raised backdrop covers the kebabs, so a tap + // at a kebab lands on that backdrop and dismisses the picker (the two + // popovers can't stack). No action menu opens from that same tap. + const kbox = await page + .locator('[data-testid="message-kebab"]') + .first() + .boundingBox(); + expect(kbox).not.toBeNull(); + if (kbox) { + await page.mouse.click(kbox.x + kbox.width / 2, kbox.y + kbox.height / 2); + } await expect(page.getByTitle(/^React with/)).toHaveCount(0); + await expect(menu).toBeHidden(); }); test("menu stays on-screen and dismisses via a far tap (narrow phone)", async ({ @@ -338,15 +355,17 @@ test.describe("Scroll-to-latest button (#402.3)", () => { await expect(button).toHaveCount(0); // Scroll the history to the top; the button must appear. - await scrollHistoryToTop(page); - await expect(button).toBeVisible({ timeout: 5_000 }); + await scrollUpUntilButtonVisible(page); + await expect(button).toBeVisible(); await button.click(); - // After clicking, the history returns to the bottom and the button hides. + // Ground truth: the animated scroll returns the history to the bottom. + await expect + .poll(() => distanceFromBottom(page), { timeout: 8_000 }) + .toBeLessThan(120); + // Once the observer sees the sentinel again, the button hides. await expect(button).toBeHidden({ timeout: 5_000 }); - // The scroll is animated (smooth), so poll until it settles at the bottom. - await expect.poll(() => distanceFromBottom(page), { timeout: 5_000 }).toBeLessThan(120); }); }); @@ -363,10 +382,7 @@ test.describe("Room-switch scroll reset (#402.3)", () => { // Enter a room and scroll up so it is no longer pinned to the bottom. await selectRoom(page, "Your Private Room"); await waitSettledAtBottom(page); - await scrollHistoryToTop(page); - await expect( - page.locator('[data-testid="scroll-to-bottom"]') - ).toBeVisible({ timeout: 5_000 }); + await scrollUpUntilButtonVisible(page); // Switch away and back. The Conversation component is reused across rooms, // so without the room-change reset the scroll position would persist near From 343b39c691961c3050b21e434065a1a48110b07f Mon Sep 17 00:00:00 2001 From: Ian Clarke Date: Tue, 14 Jul 2026 12:21:41 -0500 Subject: [PATCH 10/14] fix(ui): revert emoji-picker hoist to remove a signal-safety crash path (#402 review round 9) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ninth Codex pass found a P1 the round-7 hoist introduced: lifting `open_emoji_picker` to a shared signal made it read by every MessageGroupComponent, but the pre-existing "+" / emoji-selection handlers still write it directly, so a direct set would notify all group subscribers synchronously and hit the documented Firefox-mobile RefCell panic. The round-8 picker `z-[60]` fix already enforces single-popover behaviour — an open picker's backdrop covers every other group's kebabs and "+" buttons, so tapping one dismisses the picker instead of stacking a second popover. The shared signal is therefore redundant. Revert it to a per-group `use_signal` (narrow subscription, no new crash path); coordination comes from the z-order. Also tightened the action menu's cap to max-h-[calc(100vh-9rem)] so its internal scroll actually engages within the scrollport on short/landscape viewports. Dismissed (justified): the force_scroll-on-failed-send leak (a strict improvement over the prior unconditional is_at_bottom write; triple-rare) and the React- inherited picker flip (acceptable approximation). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01JyYK79ygyquHkDqnnBQ9Xe --- ui/src/components/conversation.rs | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/ui/src/components/conversation.rs b/ui/src/components/conversation.rs index 7247735f5..8569cf24d 100644 --- a/ui/src/components/conversation.rs +++ b/ui/src/components/conversation.rs @@ -919,10 +919,6 @@ pub fn Conversation() -> Element { // Owned by Conversation (not per message group) so only ONE menu is open at // a time across the whole history — opening one closes any other (#402). let open_action_menu: Signal> = use_signal(|| None); - // Which message's emoji reaction picker is open, by picker ID string. Also - // owned by Conversation so at most one picker is open across all groups, and - // opening an action menu can dismiss it (they'd otherwise stack) (#402). - let open_emoji_picker: Signal> = use_signal(|| None); let mut replying_to: Signal> = use_signal(|| None); // State for delete confirmation modal @@ -2004,7 +2000,6 @@ pub fn Conversation() -> Element { } }, open_action_menu: open_action_menu, - open_emoji_picker: open_emoji_picker, } } } @@ -2289,13 +2284,16 @@ fn MessageGroupComponent( on_request_delete: EventHandler, on_edit: EventHandler<(MessageId, String)>, on_reply: EventHandler, - // Shared across all groups so only one action menu / one picker is open at a - // time, and the two can coordinate (opening a menu dismisses a picker) (#402). + // Shared across all groups so only one action menu is open at a time (#402). open_action_menu: Signal>, - open_emoji_picker: Signal>, ) -> Element { let mut open_action_menu = open_action_menu; - let mut open_emoji_picker = open_emoji_picker; + // Per-group: at most one picker per group, and while a picker is open its + // raised (z-[60]) backdrop covers every other group's kebabs and "+" + // buttons, so tapping one dismisses the picker rather than stacking a + // second popover — the single-popover guarantee comes from the z-order, not + // a shared signal (#402). + let mut open_emoji_picker: Signal> = use_signal(|| None); let timestamp_ms = group.first_time.timestamp_millis(); let time_str = format_utc_as_local_time(timestamp_ms); let delay_suffix = group @@ -2891,7 +2889,7 @@ fn MessageGroupComponent( // kebab); `max-w` clamps it to the viewport as a // backstop against a narrow-screen overflow. class: format!( - "absolute z-50 min-w-[8rem] max-w-[calc(100vw-1rem)] max-h-[80vh] overflow-y-auto bg-panel rounded-lg shadow-lg border border-border py-1 flex flex-col {} {}", + "absolute z-50 min-w-[8rem] max-w-[calc(100vw-1rem)] max-h-[calc(100vh-9rem)] overflow-y-auto bg-panel rounded-lg shadow-lg border border-border py-1 flex flex-col {} {}", if *menu_show_above.read() { "bottom-full mb-1" } else { "top-full mt-1" }, if *menu_align_left.read() { "left-0" } else { "right-0" } ), From 5c1124abec9003b8b37e5d64e91da5162e2e9106 Mon Sep 17 00:00:00 2001 From: Ian Clarke Date: Tue, 14 Jul 2026 12:31:59 -0500 Subject: [PATCH 11/14] fix(ui): cap action menu to measured scrollport space (#402 review round 10) Round 10 confirmed the signal-safety crash class is fully converged (no P1). Closing the last recurring P2: the action menu's max-height now uses the space actually measured on the chosen side at tap time (space_above/space_below within the chat scrollport), applied as an inline style, instead of a viewport-relative cap. On a short/landscape scrollport where the menu fits neither side, it now scrolls internally rather than being clipped by the scroll container with Edit/Delete unreachable. Added a short-viewport regression test. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01JyYK79ygyquHkDqnnBQ9Xe --- ui/src/components/conversation.rs | 18 +++++++++++++++- ui/tests/mobile-touch-ux.spec.ts | 35 +++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/ui/src/components/conversation.rs b/ui/src/components/conversation.rs index 8569cf24d..603be44e4 100644 --- a/ui/src/components/conversation.rs +++ b/ui/src/components/conversation.rs @@ -2325,6 +2325,13 @@ fn MessageGroupComponent( // screen edge (#402 review). let mut menu_align_left: Signal = use_signal(|| false); + // Max height (px) for the kebab action menu, measured at tap time as the + // actual space available on the chosen side within the chat scrollport. The + // menu is `overflow-y-auto`, so on a very short/landscape viewport where it + // fits neither side fully it scrolls internally instead of being clipped by + // the scroll container with Edit/Delete unreachable (#402 review). + let mut menu_max_h: Signal = use_signal(|| 0.0); + // Track if emoji picker should appear above (true) or below (false) the button let mut picker_show_above: Signal = use_signal(|| false); @@ -2859,6 +2866,13 @@ fn MessageGroupComponent( let above = space_below < menu_height && space_above > space_below; let align_left = coords.x < win_w * 0.5; + // Cap the menu to the actual space on the chosen side + // (minus a small gap) so it scrolls internally rather + // than being clipped by the scroll container when it + // fits neither side. Floor so it never collapses. + let avail = + (if above { space_above } else { space_below }) - 16.0; + let max_h = avail.max(140.0); let id = msg_id_kebab_toggle.clone(); // Defer signal writes out of the event handler per // .claude/rules/dioxus-signal-safety.md (Firefox-mobile @@ -2866,6 +2880,7 @@ fn MessageGroupComponent( crate::util::defer(move || { menu_show_above.set(above); menu_align_left.set(align_left); + menu_max_h.set(max_h); // Dismiss any open reaction picker so the two // popovers can't stack (#402 review). open_emoji_picker.set(None); @@ -2889,10 +2904,11 @@ fn MessageGroupComponent( // kebab); `max-w` clamps it to the viewport as a // backstop against a narrow-screen overflow. class: format!( - "absolute z-50 min-w-[8rem] max-w-[calc(100vw-1rem)] max-h-[calc(100vh-9rem)] overflow-y-auto bg-panel rounded-lg shadow-lg border border-border py-1 flex flex-col {} {}", + "absolute z-50 min-w-[8rem] max-w-[calc(100vw-1rem)] overflow-y-auto bg-panel rounded-lg shadow-lg border border-border py-1 flex flex-col {} {}", if *menu_show_above.read() { "bottom-full mb-1" } else { "top-full mt-1" }, if *menu_align_left.read() { "left-0" } else { "right-0" } ), + style: format!("max-height: {}px", *menu_max_h.read()), "data-testid": "message-action-menu", button { class: "flex items-center gap-2 px-3 py-2 text-sm text-text hover:bg-surface text-left", diff --git a/ui/tests/mobile-touch-ux.spec.ts b/ui/tests/mobile-touch-ux.spec.ts index f6391810b..2dce64ffb 100644 --- a/ui/tests/mobile-touch-ux.spec.ts +++ b/ui/tests/mobile-touch-ux.spec.ts @@ -278,6 +278,41 @@ test.describe("Message action kebab menu (#402.1)", () => { } }); + test("menu is capped to the scrollport height on a short viewport", async ({ + page, + }) => { + await page.setViewportSize({ width: 500, height: 340 }); + await page.goto("/"); + await waitForApp(page); + await selectRoom(page, "Your Private Room"); + test.skip( + !(await isTouchOnly(page)), + "kebab menu is touch-only; desktop uses the hover action bar" + ); + + // Own message (4-row menu) opened on a short scrollport: the menu must be + // capped to the available space (and scroll internally) rather than extend + // past the scroll container with actions unreachable. + await page + .locator('[id^="msg-"]:has(.bg-accent)') + .first() + .locator('[data-testid="message-kebab"]') + .click(); + const menu = page.locator('[data-testid="message-action-menu"]'); + await expect(menu).toBeVisible(); + + const box = await menu.boundingBox(); + const scrollportH = await page.evaluate(() => { + const el = document.getElementById("chat-scroll-container"); + return el ? el.clientHeight : 0; + }); + expect(box).not.toBeNull(); + if (box) { + // Fits within the scrollport (a few px slack), so nothing is clipped away. + expect(box.height).toBeLessThanOrEqual(scrollportH + 4); + } + }); + test("never more than one menu open at a time", async ({ page }) => { await page.goto("/"); await waitForApp(page); From 2dfd657e5533fa55881dc403334d22dfca56fa30 Mon Sep 17 00:00:00 2001 From: Ian Clarke Date: Tue, 14 Jul 2026 12:44:38 -0500 Subject: [PATCH 12/14] fix(ui): set force_scroll only on successful send; cap menu to measured space (#402 review round 11) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 11 (crash class still converged — no P1): - `handle_send_message` now raises `force_scroll` inside the successful `apply_delta` branch (cloned into the async send) instead of unconditionally at the top. A rejected send — empty, over `max_message_size`, or a serialize/sign/delta failure — no longer leaves the flag set for a later unrelated incoming message to consume and yank the reader to the bottom. - The action menu's max-height now uses exactly the measured space on the chosen side (the roomier one) with only a 1px degenerate floor, so on a tiny landscape scrollport it can never exceed the scroll container and clip its own rows — the previous 140px floor could overshoot a shorter available space. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01JyYK79ygyquHkDqnnBQ9Xe --- ui/src/components/conversation.rs | 32 +++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/ui/src/components/conversation.rs b/ui/src/components/conversation.rs index 603be44e4..935f6ea39 100644 --- a/ui/src/components/conversation.rs +++ b/ui/src/components/conversation.rs @@ -1590,15 +1590,6 @@ pub fn Conversation() -> Element { let handle_send_message = { let force_scroll = force_scroll.clone(); move |(message_text, reply_ctx): (String, Option)| { - // Scroll the user's own new message into view once it mounts. Uses - // `force_scroll` (a plain Cell, consumed by the mount-triggered - // effect) rather than writing `is_at_bottom`: a send that fails - // (payload too large, delta rejected) mounts no message, so nothing - // scrolls and the scroll-to-latest button's state is left untouched - // instead of being wrongly cleared. Also avoids a signal write from - // this event handler entirely. #402 - force_scroll.set(true); - if message_text.is_empty() { warn!("Message is empty"); return; @@ -1632,6 +1623,14 @@ pub fn Conversation() -> Element { .get_secret() .map(|(secret, version)| (*secret, version)); + // Cloned into the async send so `force_scroll` (consumed by the + // mount effect when the sent message appears) is raised ONLY + // after the delta applies locally — a rejected send (empty, + // over-size, serialize/sign/delta failure) then leaves the + // scroll position and the scroll-to-latest button untouched + // rather than snapping a later unrelated message to the bottom + // (#402 review). + let force_scroll = force_scroll.clone(); spawn_local(async move { use river_core::room_state::content::{ ReplyContentV1, TextContentV1, CONTENT_TYPE_REPLY, CONTENT_TYPE_TEXT, @@ -1796,6 +1795,9 @@ pub fn Conversation() -> Element { } }); if delta_applied { + // Local apply succeeded and a message will mount: + // scroll it into view (#402 review). + force_scroll.set(true); crate::util::debug_log("[send] marking NEEDS_SYNC"); crate::components::app::mark_needs_sync(current_room); #[cfg(target_arch = "wasm32")] @@ -2870,9 +2872,15 @@ fn MessageGroupComponent( // (minus a small gap) so it scrolls internally rather // than being clipped by the scroll container when it // fits neither side. Floor so it never collapses. - let avail = - (if above { space_above } else { space_below }) - 16.0; - let max_h = avail.max(140.0); + // Exactly the space on the chosen side (minus the + // mt-1/mb-1 gap): never larger, so the overflow-y-auto + // menu can't exceed the scrollport and clip its own + // rows. `above` already selects the roomier side, so + // this is realistically ample; the 1px floor only + // guards a degenerate near-zero measurement. + let max_h = ((if above { space_above } else { space_below }) + - 16.0) + .max(1.0); let id = msg_id_kebab_toggle.clone(); // Defer signal writes out of the event handler per // .claude/rules/dioxus-signal-safety.md (Firefox-mobile From 42bafe45836de9b4f72d46687307fa754a86edef Mon Sep 17 00:00:00 2001 From: Ian Clarke Date: Tue, 14 Jul 2026 12:53:43 -0500 Subject: [PATCH 13/14] fix(ui): guard force_scroll against spurious arming (#402 review round 12) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 12 (crash class still converged — no P1). Two ways the shared force_scroll flag could be armed without a matching mount to consume it, then snap an unrelated room to the bottom on a later message: - Re-selecting the already-open room in the sidebar rewrites CURRENT_ROOM with the same key, and Dioxus re-runs the reset effect on any write. Now guarded on an actual owner_key change (tracked in a per-component Cell). - A send whose async signing/apply completes after the user switched rooms armed the conversation-wide flag while a different room was visible. Now only armed if the send's target room is still the current one. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01JyYK79ygyquHkDqnnBQ9Xe --- ui/src/components/conversation.rs | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/ui/src/components/conversation.rs b/ui/src/components/conversation.rs index 935f6ea39..e463c7aac 100644 --- a/ui/src/components/conversation.rs +++ b/ui/src/components/conversation.rs @@ -1169,14 +1169,25 @@ pub fn Conversation() -> Element { // `first_scroll` = true makes that snap instant rather than animated from an // arbitrary position. `is_at_bottom` = true hides the scroll-to-latest // button immediately on switch (the observer reconfirms after the snap). + // + // Guarded on an ACTUAL key change: Dioxus re-runs the effect on any write + // to `CURRENT_ROOM`, and re-selecting the already-open room in the sidebar + // rewrites it with the same key. Without the guard that would arm + // `force_scroll` with no new bubble to consume it, so a later message would + // snap the reader to the bottom (#402 review). { let first_scroll = first_scroll.clone(); let force_scroll = force_scroll.clone(); + let prev_room = + use_hook(|| Rc::new(std::cell::Cell::new(None::))); use_effect(move || { - let _room = CURRENT_ROOM.read().owner_key; - force_scroll.set(true); - first_scroll.set(true); - is_at_bottom.set(true); + let room = CURRENT_ROOM.read().owner_key; + if prev_room.get() != room { + prev_room.set(room); + force_scroll.set(true); + first_scroll.set(true); + is_at_bottom.set(true); + } }); } @@ -1796,8 +1807,14 @@ pub fn Conversation() -> Element { }); if delta_applied { // Local apply succeeded and a message will mount: - // scroll it into view (#402 review). - force_scroll.set(true); + // scroll it into view — but only if the user is still + // viewing the room this send targeted. Signing is + // async, so they may have switched rooms; arming the + // conversation-wide flag then would snap the NEW room + // to the bottom on its next message (#402 review). + if CURRENT_ROOM.peek().owner_key == Some(current_room) { + force_scroll.set(true); + } crate::util::debug_log("[send] marking NEEDS_SYNC"); crate::components::app::mark_needs_sync(current_room); #[cfg(target_arch = "wasm32")] From 9a65367702a833df3d58c8b7e054365b59176bc2 Mon Sep 17 00:00:00 2001 From: Ian Clarke Date: Tue, 14 Jul 2026 13:10:16 -0500 Subject: [PATCH 14/14] test(ui): fix flaky #205 edit-box test on touch (CI mobile-chrome) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The touch branch iterated every message, opening each kebab and dismissing received-message menus between iterations. The menu close is deferred, so in CI the next kebab tap raced the still-open (z-[60]) backdrop and timed out. Target a self message's kebab directly and open Edit from it — no iterate/dismiss race. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01JyYK79ygyquHkDqnnBQ9Xe --- ui/tests/message-layout.spec.ts | 60 ++++++++++++++------------------- 1 file changed, 26 insertions(+), 34 deletions(-) diff --git a/ui/tests/message-layout.spec.ts b/ui/tests/message-layout.spec.ts index b89cf0feb..5e172c06f 100644 --- a/ui/tests/message-layout.spec.ts +++ b/ui/tests/message-layout.spec.ts @@ -48,13 +48,6 @@ test.describe("Edit box width (#205)", () => { await waitForApp(page); await selectRoom(page, "Your Private Room"); - // Hover each message bubble until we find one that exposes an Edit - // button (own messages in the private room, where the owner IS self). - // Bubbles are divs with `max-w-prose` in the class list. - const bubbles = page.locator(".max-w-prose"); - const count = await bubbles.count(); - expect(count).toBeGreaterThan(0); - // On touch devices (no hover) the hover action bar is non-interactive; the // real edit path is the kebab menu (freenet/river#402). Use whichever // affordance the current device exposes. @@ -63,34 +56,33 @@ test.describe("Edit box width (#205)", () => { ); let clicked = false; - for (let i = 0; i < count; i++) { - const bubble = bubbles.nth(i); - await bubble.scrollIntoViewIfNeeded(); - // Scope the edit affordance lookup to this bubble's ancestor (the outer - // message container), so a stray control on another message doesn't mask - // the current target. - const msgContainer = bubble.locator( - "xpath=ancestor::*[starts-with(@id,'msg-')][1]" - ); - - if (touch) { - const kebab = msgContainer.locator('[data-testid="message-kebab"]'); - if (!(await kebab.isVisible({ timeout: 500 }).catch(() => false))) - continue; - await kebab.click(); - const editItem = page - .locator('[data-testid="message-action-menu"]') - .getByRole("button", { name: /edit/i }); - if (await editItem.isVisible({ timeout: 500 }).catch(() => false)) { - await editItem.click(); - clicked = true; - break; - } - // Received message (no Edit item): dismiss the menu and try the next. - await page.mouse.click(2, 2); - } else { + if (touch) { + // Target a self (accent) message directly and open Edit from its kebab — + // no iterating/dismissing, so the deferred menu-close can't race a + // following kebab tap. + const ownRow = page.locator('[id^="msg-"]:has(.bg-accent)').first(); + await expect(ownRow).toBeVisible(); + await ownRow.scrollIntoViewIfNeeded(); + await ownRow.locator('[data-testid="message-kebab"]').click(); + await page + .locator('[data-testid="message-action-menu"]') + .getByRole("button", { name: /edit/i }) + .click(); + clicked = true; + } else { + // Hover each message bubble until one exposes an Edit button (own messages + // in the private room, where the owner IS self). Bubbles are divs with + // `max-w-prose` in the class list. + const bubbles = page.locator(".max-w-prose"); + const count = await bubbles.count(); + expect(count).toBeGreaterThan(0); + for (let i = 0; i < count; i++) { + const bubble = bubbles.nth(i); + await bubble.scrollIntoViewIfNeeded(); await bubble.hover(); - const editBtn = msgContainer.getByRole("button", { name: /edit/i }); + const editBtn = bubble + .locator("xpath=ancestor::*[starts-with(@id,'msg-')][1]") + .getByRole("button", { name: /edit/i }); if (await editBtn.isVisible({ timeout: 500 }).catch(() => false)) { await editBtn.click(); clicked = true;