From 1549ef2240f0c41d971094b2d9c7ca98bbefa641 Mon Sep 17 00:00:00 2001 From: "@permadeath.com" Date: Thu, 13 Aug 2026 15:20:48 -0400 Subject: [PATCH] feat(popup): stand placeholders where a slow card's content will go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cards fetch off the render path and redraw when their answer lands, so on a slow connection the popup dropped rows in one at a time. Each card now stands a placeholder the shape and height of the row that is coming, so the answer fills boxes in rather than moving anything below it. Nothing appears for the first 150ms. Most loads here are a read of the extension's own cache and land in single-digit milliseconds; a placeholder that flickers past on every popup open would be worse than the pop-in it replaces. LoadTracker redraws at the two moments nothing else would — when a slow load crosses that delay, and when a load that was showing a placeholder ends with nothing to draw. Cards mark themselves aria-busy while a lookup is out, and the shimmer is dropped under prefers-reduced-motion, which popup.css had no rules for before. --- src/popup/cards/card.ts | 11 +++ src/popup/cards/follows.test.ts | 1 + src/popup/cards/loading.test.ts | 123 +++++++++++++++++++++++++ src/popup/cards/loading.ts | 133 ++++++++++++++++++++++++++++ src/popup/cards/owner.ts | 31 ++++++- src/popup/cards/subscribers.test.ts | 1 + src/popup/cards/subscribers.ts | 31 ++++++- src/popup/popup.css | 55 ++++++++++++ src/popup/popup.ts | 28 +++++- 9 files changed, 409 insertions(+), 5 deletions(-) create mode 100644 src/popup/cards/loading.test.ts create mode 100644 src/popup/cards/loading.ts diff --git a/src/popup/cards/card.ts b/src/popup/cards/card.ts index 7293293..f739b56 100644 --- a/src/popup/cards/card.ts +++ b/src/popup/cards/card.ts @@ -21,6 +21,11 @@ // its answer hides its own row rather than reporting an error, because // none of this is what the popup is for. // - a card touches only the elements it owns. +// - a card whose answer has not arrived may stand a placeholder where it +// will go, but only while `host.loading(id)` says so, and only where the +// answer is one that reliably arrives — a placeholder for a row that +// turns out to be empty is a layout shift the popup did to itself +// (loading.ts). import type { PageState, SessionInfo } from '../../lib/types' @@ -32,6 +37,12 @@ export interface CardHost { session(): SessionInfo | undefined /** Redraw every card: what a finished `load` calls. */ rerender(): void + /** + * Whether this card's `load` has been running long enough to be worth a + * placeholder (see loading.ts). Answered for a card id rather than for the + * caller, because `render` gets the host and not itself. + */ + loading(id: string): boolean } export interface Card { diff --git a/src/popup/cards/follows.test.ts b/src/popup/cards/follows.test.ts index ccb74f4..a317398 100644 --- a/src/popup/cards/follows.test.ts +++ b/src/popup/cards/follows.test.ts @@ -18,6 +18,7 @@ function hostFor(did?: string): CardHost { state: () => undefined, session: () => (did ? ({ did } as SessionInfo) : undefined), rerender: () => {}, + loading: () => false, } } diff --git a/src/popup/cards/loading.test.ts b/src/popup/cards/loading.test.ts new file mode 100644 index 0000000..d123e3b --- /dev/null +++ b/src/popup/cards/loading.test.ts @@ -0,0 +1,123 @@ +// The rule that decides whether a placeholder is ever seen: a load that lands +// from the extension's own cache must not draw one, and a load that goes to +// the network must. Everything here is timing, so it is tested on a fake clock +// rather than through a popup. + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { LoadTracker, SHOW_AFTER_MS, showsPlaceholder } from './loading' + +beforeEach(() => { + vi.useFakeTimers() +}) +afterEach(() => { + vi.useRealTimers() +}) + +/** A promise this test settles by hand, standing in for a card's `load`. */ +function deferred() { + let settle!: (err?: Error) => void + const work = new Promise((resolve, reject) => { + settle = (err) => (err ? reject(err) : resolve()) + }) + return { work, settle } +} + +/** Let the timers run to `ms`, and the promise callbacks behind them too. */ +const tick = (ms: number) => vi.advanceTimersByTimeAsync(ms) + +describe('showsPlaceholder', () => { + it('shows nothing for the first SHOW_AFTER_MS', () => { + expect(showsPlaceholder({ startedAt: 0 }, SHOW_AFTER_MS - 1)).toBe(false) + }) + + it('shows once a load has run past the delay', () => { + expect(showsPlaceholder({ startedAt: 0 }, SHOW_AFTER_MS)).toBe(true) + }) + + it('shows nothing for a load that has ended, however long it took', () => { + expect(showsPlaceholder({ startedAt: 0, endedAt: 9_000 }, 9_001)).toBe(false) + }) +}) + +describe('LoadTracker', () => { + it('never draws for a load that lands from cache', async () => { + const redraw = vi.fn() + const tracker = new LoadTracker(redraw) + const { work, settle } = deferred() + tracker.track('owner', work) + + await tick(20) + settle() + await tick(0) + + expect(tracker.busy('owner')).toBe(false) + expect(tracker.anyPending()).toBe(false) + // The redraw scheduled for the delay has to have been called off, or the + // popup redraws for a load that finished long before. + await tick(SHOW_AFTER_MS * 2) + expect(redraw).not.toHaveBeenCalled() + }) + + it('draws once a load crosses the delay, and again when it lands', async () => { + const redraw = vi.fn() + const tracker = new LoadTracker(redraw) + const { work, settle } = deferred() + tracker.track('owner', work) + + await tick(SHOW_AFTER_MS - 1) + expect(tracker.busy('owner')).toBe(false) + expect(redraw).not.toHaveBeenCalled() + + await tick(1) + expect(tracker.busy('owner')).toBe(true) + expect(redraw).toHaveBeenCalledTimes(1) + + settle() + await tick(0) + expect(tracker.busy('owner')).toBe(false) + // The card redraws for itself when it has an answer, but a load can end + // with nothing to draw; the placeholder still has to come down. + expect(redraw).toHaveBeenCalledTimes(2) + }) + + it('takes the placeholder down when a load fails', async () => { + const redraw = vi.fn() + const tracker = new LoadTracker(redraw) + const { work, settle } = deferred() + tracker.track('subscribers', work) + + await tick(SHOW_AFTER_MS) + expect(tracker.busy('subscribers')).toBe(true) + + settle(new Error('constellation said no')) + await tick(0) + expect(tracker.busy('subscribers')).toBe(false) + }) + + it('keeps one timer for loads that started at different times', async () => { + const redraw = vi.fn() + const tracker = new LoadTracker(redraw) + const first = deferred() + const second = deferred() + + tracker.track('owner', first.work) + await tick(100) + // A second load starting must not push back the first one's placeholder. + tracker.track('subscribers', second.work) + + await tick(50) + expect(tracker.busy('owner')).toBe(true) + expect(tracker.busy('subscribers')).toBe(false) + + await tick(100) + expect(tracker.busy('subscribers')).toBe(true) + expect(redraw).toHaveBeenCalledTimes(2) + }) + + it('is never busy for a card that does not load', () => { + const tracker = new LoadTracker(vi.fn()) + tracker.track('labels', undefined) + expect(tracker.busy('labels')).toBe(false) + expect(tracker.anyPending()).toBe(false) + }) +}) diff --git a/src/popup/cards/loading.ts b/src/popup/cards/loading.ts new file mode 100644 index 0000000..95d5cee --- /dev/null +++ b/src/popup/cards/loading.ts @@ -0,0 +1,133 @@ +// When a card is allowed to say that it is working. +// +// Every card fetches off the render path and redraws when its answer lands +// (see card.ts), so on a slow connection the popup used to sit there dropping +// rows in one at a time. A placeholder in the shape of the row that is coming +// says the same thing without the jank — but only if it is rarer than the +// answer itself. +// +// Two rules, and the first one is what makes the second bearable: +// +// - Nothing appears for the first `SHOW_AFTER_MS`. Most loads here are a +// read of the extension's own cache, which lands in single-digit +// milliseconds, and a placeholder that flickers past on every popup open +// is worse than the pop-in it replaces. +// - A placeholder stands where its content will, at roughly the height that +// content will take. There is no minimum time it stays up: it is the same +// shape as what replaces it, so an answer landing a moment after it +// appeared fills the boxes in rather than moving anything. +// +// The tracker below is the whole mechanism. `popup.ts` hands it every `load` +// it starts, cards ask it (through `CardHost.loading`) whether to draw a +// placeholder, and it redraws the popup at the one moment nothing else will: +// the instant a slow load crosses the delay. + +/** + * How long a load may run before it is worth drawing anything about it. Long + * enough that a cache hit never shows a placeholder, short enough that a real + * network round trip always does. + */ +export const SHOW_AFTER_MS = 150 + +/** One card's `load`, from the moment it started to the moment it settled. */ +export interface Load { + startedAt: number + /** Set when the promise settles, however it settled. */ + endedAt?: number +} + +/** Whether the placeholder for this load is on screen at `now`. */ +export function showsPlaceholder(load: Load, now: number): boolean { + if (load.endedAt !== undefined) return false + return now - load.startedAt >= SHOW_AFTER_MS +} + +/** + * When this load next changes what is on screen, or undefined when nothing + * more will happen on a timer. Only ever the moment it crosses the delay: the + * end of a load is an event, not a deadline, and arrives on its own. + */ +export function nextChange(load: Load, now: number): number | undefined { + if (load.endedAt !== undefined) return undefined + const shownAt = load.startedAt + SHOW_AFTER_MS + return now < shownAt ? shownAt : undefined +} + +/** + * The loads this popup has started, by card id. + * + * It redraws by itself twice per slow load: once when the placeholder is due, + * and once when a load that was showing one ends. The second one matters even + * though a card that got an answer redraws on its own — a load can end with + * nothing to draw (no publication, a failed lookup), and then nothing else + * would take the placeholder back down. + */ +export class LoadTracker { + private loads = new Map() + private timer: ReturnType | undefined + private now: () => number + + constructor( + private redraw: () => void, + now: () => number = Date.now, + ) { + this.now = now + } + + /** Watch one card's load. A card with no `load` is never busy. */ + track(id: string, work: Promise | undefined): void { + if (!work) return + const load: Load = { startedAt: this.now() } + this.loads.set(id, load) + this.schedule() + void Promise.resolve(work) + .catch(() => {}) + .then(() => { + const showing = showsPlaceholder(load, this.now()) + load.endedAt = this.now() + this.schedule() + if (showing) this.redraw() + }) + } + + /** Whether this card should be drawing a placeholder right now. */ + busy(id: string): boolean { + const load = this.loads.get(id) + return !!load && showsPlaceholder(load, this.now()) + } + + /** Whether anything at all is still being waited on, placeholder or not. */ + anyPending(): boolean { + return [...this.loads.values()].some((load) => load.endedAt === undefined) + } + + /** One timer for every load: the earliest moment any of them changes. */ + private schedule(): void { + const now = this.now() + const due = [...this.loads.values()] + .map((load) => nextChange(load, now)) + .filter((at): at is number => at !== undefined) + clearTimeout(this.timer) + this.timer = undefined + if (due.length === 0) return + this.timer = setTimeout( + () => { + this.timer = undefined + this.redraw() + this.schedule() + }, + Math.max(0, Math.min(...due) - now), + ) + } +} + +/** + * Put an element into (or out of) its placeholder state: the class the + * stylesheet draws grey boxes from, and hidden from assistive technology, + * which has `aria-busy` on the card instead of a row of empty elements. + */ +export function placeholder(el: HTMLElement, on: boolean): void { + el.classList.toggle('loading', on) + if (on) el.setAttribute('aria-hidden', 'true') + else el.removeAttribute('aria-hidden') +} diff --git a/src/popup/cards/owner.ts b/src/popup/cards/owner.ts index b19175e..54e0eab 100644 --- a/src/popup/cards/owner.ts +++ b/src/popup/cards/owner.ts @@ -19,6 +19,7 @@ import type { Viewer } from '../../lib/subscribers' import { $, failedIconUrls } from '../dom' import type { Card, CardHost } from './card' import { resetFollows, viewerFollows } from './follows' +import { placeholder } from './loading' /** Kept with the account it describes, so a redraw mid-load draws nothing wrong. */ let owner: { did: string; info: Owner } | undefined @@ -48,9 +49,13 @@ export const ownerCard: Card = { const info = blocked ? blockedIdentity(loaded) : loaded const card = $('owner') if (!info) { - card.hidden = true + // Nothing yet. An account always resolves to something — fetchOwner + // answers with the DID when everything else fails — so the card is + // coming, and standing a placeholder in its place costs no layout. + renderPlaceholder(card, did ? host.loading('owner') : false) return } + placeholder(card, false) card.hidden = false // The proven handle when there is one; bsky.app takes a DID just as well, // and an unproven handle must not become a link that vouches for it. @@ -76,6 +81,30 @@ export const ownerCard: Card = { }, } +/** + * The card's own shape in grey: avatar, name, handle, a line of bio. The same + * elements the real card uses, emptied and sized by the stylesheet, so the + * profile lands in the boxes instead of pushing them around. + */ +function renderPlaceholder(card: HTMLAnchorElement, on: boolean) { + card.hidden = !on + placeholder(card, on) + if (!on) return + // Nothing to open and nothing to tab to until there is a profile behind it. + card.removeAttribute('href') + $('owner-name').textContent = '' + $('owner-handle').textContent = '' + $('owner-handle').hidden = false + $('owner-desc').textContent = '' + $('owner-desc').hidden = false + // The reader's own relationship to an account nobody has named yet. + $('owner-follows').hidden = true + $('owner-avatar').hidden = true + const fallback = $('owner-avatar-fallback') + fallback.textContent = '' + fallback.hidden = false +} + async function loadProfile(host: CardHost, did: string, refresh: boolean) { if (!refresh && loadedFor === did) return loadedFor = did diff --git a/src/popup/cards/subscribers.test.ts b/src/popup/cards/subscribers.test.ts index d64f51f..56527d8 100644 --- a/src/popup/cards/subscribers.test.ts +++ b/src/popup/cards/subscribers.test.ts @@ -46,6 +46,7 @@ function hostFor(did?: string): CardHost { state: () => ({ url: 'https://pub.example', fetchedAt: 0, pub: { uri: PUB } }) as PageState, session: () => (did ? ({ did } as SessionInfo) : undefined), rerender: () => draws.push(draws.length), + loading: () => false, } } diff --git a/src/popup/cards/subscribers.ts b/src/popup/cards/subscribers.ts index 43a34fe..f672798 100644 --- a/src/popup/cards/subscribers.ts +++ b/src/popup/cards/subscribers.ts @@ -19,6 +19,7 @@ import { import { $ } from '../dom' import type { Card, CardHost } from './card' import { resetFollows, viewerFollows } from './follows' +import { placeholder } from './loading' let subscribers: { uri: string; summary: SubscriberSummary } | undefined let loadedFor: string | undefined @@ -83,9 +84,20 @@ export const subscribersCard: Card = { const summary = pub && subscribers?.uri === pub.uri ? subscribers.summary : undefined // A publication nobody subscribes to says nothing worth a row of its own. if (!summary || summary.total === 0) { - row.hidden = true + // A count still on its way gets the row's shape; a count that came back + // zero gets no row. Which means a slow lookup that ends in zero does + // take the row back down — the only shift this file can cause, and the + // delay in loading.ts keeps it off every fast answer. + renderPlaceholder( + row, + !summary && !!pub && host.loading('subscribers'), + // Faces are the viewer's own follows, so a signed-out row is the + // count alone and its placeholder should be too. + host.session() ? 3 : 0, + ) return } + placeholder(row, false) $('subscriber-faces').replaceChildren(...summary.followed.map(faceFor)) const parts: string[] = [] if (summary.followedTotal > 0) parts.push(`${summary.followedTotal} you follow`) @@ -106,6 +118,23 @@ export const subscribersCard: Card = { }, } +/** The row's shape in grey: a few faces, and a bar where the count goes. */ +function renderPlaceholder(row: HTMLElement, on: boolean, faces: number) { + row.hidden = !on + placeholder(row, on) + if (!on) return + $('subscriber-faces').replaceChildren( + ...Array.from({ length: faces }, () => { + const face = document.createElement('span') + face.className = 'face' + return face + }), + ) + const count = $('subscriber-count') + count.textContent = '' + count.title = '' +} + /** * Why a face the reader expected might not be here. Either cap can do it: the * subscribers we looked at, and the follows we could recognise them from. diff --git a/src/popup/popup.css b/src/popup/popup.css index ede4ed7..965a94d 100644 --- a/src/popup/popup.css +++ b/src/popup/popup.css @@ -30,6 +30,11 @@ } } +/* Ink, thinned until it is a shape rather than a word: the grey of every + loading placeholder. One definition for both palettes — it is mixed from + --fg, which each of them has already set. */ +:root { --skeleton: color-mix(in srgb, var(--fg) 12%, transparent); } + * { box-sizing: border-box; } body { /* Wide enough for the actions row's widest state — ✓ Subscribed, the @@ -204,6 +209,11 @@ actor-typeahead { /* The owner card: the account, above the publication it publishes. A card rather than a line, because it carries the same three things a profile does — avatar, name, bio — and it is a link to that profile. */ +/* The same trap as .pub-icon-fallback and .account-btn: an author-origin + `display` beats the UA's `[hidden] { display: none }`, so a card with no + profile in it yet drew an empty bordered box — which is what the placeholder + below stands in the middle of, and what it must fall back to nothing from. */ +.owner[hidden] { display: none; } .owner { display: flex; align-items: flex-start; @@ -323,6 +333,8 @@ actor-typeahead { /* Who else subscribes: overlapping faces of the people you follow, then the count. Sits between the description and the article line, so it reads as part of the publication rather than as part of the actions. */ +/* Same trap: hidden has to be restated, or the row keeps its top margin. */ +.subscribers[hidden] { display: none; } .subscribers { margin-top: 8px; display: flex; @@ -355,6 +367,49 @@ actor-typeahead { user-select: none; } .subscriber-count { min-width: 0; overflow: hidden; text-overflow: ellipsis; white-space: nowrap; } + +/* Placeholders, for a card whose lookup is still out after 150ms (the rule + lives in src/popup/cards/loading.ts; this is only how it looks). + + Every box below is an element the real content fills, given a size here so + the answer lands inside it rather than pushing it open — a placeholder that + collapsed to nothing would move more than the pop-in it replaces. Sizes are + the middle of what these fields actually hold: a name, a handle, one line of + a bio that may turn out to be two. */ +.loading { pointer-events: none; } +.loading .owner-avatar, +.loading .face, +.loading .owner-name, +.loading .owner-handle, +.loading .owner-desc, +.loading .subscriber-count { + background: var(--skeleton); + animation: placeholder-pulse 1.6s ease-in-out infinite; +} +/* Each bar sits in the line box its text would have taken — 13px/1.45 rounds + to 19, 12px/1.45 to 17 — so a card whose bio turns out to be one line does + not move at all when it arrives, and one with two lines grows by one. */ +.loading .owner-line { min-height: 19px; } +.loading .owner-name { width: 128px; height: 12px; border-radius: 3px; } +.loading .owner-handle { width: 92px; height: 10px; margin: 3.5px 0; border-radius: 3px; } +.loading .owner-desc { height: 10px; margin: 7.5px 0 3.5px; border-radius: 3px; } +.loading .subscriber-count { width: 116px; height: 10px; margin: 3.5px 0; border-radius: 3px; } +/* Barely there: the point is that something is coming, not that something is + happening. */ +@keyframes placeholder-pulse { + 0%, 100% { opacity: 1; } + 50% { opacity: 0.45; } +} +@media (prefers-reduced-motion: reduce) { + /* The pulse is the only thing in this popup that moves on its own, so + stopping every animation is the whole guard. The grey boxes stand still + and say the same thing. */ + *, + *::before, + *::after { + animation: none !important; + } +} /* Moderation labels, themed after Bluesky's: a chip per label, an alert tone for the values its own interpretation calls alerts, and a cover in front of anything a label says to put behind a click. */ diff --git a/src/popup/popup.ts b/src/popup/popup.ts index cc76eae..1b82558 100644 --- a/src/popup/popup.ts +++ b/src/popup/popup.ts @@ -26,6 +26,7 @@ import { import { shareOnBlueskyUrl } from '../lib/share' import { type StatusMessage, statusMessagesFor } from '../lib/status' import { CARDS, type CardHost } from './cards' +import { LoadTracker } from './cards/loading' import { $, TONE_ICONS, failedIconUrls } from './dom' import { send } from './send' import { wireTypeaheadSubmit } from '../lib/typeahead' @@ -33,6 +34,13 @@ import type { PageState, PubInfo, SessionInfo } from '../lib/types' const SUB_COLLECTION = 'site.standard.graph.subscription' +/** + * Which card loads are still running, and which have run long enough to be + * worth a placeholder. Redraws by itself when one crosses that line, since + * nothing else is happening at that moment (src/popup/cards/loading.ts). + */ +const loads = new LoadTracker(() => render()) + /** * What the cards can ask for. Methods rather than values: a card holds this * for the life of the popup and the answers change under it. @@ -41,6 +49,16 @@ const host: CardHost = { state: () => state, session: () => session, rerender: () => render(), + loading: (id) => loads.busy(id), +} + +/** Start every card's load, and watch it, in the one place they are started. */ +function loadCards(refresh: boolean) { + // Every card fetches on its own, in parallel, and redraws when it lands. + for (const card of CARDS) loads.track(card.id, card.load?.(host, refresh)) + // Redrawn here rather than left to the first load that finishes, so the + // card says it is busy from the moment it is (see render). + render() } @@ -171,8 +189,7 @@ async function loadState(refresh: boolean) { requestFailed = true } render() - // Every card fetches on its own, in parallel, and redraws when it lands. - for (const card of CARDS) void card.load?.(host, refresh) + loadCards(refresh) } function renderAccount() { @@ -245,6 +262,11 @@ function render() { } for (const card of CARDS) card.render(host) + // One "still working" for the whole card, rather than one per placeholder: + // the placeholders are hidden from assistive technology, which gets the + // card's own state instead of a row of empty elements. + if (loads.anyPending()) pubEl.setAttribute('aria-busy', 'true') + else pubEl.removeAttribute('aria-busy') renderSubscribe() renderOpen() renderShare() @@ -671,7 +693,7 @@ $('signout').addEventListener('click', async () => { for (const card of CARDS) card.reset?.() renderAccount() render() - for (const card of CARDS) void card.load?.(host, false) + loadCards(false) }) $('share').addEventListener('click', () => { -- 2.51.2