From 5fa79d363bcbbb6c8fa7d6f26ee429eaa099a7ab Mon Sep 17 00:00:00 2001 From: Ivan Date: Thu, 7 May 2026 20:05:10 -0500 Subject: [PATCH] feat(conversationScroll): add scroll management functions and update E2E tests for message switching --- .../components/messages/conversationScroll.js | 22 ++++++++++++ tests/e2e/helpers.js | 31 ++++++++++++++++ .../e2e/messages-conversation-scroll.spec.js | 27 ++++++++++++++ tests/frontend/conversationScroll.test.js | 36 +++++++++++++++++++ 4 files changed, 116 insertions(+) diff --git a/meshchatx/src/frontend/components/messages/conversationScroll.js b/meshchatx/src/frontend/components/messages/conversationScroll.js index 0716d39..34bc4b8 100644 --- a/meshchatx/src/frontend/components/messages/conversationScroll.js +++ b/meshchatx/src/frontend/components/messages/conversationScroll.js @@ -64,6 +64,28 @@ export function scrollContainerToBottom(container) { } } +/** + * Clears stale scroll position when reusing the same scroll element for another thread. + * `scrollMessagesToBottom` / `scrollContainerToBottom` run after content is mounted. + * @param {Element | null | undefined} container + */ +export function resetMessagesScrollSurface(container) { + if (!container) { + return; + } + container.scrollTop = 0; +} + +/** + * When the scroll area has no inner content yet, `isScrollColumnReverse` is false and + * `isNearBottom` is misleading (empty scroller looks "at bottom"). Do not use it to settle. + * @param {Element | null | undefined} container + * @returns {boolean} + */ +export function canTrustScrollNearBottomHeuristic(container) { + return Boolean(container?.firstElementChild); +} + /** * Whether the user has scrolled into the region where older messages should be loaded. * @param {Element} container diff --git a/tests/e2e/helpers.js b/tests/e2e/helpers.js index b8ac2f7..a65032e 100644 --- a/tests/e2e/helpers.js +++ b/tests/e2e/helpers.js @@ -5,6 +5,7 @@ const E2E_BACKEND_PORT = process.env.E2E_BACKEND_PORT || "18079"; const E2E_BACKEND_ORIGIN = `http://127.0.0.1:${E2E_BACKEND_PORT}`; const E2E_SCROLL_PEER_HASH = `e2e0${"0".repeat(28)}`; +const E2E_SCROLL_ALT_PEER_HASH = `e2e1${"0".repeat(28)}`; function buildE2eLxmfRow({ peerHash, localHash, index, total, inbound }) { const hash = crypto.randomBytes(16).toString("hex"); @@ -76,6 +77,34 @@ async function seedE2eLongConversationThread(request, opts = {}) { return { peerHash, localHash }; } +/** + * Second conversation for tests that switch between a long and a short thread. + * @param {import('@playwright/test').APIRequestContext} request + * @param {{ messageCount?: number }} [opts] + */ +async function seedE2eAltShortConversationThread(request, opts = {}) { + const messageCount = opts.messageCount ?? 12; + const localHash = await getE2eLocalLxmfHash(request); + const peerHash = E2E_SCROLL_ALT_PEER_HASH; + const messages = []; + for (let i = 0; i < messageCount; i++) { + const row = buildE2eLxmfRow({ + peerHash, + localHash, + index: i, + total: messageCount, + inbound: i % 2 === 0, + }); + row.content = `E2E alt short ${String(i).padStart(3, "0")} ${"x".repeat(48)}`; + messages.push(row); + } + const imp = await request.post(`${E2E_BACKEND_ORIGIN}/api/v1/maintenance/messages/import`, { + data: { messages }, + }); + expect(imp.ok()).toBeTruthy(); + return { peerHash, localHash }; +} + const PALETTE_PLACEHOLDER = /Search commands,\s*(routes|navigate),\s*or peers\.{0,3}/i; /** @@ -125,10 +154,12 @@ async function dismissMapOnboardingTooltip(page) { module.exports = { E2E_BACKEND_ORIGIN, E2E_SCROLL_PEER_HASH, + E2E_SCROLL_ALT_PEER_HASH, PALETTE_PLACEHOLDER, dismissMapOnboardingTooltip, openCommandPalette, prepareE2eSession, getE2eLocalLxmfHash, seedE2eLongConversationThread, + seedE2eAltShortConversationThread, }; diff --git a/tests/e2e/messages-conversation-scroll.spec.js b/tests/e2e/messages-conversation-scroll.spec.js index 107681f..b5364ff 100644 --- a/tests/e2e/messages-conversation-scroll.spec.js +++ b/tests/e2e/messages-conversation-scroll.spec.js @@ -2,6 +2,7 @@ const { test, expect } = require("@playwright/test"); const { prepareE2eSession, seedE2eLongConversationThread, + seedE2eAltShortConversationThread, getE2eLocalLxmfHash, E2E_SCROLL_PEER_HASH, } = require("./helpers"); @@ -84,6 +85,7 @@ test.describe("Messages conversation scroll", () => { test.beforeAll(async ({ request }) => { await prepareE2eSession(request); await seedE2eLongConversationThread(request, { messageCount: 120 }); + await seedE2eAltShortConversationThread(request, { messageCount: 12 }); }); test.beforeEach(async ({ request }) => { @@ -211,6 +213,31 @@ test.describe("Messages conversation scroll", () => { expect(await messagesNearBottom(page)).toBe(true); }); + test("stays near bottom when switching between long and short threads repeatedly", async ({ page }) => { + await page.goto("/#/messages"); + await expect(page.getByText("Conversations", { exact: true }).first()).toBeVisible({ timeout: 25000 }); + const longRow = page + .locator(".conversation-item") + .filter({ hasText: /E2E scroll seed/ }) + .first(); + const shortRow = page + .locator(".conversation-item") + .filter({ hasText: /E2E alt short/ }) + .first(); + for (let i = 0; i < 5; i++) { + await longRow.click(); + await waitForMessagesViewportReady(page); + await expect(page.locator("#messages")).toBeVisible({ timeout: 25000 }); + await waitForMessagesOverflow(page); + expect(await messagesNearBottom(page)).toBe(true); + + await shortRow.click(); + await waitForMessagesViewportReady(page); + await expect(page.locator("#messages")).toBeVisible({ timeout: 25000 }); + expect(await messagesNearBottom(page)).toBe(true); + } + }); + test("preserves scroll anchor when loading older messages from the top", async ({ page }) => { await page.goto("/#/messages"); await expect(page.getByText("Conversations", { exact: true }).first()).toBeVisible({ timeout: 25000 }); diff --git a/tests/frontend/conversationScroll.test.js b/tests/frontend/conversationScroll.test.js index df1cfa2..db7f498 100644 --- a/tests/frontend/conversationScroll.test.js +++ b/tests/frontend/conversationScroll.test.js @@ -1,9 +1,11 @@ import { describe, it, expect } from "vitest"; import { SCROLL_BOTTOM_EPS_PX, + canTrustScrollNearBottomHeuristic, isNearBottom, isScrollColumnReverse, maxScrollTop, + resetMessagesScrollSurface, scrollContainerToBottom, shouldLoadPreviousMessages, } from "@/components/messages/conversationScroll.js"; @@ -134,4 +136,38 @@ describe("conversationScroll.js", () => { expect(shouldLoadPreviousMessages(el)).toBe(false); el.remove(); }); + + it("resetMessagesScrollSurface forces scrollTop to 0", () => { + const el = makeScrollContainer({ + reverse: false, + scrollTop: 400, + scrollHeight: 900, + clientHeight: 100, + }); + resetMessagesScrollSurface(el); + expect(el.scrollTop).toBe(0); + el.remove(); + }); + + it("canTrustScrollNearBottomHeuristic requires an inner child", () => { + const outer = document.createElement("div"); + document.body.appendChild(outer); + expect(canTrustScrollNearBottomHeuristic(outer)).toBe(false); + const inner = document.createElement("div"); + outer.appendChild(inner); + expect(canTrustScrollNearBottomHeuristic(outer)).toBe(true); + outer.remove(); + }); + + it("isNearBottom is misleading on empty container without inner (trust heuristic false)", () => { + const el = document.createElement("div"); + document.body.appendChild(el); + Object.defineProperty(el, "scrollHeight", { value: 400, configurable: true }); + Object.defineProperty(el, "clientHeight", { value: 400, configurable: true }); + el.scrollTop = 0; + expect(isScrollColumnReverse(el)).toBe(false); + expect(isNearBottom(el, SCROLL_BOTTOM_EPS_PX)).toBe(true); + expect(canTrustScrollNearBottomHeuristic(el)).toBe(false); + el.remove(); + }); });