From 49992c1db068cf2435c872c2386e0c37e6812cf3 Mon Sep 17 00:00:00 2001 From: Simon Date: Wed, 9 Sep 2026 09:47:43 +0000 Subject: [PATCH] Recycle grid cards instead of rebuilding them Scrolling the grid did nothing but destroy cards and build near-identical ones back: a template string parsed as innerHTML, ten querySelectors, and a listener per interactive element, every time a card entered the window. The virtualizer now keeps a pool and rebinds a card it already has -- 18us against 136us to build one, and 74-83% of mounts are served from it. Two things had to change first. Nothing on a card may close over the video it is showing, because the card outlives the video, so every interaction moved to one delegated listener per event type on the grid. And every card now has the same shape whatever it shows: the optional parts are always present and hidden when unused, so any pooled card fits any video. That needed a global [hidden] rule, since .live-badge and .video-tags carry their own display. The rest is the release path, which is where this design lives or dies. A thumbnail carries a generation, so a race or a proxy fallback settling after the card moved on cannot paint over the video now showing. The player stamps the card it was opened from, so a recycled element stops answering for it. The reveal handler, the entrance-animation listener and the hover preview are all taken back off. Anything missed here surfaces as one video's title, thumbnail or heart on another video's card, which is what the smoke suite scrolls back and forth to catch. Two incidental fixes found while measuring: bindCard no longer writes a data-tag per tag button (dataset is a proxy, and that alone cost more than the rest of a rebind put together -- the handler reads the label off the button), and favorites.has no longer parses a URL for every card that isn't a favorite by key. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01QBDkEXP4htyXTCZUwMLphd --- frontend/css/style.css | 6 + frontend/js/enhance.js | 18 +- frontend/js/favorites.js | 4 + frontend/js/player.js | 35 ++- frontend/js/videos.js | 508 ++++++++++++++++++++++++++++----------- tests/smoke_grid.py | 54 ++++- 6 files changed, 470 insertions(+), 155 deletions(-) diff --git a/frontend/css/style.css b/frontend/css/style.css index 2ab8a55..390df84 100644 --- a/frontend/css/style.css +++ b/frontend/css/style.css @@ -1,5 +1,11 @@ * { margin: 0; padding: 0; box-sizing: border-box; } +/* A card keeps its optional parts (live badge, uploader, duration, tags) at all + times and hides the ones this video doesn't need, so any pooled card fits any + video -- see bindCard. Several of those carry their own `display`, which beats + the UA rule for [hidden], so say it once and mean it. */ +[hidden] { display: none !important; } + :root { /* Warm "classic" dark: deep charcoal with a hint of brown, never pure black. */ --bg-primary: #14110d; diff --git a/frontend/js/enhance.js b/frontend/js/enhance.js index a9c5d38..a7efb8d 100644 --- a/frontend/js/enhance.js +++ b/frontend/js/enhance.js @@ -60,6 +60,15 @@ App.enhance = App.enhance || {}; if (!grid || !fineHover) return; let dwellTimer = null; let activeCard = null; + // The card element alone doesn't identify what is being previewed: the + // grid pools its cards, so the same element can come back showing a + // different video (via relayout or a new search, neither of which + // scrolls, so clearPreview never runs). Remembering the video too keeps + // the "already previewing this" check honest. + let activeVideo = null; + + const isActive = (card) => card === activeCard && + (!App.videos.getVideoForCard || App.videos.getVideoForCard(card) === activeVideo); const clearPreview = () => { if (dwellTimer) { clearTimeout(dwellTimer); dwellTimer = null; } @@ -68,6 +77,7 @@ App.enhance = App.enhance || {}; if (vid) { try { vid.pause(); } catch (e) {} vid.remove(); } activeCard.classList.remove('is-previewing'); activeCard = null; + activeVideo = null; } }; @@ -105,10 +115,14 @@ App.enhance = App.enhance || {}; grid.addEventListener('pointerover', (e) => { const card = e.target.closest('.video-card'); - if (!card || card === activeCard) return; + if (!card || isActive(card)) return; clearPreview(); activeCard = card; - dwellTimer = setTimeout(() => { dwellTimer = null; if (activeCard === card) startPreview(card); }, 600); + activeVideo = App.videos.getVideoForCard ? App.videos.getVideoForCard(card) : null; + dwellTimer = setTimeout(() => { + dwellTimer = null; + if (isActive(card)) startPreview(card); + }, 600); }); grid.addEventListener('pointerout', (e) => { const card = e.target.closest('.video-card'); diff --git a/frontend/js/favorites.js b/frontend/js/favorites.js index 447824b..da595db 100644 --- a/frontend/js/favorites.js +++ b/frontend/js/favorites.js @@ -224,6 +224,10 @@ App.favorites = App.favorites || {}; const index = identities(); const key = App.favorites.getKey(video); if (key && index.keys.has(key)) return true; + // Normalising a URL means parsing one, which is the expensive half of + // this and runs for every card that isn't a favorite by key. With + // nothing stored to match against, there is nothing to parse it for. + if (!index.urls.size) return false; const meta = (video && video.meta) || video || {}; const urlKey = App.favorites.urlKey(video && (video.url || meta.url)); return !!(urlKey && index.urls.has(urlKey)); diff --git a/frontend/js/player.js b/frontend/js/player.js index e2fb464..815b1b3 100644 --- a/frontend/js/player.js +++ b/frontend/js/player.js @@ -23,6 +23,7 @@ App.player = App.player || {}; historyPushed: false, idleTimer: null, originEl: null, + originToken: null, // stamp proving originEl is still the card we opened hudHovered: false, // mouse resting on the controls (desktop) activeUrl: '', // media URL actually playing, for the format menu's tick attemptToken: 0, // bumps on every open()/format switch to void stale async callbacks @@ -59,6 +60,24 @@ App.player = App.player || {}; } } + // The card the player was opened from is owned by the grid, which pools and + // reuses its cards (see resetCard in videos.js). By the time the player lets + // go, that element may be showing a different video -- so it is stamped at + // open, and every later touch checks the stamp still matches. A recycled + // card has had it wiped, and simply stops answering. + let originSeq = 0; + + const claimOrigin = function(el) { + if (!el) return null; + const token = String(++originSeq); + el.dataset.playerToken = token; + return token; + }; + + const withOrigin = function(el, token, fn) { + if (el && el.dataset.playerToken === token) fn(el); + }; + const addCleanup = (fn) => cp.cleanups.push(fn); const runCleanups = () => { cp.cleanups.forEach((fn) => { try { fn(); } catch (err) { /* ignore */ } }); @@ -695,9 +714,10 @@ App.player = App.player || {}; // the wrong card loaded or (via the token guard below) never clear // this card's spinner at all. const originEl = (opts && opts.originEl) || null; + const originToken = originEl ? originEl.dataset.playerToken : null; const sources = resolveSources(videoData); const clearLoading = () => { - if (originEl) originEl.classList.remove('is-loading'); + withOrigin(originEl, originToken, (el) => el.classList.remove('is-loading')); }; const sourceUrl = (videoData && (videoData.url || (videoData.meta && videoData.meta.url))) || ''; @@ -921,11 +941,12 @@ App.player = App.player || {}; cp.attemptToken++; cancelInFlight(); clearIdleTimer(); - if (cp.originEl) cp.originEl.classList.remove('is-loading'); + withOrigin(cp.originEl, cp.originToken, (el) => el.classList.remove('is-loading')); } runCleanups(); cp.originEl = opts && opts.originEl ? opts.originEl : null; + cp.originToken = claimOrigin(cp.originEl); if (cp.originEl) cp.originEl.classList.add('is-loading'); cp.container = buildContainer(); @@ -1031,10 +1052,12 @@ App.player = App.player || {}; cp.historyPushed = false; } - if (cp.originEl) { - cp.originEl.classList.remove('is-loading'); - cp.originEl = null; - } + withOrigin(cp.originEl, cp.originToken, (el) => { + el.classList.remove('is-loading'); + delete el.dataset.playerToken; + }); + cp.originEl = null; + cp.originToken = null; cp.data = null; cp.source = null; // voids a still-pending format resolve for this open cp.formatOverride = null; diff --git a/frontend/js/videos.js b/frontend/js/videos.js index e825500..f0b51e4 100644 --- a/frontend/js/videos.js +++ b/frontend/js/videos.js @@ -147,6 +147,7 @@ App.videos = App.videos || {}; const imageRoutes = new Map(); // host -> winning route; absent = unknown const imageRacing = new Set(); // hosts with a race already deciding const imageWaiting = new Map(); // host -> images held until it decides + let thumbSeq = 0; // generation, so stale work can be dropped // A page of cards is built in one go, so every thumbnail from a host is // attached before the first one has come back. Sending them all down the @@ -188,7 +189,13 @@ App.videos = App.videos || {}; // and put on only once there is an image to caption -- otherwise every card // spells out its own title over the placeholder while the host is being // decided, and permanently for an item that has no thumbnail at all. - const showThumbnail = function(img, url) { + // `token` is the generation of the attachThumbnail call that started this + // work. A card can be recycled while its thumbnail is still being decided, + // and the callbacks that eventually fire still hold the old element -- so + // anything arriving for a generation the element has moved past is dropped + // rather than painted onto whatever video the card now shows. + const showThumbnail = function(img, url, token) { + if (token !== undefined && img.dataset.thumbToken !== token) return; if (img.dataset.alt !== undefined) { img.alt = img.dataset.alt; delete img.dataset.alt; @@ -198,9 +205,22 @@ App.videos = App.videos || {}; // Last resort on a route that normally works: one expired or missing image // shouldn't be left broken just because its host is fine in general. - const attachProxyFallback = function(img, proxyUrl) { + const attachProxyFallback = function(img, proxyUrl, token) { if (!proxyUrl) return; - img.addEventListener('error', () => { showThumbnail(img, proxyUrl); }, { once: true }); + // Held on the element so detachThumbnail can take it off again. On the + // happy path it never fires and `once` never collects it, so a pooled + // image would otherwise accumulate one closure per mount it has served. + detachProxyFallback(img); + const onError = () => { showThumbnail(img, proxyUrl, token); }; + img._thumbFallback = onError; + img.addEventListener('error', onError, { once: true }); + }; + + const detachProxyFallback = function(img) { + if (img && img._thumbFallback) { + img.removeEventListener('error', img._thumbFallback); + img._thumbFallback = null; + } }; // Releases the images held for `host`. `route` is the winner, or null when @@ -213,11 +233,11 @@ App.videos = App.videos || {}; imageWaiting.delete(host); waiting.forEach((entry) => { if (route === IMAGE_PROXY) { - showThumbnail(entry.img, entry.proxyUrl); + showThumbnail(entry.img, entry.proxyUrl, entry.token); return; } - if (route !== IMAGE_DIRECT) attachProxyFallback(entry.img, entry.proxyUrl); - showThumbnail(entry.img, entry.directUrl); + if (route !== IMAGE_DIRECT) attachProxyFallback(entry.img, entry.proxyUrl, entry.token); + showThumbnail(entry.img, entry.directUrl, entry.token); }); }; @@ -238,7 +258,7 @@ App.videos = App.videos || {}; // pointed at the first one home. Racing on the visible element instead // would abort the loser -- and the loser is the request that answers the // second question. - const raceThumbnail = function(img, directUrl, proxyUrl, host) { + const raceThumbnail = function(img, directUrl, proxyUrl, host, token) { let shown = false; let outstanding = 2; let routeFinal = false; // the direct verdict is in; nothing can revise it @@ -250,7 +270,7 @@ App.videos = App.videos || {}; const show = function(url) { if (shown) return; shown = true; - showThumbnail(img, url); // a cache hit; the probe has the bytes + showThumbnail(img, url, token); // a cache hit; the probe has the bytes }; // Records the host's route and lets go of everything held for it. The @@ -278,11 +298,11 @@ App.videos = App.videos || {}; if (shown) return; shown = true; if (imageRoutes.get(host) === IMAGE_PROXY) { - showThumbnail(img, proxyUrl); + showThumbnail(img, proxyUrl, token); return; } - attachProxyFallback(img, proxyUrl); - showThumbnail(img, directUrl); + attachProxyFallback(img, proxyUrl, token); + showThumbnail(img, directUrl, token); }; const decide = function() { @@ -383,9 +403,13 @@ App.videos = App.videos || {}; const proxyUrl = App.videos.buildImageProxyUrl(directUrl); const host = imageHostOf(directUrl); const route = imageRoutes.get(host); + // Every attach is a new generation, so work started for a previous one + // stops being able to touch this element. + const token = String(++thumbSeq); + img.dataset.thumbToken = token; if (route === IMAGE_PROXY) { - showThumbnail(img, proxyUrl || directUrl); + showThumbnail(img, proxyUrl || directUrl, token); return; } if (imageRacing.has(host)) { @@ -393,18 +417,28 @@ App.videos = App.videos || {}; // guessing: guessing wrong costs this image a whole failed request // before it even asks the route that was about to be proven. const waiting = imageWaiting.get(host) || []; - waiting.push({ img: img, directUrl: directUrl, proxyUrl: proxyUrl }); + waiting.push({ img: img, directUrl: directUrl, proxyUrl: proxyUrl, token: token }); imageWaiting.set(host, waiting); return; } if (route === IMAGE_DIRECT || !host || !proxyUrl) { // Known good, or nothing to race against: take the provider and keep // the proxy as this image's own fallback. - attachProxyFallback(img, proxyUrl); - showThumbnail(img, directUrl); + attachProxyFallback(img, proxyUrl, token); + showThumbnail(img, directUrl, token); return; } - raceThumbnail(img, directUrl, proxyUrl, host); + raceThumbnail(img, directUrl, proxyUrl, host, token); + }; + + // Voids whatever is still in flight for this element's thumbnail. Its + // generation moves on, so a race that settles later, or a proxy fallback + // that fires later, finds a token that no longer matches and does nothing. + App.videos.detachThumbnail = function(img) { + if (!img) return; + img.dataset.thumbToken = String(++thumbSeq); + delete img.dataset.alt; + detachProxyFallback(img); }; // Each channel in a group sends back a different number of videos per @@ -710,132 +744,268 @@ App.videos = App.videos || {}; } }; - // Builds a fully-wired video card element for `v`. Kept separate from - // mounting so the virtualizer can create a card the moment it needs to be - // on screen and throw it away once it scrolls out of the window. - App.videos.buildCard = function(v, options) { - const card = document.createElement('div'); - card.className = 'video-card'; + // --------------------------------------------------------------------- + // Cards + // + // A card is built once and then reused: the virtualizer keeps a pool of + // them and rebinds one to a new video rather than constructing another + // (see acquire/release in App.virtualGrid). Two things follow from that, + // and both are load-bearing. + // + // Nothing on a card may close over the video it is currently showing -- + // the card outlives the video. Every interaction is therefore handled by + // one delegated listener per event type on the grid, which resolves the + // video from the card under the pointer (see bindGridDelegation). + // + // And every card has the same shape whatever video it shows: the optional + // parts -- live badge, uploader, duration, tags -- are always present and + // hidden when unused, so any pooled card fits any video. + // --------------------------------------------------------------------- + const CARD_TEMPLATE_HTML = ` + + + + +
+ + + + +
+

+ + `; + + // Parsed once. Cloning this is roughly six times cheaper than asking the + // parser to read the same markup again for every card. + let cardTemplate = null; + + // The parts bindCard writes to, found once when the card is created rather + // than looked up again on every rebind. Searching the subtree seven times + // per mount was most of what rebinding cost. + const cardRefs = function(card) { + if (!card._refs) { + card._refs = { + live: card.querySelector('.live-badge'), + favorite: card.querySelector('.favorite-btn'), + title: card.querySelector('.video-title-text'), + uploader: card.querySelector('.video-uploader'), + duration: card.querySelector('.video-duration'), + tags: card.querySelector('.video-tags'), + img: card.querySelector('img') + }; + } + return card._refs; + }; + + const createCardShell = function() { + if (!cardTemplate) { + cardTemplate = document.createElement('template'); + cardTemplate.innerHTML = `
${CARD_TEMPLATE_HTML}
`; + } + const card = cardTemplate.content.firstElementChild.cloneNode(true); + cardRefs(card); + return card; + }; + + // Tag buttons are the only part whose *count* varies, so they are adjusted + // rather than rebuilt: usually the card already has the right number. + const bindTags = function(container, tags) { + const list = Array.isArray(tags) ? tags.filter((tag) => tag) : []; + container.hidden = list.length === 0; + while (container.childElementCount > list.length) { + container.removeChild(container.lastElementChild); + } + while (container.childElementCount < list.length) { + const button = document.createElement('button'); + button.className = 'video-tag'; + button.type = 'button'; + button.dataset.action = 'tag'; + container.appendChild(button); + } + // Only the label is written: the delegated handler reads the tag off + // the button's own text. Writing it a second time into a data attribute + // cost more than everything else in a rebind put together -- dataset is + // a proxy, and this runs once per tag per card. + list.forEach((tag, index) => { + const button = container.children[index]; + if (button.textContent !== tag) button.textContent = tag; + }); + }; + + // Points an existing card at `v`. This is the whole per-mount cost. + App.videos.bindCard = function(card, v) { + const refs = cardRefs(card); card.dataset.videoId = v.id; - const durationText = App.videos.formatDuration(v.duration); + cardVideo.set(card, v); + + refs.live.hidden = !v.isLive; + const favoriteKey = App.favorites.getKey(v); + refs.favorite.dataset.favKey = favoriteKey || ''; + refs.favorite.dataset.favUrl = v.url || ''; + // By either identity: a favorite imported from a backup is keyed by its + // URL, not by the id this card carries. + App.favorites.setButtonState(refs.favorite, !!favoriteKey && App.favorites.has(v)); + + refs.title.textContent = v.title || ''; + const uploaderText = v.uploader || ''; - const tags = Array.isArray(v.tags) ? v.tags.filter(tag => tag) : []; - const tagsMarkup = tags.length - ? `
${tags.map(tag => ``).join('')}
` - : ''; - const liveBadge = v.isLive ? '● LIVE' : ''; - card.innerHTML = ` - ${liveBadge} - - - -
- ${v.title} - - ${uploaderText ? `` : ''} - ${durationText ? `${durationText}` : ''} -
-

${v.title}

- ${tagsMarkup} - `; - const thumb = card.querySelector('img'); + refs.uploader.hidden = !uploaderText; + refs.uploader.textContent = uploaderText; + refs.uploader.dataset.uploader = uploaderText; + + const durationText = App.videos.formatDuration(v.duration); + refs.duration.hidden = !durationText; + refs.duration.textContent = durationText; + + bindTags(refs.tags, v.tags); + + // Set before attachThumbnail, which holds the caption back until there + // is an image to caption. + refs.img.alt = v.title || ''; + return card; + }; + + // A card ready to show `v`, thumbnail and all. + App.videos.buildCard = function(v, options) { + const card = App.videos.bindCard(createCardShell(), v); // The layout probe (see shapeHeight) needs the card's shape, never its // pixels: it measures against the CSS 16:9 placeholder and is removed in // the same frame, so loading a thumbnail for it -- let alone racing one // -- would be pure waste. if (!(options && options.skipThumbnail)) { - App.videos.attachThumbnail(thumb, v.thumb); + App.videos.attachThumbnail(cardRefs(card).img, v.thumb); } - const favoriteBtn = card.querySelector('.favorite-btn'); - if (favoriteBtn && favoriteKey) { - // By either identity: a favorite imported from a backup is keyed - // by its URL, not by the id this card carries. - App.favorites.setButtonState(favoriteBtn, App.favorites.has(v)); - favoriteBtn.onclick = (event) => { - event.stopPropagation(); - App.favorites.toggle(v); - }; - } - const titleWrap = card.querySelector('.video-title'); - const titleText = card.querySelector('.video-title-text'); - if (titleWrap && titleText) { - card.addEventListener('focusin', () => { - card.dataset.titleFocused = '1'; - updateTitleActive(card); - }); - card.addEventListener('focusout', () => { - card.dataset.titleFocused = '0'; - updateTitleActive(card); - }); - if (titleEnv.useHoverFocus) { - card.addEventListener('mouseenter', () => { - card.dataset.titleHovered = '1'; - updateTitleActive(card); - }); - card.addEventListener('mouseleave', () => { - card.dataset.titleHovered = '0'; - updateTitleActive(card); - }); - } - // On touch devices the marquee observer is attached lazily by - // measureTitle (called on mount), and only for overflowing titles. - } - const uploaderBtn = card.querySelector('.uploader-link'); - if (uploaderBtn) { - uploaderBtn.onclick = (event) => { - event.stopPropagation(); - const uploader = uploaderBtn.dataset.uploader || uploaderBtn.textContent || ''; - App.videos.handleSearch(uploader); - }; - } - const tagButtons = card.querySelectorAll('.video-tag'); - if (tagButtons.length) { - tagButtons.forEach((tagBtn) => { - tagBtn.onclick = (event) => { - event.stopPropagation(); - const tag = tagBtn.dataset.tag || tagBtn.textContent || ''; - App.videos.handleSearch(tag); - }; - }); - } - const menuBtn = card.querySelector('.video-menu-btn'); + return card; + }; + + // Returns a card to a state where it shows nothing and remembers nothing, + // ready to be bound to another video. Anything left behind here surfaces as + // one video's content on another video's card. + App.videos.resetCard = function(card) { + card.classList.remove('is-loading', 'is-title-active', 'is-revealing', 'is-previewing'); + delete card.dataset.videoId; + delete card.dataset.titleFocused; + delete card.dataset.titleHovered; + delete card.dataset.titlePrimary; + // The player stamps this to recognise the card it was opened from; a + // recycled card must stop answering to it. + delete card.dataset.playerToken; + const menu = card.querySelector('.video-menu'); - const showInfoBtn = card.querySelector('.video-menu-item[data-action="info"]'); - const downloadBtn = card.querySelector('.video-menu-item[data-action="download"]'); - if (menuBtn && menu) { - menuBtn.onclick = (event) => { - event.stopPropagation(); - App.videos.toggleMenu(menu, menuBtn); - }; + if (menu) menu.classList.remove('open'); + const menuBtn = card.querySelector('.video-menu-btn'); + if (menuBtn) menuBtn.setAttribute('aria-expanded', 'false'); + + const titleWrap = card.querySelector('.video-title'); + if (titleWrap) titleWrap.classList.remove('has-marquee'); + const titleText = card.querySelector('.video-title-text'); + if (titleText) { + titleText.style.removeProperty('--marquee-distance'); + titleText.style.removeProperty('--marquee-duration'); } - if (showInfoBtn) { - showInfoBtn.onclick = (event) => { - event.stopPropagation(); - App.ui.openInfo(v); - App.videos.closeAllMenus(); - }; + + // The hover preview (enhance.js) parks a