diff --git a/e2e/USER_STORIES.md b/e2e/USER_STORIES.md index b5a6fc0f..d481fba7 100644 --- a/e2e/USER_STORIES.md +++ b/e2e/USER_STORIES.md @@ -711,21 +711,23 @@ Acceptance: before submitting. - A reply to a thread in a room is tagged into that room, so the relay handles it as part of the group. -- The thread's opening post carries an "OP" badge on every page. +- The opening post stays above the replies, and the thread's author carries an + "OP" badge wherever their posts turn up. ### US-044 — Navigate a long thread -As bob, I want a long thread paginated and its posts individually linkable, so -that I can move around it and point people at one message. +As bob, I want a long thread to open on its newest replies with the earlier ones +within reach, and its posts individually linkable, so that I can catch up on it +and point people at one message. Acceptance: -- A thread with more than 20 posts shows pagination controls, and the page - number, next/prev, and first/last controls each move to the matching slice - with the "Page X of Y" indicator updating. +- A thread with more than 20 replies opens on its newest 20, under the opening + post, with a "Show earlier replies" control for the rest. +- That control reveals the next 20 without leaving the page. - "Permalink" on a post copies a link to that post. -- Opening that link as carol loads the thread, navigates to the page holding - that post, and scrolls it into view. +- Opening that link as carol loads the thread, reveals the post it names however + far back it is, and scrolls it into view. ### US-045 — Turn a chat message into a thread diff --git a/e2e/specs/articles-threads.spec.ts b/e2e/specs/articles-threads.spec.ts index 4005585f..b0be56e8 100644 --- a/e2e/specs/articles-threads.spec.ts +++ b/e2e/specs/articles-threads.spec.ts @@ -718,8 +718,8 @@ test("US-043 reply to a thread and to a specific post", async ({seed, as}) => { at(3, HOUR), ) - // Twenty replies plus the opening post is one post past the first page, and the last of them - // is alice's, so her OP badge has to survive the page change. + // Twenty replies is exactly the window a thread opens with, and the last of them is alice's, + // so her OP badge has to survive bob's own reply pushing the oldest one out of it. for (let i = 1; i <= 20; i++) { space.event( i === 20 ? user.alice : i % 2 === 0 ? user.carol : user.bob, @@ -762,6 +762,7 @@ test("US-043 reply to a thread and to a specific post", async ({seed, as}) => { await threadReply.getByRole("button", {name: "Post Reply"}).click() await expect(bob.getByText("21 replies")).toBeVisible() + await expect(bob.getByText("Twice a year here.")).toBeVisible() // The same for a thread reply, which goes out through a different composer. await expect @@ -770,16 +771,28 @@ test("US-043 reply to a thread and to a specific post", async ({seed, as}) => { ) .toEqual(["lounge"]) - // The thread's author is marked OP wherever their posts turn up, second page included. - await bob.getByRole("button", {name: "2", exact: true}).click() - await expect(bob.getByText("Page 2 of 2")).toBeVisible() + // His is the twenty first reply, so the oldest one drops out of the window, and the opening + // post stays above whatever the window holds. + const showEarlier = bob.getByRole("button", {name: "Show earlier replies"}) + await expect(showEarlier).toBeVisible() + await expect(bob.getByText("Reply 01", {exact: true})).toHaveCount(0) + await expect(openingPost).toBeVisible() + + // The thread's author is marked OP wherever their posts turn up. const alicesLastPost = bob.locator("article").filter({hasText: "Reply 20"}) await expect(alicesLastPost.getByText("OP", {exact: true})).toBeVisible() - await expect(bob.getByText("Twice a year here.")).toBeVisible() + + await showEarlier.click() + + await expect(bob.getByText("Reply 01", {exact: true})).toBeVisible() + await expect(showEarlier).toHaveCount(0) const carol = await as(users.carol, threadPath) + + await carol.getByRole("button", {name: "Show earlier replies"}).click() + const bobsFirstPost = carol.locator("article").filter({hasText: "Reply 01"}) await expect(bobsFirstPost).toBeVisible() @@ -798,23 +811,13 @@ test("US-043 reply to a thread and to a specific post", async ({seed, as}) => { await noteEditor(postReply).pressSequentially("Answering the thread instead.") await postReply.getByRole("button", {name: "Post Reply"}).click() - // Twenty one replies before hers is already a page and a bit, so her post is appended to the - // last one rather than to the page she wrote it from. The last control in the join is "go last", - // which is the same button whatever the page count turns out to be. - await carol - .getByText(/^Page \d+ of \d+$/) - .locator("xpath=..") - .locator(".join") - .getByRole("button") - .last() - .click() - + // Her post lands at the end of the one list, so there is nothing to click to reach it. await expect(carol.getByText("Answering the thread instead.")).toBeVisible() }) test("US-044 navigate a long thread", async ({seed, as}) => { let thread!: Seeded - let lastPost!: Seeded + let firstReply!: Seeded const scenario = await seed(({relay, user, at}) => { const space = relay("space") @@ -840,7 +843,7 @@ test("US-044 navigate a long thread", async ({seed, as}) => { at(4, HOUR), ) - // Forty one replies plus the opening post is three pages of twenty. + // Forty one replies is two reveals past the twenty a thread opens with. for (let i = 1; i <= 41; i++) { const reply = space.event( i % 2 === 0 ? user.alice : user.bob, @@ -855,8 +858,8 @@ test("US-044 navigate a long thread", async ({seed, as}) => { at(4, HOUR) + i * 60, ) - if (i === 41) { - lastPost = reply + if (i === 1) { + firstReply = reply } } @@ -869,55 +872,48 @@ test("US-044 navigate a long thread", async ({seed, as}) => { context: {permissions: ["clipboard-read", "clipboard-write"]}, }) - const indicator = bob.getByText(/^Page \d+ of \d+$/) + const showEarlier = bob.getByRole("button", {name: "Show earlier replies"}) - await expect(indicator).toHaveText("Page 1 of 3") - await expect(bob.getByText("Reply 19", {exact: true})).toBeVisible() + // Every assertion below is about which of the replies are in the window, so wait until they + // have all arrived. + await expect(bob.getByText("41 replies")).toBeVisible() - // ThreadPagination is first, prev, a button per page, next, last, and only the page numbers - // carry an accessible name — the rest are icons — so with three pages the ends are 0, 1, 5, 6. - const controls = indicator.locator("xpath=..").locator(".join").getByRole("button") - const goFirst = controls.nth(0) - const goPrev = controls.nth(1) - const goNext = controls.nth(5) - const goLast = controls.nth(6) + // The thread opens on its newest twenty replies, with the opening post above them. + await expect(bob.locator(`[data-event="${thread.id}"]`)).toBeVisible() + await expect(bob.getByText("Reply 41", {exact: true})).toBeVisible() + await expect(bob.getByText("Reply 22", {exact: true})).toBeVisible() + await expect(bob.getByText("Reply 21", {exact: true})).toHaveCount(0) - await goNext.click() - await expect(indicator).toHaveText("Page 2 of 3") - await expect(bob.getByText("Reply 20", {exact: true})).toBeVisible() + // Each reveal reaches twenty further back without leaving the page. + await showEarlier.click() - await goLast.click() - await expect(indicator).toHaveText("Page 3 of 3") + await expect(bob.getByText("Reply 02", {exact: true})).toBeVisible() + await expect(bob.getByText("Reply 01", {exact: true})).toHaveCount(0) await expect(bob.getByText("Reply 41", {exact: true})).toBeVisible() - await goPrev.click() - await expect(indicator).toHaveText("Page 2 of 3") - await expect(bob.getByText("Reply 20", {exact: true})).toBeVisible() + await showEarlier.click() - await goFirst.click() - await expect(indicator).toHaveText("Page 1 of 3") - await expect(bob.getByText("Reply 19", {exact: true})).toBeVisible() + await expect(bob.getByText("Reply 01", {exact: true})).toBeVisible() + await expect(showEarlier).toHaveCount(0) - await bob.getByRole("button", {name: "3", exact: true}).click() - await expect(indicator).toHaveText("Page 3 of 3") + const oldestPost = bob.locator(`[data-event="${firstReply.id}"]`) - const finalPost = bob.locator(`[data-event="${lastPost.id}"]`) - - await finalPost.getByRole("button", {name: "Permalink"}).click() + await oldestPost.getByRole("button", {name: "Permalink"}).click() await expect(bob.getByRole("alert")).toContainText("Copied to clipboard!") const permalink = await bob.evaluate(() => navigator.clipboard.readText()) const {pathname, hash} = new URL(permalink) expect(pathname).toBe(threadPath) - expect(hash).toBe(`#${nip19.neventEncode({id: lastPost.id, relays: [url]})}`) + expect(hash).toBe(`#${nip19.neventEncode({id: firstReply.id, relays: [url]})}`) + // A permalink reaches back as far as it has to on its own, so carol never sees the control. const carol = await as(users.carol, pathname + hash) - const target = carol.locator(`[data-event="${lastPost.id}"]`) + const target = carol.locator(`[data-event="${firstReply.id}"]`) - await expect(carol.getByText(/^Page \d+ of \d+$/)).toHaveText("Page 3 of 3") await expect(target).toBeVisible() await expect(target).toBeInViewport() + await expect(carol.getByRole("button", {name: "Show earlier replies"})).toHaveCount(0) }) test("US-045 turn a chat message into a thread", async ({seed, as}) => { diff --git a/src/app/components/ThreadPagination.svelte b/src/app/components/ThreadPagination.svelte deleted file mode 100644 index a091084d..00000000 --- a/src/app/components/ThreadPagination.svelte +++ /dev/null @@ -1,67 +0,0 @@ - - -
-
-

Page {page} of {pageCount}

-
- - - {#each pages as p, i (p)} - {#if i > 0 && p - pages[i - 1] > 1} - - {/if} - - {/each} - - -
-
diff --git a/src/routes/spaces/[relay]/threads/[id]/+page.svelte b/src/routes/spaces/[relay]/threads/[id]/+page.svelte index c2655a10..3a89bd6a 100644 --- a/src/routes/spaces/[relay]/threads/[id]/+page.svelte +++ b/src/routes/spaces/[relay]/threads/[id]/+page.svelte @@ -2,7 +2,6 @@ import {onDestroy} from "svelte" import * as nip19 from "nostr-tools/nip19" import {page} from "$app/stores" - import {goto} from "$app/navigation" import {call, sleep, spec, tryCatch} from "@welshman/lib" import type {MakeNonOptional} from "@welshman/lib" import type {TrustedEvent} from "@welshman/util" @@ -16,7 +15,6 @@ import Link from "@lib/components/Link.svelte" import SpaceBar from "@app/components/SpaceBar.svelte" import ThreadPost from "@app/components/ThreadPost.svelte" - import ThreadPagination from "@app/components/ThreadPagination.svelte" import EventReply from "@app/components/EventReply.svelte" import RoomName from "@app/components/RoomName.svelte" import {deriveEvent, deriveEventsById} from "@app/repository" @@ -25,7 +23,7 @@ import {decodeRelay} from "@app/relays" import {makeSpacePath, scrollToEvent} from "@app/routes" - const POSTS_PER_PAGE = 20 + const REPLY_BATCH_SIZE = 20 const {relay, id} = $page.params as MakeNonOptional const url = decodeRelay(relay) @@ -37,45 +35,46 @@ const back = () => history.back() - const posts = $derived.by(() => { - if (!$event) return [] - - return [$event, ...$replies] - }) - - const replyCount = $derived(Math.max(0, posts.length - 1)) + const replyCount = $derived($replies.length) const h = $derived(tagValue(tagSpec("h"), $event?.tags || [])) - const pageCount = $derived(Math.max(1, Math.ceil(posts.length / POSTS_PER_PAGE))) + // A permalink's target. Replies stream in newest first, so the post it names is usually here + // long before the ones above it — its position is only right once the thread has finished + // arriving, so this is kept and re-read rather than acted on the first time it shows up. + let target: string | undefined = $state( + call(() => { + const hash = window.location.hash.replace(/^#/, "") - const currentPage = $derived.by(() => { - const raw = parseInt($page.url.searchParams.get("page") || "1") + if (hash.startsWith("nevent1")) { + const decoded = tryCatch(() => nip19.decode(hash)) - if (Number.isNaN(raw) || raw < 1) return 1 - if (raw > pageCount) return pageCount - - return raw - }) - - const pagePosts = $derived( - posts.slice((currentPage - 1) * POSTS_PER_PAGE, currentPage * POSTS_PER_PAGE), + if (decoded?.type === "nevent") { + return decoded.data.id + } + } + }), ) - const setPage = (nextPage: number) => { - const params = new URLSearchParams($page.url.searchParams) + let revealed = $state(REPLY_BATCH_SIZE) - if (nextPage <= 1) { - params.delete("page") - } else { - params.set("page", String(nextPage)) - } + const targetIndex = $derived(target ? $replies.findIndex(spec({id: target})) : -1) - const search = params.toString() + // A thread reads from its newest end, and the reader pulls earlier replies in from there. A + // permalink reaches back as far as it has to on its own. + const visibleCount = $derived( + targetIndex < 0 ? revealed : Math.max(revealed, $replies.length - targetIndex), + ) - goto(`${$page.url.pathname}${search ? `?${search}` : ""}`, { - keepFocus: true, - noScroll: true, - }) + const visibleReplies = $derived($replies.slice(Math.max(0, $replies.length - visibleCount))) + + const hiddenCount = $derived($replies.length - visibleReplies.length) + + // Reaching back by hand hands control back to the reader. + const showEarlier = () => { + const nextCount = visibleCount + REPLY_BATCH_SIZE + + target = undefined + revealed = nextCount } const openReply = (post: TrustedEvent) => { @@ -105,45 +104,10 @@ let showReply = $state(false) let replyTo: TrustedEvent | undefined = $state() - // A permalink's target, read once because setPage drops the hash. Replies stream in newest - // first, so the post a permalink names is usually here long before the ones above it — its - // index, and the page it lands on, are only right once the thread has finished arriving. So - // this is kept and re-evaluated rather than acted on the first time the post shows up. - let target: string | undefined = $state( - call(() => { - const hash = window.location.hash.replace(/^#/, "") - - if (hash.startsWith("nevent1")) { - const decoded = tryCatch(() => nip19.decode(hash)) - - if (decoded?.type === "nevent") { - return decoded.data.id - } - } - }), - ) - - // Paginating by hand hands control back to the reader. - const goToPage = (nextPage: number) => { - target = undefined - - setPage(nextPage) - } - $effect(() => { - if (!target) return - - const index = posts.findIndex(spec({id: target})) - - if (index < 0) return - - const targetPage = Math.ceil((index + 1) / POSTS_PER_PAGE) - - if (targetPage !== currentPage) { - setPage(targetPage) + if (target && (target === $event?.id || targetIndex >= 0)) { + setTimeout(() => scrollToEvent(target!), 100) } - - setTimeout(() => scrollToEvent(target!), 100) }) $effect(() => { @@ -177,12 +141,26 @@ {#if $event}
- {#each pagePosts as post (post.id)} - - {/each} +
- {#if pageCount > 1} - + {#if hiddenCount > 0} +
+ +
+ {/if} + {#if visibleReplies.length > 0} +
+ {#each visibleReplies as reply (reply.id)} + + {/each} +
{/if} {#if showReply && replyTo && $event}