From f3d1ae1c8c303153ac8a8f97bfb3306ea3ac16d0 Mon Sep 17 00:00:00 2001 From: Logan Date: Wed, 19 Aug 2026 20:27:22 -0400 Subject: [PATCH 1/7] feat(jobs): show background work where the work is started MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reading the app before moving anything turned up that four of the five job kinds already have a home showing their work: Settings → Search Index draws per-tier index progress, `downloads-view` draws every download's lifecycle state, `autotag-view` draws its own apply ring, and only `library-scan` had nowhere but the Jobs tab. What none of the four had is the *generic* affordances — pause, cancel, Details, the log, and a finished job you can dismiss. So this is a panel embedded beside each of them rather than one "Background jobs" section in Settings, which would have been the tab again under another name. Three rules in it. The controls are `applyJobControl`, not a reimplementation, which is what keeps the index build's "you will discard hours of downloading" confirmation alive across the move. A panel with nothing to say is `hidden` rather than empty, host margin included, because an idle panel in four places is four pieces of furniture describing an absence. And there is no "Clear finished": `ClearFinishedJobs` is global, so a Clear under Libraries would discard the index build's history too. `JobKind` also gains `download`, which the backend has had all along. --- frontend/src/components/jobs/job-panel.ts | 229 +++++++++++++++++++++ frontend/src/store/job-store.ts | 1 + frontend/test/components/job-panel.test.ts | 160 ++++++++++++++ 3 files changed, 390 insertions(+) create mode 100644 frontend/src/components/jobs/job-panel.ts create mode 100644 frontend/test/components/job-panel.test.ts diff --git a/frontend/src/components/jobs/job-panel.ts b/frontend/src/components/jobs/job-panel.ts new file mode 100644 index 0000000..5bc5808 --- /dev/null +++ b/frontend/src/components/jobs/job-panel.ts @@ -0,0 +1,229 @@ +import { LitElement, html, css, nothing } from 'lit'; +import { customElement, property, state } from 'lit/decorators.js'; +import { designTokens } from '../../styles/tokens.css'; +import { jobStore } from '@store/job-store'; +import type { Job, JobKind } from '@store/job-store'; +import { isTerminal } from '@store/job-store'; +import './job-row'; +import './job-details-drawer'; +import { applyJobControl } from './job-controls'; +import { jobStateStyles } from './job-format'; + +/** + * The background work of one kind, rendered wherever that work is + * started or configured. + * + * #27 folded the Jobs tab away, and the shape it folded into is this + * rather than one "Background jobs" panel in Settings — which would + * have been the tab again under another name. Reading the app first + * turned up that **four of the five job kinds already had a home** + * showing their work: Settings → Search Index draws per-tier index + * progress, `downloads-view` draws every download's lifecycle state, + * `autotag-view` draws its own apply ring, and only `library-scan` had + * nowhere but the tab. What none of those four had is the *generic* + * affordances — pause, cancel, "Details", the log, and a finished job + * you can dismiss — which is what this carries to each of them. + * + * Three things about it are load-bearing. + * + * **The controls are `applyJobControl`, not a reimplementation.** That + * is what keeps the "you will discard hours of downloading" + * confirmation on an index build alive across the move: it is keyed on + * `KindIndexBuild` inside the shared handler, and a host that rendered + * its own buttons would silently drop it. + * + * **A panel with nothing to say renders nothing at all**, host padding + * included — an idle panel in four places is four pieces of furniture + * describing an absence. That is the rule `startBackfillJob` follows + * for the indicator, one layer up. + * + * **There is no "Clear finished" here**, because `ClearFinishedJobs` is + * global: a Clear in the Libraries panel would silently discard the + * index build's history too. A finished row dismisses itself, which is + * per-job and is what `job-row` already offers. + */ +@customElement('job-panel') +export class JobPanel extends LitElement { + /** + * Comma-separated job kinds, e.g. `index-build,catalog-enrich`. + * + * An attribute rather than a property because every call site is a + * literal in a template, and one of them is inside an HTMX-adjacent + * settings page where a property binding would be one more thing to + * remember. + */ + @property({ type: String }) + kinds = ''; + + /** Heading above the rows. Omitted renders no heading. */ + @property({ type: String }) + heading = ''; + + @state() + private jobs: Job[] = []; + + @state() + private drawerJobId = ''; + + @state() + private drawerOpen = false; + + private unsubscribe: (() => void) | null = null; + + static override styles = [ + designTokens, + jobStateStyles, + css` + :host { + display: block; + margin-top: 1em; + } + + /* An empty panel takes no room at all, margin included. */ + :host([hidden]) { + display: none; + } + + h3 { + font-size: var(--yj-text-sm); + text-transform: uppercase; + letter-spacing: 0.06em; + color: var(--yj-text-tertiary, #868e96); + margin: 0 0 0.5em; + } + + .card { + background: var(--yj-bg-surface, #2b3035); + border: 1px solid var(--yj-border, #495057); + border-radius: 6px; + overflow: hidden; + } + + .job-entry { + display: flex; + align-items: center; + gap: 0.75em; + padding: 0.6em 0.8em; + border-bottom: 1px solid var(--yj-border-subtle, #3a4046); + } + + .job-entry:last-child { + border-bottom: none; + } + + job-row { + flex: 1; + /* A grid child's implicit minimum is its content, and a + job title is long. */ + min-width: 0; + } + + .details-btn { + background: none; + border: 1px solid var(--yj-border, #495057); + border-radius: 4px; + color: var(--yj-text-secondary, #adb5bd); + cursor: pointer; + font-family: inherit; + font-size: var(--yj-text-sm); + padding: 0.3em 0.6em; + white-space: nowrap; + } + + .details-btn:hover { + color: var(--yj-text-primary, #e9ecef); + } + `, + ]; + + override connectedCallback() { + super.connectedCallback(); + this.unsubscribe = jobStore.subscribe(() => { + this.jobs = jobStore.jobs; + }); + this.jobs = jobStore.jobs; + void jobStore.init(); + } + + override disconnectedCallback() { + super.disconnectedCallback(); + this.unsubscribe?.(); + this.unsubscribe = null; + } + + /** The kinds this panel answers for. */ + private get wanted(): ReadonlySet { + return new Set( + this.kinds + .split(',') + .map((k) => k.trim()) + .filter(Boolean), + ); + } + + private get mine(): Job[] { + const wanted = this.wanted; + + return this.jobs.filter((job) => wanted.has(job.kind as JobKind)); + } + + private openDetails(id: string) { + this.drawerJobId = id; + this.drawerOpen = true; + } + + private onDrawerClosed = () => { + this.drawerOpen = false; + }; + + override render() { + const mine = this.mine; + + // Hidden rather than empty: see the class comment. The drawer + // goes with it, since it can only have been opened from a row. + this.hidden = mine.length === 0; + + if (mine.length === 0) return nothing; + + const active = mine.filter((job) => !isTerminal(job)); + const finished = mine.filter(isTerminal); + + return html` + ${this.heading ? html`

${this.heading}

` : nothing} +
+ ${[...active, ...finished].map( + (job) => html` +
+ + +
+ `, + )} +
+ + + `; + } +} + +declare global { + interface HTMLElementTagNameMap { + 'job-panel': JobPanel; + } +} diff --git a/frontend/src/store/job-store.ts b/frontend/src/store/job-store.ts index 29e83f8..7f06c49 100644 --- a/frontend/src/store/job-store.ts +++ b/frontend/src/store/job-store.ts @@ -30,6 +30,7 @@ export type JobState = export type JobKind = | 'library-scan' | 'index-build' + | 'download' | 'autotag-apply' | 'catalog-enrich'; diff --git a/frontend/test/components/job-panel.test.ts b/frontend/test/components/job-panel.test.ts new file mode 100644 index 0000000..978d42b --- /dev/null +++ b/frontend/test/components/job-panel.test.ts @@ -0,0 +1,160 @@ +/** + * Background work, shown where the work is started (#27). + * + * The Jobs tab is gone; each kind's rows now live beside the thing that + * starts it — scans in Settings → Libraries, index work in Search + * Index, downloads under the download clients, the autotag apply in the + * Autotag view. What those four surfaces never had, and what this + * carries to them, is the *generic* affordances: pause, cancel, + * "Details", and a finished job you can dismiss. + * + * The assertions are about which rows a panel owns and what its buttons + * do, not about the store — `job-store` already has the snapshot + * covered, and a panel that renders the right rows for the wrong reason + * would pass either way. + */ +import { describe, expect, it, beforeEach } from 'vitest'; + +import '@components/jobs/job-panel'; +import { emit, flush, lastArgs, calls, resetHarness, stub } from '@test/support/harness'; +import { Events } from '../../src/events'; +import { fixture, shadow, shadowAll } from '@test/support/render'; +import type { LitElement } from 'lit'; + +/** A job snapshot entry, with the fields `job-row` actually reads. */ +const job = (over: Record = {}) => ({ + id: 'scan:1', + kind: 'library-scan', + title: 'Scanning Music', + state: 'running', + current: 3, + total: 10, + caps: { pausable: true, cancellable: true }, + stages: null, + stats: null, + startedAt: Date.now(), + updatedAt: Date.now(), + logCount: 0, + warnCount: 0, + errorCount: 0, + ...over, +}); + +/** Push a full snapshot, which is what the backend emits. */ +async function snapshot(jobs: unknown[]): Promise { + emit(Events.JobsChanged, jobs); + await flush(); +} + +const rows = (el: HTMLElement) => shadowAll(el, 'job-row'); + +const titles = (el: HTMLElement) => + rows(el).map((row) => (row as HTMLElement & { job: { title: string } }).job.title); + +describe('', () => { + beforeEach(async () => { + resetHarness(); + stub('jobs.Service.GetJobs', []); + await snapshot([]); + }); + + it('renders only the kinds it was asked for', async () => { + const el = await fixture('job-panel', { + kinds: 'index-build,catalog-enrich', + }); + + await snapshot([ + job({ id: 'scan:1', kind: 'library-scan', title: 'Scanning Music' }), + job({ id: 'idx', kind: 'index-build', title: 'Building the index' }), + job({ id: 'enrich', kind: 'catalog-enrich', title: 'Filling in artists' }), + ]); + await el.updateComplete; + + // The title is inside `job-row`'s own shadow root, so this asks + // the rows what they are drawing rather than reading the panel's + // text -- which would pass whether or not a row rendered. + expect(titles(el)).toEqual(['Building the index', 'Filling in artists']); + }); + + /** + * An idle panel in four places is four pieces of furniture describing + * an absence — and `hidden` rather than an empty render, because the + * host's own margin would otherwise still be spent. + */ + it('takes up no room when it has nothing to say', async () => { + const el = await fixture('job-panel', { kinds: 'download' }); + + await snapshot([job({ id: 'scan:1', kind: 'library-scan' })]); + await el.updateComplete; + + expect(el.hidden).toBe(true); + expect(rows(el)).toHaveLength(0); + + await snapshot([ + job({ id: 'dl:1', kind: 'download', title: 'Downloading Glass Harbour' }), + ]); + await el.updateComplete; + + expect(el.hidden).toBe(false); + expect(rows(el)).toHaveLength(1); + }); + + /** + * The controls go through `applyJobControl`, which is what carries + * the index build's "you will discard hours of downloading" + * confirmation across this move. A host drawing its own buttons would + * have dropped it silently. + */ + it('pauses through the shared handler', async () => { + const el = await fixture('job-panel', { kinds: 'library-scan' }); + + await snapshot([job()]); + await el.updateComplete; + + const row = rows(el)[0]!; + + shadow(row, 'button[aria-label^="Pause"]')?.click(); + await flush(); + + expect(lastArgs('jobs.Service.PauseJob')).toEqual(['scan:1']); + }); + + /** + * Cancelling an index build asks first; cancelling a scan does not, + * because a scan is cheap to re-run. Both answers live in + * `applyJobControl` and both had to survive the move. + */ + it('does not ask before cancelling a scan', async () => { + const el = await fixture('job-panel', { kinds: 'library-scan' }); + + await snapshot([job()]); + await el.updateComplete; + + const row = rows(el)[0]!; + + shadow(row, 'button[aria-label^="Stop"]')?.click(); + await flush(); + + expect(lastArgs('jobs.Service.CancelJob')).toEqual(['scan:1']); + }); + + /** + * A finished job is dismissed one at a time. There is deliberately no + * "Clear finished" here: `ClearFinishedJobs` is global, so a Clear in + * the Libraries panel would discard the index build's history too. + */ + it('keeps finished jobs, dismissible one by one', async () => { + const el = await fixture('job-panel', { kinds: 'library-scan' }); + + await snapshot([job({ state: 'complete' })]); + await el.updateComplete; + + const row = rows(el)[0]!; + + shadow(row, 'button[aria-label^="Dismiss"]')?.click(); + await flush(); + + expect(lastArgs('jobs.Service.DismissJob')).toEqual(['scan:1']); + expect(calls('jobs.Service.ClearFinishedJobs')).toHaveLength(0); + }); +}); -- 2.54.0 From 12af6ec1f74101fca5ab0bcbb1849fcf376d2344 Mon Sep 17 00:00:00 2001 From: Logan Date: Wed, 19 Aug 2026 20:27:33 -0400 Subject: [PATCH 2/7] feat(settings): scan from Settings, and watch every job in place Scanning goes back where libraries are managed -- the Jobs tab's own comment said the per-library controls had been taken out of Settings to build it. "Scan All" and "Full Rescan" join "Add Library", "Scan now" joins the per-library overflow menu beside Rename and Remove, and a library being scanned says so where its track count goes. Each surface then gets the job rows for its own kind: scans under Libraries, index and enrichment under Search Index, downloads under the download clients, and the autotag apply in the Autotag view -- where stopping a run matters most, since applying rewrites tags on disk and that view had no cancel at all. The autotag panel shares the header's grid row through a wrapper rather than taking a third row: it is display:none while nothing is applying, which is nearly always, and a grid row would still spend the container's gap on it. The Launch Page select is derived from `VIEW_META` instead of listing its ten options, since removing a destination is exactly the change that leaves two hand-written copies disagreeing. --- .../components/autotag-view/autotag-view.ts | 30 ++- .../src/components/config-page/config-page.ts | 219 ++++++++++++++++-- .../config-page/download-clients.ts | 12 + .../download-picker/download-picker.ts | 2 +- 4 files changed, 243 insertions(+), 20 deletions(-) diff --git a/frontend/src/components/autotag-view/autotag-view.ts b/frontend/src/components/autotag-view/autotag-view.ts index 6d4f6e6..56e2fc4 100644 --- a/frontend/src/components/autotag-view/autotag-view.ts +++ b/frontend/src/components/autotag-view/autotag-view.ts @@ -29,6 +29,7 @@ import { nameDialogsIn } from '../../utils/name-dialog'; import { ViewLifecycleMixin } from '../../utils/view-lifecycle'; import { confirmAction } from '../confirm-dialog/confirm-dialog'; import '@awesome.me/webawesome/dist/components/dialog/dialog.js'; +import '@components/jobs/job-panel'; import { list } from '@utils/binding'; type PendingItem = autotagservice.PendingItem; @@ -135,8 +136,20 @@ export class AutotagView extends ViewLifecycleMixin(LitElement) { padding: 0.75rem 1rem; } - .header { + /* The header and the apply-job panel share the header row. + A wrapper rather than a third grid row, because the panel + is display:none while nothing is applying and a grid + row would still spend the container's gap on it -- the + idle case, which is nearly always. */ + .header-area { grid-area: header; + display: flex; + flex-direction: column; + gap: 0.5rem; + min-width: 0; + } + + .header { display: flex; align-items: center; gap: 0.75rem; @@ -3207,7 +3220,20 @@ export class AutotagView extends ViewLifecycleMixin(LitElement) { // sees a blank full-screen "Loading\u2026". return html`
- ${this.renderHeader()} +
+ ${this.renderHeader()} + + +
${this.renderFolderSidebar()} ${this.renderMain()}
diff --git a/frontend/src/components/config-page/config-page.ts b/frontend/src/components/config-page/config-page.ts index 70078fb..9d9236b 100644 --- a/frontend/src/components/config-page/config-page.ts +++ b/frontend/src/components/config-page/config-page.ts @@ -9,7 +9,13 @@ import { RemoveLibrary, GetRemovalImpact, GetAllLibrariesWithTrackCounts, + ScanLibrary, + ScanAllLibraries, + FullRescan, } from '@go/library/library.js'; +import { jobStore } from '@store/job-store'; +import type { Job } from '@store/job-store'; +import '@components/jobs/job-panel'; import { GetScanConcurrency, SetScanConcurrency, @@ -77,6 +83,24 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) { /** Which destinations the navigation offers (#25). */ private viewsCtrl = new ViewVisibilityController(this); + /** + * The job snapshot, for the per-library scan status (#27). + * + * Held as state rather than read from the store in `render()` so + * Lit sees the dependency: the store notifies, and a getter read + * inside a template is not a reactive input. + */ + @state() private jobs: Job[] = []; + + /** + * Set between pressing a scan button and the job snapshot that + * proves it started -- `JobsChanged` is coalesced at 250 ms, which + * is long enough for a second click to start a second scan. + */ + @state() private startingScan = false; + + private unsubscribeJobs: (() => void) | null = null; + // --- Shortcuts controller --- private shortcutsCtrl = new ShortcutsController(this); @@ -864,8 +888,14 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) { this.scrollMode = localStorage.getItem(SCROLL_STORAGE_KEY) || 'hover'; - // Scan progress lives in the jobs panel now — this page only - // needs to know when the library list itself changes. + // Scanning is started and watched here (#27), so the job + // snapshot is a live input to this page. + this.unsubscribeJobs = jobStore.subscribe(() => { + this.jobs = jobStore.jobs; + }); + this.jobs = jobStore.jobs; + void jobStore.init(); + this.cancelLibraryAdded = EventsOn( Events.LibraryAdded, () => void this.loadLibraries(), @@ -896,6 +926,9 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) { } protected override onViewDeactivate(): void { + this.unsubscribeJobs?.(); + this.unsubscribeJobs = null; + this.cancelLibraryAdded?.(); this.cancelLibraryRenamed?.(); this.cancelLibraryRemoved?.(); @@ -1096,6 +1129,114 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) { } } + // =================================================================== + // SCANNING (#27 — back from the Jobs tab) + // =================================================================== + + /** The scan job for a library, if one is registered. */ + private jobForLibrary(id: number): Job | undefined { + return this.jobs.find((job) => job.id === `scan:${id}`); + } + + /** The status line under a library name while it is being scanned. */ + private libraryScanStatus(id: number): string | null { + const job = this.jobForLibrary(id); + + if (!job) return null; + + switch (job.state) { + case 'running': + return job.phase ? `Scanning · ${job.phase}` : 'Scanning'; + case 'queued': + return 'Queued'; + case 'paused': + return 'Paused'; + case 'pausing': + return 'Pausing…'; + case 'cancelling': + return 'Stopping…'; + default: + return null; + } + } + + private get anyScanning(): boolean { + return this.libraries.some( + (lib) => this.libraryScanStatus(lib.id) !== null, + ); + } + + /** + * Run something that starts a job, holding the buttons until the + * snapshot lands and saying so when it does not start at all. + * + * Persistent, not a toast: the user asked for work to happen, it + * did not, and retrying is exactly the useful response. + */ + private async startJob( + what: string, + start: () => Promise, + retry: () => void, + ): Promise { + if (this.startingScan) return; + + this.startingScan = true; + + try { + await start(); + } catch (err) { + console.error(`${what} failed:`, err); + notificationStore.persistent({ + key: 'scan-start', + title: 'Scan did not start', + text: `${what} failed. ${describeError(err)}`, + detail: String(err), + action: { label: 'Try again', run: retry }, + }); + } finally { + this.startingScan = false; + } + } + + private handleScanLibrary = (id: number): void => { + this.activeMenuId = null; + void this.startJob( + 'Scanning that library', + () => ScanLibrary(id), + () => this.handleScanLibrary(id), + ); + }; + + private handleScanAll = (): void => { + void this.startJob( + 'Scanning your libraries', + () => ScanAllLibraries(), + () => this.handleScanAll(), + ); + }; + + private handleFullRescan = async (): Promise => { + const ok = await confirmAction({ + title: 'Full rescan', + message: + 'This deletes all library data — including downloaded ' + + 'cover art — and rebuilds it from your files.', + impact: + 'It is not the same as “Scan now”, which only picks up ' + + 'what changed.', + confirmLabel: 'Rebuild everything', + danger: true, + }); + + if (!ok) return; + + await this.startJob( + 'The full rescan', + () => FullRescan(), + () => void this.handleFullRescan(), + ); + }; + private handleViewToggle = ( view: string, visible: boolean, @@ -1554,6 +1695,20 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) { .value=${this.allowMeteredCatalogDownload} @config-change=${this.handleAllowMeteredChange} > + + + `; } @@ -1674,18 +1829,15 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) { description: 'The page the app opens to on launch.', type: 'select' as const, - options: [ - { value: 'home', label: 'Home' }, - { value: 'tracks', label: 'Tracks' }, - { value: 'albums', label: 'Albums' }, - { value: 'artists', label: 'Artists' }, - { value: 'genres', label: 'Genres' }, - { value: 'playlists', label: 'Playlists' }, - { value: 'explore', label: 'Explore' }, - { value: 'downloads', label: 'Downloads' }, - { value: 'autotag', label: 'Autotag' }, - { value: 'jobs', label: 'Jobs' }, - ], + // Derived, not written out: this list was a + // second copy of the launchable set, and #27 + // removing a destination is exactly the change + // that would have left the two disagreeing. + // Settings is excluded because it is the one + // view the backend refuses to launch into. + options: VIEW_META + .filter((v) => v.alwaysShown !== true) + .map((v) => ({ value: v.id, label: v.label })), }} .value=${this.defaultPage} @config-change=${this.handleDefaultPageChange} @@ -2161,8 +2313,8 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) { return html`
@@ -2172,6 +2324,23 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) { > Add Library + +
${this.libraries.length > 0 @@ -2209,7 +2378,8 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) { ${this.removingLibraryId === lib.id ? 'Removing…' - : html`${lib.trackCount} tracks`} + : this.libraryScanStatus(lib.id) + ?? html`${lib.trackCount} tracks`}
+ ${this.libraryScanStatus(lib.id) === null + ? html` +
this.handleScanLibrary(lib.id)} + > + Scan now +
+ ` + : nothing}
void this.handleRemoveClick(lib.id)} @@ -2275,6 +2455,11 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) { @config-change=${this.handleConcurrencyChange} > + + `; } diff --git a/frontend/src/components/config-page/download-clients.ts b/frontend/src/components/config-page/download-clients.ts index 0fb7052..6aef7a3 100644 --- a/frontend/src/components/config-page/download-clients.ts +++ b/frontend/src/components/config-page/download-clients.ts @@ -22,6 +22,7 @@ import { compact } from '@utils/binding'; import { describeError, explainError } from '@utils/describe-error'; import { confirmAction } from '@components/confirm-dialog/confirm-dialog'; import './config-section'; +import '@components/jobs/job-panel'; import { pickDirectory } from '../../utils/pick-directory'; /** @@ -274,6 +275,17 @@ export class DownloadClients extends LitElement {
`} + + + Found a clear match and started downloading it. Progress is - in the background jobs panel. + on the Downloads page. `; } -- 2.54.0 From 8efed2dd2bbf887395549e537037a7a82ea2dd79 Mon Sep 17 00:00:00 2001 From: Logan Date: Wed, 19 Aug 2026 20:27:40 -0400 Subject: [PATCH 3/7] refactor(shell): retire the Jobs destination Nothing it carried is gone -- the two commits before this put all of it somewhere the work is already being done. A retired destination is the one shape #25's storage decision does not make free. A visibility entry is a map key and an unknown key is dropped on load; a launch page is a *value*, and an unknown one fails validation -- which on the load path means the app refuses to start for whoever had Jobs selected. `RetiredViews` is that list, read by `ApplyDefaults`, which treats a retired name as a zero value. An unknown-but-not-retired name still errors, because that is a typo and saying so is the useful answer. --- backend/config/general.go | 10 + backend/config/views.go | 18 +- backend/config/views_test.go | 64 ++- frontend/index.ts | 2 - frontend/src/components/jobs/jobs-view.ts | 568 ---------------------- frontend/src/services/view-meta.ts | 2 - 6 files changed, 84 insertions(+), 580 deletions(-) delete mode 100644 frontend/src/components/jobs/jobs-view.ts diff --git a/backend/config/general.go b/backend/config/general.go index 750ec65..5e6609e 100644 --- a/backend/config/general.go +++ b/backend/config/general.go @@ -57,7 +57,17 @@ type GeneralConfig struct { } // ApplyDefaults fills zero-value fields with sensible defaults. +// +// A launch page naming a *retired* view is treated as a zero value +// rather than as an error, because the alternative is an app that will +// not start for anyone who had that page selected when it was removed. +// An unknown-but-not-retired name still fails Validate: that is a typo, +// and telling someone about it is the useful answer. func (c *GeneralConfig) ApplyDefaults() { + if _, retired := RetiredViews[c.DefaultPage]; retired { + c.DefaultPage = "" + } + if c.DefaultPage == "" { c.DefaultPage = DefaultDefaultPage } diff --git a/backend/config/views.go b/backend/config/views.go index 87eb47b..fdafd37 100644 --- a/backend/config/views.go +++ b/backend/config/views.go @@ -24,10 +24,25 @@ const ( ViewExplore View = "explore" ViewDownloads View = "downloads" ViewAutotag View = "autotag" - ViewJobs View = "jobs" ViewSettings View = "settings" ) +// RetiredViews are destinations that used to exist and no longer do. +// +// A *visibility* entry for a removed view needs no such list: it is a +// key in a map, and an unknown key is dropped on load. A `DefaultPage` +// is a **value**, and an unknown one fails validation -- which on the +// load path means the app refuses to start rather than a setting being +// ignored. So the one shape that cannot be retired for free is named +// here and reset to the default instead. +// +// `jobs` was folded into Settings by #27: library scans under +// Libraries, index work under Search Index, downloads under the +// download clients, and the autotag apply into the Autotag view. +var RetiredViews = map[View]struct{}{ + "jobs": {}, +} + // ViewSpec is what the backend knows about a destination. The label and // the icon are deliberately absent: those are presentation, they live // beside the rest of the app's icon vocabulary in @@ -73,7 +88,6 @@ var Views = []ViewSpec{ {ID: ViewExplore, VisibleByDefault: true, Hideable: true, CanLaunch: true}, {ID: ViewDownloads, VisibleByDefault: true, Hideable: true, CanLaunch: true}, {ID: ViewAutotag, VisibleByDefault: false, Hideable: true, CanLaunch: true}, - {ID: ViewJobs, VisibleByDefault: true, Hideable: true, CanLaunch: true}, {ID: ViewSettings, VisibleByDefault: true, Hideable: false, CanLaunch: false}, } diff --git a/backend/config/views_test.go b/backend/config/views_test.go index f14b14f..5d3916f 100644 --- a/backend/config/views_test.go +++ b/backend/config/views_test.go @@ -66,7 +66,7 @@ func TestViewVisibilityStoredWins(t *testing.T) { general := &GeneralConfig{ ViewVisibility: map[string]bool{ string(ViewAutotag): true, - string(ViewJobs): false, + string(ViewExplore): false, }, } general.ApplyDefaults() @@ -77,8 +77,8 @@ func TestViewVisibilityStoredWins(t *testing.T) { t.Error("autotag was switched on and should be visible") } - if resolved[string(ViewJobs)] { - t.Error("jobs was switched off and should be hidden") + if resolved[string(ViewExplore)] { + t.Error("explore was switched off and should be hidden") } } @@ -136,6 +136,58 @@ func TestValidateRevealsAHiddenLaunchPage(t *testing.T) { } } +// A launch page naming a view that no longer exists resets to the +// default instead of failing validation, which on the load path would +// mean the app refusing to start for whoever had it selected. +// +// This is the one shape #25's storage decision does *not* make free: a +// visibility entry is a key and an unknown key is dropped, but a launch +// page is a value. +func TestARetiredLaunchPageFallsBackToTheDefault(t *testing.T) { + t.Parallel() + + general := &GeneralConfig{DefaultPage: "jobs"} + + if err := general.Validate(); err != nil { + t.Fatalf("Validate() error: %v", err) + } + + if general.DefaultPage != DefaultDefaultPage { + t.Errorf("DefaultPage = %q, want %q", general.DefaultPage, DefaultDefaultPage) + } +} + +// A name that is merely wrong is still an error: that is a typo, and +// saying so is more useful than ignoring it. +func TestAnUnknownLaunchPageIsStillAnError(t *testing.T) { + t.Parallel() + + general := &GeneralConfig{DefaultPage: "nonsense"} + + if err := general.Validate(); !errors.Is(err, errUnknownDefaultPage) { + t.Fatalf("Validate() error = %v, want errUnknownDefaultPage", err) + } +} + +// A retired view is not a view, so nothing offers it and nothing +// resolves it -- the visibility map included. +func TestARetiredViewIsGone(t *testing.T) { + t.Parallel() + + for id := range RetiredViews { + if _, ok := LookupView(string(id)); ok { + t.Errorf("%s is retired but still in Views", id) + } + + general := &GeneralConfig{} + general.ApplyDefaults() + + if _, ok := general.ResolvedViewVisibility()[string(id)]; ok { + t.Errorf("%s is retired but still resolves a visibility", id) + } + } +} + // Settings may not be the launch page, which is the shape the old // DefaultPage enum had and is now read off the same table. func TestValidateRejectsAnUnlaunchablePage(t *testing.T) { @@ -213,7 +265,7 @@ func TestViewVisibilityRoundTrips(t *testing.T) { t.Fatalf("SetViewVisible() error: %v", err) } - if err := original.SetViewVisible(string(ViewJobs), false); err != nil { + if err := original.SetViewVisible(string(ViewExplore), false); err != nil { t.Fatalf("SetViewVisible() error: %v", err) } @@ -228,8 +280,8 @@ func TestViewVisibilityRoundTrips(t *testing.T) { t.Error("autotag should have loaded as visible") } - if resolved[string(ViewJobs)] { - t.Error("jobs should have loaded as hidden") + if resolved[string(ViewExplore)] { + t.Error("explore should have loaded as hidden") } } diff --git a/frontend/index.ts b/frontend/index.ts index 7b51668..3ebfa2b 100644 --- a/frontend/index.ts +++ b/frontend/index.ts @@ -101,7 +101,6 @@ const VIEW_TAGS: Record = { explore: 'explore-view', autotag: 'autotag-view', downloads: 'downloads-view', - jobs: 'jobs-view', settings: 'config-page', }; @@ -119,7 +118,6 @@ const VIEW_LOADERS: Record Promise> = { explore: () => import('@components/explore-view/explore-view.ts'), autotag: () => import('@components/autotag-view/autotag-view.ts'), downloads: () => import('@components/downloads-view/downloads-view.ts'), - jobs: () => import('@components/jobs/jobs-view.ts'), settings: () => import('@components/config-page/config-page.ts'), }; diff --git a/frontend/src/components/jobs/jobs-view.ts b/frontend/src/components/jobs/jobs-view.ts deleted file mode 100644 index cdd1261..0000000 --- a/frontend/src/components/jobs/jobs-view.ts +++ /dev/null @@ -1,568 +0,0 @@ -import { LitElement, html, css, nothing } from 'lit'; -import { customElement, state } from 'lit/decorators.js'; -import '@awesome.me/webawesome/dist/components/icon/icon.js'; -import '@components/page-header/page-header'; -import { designTokens } from '../../styles/tokens.css'; -import { - GetAllLibrariesWithTrackCounts, - ScanLibrary, - ScanAllLibraries, - FullRescan, -} from '@go/library/library.js'; -import type * as library from '@go/library/models.js'; -import { EventsOn } from '@runtime/runtime'; -import { Events } from '../../events'; -import { jobStore } from '@store/job-store'; -import type { Job } from '@store/job-store'; -import { notificationStore } from '@store/notification-store'; -import { describeError } from '@utils/describe-error'; -import { confirmAction } from '../confirm-dialog/confirm-dialog'; -import './job-row'; -import './job-details-drawer'; -import { applyJobControl } from './job-controls'; -import { jobStateStyles } from './job-format'; -import { ViewLifecycleMixin } from '../../utils/view-lifecycle'; - -type LibraryInfo = library.Info; - -/** Job states meaning the job will not progress further. */ -const TERMINAL_STATES: ReadonlySet = new Set([ - 'complete', - 'cancelled', - 'error', -]); - -/** - * Full-page view of background work: everything running right now, the - * per-library scan controls that used to live in Settings, and a short - * history of what recently finished. - * - * This is the same job rows as the top-bar popover at a larger density — - * one implementation, two placements, so the two can never disagree. - */ -@customElement('jobs-view') -export class JobsView extends ViewLifecycleMixin(LitElement) { - @state() - private jobs: Job[] = []; - - @state() - private libraries: LibraryInfo[] = []; - - @state() - private drawerJobId = ''; - - @state() - private drawerOpen = false; - - /** - * Set between pressing a scan button and the job snapshot that - * proves it started. `anyScanning` is derived from `JobsChanged`, - * which is coalesced at 250 ms — long enough for a second click to - * start a second scan (errors.M5). - */ - @state() - private starting = false; - - private unsubscribe: (() => void) | null = null; - - private eventCleanups: Array<() => void> = []; - - static override styles = [ - designTokens, - jobStateStyles, - css` - :host { - display: block; - overflow-y: auto; - height: 100%; - padding: 1.5em 1.75em 3em; - box-sizing: border-box; - } - - /* The header supplies its own padding and rule, so it runs - to the edge of a host that pads its own content. */ - page-header { - margin: -1.5em -1.75em 1em; - } - - h1 { - font-size: var(--yj-text-xl); - color: var(--yj-text-primary, #e9ecef); - margin: 0 0 0.2em; - } - - .page-sub { - font-size: var(--yj-text-md); - color: var(--yj-text-tertiary, #868e96); - margin: 0 0 1.75em; - } - - section { - margin-bottom: 2em; - } - - .section-head { - display: flex; - align-items: center; - justify-content: space-between; - gap: 1em; - margin-bottom: 0.75em; - } - - h2 { - font-size: var(--yj-text-sm); - text-transform: uppercase; - letter-spacing: 0.06em; - color: var(--yj-text-tertiary, #868e96); - margin: 0; - } - - .card { - background: rgba(255, 255, 255, 0.03); - border: 1px solid rgba(255, 255, 255, 0.07); - border-radius: 10px; - overflow: hidden; - } - - .card > * + * { - border-top: 1px solid rgba(255, 255, 255, 0.06); - } - - .job-entry { - display: grid; - grid-template-columns: 1fr auto; - align-items: center; - gap: 0.5em; - padding-right: 0.75em; - } - - .empty { - padding: 1.1em; - font-size: var(--yj-text-md); - color: var(--yj-text-tertiary, #868e96); - font-style: italic; - } - - .library-row { - display: grid; - grid-template-columns: 1fr auto; - align-items: center; - gap: 1em; - padding: 0.75em 0.9em; - } - - .library-name { - font-size: var(--yj-text-md); - color: var(--yj-text-primary, #e9ecef); - } - - .library-meta { - font-size: var(--yj-text-sm); - color: var(--yj-text-tertiary, #868e96); - margin-top: 0.15em; - overflow-wrap: anywhere; - } - - .library-state { - font-size: var(--yj-text-sm); - color: var(--job-tone); - margin-top: 0.15em; - } - - button.action { - display: inline-flex; - align-items: center; - gap: 0.45em; - border: 1px solid rgba(255, 255, 255, 0.14); - border-radius: 7px; - background: rgba(255, 255, 255, 0.05); - color: var(--yj-text-primary, #e9ecef); - font-size: var(--yj-text-sm); - padding: 0.42em 0.85em; - cursor: pointer; - white-space: nowrap; - transition: - background-color 120ms ease, - border-color 120ms ease; - } - - button.action:hover:not(:disabled) { - background: rgba(255, 255, 255, 0.11); - } - - button.action:disabled { - opacity: 0.4; - cursor: default; - } - - button.action:focus-visible { - outline: 2px solid var(--yj-accent, #ffd43b); - outline-offset: 2px; - } - - button.action.danger { - color: var(--yj-error-text, #ff8787); - border-color: rgba(255, 107, 107, 0.35); - } - - button.action.danger:hover:not(:disabled) { - background: rgba(255, 107, 107, 0.12); - } - - button.link { - border: none; - background: transparent; - color: var(--yj-accent-text, #ffd43b); - font-size: var(--yj-text-sm); - cursor: pointer; - padding: 0.2em 0.4em; - border-radius: 5px; - } - - button.link:hover { - text-decoration: underline; - } - - .details-btn { - border: none; - background: transparent; - color: var(--yj-text-secondary, #adb5bd); - font-size: var(--yj-text-sm); - cursor: pointer; - padding: 0.3em 0.5em; - border-radius: 6px; - white-space: nowrap; - } - - .details-btn:hover { - background: rgba(255, 255, 255, 0.1); - color: var(--yj-text-primary, #e9ecef); - } - `, - ]; - - protected override onViewActivate(): void { - this.unsubscribe = jobStore.subscribe(() => { - this.jobs = jobStore.jobs; - }); - void jobStore.init(); - this.jobs = jobStore.jobs; - void this.loadLibraries(); - - // Library CRUD happens elsewhere; keep the picker in step. - for (const event of [ - Events.LibraryAdded, - Events.LibraryRemoved, - Events.LibraryRenamed, - Events.LibraryScanComplete, - ]) { - this.eventCleanups.push( - EventsOn(event, () => void this.loadLibraries()), - ); - } - } - - protected override onViewDeactivate(): void { - this.unsubscribe?.(); - this.unsubscribe = null; - this.eventCleanups.forEach((off) => off()); - this.eventCleanups = []; - } - - private async loadLibraries(): Promise { - try { - this.libraries = (await GetAllLibrariesWithTrackCounts()) ?? []; - } catch (err) { - console.error('Failed to load libraries:', err); - } - } - - /** The scan job for a library, if one is registered. */ - private jobForLibrary(id: number): Job | undefined { - return jobStore.getJob(`scan:${id}`); - } - - private openDetails(id: string) { - this.drawerJobId = id; - this.drawerOpen = true; - } - - private onDrawerClosed = () => { - this.drawerOpen = false; - }; - - /** - * Run something that starts a job, holding the buttons until the - * snapshot lands and saying so when it does not start at all. - * - * Persistent, not a toast: the user asked for work to happen, it - * did not, and retrying is exactly the useful response. - */ - private async startJob( - what: string, - start: () => Promise, - retry: () => void, - ): Promise { - if (this.starting) return; - - this.starting = true; - - try { - await start(); - } catch (err) { - console.error(`${what} failed:`, err); - notificationStore.persistent({ - key: 'scan-start', - title: 'Scan did not start', - text: `${what} failed. ${describeError(err)}`, - detail: String(err), - action: { label: 'Try again', run: retry }, - }); - } finally { - this.starting = false; - } - } - - private async startScan(id: number) { - await this.startJob( - 'Scanning that library', - () => ScanLibrary(id), - () => void this.startScan(id), - ); - } - - private async startAllScans() { - await this.startJob( - 'Scanning your libraries', - () => ScanAllLibraries(), - () => void this.startAllScans(), - ); - } - - private async clearFinished() { - await jobStore.clearFinished(); - } - - private async fullRescan() { - const ok = await confirmAction({ - title: 'Full rescan', - message: - 'This deletes all library data — including downloaded ' + - 'cover art — and rebuilds it from your files.', - impact: - 'It is not the same as “Scan now”, which only picks up ' + - 'what changed.', - confirmLabel: 'Rebuild everything', - danger: true, - }); - - if (!ok) return; - - await this.startJob( - 'The full rescan', - () => FullRescan(), - () => void this.fullRescan(), - ); - } - - private renderJobList(list: Job[], emptyText: string) { - if (list.length === 0) { - return html`
-
${emptyText}
-
`; - } - - return html` -
- ${list.map( - (job) => html` -
- - -
- `, - )} -
- `; - } - - /** The status line under a library name in the scan-control list. */ - private libraryStatus(job: Job | undefined): string | null { - if (!job) return null; - - switch (job.state) { - case 'running': - return job.phase ? `Scanning · ${job.phase}` : 'Scanning'; - case 'queued': - return 'Queued'; - case 'paused': - return 'Paused'; - case 'pausing': - return 'Pausing…'; - case 'cancelling': - return 'Stopping…'; - default: - return null; - } - } - - private renderLibraryRow(lib: LibraryInfo) { - const job = this.jobForLibrary(lib.id); - const status = this.libraryStatus(job); - const busy = status !== null; - - return html` -
-
-
${lib.name}
-
- ${lib.trackCount.toLocaleString()} tracks · ${lib.path} -
- ${status - ? html`
${status}
` - : nothing} -
- - ${busy - ? html` - - ` - : html` - - `} -
- `; - } - - override render() { - // Derived from `this.jobs` rather than the store getters so Lit - // sees the reactive dependency and re-renders on every snapshot. - const active = this.jobs.filter((j) => !TERMINAL_STATES.has(j.state)); - const finished = this.jobs.filter((j) => TERMINAL_STATES.has(j.state)); - const anyScanning = this.libraries.some((lib) => - Boolean(this.libraryStatus(this.jobForLibrary(lib.id))), - ); - - return html` - -

- Library scans and search index builds, with their progress and - output. -

- -
-
-

Running now

-
- ${this.renderJobList(active, 'Nothing is running.')} -
- -
-
-

Libraries

- -
- -
- ${this.libraries.length === 0 - ? html`
- No libraries yet — add one in Settings. -
` - : this.libraries.map((lib) => - this.renderLibraryRow(lib), - )} -
-
- -
-
-

Maintenance

-
-
-
-
-
Full rescan
-
- Wipes all library data and cover art, then - rebuilds from your files. Only needed when the - library is corrupt — a normal scan already - picks up changes. -
-
- -
-
-
- - ${finished.length > 0 - ? html` -
-
-

Recently finished

- -
- ${this.renderJobList(finished, '')} -
- ` - : nothing} - - - `; - } -} - -declare global { - interface HTMLElementTagNameMap { - 'jobs-view': JobsView; - } -} diff --git a/frontend/src/services/view-meta.ts b/frontend/src/services/view-meta.ts index 871bab1..e51bbea 100644 --- a/frontend/src/services/view-meta.ts +++ b/frontend/src/services/view-meta.ts @@ -15,7 +15,6 @@ export type View = | 'explore' | 'downloads' | 'autotag' - | 'jobs' | 'settings'; export interface ViewMeta { @@ -61,6 +60,5 @@ export const VIEW_META: ViewMeta[] = [ { id: 'explore', label: 'Explore', icon: 'globe' }, { id: 'downloads', label: 'Downloads', icon: ICON_REQUESTED }, { id: 'autotag', label: 'Autotag', icon: ICON_AUTOTAG }, - { id: 'jobs', label: 'Jobs', icon: 'list-check' }, { id: 'settings', label: 'Settings', icon: 'gear', alwaysShown: true }, ]; -- 2.54.0 From c79d4d47a3b67b3e0631a0b577f3e1bf730338d7 Mon Sep 17 00:00:00 2001 From: Logan Date: Wed, 19 Aug 2026 20:27:48 -0400 Subject: [PATCH 4/7] test: cover jobs in Settings, and unpick two shared selectors The spec worth having is not that the tab is gone -- that is one line of a table -- but that nothing became unreachable when it went. #24 promises that no action is ever unreachable at any supported size, and deleting a destination is exactly the change that quietly breaks it. Two existing selectors had to give. `config-section .header` is ambiguous the moment a section holds a job, because `job-details-drawer` carries that class too -- so `settings-reach.spec.ts` locates a disclosure by role and name instead. And `page-header`'s and `offline-icons`'s view lists lose an entry each. Closes #27 --- e2e/specs/jobs-in-settings.spec.ts | 124 ++++++++++++++++++ e2e/specs/layout-overflow.spec.ts | 7 +- e2e/specs/offline-icons.spec.ts | 2 +- e2e/specs/page-header.spec.ts | 2 - e2e/specs/settings-reach.spec.ts | 11 +- frontend/test/components/chrome.test.ts | 1 - .../test/components/keyboard-reach.test.ts | 2 +- frontend/test/components/smoke.test.ts | 4 +- .../test/components/view-lifecycle.test.ts | 2 - .../test/components/view-visibility.test.ts | 6 +- 10 files changed, 141 insertions(+), 20 deletions(-) create mode 100644 e2e/specs/jobs-in-settings.spec.ts diff --git a/e2e/specs/jobs-in-settings.spec.ts b/e2e/specs/jobs-in-settings.spec.ts new file mode 100644 index 0000000..dafd552 --- /dev/null +++ b/e2e/specs/jobs-in-settings.spec.ts @@ -0,0 +1,124 @@ +import { test, expect, waitForEvent } from '../support/fixtures.js'; + +/** + * The Jobs tab folded into the places the work is started (#27). + * + * The assertion worth making is not that the tab is gone — that is one + * line of a table — but that **nothing became unreachable when it + * went**. Scanning is the case that mattered: the per-library controls + * lived only on that page, and the tab's own comment says they had been + * moved there out of Settings in the first place. + * + * `#24` wrote down one sentence covering all three size bands: *no + * action is ever unreachable at any supported size*. Deleting a + * destination is exactly the change that can quietly break it. + */ +type Page = import('@playwright/test').Page; + +const section = (page: Page, heading: string) => + page.locator(`config-page config-section[heading="${heading}"]`); + +async function openSettings(page: Page, heading: string): Promise { + await page.getByTestId('nav-settings').click(); + + // The section's own disclosure, by role rather than by `.header`: + // an open Libraries section also contains `job-details-drawer`, + // whose own header matches that class and makes it ambiguous. + const header = section(page, heading) + .getByRole('button', { name: heading }) + .first(); + + await expect(header).toBeVisible(); + + if ((await header.getAttribute('aria-expanded')) === 'false') { + await header.click(); + } + + await expect(header).toHaveAttribute('aria-expanded', 'true'); +} + +test.describe('background jobs live where the work is started', () => { + test('the Jobs destination is gone', async ({ app }) => { + await expect(app.getByTestId('nav-jobs')).toHaveCount(0); + + // And it is not offered as a launch page either, which is the copy + // of the destination list that is easiest to forget. + await openSettings(app, 'General'); + + const options = await section(app, 'General') + .locator('select') + .first() + .locator('option') + .allTextContents(); + + expect(options).not.toContain('Jobs'); + }); + + /** + * Scanning is startable from Settings → Libraries, and the job that + * results is visible there with its controls. One assertion covers + * both halves, because a Scan All that started nothing would leave + * the panel empty and read exactly like a panel that does not work. + */ + test('a scan is started and watched in Settings', async ({ app }) => { + await openSettings(app, 'Libraries'); + + const libraries = section(app, 'Libraries'); + + await libraries.getByRole('button', { name: 'Scan All' }).click(); + + await waitForEvent(app, 'LibraryScanComplete', { timeoutMs: 60_000 }); + + const panel = libraries.locator('job-panel'); + + await expect(panel.locator('job-row')).toHaveCount(1, { timeout: 10_000 }); + + // The generic affordances are the point of the panel: the tier + // list and the progress rings the other surfaces already had + // cannot open a log. + await expect( + panel.getByRole('button', { name: /^Details/ }), + ).toBeVisible(); + }); + + /** A finished job dismisses from where it is shown. */ + test('a finished scan can be dismissed in place', async ({ app }) => { + await openSettings(app, 'Libraries'); + + const panel = section(app, 'Libraries').locator('job-panel'); + const dismiss = panel.getByRole('button', { name: /^Dismiss/ }).first(); + + await expect(dismiss).toBeVisible({ timeout: 10_000 }); + await dismiss.click(); + + await expect(panel.locator('job-row')).toHaveCount(0); + }); + + /** + * Full rescan is destructive and asks first. It is asserted at the + * dialog rather than through it — running one against the seeded app + * would delete the library the rest of the suite reads. + */ + test('Full Rescan asks before it does anything', async ({ app }) => { + await openSettings(app, 'Libraries'); + + await section(app, 'Libraries') + .getByRole('button', { name: 'Full Rescan' }) + .click(); + + const dialog = app.getByRole('dialog', { name: 'Full rescan' }); + + await expect(dialog).toBeVisible(); + + // The message is read off the *host*, not the dialog: a wa-dialog + // keeps its slotted content in the host's shadow root, so + // `toContainText` on the dialog itself sees only Web Awesome's + // chrome. + await expect(app.locator('confirm-dialog')).toContainText( + 'deletes all library data', + ); + + await app.getByRole('button', { name: 'Cancel' }).click(); + await expect(dialog).toBeHidden(); + }); +}); diff --git a/e2e/specs/layout-overflow.spec.ts b/e2e/specs/layout-overflow.spec.ts index da0a296..c5b6401 100644 --- a/e2e/specs/layout-overflow.spec.ts +++ b/e2e/specs/layout-overflow.spec.ts @@ -93,9 +93,10 @@ test.describe('the app fits in its own window', () => { ) .toBe(true); - // Settings and Jobs are the two that were unreachable: they are - // last in the nav, and the pane used to clip rather than scroll. - for (const view of ['jobs', 'settings'] as const) { + // Settings is the one that was unreachable: it is last in the nav, + // and the pane used to clip rather than scroll. (Jobs was the other + // half of this until #27 folded it into Settings.) + for (const view of ['explore', 'settings'] as const) { const item = app.getByTestId(`nav-${view}`); await item.scrollIntoViewIfNeeded(); diff --git a/e2e/specs/offline-icons.spec.ts b/e2e/specs/offline-icons.spec.ts index cf882ff..be81f2f 100644 --- a/e2e/specs/offline-icons.spec.ts +++ b/e2e/specs/offline-icons.spec.ts @@ -28,7 +28,7 @@ const EXPECTED_MIN_ICONS = 5; const VIEWS = [ 'home', 'tracks', 'albums', 'artists', 'genres', 'playlists', - 'explore', 'downloads', 'jobs', 'settings', + 'explore', 'downloads', 'autotag', 'settings', ]; type IconState = { name: string; hasSvg: boolean }; diff --git a/e2e/specs/page-header.spec.ts b/e2e/specs/page-header.spec.ts index 71ce42a..42de153 100644 --- a/e2e/specs/page-header.spec.ts +++ b/e2e/specs/page-header.spec.ts @@ -21,7 +21,6 @@ const VIEWS: [string, string, boolean][] = [ ['tracks', 'Tracks', true], ['explore', 'Explore', false], ['downloads', 'Downloads', false], - ['jobs', 'Background jobs', false], ]; /** The header lives in the view's shadow root, inside its own. */ @@ -53,7 +52,6 @@ const TAGS: Record = { tracks: 'track-list', explore: 'explore-view', downloads: 'downloads-view', - jobs: 'jobs-view', }; test.describe('every primary view says what it is', () => { diff --git a/e2e/specs/settings-reach.spec.ts b/e2e/specs/settings-reach.spec.ts index 1ddddea..37d7700 100644 --- a/e2e/specs/settings-reach.spec.ts +++ b/e2e/specs/settings-reach.spec.ts @@ -18,16 +18,19 @@ test.describe('Settings is reachable without a mouse', () => { }) => { await app.getByTestId('nav-settings').click(); - const headers = app.locator('config-page config-section .header'); + // Per *section*, not per `.header`: a section holding a + // `job-panel` (#27) also contains `job-details-drawer`, whose own + // header carries that class and is not a disclosure. + const sections = app.locator('config-page config-section'); - await expect(headers.first()).toBeVisible(); + await expect(sections.first()).toBeVisible(); - const count = await headers.count(); + const count = await sections.count(); expect(count).toBeGreaterThan(4); for (let i = 0; i < count; i++) { - const header = headers.nth(i); + const header = sections.nth(i).locator('.header').first(); expect(await header.evaluate((el) => el.tagName)).toBe('BUTTON'); expect(['true', 'false']).toContain( diff --git a/frontend/test/components/chrome.test.ts b/frontend/test/components/chrome.test.ts index 971793e..5cb7290 100644 --- a/frontend/test/components/chrome.test.ts +++ b/frontend/test/components/chrome.test.ts @@ -63,7 +63,6 @@ describe('', () => { 'nav-explore', 'nav-downloads', 'nav-autotag', - 'nav-jobs', 'nav-settings', ]); }); diff --git a/frontend/test/components/keyboard-reach.test.ts b/frontend/test/components/keyboard-reach.test.ts index c89177e..1aabdd0 100644 --- a/frontend/test/components/keyboard-reach.test.ts +++ b/frontend/test/components/keyboard-reach.test.ts @@ -49,7 +49,7 @@ describe(' is reachable', () => { const items = shadowAll(el, 'li button'); - expect(items).toHaveLength(11); + expect(items).toHaveLength(10); expect(items.every((item) => item.tagName === 'BUTTON')).toBe(true); }); diff --git a/frontend/test/components/smoke.test.ts b/frontend/test/components/smoke.test.ts index c9125b7..e4c3f16 100644 --- a/frontend/test/components/smoke.test.ts +++ b/frontend/test/components/smoke.test.ts @@ -40,7 +40,7 @@ import '@components/jobs/job-details-drawer'; import '@components/jobs/job-indicator'; import '@components/jobs/job-log-view'; import '@components/jobs/job-row'; -import '@components/jobs/jobs-view'; +import '@components/jobs/job-panel'; import '@components/library-filter/library-filter'; import '@components/library-status-indicator/library-status-indicator'; import '@components/now-playing/now-playing'; @@ -87,8 +87,8 @@ const TAGS = [ 'job-details-drawer', 'job-indicator', 'job-log-view', + 'job-panel', 'job-row', - 'jobs-view', 'library-filter', 'library-status-indicator', 'now-playing', diff --git a/frontend/test/components/view-lifecycle.test.ts b/frontend/test/components/view-lifecycle.test.ts index 6351f85..14b3ed8 100644 --- a/frontend/test/components/view-lifecycle.test.ts +++ b/frontend/test/components/view-lifecycle.test.ts @@ -27,7 +27,6 @@ import '@components/explore-view/explore-view'; import '@components/home-view/home-view'; import '@components/downloads-view/downloads-view'; -import '@components/jobs/jobs-view'; import '@components/playlist-view/playlist-view'; import { fixture } from '@test/support/render'; import { stub, flush } from '@test/support/harness'; @@ -223,7 +222,6 @@ const CACHED_VIEWS = [ 'artists-view', 'genres-view', 'downloads-view', - 'jobs-view', 'playlist-view', 'explore-view', 'home-view', diff --git a/frontend/test/components/view-visibility.test.ts b/frontend/test/components/view-visibility.test.ts index d05d2cc..7a00c40 100644 --- a/frontend/test/components/view-visibility.test.ts +++ b/frontend/test/components/view-visibility.test.ts @@ -72,7 +72,6 @@ const ALL_VISIBLE = { explore: true, downloads: true, autotag: true, - jobs: true, settings: true, }; @@ -96,7 +95,6 @@ describe('view visibility', () => { 'explore', 'downloads', 'autotag', - 'jobs', 'settings', ]); }); @@ -104,11 +102,11 @@ describe('view visibility', () => { it('drops the ones the user switched off', async () => { const el = await fixture('app-sidebar'); - await setViews({ ...ALL_VISIBLE, autotag: false, jobs: false }); + await setViews({ ...ALL_VISIBLE, autotag: false, explore: false }); await el.updateComplete; expect(navIDs(el)).not.toContain('autotag'); - expect(navIDs(el)).not.toContain('jobs'); + expect(navIDs(el)).not.toContain('explore'); expect(navIDs(el)).toContain('settings'); }); -- 2.54.0 From 99f355b2fc5f8ac705993ba81f6124cfd76e8b72 Mon Sep 17 00:00:00 2001 From: Logan Date: Wed, 19 Aug 2026 20:27:55 -0400 Subject: [PATCH 5/7] docs: record where background jobs went, and two traps The retired-destination note sits beside #25's storage paragraph because it is the exception to it: an absent key is free, a value is not. `config-section .header` resolving to two elements is in NOTES because it cannot be reproduced by opening Settings and looking -- the drawer only exists once a job does. --- .planning/NOTES.md | 19 +++++++++++++++++++ CLAUDE.md | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+) diff --git a/.planning/NOTES.md b/.planning/NOTES.md index c7d3adf..3c0fa9c 100644 --- a/.planning/NOTES.md +++ b/.planning/NOTES.md @@ -3732,3 +3732,22 @@ so it is the mechanism and not a test-only door. Use the nav item when the *nav* is the subject, and `navigateTo` when the view is. + +## `config-section .header` is ambiguous once a job exists (2026-08-19) + +#27 embeds `` inside four Settings sections, and a panel with +any job in it also mounts a `job-details-drawer` — whose own header +carries the class `.header`. So `config-page config-section .header`, +which `settings-reach.spec.ts` had used since plan 007, resolves to two +elements and fails Playwright's strict mode the moment a scan has run. + +Two things follow. A spec asserting on a section's *disclosure* should +locate it by role and name (`getByRole('button', {name: heading})`) or +scope per section and take `.first()`, not by that class. And this is a +worked example of the more general trap: a class name is not a +selector's contract, and a component that embeds another inherits its +class names into every ancestor query. + +It also only appears in a suite that has *done* something — the +sections are empty on a fresh app, so this cannot be reproduced by +opening Settings and looking. diff --git a/CLAUDE.md b/CLAUDE.md index 4664818..1052624 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -514,6 +514,36 @@ rather than renaming them. the autotag apply are registered; anything that is not registered has none of that, which is exactly how the three gaps the audit found came about. + + **Its rows are shown where the work is started, not on a page of + their own.** #27 folded the Jobs destination away, and the shape it + folded into is `` embedded four times — scans in + Settings → Libraries, index and enrichment in Settings → Search + Index, downloads under the download clients, the autotag apply in + `autotag-view`. One "Background jobs" section in Settings was the + obvious reading of the report and is the tab again under another + name. + + Four things about it are load-bearing. **Four of the five kinds + already had a home** that showed their work — the tier list, the + download list, the apply ring — and what none of them had is the + *generic* affordances, so the panel carries pause, cancel, Details + and the log to each rather than replacing what is there. **The + controls are `applyJobControl`**, not a reimplementation, which is + what keeps the "you will discard hours of downloading" confirmation + alive: it is keyed on `KindIndexBuild` inside the shared handler, and + a host drawing its own buttons would drop it silently. **A panel with + nothing to say is `hidden`**, host margin included, because an idle + panel in four places is four pieces of furniture describing an + absence. And **there is no "Clear finished"** in it, because + `ClearFinishedJobs` is global — a Clear under Libraries would discard + the index build's history too; a finished row dismisses itself. + + The header `job-indicator` is untouched and is still the one view of + everything at once, from every page. One consequence worth knowing + before writing a spec: a section holding a `job-panel` also holds a + `job-details-drawer`, whose own header carries `.header` — so + `config-section .header` is ambiguous the moment a job exists. - `config` — TOML-based settings. Settings page uses HTMX + templ for server-rendered HTML fragments. - `playlist` / `smartplaylist` — Playlist CRUD and rule-based smart playlists. - `mediacontrols` — OS media controls behind one `Handler`: MPRIS over @@ -1086,6 +1116,15 @@ tap away. Which four tabs is still plan 016's committed subset; this only removes from it, and "More" is never filtered because it is how everything else stays reachable. +**A retired destination is the one shape this does not make free.** An +absent visibility key takes its default and an unknown one is dropped, +but `DefaultPage` is a *value*: a launch page naming a view that no +longer exists fails validation, and on the load path that means the app +refuses to start for whoever had it selected. `RetiredViews` is that +list, and `ApplyDefaults` treats a retired name as a zero value while +an unknown-but-not-retired one still errors — a typo is worth being +told about. #27 retiring `jobs` is its first entry. + **The list of destinations is `services/view-meta.ts`**, on `shortcut-meta.ts`'s pattern, because #25 gave it a second reader: Settings renders a toggle per view and needs the same labels in the -- 2.54.0 From e4efec6f0c6fe516bf995f8b9a6927bc2fc192a9 Mon Sep 17 00:00:00 2001 From: Logan Date: Wed, 19 Aug 2026 20:44:02 -0400 Subject: [PATCH 6/7] fix(e2e): the third spec that located a disclosure by class `config-section .header` is ambiguous once a section holds a job, and `failure-voice.spec.ts` was the one I did not grep for. It passed on chromium and failed on webkit in the same CI run, which is the tell: the two engines share one app, so the second one runs with a finished scan the first one left behind. The NOTES entry already says a class name is not a selector's contract; this is the same fix, by role and name. --- e2e/specs/failure-voice.spec.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/e2e/specs/failure-voice.spec.ts b/e2e/specs/failure-voice.spec.ts index baafe60..6d3bc82 100644 --- a/e2e/specs/failure-voice.spec.ts +++ b/e2e/specs/failure-voice.spec.ts @@ -36,9 +36,16 @@ test.describe('a failed binding says so', () => { // Libraries is the one section that starts expanded (H-22), so ask // the disclosure what state it is in rather than assuming one — a // blind click used to expand it and now collapses it. + // + // By role and name, not by `.header`: since #27 the section also + // contains a `job-panel`, and an open `job-details-drawer` inside + // it carries the same class. That only bites once a job exists, + // which is why it showed up on the *second* engine of a CI run and + // not the first. const disclosure = page .locator('config-section[heading="Libraries"]') - .locator('.header'); + .getByRole('button', { name: 'Libraries' }) + .first(); if ((await disclosure.getAttribute('aria-expanded')) === 'false') { await disclosure.click(); -- 2.54.0 From f9ba9a87d7fd10bb12033cb4b68c9047b9aada8f Mon Sep 17 00:00:00 2001 From: Logan Date: Wed, 19 Aug 2026 20:44:16 -0400 Subject: [PATCH 7/7] docs: note that CI's two engines share one app Which is why a shared-selector fault can be green on chromium and red on webkit in the same run, and how to reproduce it locally. --- .planning/NOTES.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/.planning/NOTES.md b/.planning/NOTES.md index 3c0fa9c..ee3cd36 100644 --- a/.planning/NOTES.md +++ b/.planning/NOTES.md @@ -3751,3 +3751,13 @@ class names into every ancestor query. It also only appears in a suite that has *done* something — the sections are empty on a fresh app, so this cannot be reproduced by opening Settings and looking. + +**And it appears on the second engine, not the first.** CI runs +chromium then webkit against **one app**, so a spec that scans in the +chromium pass leaves a finished job the webkit pass then trips over. +Three specs used that selector; two failed locally and the third +(`failure-voice.spec.ts`) was green on chromium and red on webkit in +the same run. Reproducing it locally is running the suite twice against +one `make dev-headless` — which is worth doing for any change that +leaves state behind, since it is the only place a cross-engine order +dependency shows up. -- 2.54.0