diff --git a/.planning/NOTES.md b/.planning/NOTES.md index c7d3adf..ee3cd36 100644 --- a/.planning/NOTES.md +++ b/.planning/NOTES.md @@ -3732,3 +3732,32 @@ 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. + +**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. 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 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/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(); 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/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/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. `; } 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/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 }, ]; 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/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/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); + }); +}); 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'); });