Discover cleanup: controller extensions, toast errors, migrate skipped sections

Follow-up to the controller migration commits. Closes out the
extension list the per-section migrations surfaced as needed.

CONTROLLER EXTENSIONS

- Callable `fetchUrl: () => string` — resolves the seasonal-playlist
  recreate-on-key-change hack from the prior commit.
- No-fetch `data:` mode — value or `() => value`. Lets render-only
  sections like Seasonal Albums use the controller without inventing
  a fake endpoint. Mutually exclusive with `fetchUrl`; validated up
  front so misuse fails at register-time.
- `beforeLoad(ctx)` hook — runs before the spinner shows. Lets
  dynamically-inserted sections like Because You Listen To ensure
  their `contentEl` exists before the visibility check.
- `onSuccess(data, ctx)` hook — runs after the success gate but
  before isEmpty / isStale. Cleaner home for sibling header /
  subtitle / button updates than folding them into renderItems.
- `isStale(items, data)` + `onStale(ctx)` + `renderStale(items, data)`
  + `staleMessage` — third render state for "data is empty BUT
  upstream is still discovering". Stale wins over empty when both
  apply. Default stale UI is the same spinner block used elsewhere.
- `showErrorToast: true` config — opens a global `showToast(...)` in
  addition to the in-section error block. Default off; sections that
  have no recovery action shouldn't shout at the user.
- `renderItems` returning null/undefined now leaves contentEl
  untouched. Lets a renderer do its own DOM manipulation (e.g.
  delegating to an existing grid-render fn that targets a child
  element) without fighting the controller's innerHTML swap.

MIGRATED THE 2 SKIPPED SECTIONS

- `loadYourAlbums` — uses `isStale`/`onStale`/`renderStale` for the
  stale-fetch state, `onSuccess` for the subtitle/filters/download
  side-effects, `hideWhenEmpty` + `sectionEl` for the truly-empty
  case, `renderItems` returning null since it delegates to the
  existing `_renderYourAlbumsGrid` + `_renderYourAlbumsPagination`.
- `loadSeasonalAlbums` — uses no-fetch `data:` mode because the
  parent `loadSeasonalContent` already fetched the season payload.
  `beforeLoad` updates the sibling title/subtitle text.

ERROR TOASTS ON ALL MIGRATED SECTIONS

Every migrated section now has `showErrorToast: true`. Section load
failures surface a global toast instead of silently spinning forever
or swallowing into console.debug. Same pattern JohnBaumb #369 asked
for at the Python layer, applied at the UI layer.

SHARED SYNC-STATUS BLOCK

Lifted the duplicated decade-tab + genre-tab sync-status HTML
(✓ completed |  pending | ✗ failed | percentage) into a single
`_renderSyncStatusBlock(idPrefix)` helper. Two call sites now share
one implementation. ListenBrainz playlists keep their own block
because the semantics differ — matching progress (total / matched /
failed) vs download progress.

DEAD-SECTION AUDIT — NONE DEAD

Audited the 13 supposedly-dead hidden sections from
DISCOVER_REVIEW.md. All 13 are alive: gated on user data (discovery
pool, library content, metadata cache) and self-surface when their
data exists via `style.display = 'block'` on the success path. The
review's grep missed the toggle. No deletions made.

DAILY MIXES ORPHAN CALL

Removed the orphaned `loadPersonalizedDailyMixes()` call from
`blockDiscoveryArtist` — Daily Mixes is intentionally paused (its
load call in `loadDiscoverPage` is commented out) so refreshing it
from the post-block hook was a no-op.

2204/2204 full suite green. JS parses clean (`node --check`).
This commit is contained in:
Broque Thomas 2026-05-07 20:05:39 -07:00
parent 4ee78bb973
commit dc2323cde6
3 changed files with 328 additions and 199 deletions

View file

@ -5,17 +5,17 @@
* Owns the lifecycle every discover-page section already does by hand:
*
* 1. show a loading spinner in the carousel container
* 2. fetch the section's endpoint
* 2. fetch the section's endpoint (or use pre-fetched data)
* 3. parse the response, decide whether the data is empty
* 4. either show the empty state, render the items, or show an error
* 4. either show the empty state, render the items, show a stale
* "still updating" state, or show an error
* 5. wire any post-render handlers (download buttons, hover, etc)
* 6. expose a refresh() method so the same lifecycle can re-fire
*
* Each section currently re-implements this 30 times in `discover.js`
* Each section currently re-implements this by hand in `discover.js`
* with subtle drift different empty-state messages, inconsistent
* error handling (some console.debug, some silently swallowed, some
* leave the spinner spinning forever), inconsistent refresh-button
* feedback. This controller is the "lift what's truly shared"
* error handling, inconsistent refresh-button feedback, no consistent
* error toast. This controller is the "lift what's truly shared"
* extraction: register a section once, the controller handles the
* lifecycle, the section provides only its renderer.
*
@ -24,12 +24,6 @@
* rows). The controller is the lifecycle wrapper around those
* renderers, not a forced visual abstraction.
*
* MIGRATION STATUS: this is the foundation commit. Only `Recent
* Releases` has been migrated as a proof. The other sections
* (Your Artists, Your Albums, Seasonal, Fresh Tape, The Archives,
* etc) still use their hand-rolled load functions in discover.js
* and will migrate one section per commit.
*
* USAGE:
*
* const ctrl = createDiscoverSectionController({
@ -45,19 +39,48 @@
* });
* ctrl.load();
*
* Future enhancements (not in this foundation commit):
* - global error toast wrapper (so users see something when an
* endpoint fails instead of the silent-empty-state default)
* - registry-driven section list (so the dead-section audit
* becomes registry edits, not section-by-section deletions)
* - per-section "requires X primary source" gate
* EXTENSIONS:
*
* `fetchUrl` accepts a function returning a string for sections
* whose endpoint depends on runtime state (e.g. seasonal playlist
* keyed by `currentSeasonKey`).
*
* `data` lets a section bypass fetch entirely the controller still
* runs success / empty / render / onRendered, just without going to
* the network. Use when a parent already fetched and just wants the
* shared lifecycle. `data` may be a value or a `() => value`
* function. Sections must supply EITHER `fetchUrl` OR `data`, not
* both.
*
* `beforeLoad(ctx)` runs before the spinner shows. Useful for
* ensuring `contentEl` exists (e.g. dynamically inserted sections)
* or updating sibling headers / subtitles before any visual change.
*
* `onSuccess(data, ctx)` runs after the success check passes but
* before isEmpty / isStale checks. Cleaner home for header text
* updates that depend on response data (vs folding them into
* renderItems).
*
* `isStale(items, data)` + `onStale(ctx)` give sections a third
* render state for "data is empty but the upstream is still
* discovering". Returning true from `isStale` renders the stale
* state (default: spinner + "Updating..." copy, override via
* `renderStale` or `staleMessage`) and fires `onStale` so the
* section can start a poller. Stale wins over empty when both apply.
*
* `showErrorToast: true` opens a global `showToast(...)` on error
* in addition to the in-section error block. Default off sections
* that have no recovery action shouldn't shout at the user.
*
* If `renderItems` returns null / undefined, the controller leaves
* `contentEl` untouched. Lets a renderer do its own DOM manipulation
* (e.g. dynamic per-item child containers) without fighting the
* controller's `innerHTML` swap.
*/
(function () {
'use strict';
// Validate the config object up front — bad configs fail fast at
// section-register time instead of silently breaking on load.
function _validateConfig(cfg) {
if (!cfg || typeof cfg !== 'object') {
throw new Error('createDiscoverSectionController: config required');
@ -68,8 +91,13 @@
if (typeof cfg.contentEl !== 'string' && !(cfg.contentEl instanceof Element)) {
throw new Error(`[discover:${cfg.id}] config.contentEl required (selector or Element)`);
}
if (typeof cfg.fetchUrl !== 'string' || !cfg.fetchUrl) {
throw new Error(`[discover:${cfg.id}] config.fetchUrl required`);
const hasFetch = (typeof cfg.fetchUrl === 'string' && cfg.fetchUrl) || typeof cfg.fetchUrl === 'function';
const hasData = cfg.data !== undefined;
if (!hasFetch && !hasData) {
throw new Error(`[discover:${cfg.id}] either config.fetchUrl or config.data required`);
}
if (hasFetch && hasData) {
throw new Error(`[discover:${cfg.id}] config.fetchUrl and config.data are mutually exclusive`);
}
if (typeof cfg.renderItems !== 'function') {
throw new Error(`[discover:${cfg.id}] config.renderItems required (function)`);
@ -90,47 +118,42 @@
_validateConfig(cfg);
const config = Object.assign({
// Wrapper element to show/hide when section becomes empty.
// Null = always visible, only the contents change.
sectionEl: null,
// Hide the whole section if the response is empty
// (vs showing an empty-state message inside the carousel).
hideWhenEmpty: false,
// For sections that need to show data even on empty (e.g.
// "Recent Releases" with "no recent releases" copy).
// When true and items.length === 0, render the empty state.
renderEmptyState: true,
// HTTP method + options for the fetch call. Default GET, no
// body. fetchOptions is a function so callers can compute it
// at load time (e.g. read filter selects).
fetchMethod: 'GET',
fetchOptions: null,
// Pull the items array from the response. Default looks
// for `data.items` then `data.albums` then `data.artists`.
// Either fetchUrl (string or () => string) or data
// (value or () => value). Validated mutually exclusive above.
extractItems: null,
// Override the success check. Default: require data.success
// when present, otherwise treat any 2xx as success.
isSuccess: null,
// Override empty-detection. Default: items.length === 0.
isEmpty: null,
// Post-render hook — attach event handlers, etc.
onRendered: null,
// Lifecycle copy. Empty string = no message.
// Stale = data is empty but upstream is still discovering.
// Returning true here renders the stale state instead of
// empty, and fires onStale so the section can poll.
isStale: null,
renderStale: null,
staleMessage: 'Updating...',
// Hooks
beforeLoad: null, // (ctx) => void — before spinner shows
onSuccess: null, // (data, ctx) => void — after success gate
onStale: null, // (ctx) => void — when stale state renders
onRendered: null, // (ctx) => void — after content renders
// UX copy
loadingMessage: 'Loading...',
emptyMessage: 'Nothing to show',
errorMessage: 'Failed to load',
// CSS class names — let surfaces override for styling.
loadingClass: 'discover-loading',
emptyClass: 'discover-empty',
errorClass: 'discover-empty',
// When true, log full error stacks to console.error. Default
// logs to console.debug — keeps the console quiet for users
// while staying inspectable when devtools is open.
staleClass: 'discover-loading',
// Errors
verboseErrors: false,
showErrorToast: false, // also fire window.showToast on error
}, cfg);
const state = {
phase: 'idle', // idle | loading | rendered | empty | error
phase: 'idle', // idle | loading | rendered | empty | stale | error
lastData: null,
lastError: null,
inFlight: null,
@ -140,6 +163,13 @@
if (el) el.innerHTML = html;
}
function _ctx(extra) {
return Object.assign(
{ contentEl: _resolveEl(config.contentEl), config },
extra || {},
);
}
function _showLoading() {
const contentEl = _resolveEl(config.contentEl);
if (!contentEl) return;
@ -176,6 +206,40 @@
state.phase = 'empty';
}
function _showStale(items, data) {
const contentEl = _resolveEl(config.contentEl);
if (!contentEl) return;
_showSection();
// Custom renderStale wins. Otherwise default spinner + copy.
let html;
if (typeof config.renderStale === 'function') {
try {
html = config.renderStale(items, data, _ctx({ items, data }));
} catch (err) {
console.debug(`[discover:${config.id}] renderStale threw:`, err);
html = null;
}
}
if (html === null || html === undefined) {
html = `
<div class="${config.staleClass}">
<div class="loading-spinner"></div>
<p>${config.staleMessage}</p>
</div>
`;
}
_setHtml(contentEl, html);
state.phase = 'stale';
if (typeof config.onStale === 'function') {
try {
config.onStale(_ctx({ items, data }));
} catch (err) {
console.debug(`[discover:${config.id}] onStale hook threw:`, err);
}
}
}
function _showError(error) {
const contentEl = _resolveEl(config.contentEl);
if (!contentEl) return;
@ -188,6 +252,13 @@
state.lastError = error;
const log = config.verboseErrors ? console.error : console.debug;
log(`[discover:${config.id}]`, error);
if (config.showErrorToast && typeof window.showToast === 'function') {
try {
window.showToast(config.errorMessage, 'error');
} catch (toastErr) {
console.debug(`[discover:${config.id}] toast failed:`, toastErr);
}
}
}
function _showSection() {
@ -197,7 +268,6 @@
function _extractItems(data) {
if (config.extractItems) return config.extractItems(data) || [];
// Sensible defaults for the most common response shapes.
if (Array.isArray(data?.items)) return data.items;
if (Array.isArray(data?.albums)) return data.albums;
if (Array.isArray(data?.artists)) return data.artists;
@ -208,8 +278,6 @@
function _isSuccess(data) {
if (config.isSuccess) return config.isSuccess(data);
// If `success` is present, require it to be truthy. Otherwise
// a 2xx response with parseable JSON counts as success.
if (data && Object.prototype.hasOwnProperty.call(data, 'success')) {
return Boolean(data.success);
}
@ -221,11 +289,40 @@
return !Array.isArray(items) || items.length === 0;
}
function _isStale(items, data) {
if (typeof config.isStale !== 'function') return false;
try {
return Boolean(config.isStale(items, data));
} catch (err) {
console.debug(`[discover:${config.id}] isStale threw:`, err);
return false;
}
}
function _resolveFetchUrl() {
if (typeof config.fetchUrl === 'function') return config.fetchUrl();
return config.fetchUrl;
}
function _resolveStaticData() {
if (typeof config.data === 'function') return config.data();
return config.data;
}
async function load() {
// Coalesce concurrent loads — if a fetch is already in flight,
// return the same promise rather than firing a second call.
// Coalesce concurrent loads — refresh() bypasses the coalesce.
if (state.inFlight) return state.inFlight;
// Run beforeLoad first so it can set up `contentEl` (dynamic
// section creation) before the visibility check below.
if (typeof config.beforeLoad === 'function') {
try {
config.beforeLoad(_ctx());
} catch (err) {
console.debug(`[discover:${config.id}] beforeLoad hook threw:`, err);
}
}
const contentEl = _resolveEl(config.contentEl);
if (!contentEl) {
console.debug(`[discover:${config.id}] contentEl not found, skipping load`);
@ -234,49 +331,70 @@
_showLoading();
const fetchOpts = (typeof config.fetchOptions === 'function')
? (config.fetchOptions() || {})
: {};
const init = Object.assign(
{ method: config.fetchMethod },
fetchOpts,
);
const promise = (async () => {
try {
const resp = await fetch(config.fetchUrl, init);
if (!resp.ok) {
throw new Error(`HTTP ${resp.status}`);
let data;
if (config.data !== undefined) {
// No-fetch mode — parent already has the data.
data = _resolveStaticData();
} else {
const fetchOpts = (typeof config.fetchOptions === 'function')
? (config.fetchOptions() || {})
: {};
const init = Object.assign(
{ method: config.fetchMethod },
fetchOpts,
);
const url = _resolveFetchUrl();
const resp = await fetch(url, init);
if (!resp.ok) {
throw new Error(`HTTP ${resp.status}`);
}
data = await resp.json();
}
const data = await resp.json();
state.lastData = data;
if (!_isSuccess(data)) {
// Treat success=false as empty rather than error so
// the user sees the "nothing here" copy. Endpoints
// returning success=false with a network/auth reason
// can opt into error treatment via isSuccess.
_showEmpty();
return;
}
if (typeof config.onSuccess === 'function') {
try {
config.onSuccess(data, _ctx({ data }));
} catch (err) {
console.debug(`[discover:${config.id}] onSuccess hook threw:`, err);
}
}
const items = _extractItems(data);
// Stale wins over empty — section is empty *now* but
// upstream is still discovering, so show updating UI
// rather than the bare "nothing here" copy.
if (_isStale(items, data)) {
_showStale(items, data);
return;
}
if (_isEmpty(items, data)) {
_showEmpty();
return;
}
_showSection();
const html = config.renderItems(items, data, { contentEl, config });
_setHtml(contentEl, html || '');
const html = config.renderItems(items, data, _ctx({ items, data }));
// null / undefined return = renderer is doing its own
// DOM work, leave the container alone.
if (html !== null && html !== undefined) {
_setHtml(contentEl, html);
}
state.phase = 'rendered';
if (typeof config.onRendered === 'function') {
try {
config.onRendered({ contentEl, items, data, config });
config.onRendered(_ctx({ items, data }));
} catch (hookErr) {
// Don't let a renderer hook error rip down the
// controller — log + continue.
console.debug(`[discover:${config.id}] onRendered hook threw:`, hookErr);
}
}
@ -292,8 +410,6 @@
}
async function refresh() {
// Clear in-flight first so refresh() always re-fires the
// network call (load() coalesces, refresh() bypasses).
state.inFlight = null;
return load();
}
@ -306,8 +422,6 @@
}
function getState() {
// Expose a copy so callers can inspect without holding onto
// mutable internal state.
return {
phase: state.phase,
hasData: state.lastData !== null,
@ -318,7 +432,5 @@
return { load, refresh, destroy, getState };
}
// Expose globally — the discover page is one big shared script
// surface, no module system in play.
window.createDiscoverSectionController = createDiscoverSectionController;
})();

View file

@ -885,6 +885,7 @@ async function loadDiscoverRecentReleases() {
loadingMessage: 'Loading recent releases...',
emptyMessage: 'No recent releases found',
errorMessage: 'Failed to load recent releases',
showErrorToast: true,
});
}
return _recentReleasesCtrl.load();
@ -909,46 +910,64 @@ function debouncedYourAlbumsSearch() {
}, 400);
}
let _yourAlbumsCtrl = null;
async function loadYourAlbums() {
const section = document.getElementById('your-albums-section');
if (!section) return;
try {
const resp = await fetch('/api/discover/your-albums?page=1&per_page=48&status=all');
if (!resp.ok) return;
const data = await resp.json();
if (!data.success) return;
const totalCount = (data.stats && data.stats.total) || 0;
if (totalCount === 0 && !data.stale) return; // Nothing to show yet
section.style.display = '';
yourAlbums = data.albums || [];
yourAlbumsTotal = data.total || 0;
yourAlbumsPage = 1;
const subtitle = document.getElementById('your-albums-subtitle');
if (subtitle && data.stats) {
const s = data.stats;
subtitle.textContent = `${s.total} albums \u00B7 ${s.owned} owned \u00B7 ${s.missing} missing`;
}
const filters = document.getElementById('your-albums-filters');
if (filters && totalCount > 0) filters.style.display = '';
const downloadBtn = document.getElementById('your-albums-download-btn');
if (downloadBtn && data.stats && data.stats.missing > 0) downloadBtn.style.display = '';
_renderYourAlbumsGrid(yourAlbums);
_renderYourAlbumsPagination(yourAlbumsTotal, yourAlbumsPage);
if (data.stale && totalCount === 0) {
const grid = document.getElementById('your-albums-grid');
if (grid) grid.innerHTML = '<div class="discover-loading"><div class="loading-spinner"></div><p>Fetching your albums from connected services...</p></div>';
_pollYourAlbums();
}
} catch (e) {
console.error('Error loading your albums:', e);
if (!_yourAlbumsCtrl) {
_yourAlbumsCtrl = createDiscoverSectionController({
id: 'your-albums',
sectionEl: '#your-albums-section',
contentEl: '#your-albums-grid',
fetchUrl: '/api/discover/your-albums?page=1&per_page=48&status=all',
extractItems: (data) => data.albums || [],
// Truly empty (no data + not stale) \u2192 hide the whole section
// (matches the legacy "Nothing to show yet" early-return). The
// outer hideWhenEmpty + sectionEl handle the visibility flip.
isEmpty: (items, data) => {
const total = (data && data.stats && data.stats.total) || 0;
return total === 0 && !data.stale;
},
hideWhenEmpty: true,
// Stale + no albums yet \u2192 show the "fetching from connected
// services" UI and start the poller. Fires before isEmpty.
isStale: (items, data) => {
const total = (data && data.stats && data.stats.total) || 0;
return Boolean(data && data.stale) && total === 0;
},
renderStale: () =>
'<div class="discover-loading"><div class="loading-spinner"></div><p>Fetching your albums from connected services...</p></div>',
onStale: () => _pollYourAlbums(),
// Side-effects against sibling DOM (subtitle / filters /
// download button) belong here, not in renderItems.
onSuccess: (data) => {
const subtitle = document.getElementById('your-albums-subtitle');
if (subtitle && data.stats) {
const s = data.stats;
subtitle.textContent = `${s.total} albums \u00B7 ${s.owned} owned \u00B7 ${s.missing} missing`;
}
const totalCount = (data.stats && data.stats.total) || 0;
const filters = document.getElementById('your-albums-filters');
if (filters && totalCount > 0) filters.style.display = '';
const downloadBtn = document.getElementById('your-albums-download-btn');
if (downloadBtn && data.stats && data.stats.missing > 0) downloadBtn.style.display = '';
},
// Renderer delegates to the existing grid renderer, which
// writes its own DOM into `#your-albums-grid`. Returning
// null keeps the controller from clobbering it.
renderItems: (items, data) => {
yourAlbums = items;
yourAlbumsTotal = data.total || 0;
yourAlbumsPage = 1;
_renderYourAlbumsGrid(yourAlbums);
_renderYourAlbumsPagination(yourAlbumsTotal, yourAlbumsPage);
return null;
},
errorMessage: 'Failed to load your albums',
verboseErrors: true,
showErrorToast: true,
});
}
return _yourAlbumsCtrl.load();
}
function _pollYourAlbums() {
@ -1368,6 +1387,7 @@ async function loadDiscoverReleaseRadar() {
emptyMessage: 'No new releases available',
errorMessage: 'Failed to load release radar',
verboseErrors: true,
showErrorToast: true,
});
}
return _releaseRadarCtrl.load();
@ -1391,6 +1411,7 @@ async function loadDiscoverWeekly() {
emptyMessage: 'No tracks available yet',
errorMessage: 'Failed to load discovery weekly',
verboseErrors: true,
showErrorToast: true,
});
}
return _weeklyCtrl.load();
@ -1434,6 +1455,7 @@ async function loadDecadeBrowser() {
emptyMessage: 'No decade content available yet. Run a watchlist scan to populate your discovery pool!',
errorMessage: 'Failed to load decades',
verboseErrors: true,
showErrorToast: true,
});
}
return _decadeBrowserCtrl.load();
@ -1525,6 +1547,7 @@ async function loadGenreBrowser() {
emptyMessage: 'No genre content available yet. Run a watchlist scan to populate your discovery pool!',
errorMessage: 'Failed to load genres',
verboseErrors: true,
showErrorToast: true,
});
}
return _genreBrowserCtrl.load();
@ -1654,6 +1677,31 @@ async function openGenrePlaylist(genre) {
let decadeTracksCache = {}; // Store tracks for each decade
let activeDecade = null;
// Shared sync-status display block. Used by per-tab playlists
// (decade browser, genre browser) where we show download progress
// in the standard "✓ completed | ⏳ pending | ✗ failed (N%)" format.
// ListenBrainz playlists use a different shape (total/matched/failed)
// because they show MATCHING progress against the library, not
// download progress, so they intentionally don't use this helper.
function _renderSyncStatusBlock(idPrefix) {
return `
<div class="discover-sync-status" id="${idPrefix}-sync-status" style="display: none;">
<div class="sync-status-content">
<div class="sync-status-label">
<span class="sync-icon"></span>
<span>Syncing to media server...</span>
</div>
<div class="sync-status-stats">
<span class="sync-stat"> <span id="${idPrefix}-sync-completed">0</span></span>
<span class="sync-stat"> <span id="${idPrefix}-sync-pending">0</span></span>
<span class="sync-stat"> <span id="${idPrefix}-sync-failed">0</span></span>
<span class="sync-stat">(<span id="${idPrefix}-sync-percentage">0</span>%)</span>
</div>
</div>
</div>
`;
}
async function loadDecadeBrowserTabs() {
try {
const tabsContainer = document.getElementById('decade-tabs');
@ -1713,21 +1761,7 @@ async function loadDecadeBrowserTabs() {
</div>
</div>
<!-- Sync Status Display -->
<div class="discover-sync-status" id="${tabId}-sync-status" style="display: none;">
<div class="sync-status-content">
<div class="sync-status-label">
<span class="sync-icon"></span>
<span>Syncing to media server...</span>
</div>
<div class="sync-status-stats">
<span class="sync-stat"> <span id="${tabId}-sync-completed">0</span></span>
<span class="sync-stat"> <span id="${tabId}-sync-pending">0</span></span>
<span class="sync-stat"> <span id="${tabId}-sync-failed">0</span></span>
<span class="sync-stat">(<span id="${tabId}-sync-percentage">0</span>%)</span>
</div>
</div>
</div>
${_renderSyncStatusBlock(tabId)}
</div>
<!-- Track List -->
@ -2110,21 +2144,7 @@ async function loadGenreBrowserTabs() {
</div>
</div>
<!-- Sync Status Display -->
<div class="discover-sync-status" id="${tabId}-sync-status" style="display: none;">
<div class="sync-status-content">
<div class="sync-status-label">
<span class="sync-icon"></span>
<span>Syncing to media server...</span>
</div>
<div class="sync-status-stats">
<span class="sync-stat"> <span id="${tabId}-sync-completed">0</span></span>
<span class="sync-stat"> <span id="${tabId}-sync-pending">0</span></span>
<span class="sync-stat"> <span id="${tabId}-sync-failed">0</span></span>
<span class="sync-stat">(<span id="${tabId}-sync-percentage">0</span>%)</span>
</div>
</div>
</div>
${_renderSyncStatusBlock(tabId)}
</div>
<!-- Track List -->
@ -3533,59 +3553,52 @@ async function loadSeasonalContent() {
}
}
function _renderSeasonalAlbumCard(album, index) {
const coverUrl = album.album_cover_url || '/static/placeholder-album.png';
return `
<div class="discover-card" onclick="openDownloadModalForSeasonalAlbum(${index})" style="cursor: pointer;">
<div class="discover-card-image">
<img src="${coverUrl}" alt="${album.album_name}" loading="lazy">
</div>
<div class="discover-card-info">
<h4 class="discover-card-title">${album.album_name}</h4>
<p class="discover-card-subtitle">${album.artist_name}</p>
${album.release_date ? `<p class="discover-card-meta">${album.release_date}</p>` : ''}
</div>
</div>
`;
}
// Seasonal Albums uses no-fetch `data:` mode — the parent
// `loadSeasonalContent` already fetched the season payload and passes
// the album list directly. Controller is recreated per call so the
// per-season `data` snapshot is current.
async function loadSeasonalAlbums(seasonData) {
try {
const carousel = document.getElementById('seasonal-albums-carousel');
if (!carousel) return;
// Show seasonal section
const seasonalSection = document.getElementById('seasonal-albums-section');
if (seasonalSection) {
seasonalSection.style.display = 'block';
}
// Update header
const seasonalTitle = document.getElementById('seasonal-albums-title');
const seasonalSubtitle = document.getElementById('seasonal-albums-subtitle');
if (seasonalTitle) {
seasonalTitle.textContent = `${seasonData.icon} ${seasonData.name}`;
}
if (seasonalSubtitle) {
seasonalSubtitle.textContent = seasonData.description;
}
// Store albums for download functionality
discoverSeasonalAlbums = seasonData.albums || [];
if (discoverSeasonalAlbums.length === 0) {
carousel.innerHTML = '<div class="discover-empty"><p>No seasonal albums found</p></div>';
return;
}
// Build carousel HTML
let html = '';
discoverSeasonalAlbums.forEach((album, index) => {
const coverUrl = album.album_cover_url || '/static/placeholder-album.png';
html += `
<div class="discover-card" onclick="openDownloadModalForSeasonalAlbum(${index})" style="cursor: pointer;">
<div class="discover-card-image">
<img src="${coverUrl}" alt="${album.album_name}" loading="lazy">
</div>
<div class="discover-card-info">
<h4 class="discover-card-title">${album.album_name}</h4>
<p class="discover-card-subtitle">${album.artist_name}</p>
${album.release_date ? `<p class="discover-card-meta">${album.release_date}</p>` : ''}
</div>
</div>
`;
});
carousel.innerHTML = html;
} catch (error) {
console.error('Error loading seasonal albums:', error);
}
const albums = (seasonData && seasonData.albums) || [];
const ctrl = createDiscoverSectionController({
id: 'seasonal-albums',
sectionEl: '#seasonal-albums-section',
contentEl: '#seasonal-albums-carousel',
data: { success: true, albums },
extractItems: (data) => data.albums,
beforeLoad: () => {
const section = document.getElementById('seasonal-albums-section');
if (section) section.style.display = 'block';
const titleEl = document.getElementById('seasonal-albums-title');
const subtitleEl = document.getElementById('seasonal-albums-subtitle');
if (titleEl && seasonData) titleEl.textContent = `${seasonData.icon} ${seasonData.name}`;
if (subtitleEl && seasonData) subtitleEl.textContent = seasonData.description;
},
renderItems: (items) => {
discoverSeasonalAlbums = items;
return items.map((album, i) => _renderSeasonalAlbumCard(album, i)).join('');
},
emptyMessage: 'No seasonal albums found',
errorMessage: 'Failed to load seasonal albums',
verboseErrors: true,
showErrorToast: true,
});
return ctrl.load();
}
let _seasonalPlaylistCtrl = null;
@ -3629,6 +3642,7 @@ async function loadSeasonalPlaylist(seasonData) {
emptyMessage: 'No tracks available yet',
errorMessage: 'Failed to load playlist',
verboseErrors: true,
showErrorToast: true,
});
_seasonalPlaylistCtrlKey = currentSeasonKey;
}
@ -3983,10 +3997,11 @@ async function blockDiscoveryArtist(artistName) {
const data = await res.json();
if (data.success) {
showToast(`Blocked ${artistName} from discovery`, 'success');
// Refresh all discovery sections to remove the artist
// Refresh discovery sections to remove the artist. Daily Mixes
// is intentionally paused (see loadDiscoverPage), so don't
// refresh it — the section isn't on the page anyway.
loadPersonalizedHiddenGems();
loadDiscoveryShuffle();
loadPersonalizedDailyMixes();
} else {
showToast(data.error || 'Failed to block artist', 'error');
}
@ -4181,6 +4196,7 @@ async function loadYourArtists() {
emptyMessage: 'No followed artists found',
errorMessage: 'Failed to load your artists',
verboseErrors: true,
showErrorToast: true,
});
}
return _yourArtistsCtrl.load();

View file

@ -3432,6 +3432,7 @@ const WHATS_NEW = {
'2.4.3': [
// --- post-2.4.2 dev work — entries hidden by _getLatestWhatsNewVersion until the build version bumps ---
{ date: 'Unreleased — 2.4.3 dev cycle' },
{ title: 'Internal: Discover Cleanup Round — Toast Errors, Stale State, Skipped Sections', desc: 'follow-up to the controller migration. extended `createDiscoverSectionController` with the hooks the per-section migrations surfaced as needed: callable `fetchUrl` (resolves the seasonal-playlist recreate-on-key-change hack), no-fetch `data:` mode (lets render-only sections like seasonal albums use the controller without inventing a fake endpoint), `beforeLoad` hook (lets dynamically-inserted sections like because-you-listen-to ensure their container exists before the spinner shows), `onSuccess(data)` hook (cleaner home for sibling header / subtitle / button updates than folding them into renderItems), and an `isStale` / `onStale` / `renderStale` triple for the third render state (data is empty BUT upstream is still discovering — show updating UI + start a poller, instead of the bare empty-state copy). turned on `showErrorToast: true` for every migrated section — section load failures now surface a global toast instead of silently spinning forever or swallowing into console.debug. that\'s the JohnBaumb #369 pattern applied at the UI layer. migrated the two sections that didn\'t fit the original controller contract: `loadYourAlbums` (uses isStale/onStale for stale-fetch UI + onSuccess for subtitle/filters/download-button side-effects + renderItems returning null since it delegates to the existing grid renderer) and `loadSeasonalAlbums` (uses no-fetch data mode since the parent `loadSeasonalContent` already fetched the season payload). also lifted the duplicated decade-tab + genre-tab sync-status block (✓/⏳/✗/percentage) into a `_renderSyncStatusBlock(idPrefix)` helper — two call sites now share one implementation. listenbrainz playlists keep their own block because the semantics differ (matching progress vs download progress). audit found the 13 supposedly-dead hidden sections aren\'t dead at all — they\'re gated on user data (discovery pool, library content, metadata cache) and self-surface when their data exists. removed one orphaned `loadPersonalizedDailyMixes()` call from `blockDiscoveryArtist` — daily mixes is intentionally paused, refreshing it from there was a no-op.', page: 'discover' },
{ title: 'Internal: Migrate 7 More Discover Sections to the Controller', desc: 'follow-up to the foundation commit. migrated fresh tape, the archives, time machine intro carousel, browse by genre intro carousel, seasonal mix, your artists, and because-you-listen-to onto `createDiscoverSectionController`. each one drops its own hand-rolled try/catch + spinner injection + empty-state HTML + error swallow in favor of a config object — controller owns the lifecycle. net 76 lines smaller in discover.js even after adding the per-section render helpers. skipped two sections that don\'t fit the controller\'s single-fetch / single-render-target shape: `loadYourAlbums` (paginated grid + filters, four separate UI elements updated) and `loadSeasonalAlbums` (no fetch — receives pre-fetched data from parent). hidden / dead sections (~13 of them) untouched in this pass — separate audit commit will surface or kill them. controller extension candidates surfaced for follow-up: callable `fetchUrl` (so seasonal playlist doesn\'t need controller-recreate-on-key-change), explicit `isStale` / `onStale` hook (so your-artists doesn\'t fold stale handling into renderItems), `beforeLoad` hook (so because-you-listen-to can let the controller own the dynamic container creation), and a no-fetch `data:` mode (so render-only sections like seasonal albums can use the controller). zero behavior changes — every public load function keeps its name + signature so existing callers, refresh buttons, and dashboard wiring don\'t notice the swap.', page: 'discover' },
{ title: 'Internal: Discover Section Controller Foundation', desc: 'every section on the discover page (recent releases, your artists, your albums, seasonal, fresh tape, the archives, etc) re-implements the same lifecycle by hand: show spinner → fetch endpoint → parse → either render or show empty state or show error → maybe wire post-render handlers → maybe expose refresh. ~30 sections, all subtly drifting — different empty messages, different error handling (some console.debug, some silently swallowed, some leave the spinner spinning forever), different sync-status icons, no consistent error toast. lifted that lifecycle into a shared `createDiscoverSectionController` (renderers stay per-section because section data shapes legitimately differ — album cards vs artist circles vs playlist tiles vs track rows; the controller is the wrapper, not a forced visual abstraction). this commit is the foundation: built the controller + migrated `recent releases` as proof. each remaining section will migrate in its own follow-up commit (keeps reviews small + lets us sequence the work). once everything is on the controller, the discover-page cleanup work (kill 13 dead sections, standardize sync-status icons, add error toasts) becomes single-line registry edits instead of section-by-section rewrites.', page: 'discover' },
],