diff --git a/e2e/USER_STORIES.md b/e2e/USER_STORIES.md index 8138c973..6f9943ad 100644 --- a/e2e/USER_STORIES.md +++ b/e2e/USER_STORIES.md @@ -401,8 +401,20 @@ Acceptance: - Clicking a result closes search and scrolls the timeline to the message, highlighted in view. - Opening a permalink url for a specific message lands on that message directly, - with a "scroll to bottom" control shown since the view is no longer at the - newest message. + with a "jump to newest" control shown since the view is no longer at the newest + message. Using it returns to the live end and the control goes away. + +### US-027a — Follow a link to a recent message + +As someone opening a push notification, I want the room to behave as though I +had scrolled to the bottom, so that I am not offered a way back to where I +already am. + +Acceptance: + +- A permalink to a message near the newest end lands with the room's last + message on screen and no "jump to newest" control, even when other events were + published after the one linked to. ### US-028 — Share a message somewhere else diff --git a/e2e/specs/rooms.spec.ts b/e2e/specs/rooms.spec.ts index 7004edb4..fe9a235e 100644 --- a/e2e/specs/rooms.spec.ts +++ b/e2e/specs/rooms.spec.ts @@ -1,4 +1,4 @@ -import {DAY, HOUR, WEEK, bech32ToHex} from "@welshman/lib" +import {DAY, HOUR, MINUTE, WEEK, bech32ToHex} from "@welshman/lib" import {getLnUrl} from "@welshman/util" import {MessagingRelayList, Profile, RelayList, displayPubkey} from "@welshman/domain" import type {Locator, Page} from "@playwright/test" @@ -28,6 +28,9 @@ const messages = (page: Page) => page.locator(".room__item") const message = (page: Page, text: string) => messages(page).filter({hasText: text}) +// The room's way back to the live end, up only while the bottom of the container isn't it. +const jumpToNewest = (page: Page) => page.getByRole("button", {name: "Jump to newest"}) + // RoomItem gives its hover actions no accessible names — every one is an icon. Their order is // fixed by the component: zap, emoji, reply, edit (only on your own recent message), menu. const messageActions = (page: Page, text: string) => @@ -706,7 +709,47 @@ test("US-027 find a past message and jump to it", async ({seed, as}) => { await page.goto(`${path}?at=${older.event.created_at}`) await expect(page.locator(`[data-event="${older.id}"]`)).toBeInViewport() - await expect(page.locator(".chat__scroll-down")).toBeVisible() + await expect(jumpToNewest(page)).toBeVisible() + + await jumpToNewest(page).click() + + await expect(message(page, "harbor lights are on tonight")).toBeInViewport() + await expect(jumpToNewest(page)).toHaveCount(0) +}) + +// A push notification links to the message it announced, which is near the newest end of the room, +// so the jump lands at the bottom with nowhere left to scroll down to. Anything published since — +// another message, a membership event — is newer than the one linked to, so "is this the last +// event in the room" is the wrong question to hang the button on; whether the loaded window has +// caught up to the present is the right one. +test("US-027a a permalink near the newest end lands at the bottom", async ({seed, as}) => { + let recent!: Seeded + + const scenario = await seed(({relay, user, at}) => { + const space = relay("space") + + space.room("general", {name: "General"}) + space.join(user.alice, "general") + space.join(user.bob, "general") + + for (let i = 0; i < 20; i++) { + space.message(user.bob, "general", `harbor watch note ${i}`, at(40 - i, MINUTE)) + } + + recent = space.message(user.bob, "general", "the harbor pilot is booked", at(5, MINUTE)) + + space.message(user.bob, "general", "and the tide is with us", at(2, MINUTE)) + }) + + const {url} = scenario.space("space") + const path = roomPath(url, "general") + const page = await as(users.alice, `${path}?at=${recent.event.created_at}`) + + await expect(page.locator(`[data-event="${recent.id}"]`)).toBeInViewport() + + // The newest message in the room is on screen with it, so this is the live end + await expect(message(page, "and the tide is with us")).toBeInViewport() + await expect(jumpToNewest(page)).toHaveCount(0) }) test("US-028 share a message somewhere else", async ({seed, as}) => { diff --git a/src/app/components/RoomChat.svelte b/src/app/components/RoomChat.svelte index 096824c0..7defa71b 100644 --- a/src/app/components/RoomChat.svelte +++ b/src/app/components/RoomChat.svelte @@ -298,12 +298,7 @@ } const manageScrollPosition = () => { - // Only treat an `at` jump as "scrolled up" when it targets an event below the - // newest one; jumping to the most recent message already lands us at the bottom. - const newestEvent = $events[$events.length - 1] - const atIsBelowNewest = !isNaN(at) && newestEvent !== undefined && at < newestEvent.created_at - - showScrollButton = atIsBelowNewest || Math.abs(element?.scrollTop || 0) > 1500 + scrolledUp = Math.abs(element?.scrollTop || 0) > 1500 const newMessages = document.getElementById("new-messages") @@ -382,7 +377,7 @@ let newMessagesBefore = $state(now()) let newMessagesSeen = false let showFixedNewMessages = $state(false) - let showScrollButton = $state(false) + let scrolledUp = $state(false) let cleanup: () => void let events: Readable = $state(readable([])) let compose: RoomCompose | undefined = $state() @@ -432,13 +427,19 @@ }), ) - // Newer messages are only worth waiting for when the window stops short of the present, which - // only happens after jumping into history — anything published from here on arrives through the - // repository rather than through a forward walk. And with no messages between them the two - // loaders would sit against each other, so this one yields while the other is still running. - const loadingForward = $derived( - !isNaN(at) && $newer?.status !== "exhausted" && !(elements.length === 0 && loadingBackward), - ) + // The window only stops short of the present after jumping into history — anything published + // from here on arrives through the repository rather than through a forward walk. + const windowStopsShort = $derived(!isNaN(at) && $newer?.status !== "exhausted") + + // With no messages between them the two loaders would sit against each other, so this one yields + // while the other is still running. + const loadingForward = $derived(windowStopsShort && !(elements.length === 0 && loadingBackward)) + + // While the window stops short, the bottom of the container is not the bottom of the + // conversation, so the button is the way back to the live end rather than a scroll — which is + // why it clears `at` instead of scrolling. Once the two are the same place, scroll position is + // the whole answer. + const showScrollButton = $derived(scrolledUp || windowStopsShort) $effect(() => { if (elements.length > 0 && !isUserScrolling) { @@ -665,7 +666,10 @@ {#if showScrollButton}
-