diff --git a/frontend/src/components/autotag-view/autotag-view.ts b/frontend/src/components/autotag-view/autotag-view.ts index e9a51fa..f7e172d 100644 --- a/frontend/src/components/autotag-view/autotag-view.ts +++ b/frontend/src/components/autotag-view/autotag-view.ts @@ -26,6 +26,8 @@ import { libraryStore } from '../../store/library-store'; import { notificationStore } from '../../store/notification-store'; import { describeError, explainError } from '../../utils/describe-error'; import { ViewLifecycleMixin } from '../../utils/view-lifecycle'; +import { confirmAction } from '../confirm-dialog/confirm-dialog'; +import '@awesome.me/webawesome/dist/components/dialog/dialog.js'; type PendingItem = autotagservice.PendingItem; type ScoreView = autotagservice.ScoreView; @@ -1026,44 +1028,19 @@ export class AutotagView extends ViewLifecycleMixin(LitElement) { border: 1px solid currentColor; } - /* ── Dialogs (carried over) ── */ + /* ── Dialogs ── + The frame, the backdrop, the focus trap and the Escape + handling belong to wa-dialog; what is left here is the + content these two put inside it. */ - .dialog-overlay { - position: fixed; - inset: 0; - background: rgba(0, 0, 0, 0.5); - display: flex; - align-items: center; - justify-content: center; - z-index: 100; + wa-dialog::part(dialog) { + background: var(--yj-bg-surface, #212529); + color: var(--yj-text-primary, #fff); } - .dialog { - background: var(--yj-bg-surface, #222); - padding: 1.25rem; - border-radius: 6px; - min-width: 420px; - max-width: 560px; - border: 1px solid var(--yj-bg-overlay, rgba(255, 255, 255, 0.1)); - } + wa-dialog p { margin: 0 0 1rem 0; font-size: 0.9rem; } - .dialog-header { display: flex; align-items: center; margin-bottom: 0.75rem; } - .dialog-header h3 { margin: 0; } - - .dialog-close { - margin-left: auto; - background: transparent; - color: var(--yj-text-secondary, #b3b3b3); - border: 0; - font-size: 1.2rem; - cursor: pointer; - padding: 0 0.25rem; - } - - .dialog-close:hover { color: var(--yj-text-primary, #fff); } - .dialog p { margin: 0 0 1rem 0; font-size: 0.9rem; } - - .dialog input[type="url"] { + .url-input { width: 100%; padding: 0.4rem; font: inherit; @@ -1074,16 +1051,15 @@ export class AutotagView extends ViewLifecycleMixin(LitElement) { box-sizing: border-box; } - .dialog .row { + .row { display: flex; gap: 0.5rem; justify-content: flex-end; - margin-top: 1rem; } /* ── In-app search dialog ── */ - .search-dialog { min-width: 480px; } + .search-dialog::part(dialog) { min-width: 480px; } .search-kind { display: flex; @@ -1119,8 +1095,15 @@ export class AutotagView extends ViewLifecycleMixin(LitElement) { } .search-result { + display: block; + width: 100%; + text-align: left; + background: transparent; + color: inherit; + font: inherit; padding: 0.45rem 0.5rem; cursor: pointer; + border: 0; border-bottom: 1px solid var(--yj-bg-overlay, rgba(255, 255, 255, 0.05)); border-radius: 4px; } @@ -1166,7 +1149,10 @@ export class AutotagView extends ViewLifecycleMixin(LitElement) { @state() private loading = false; @state() private foldersLoading = false; @state() private errorMessage = ''; - @state() private dialog: 'none' | 'paste' | 'warning' | 'leave' | 'search' = 'none'; + // Two of the four dialogs this view used to hand-roll were plain + // confirmations and are `confirmAction()` calls now; the two that + // remain carry input and so are ``s in the template. + @state() private dialog: 'none' | 'paste' | 'search' = 'none'; @state() private pasteURL = ''; // In-app MusicBrainz search ("suggest a candidate") dialog state. @@ -1175,6 +1161,9 @@ export class AutotagView extends ViewLifecycleMixin(LitElement) { @state() private searchArtist = ''; @state() private searchResults: SearchHitView[] = []; @state() private searchLoading = false; + /** Whether a search has actually been run, as opposed to the fields + * merely being seeded from the current folder. */ + @state() private searchRan = false; @state() private searchError = ''; @state() private queueMenuOpen = false; // Collapsible sidebar sections. Pending stays expanded so the @@ -1197,7 +1186,11 @@ export class AutotagView extends ViewLifecycleMixin(LitElement) { * when it is disconnected — which never happens, because the view * is cached (see utils/view-lifecycle.ts). */ protected override onViewActivate(): void { - this.listenWhileActive(document, 'keydown', this.onKeydown as EventListener); + // This view owned one last document keydown listener, for Escape, + // because its dialogs were hand-rolled and nothing else would + // close them. They are `wa-dialog`s now and close themselves, so + // the shortcut service is the only thing on this page deciding + // what a key means — which is what Phase 1 was for. this.listenWhileActive( document, 'mousedown', @@ -1391,49 +1384,42 @@ export class AutotagView extends ViewLifecycleMixin(LitElement) { private onPasteKeydown = (e: KeyboardEvent) => { if (e.key === 'Enter') this.onPasteSubmit(); }; - private onWarningCancel = () => { this.dialog = 'none'; }; - private onWarningContinue = async () => { - if (!this.current) return; + /** Acknowledge the once-per-library "this rewrites files" warning, + * then apply. Returns without applying if the user backed out. */ + private async confirmWarningThenApply(): Promise { + const current = this.current; - this.markWarningAcked(this.current.libraryId); + if (!current) return; - // Unwrapped, a rejection here meant the lines after it — the - // one that closes the dialog — never ran, and the dialog sat - // there forever (errors.m6). + const library = current.libraryName || `library #${current.libraryId}`; + const ok = await confirmAction({ + title: 'Heads up: this rewrites audio files', + message: `Applying autotag writes new metadata directly to every track in ${current.albumName}. ` + + 'The change lands on disk and is not automatically reversible. ' + + 'Files outside this library are not touched.', + impact: `This warning shows once per library. Continue to acknowledge for ${library}.`, + confirmLabel: 'Continue', + }); + + if (!ok) return; + + this.markWarningAcked(current.libraryId); + + // Unwrapped, a rejection here meant the lines after it never ran + // and the dialog sat there forever (errors.m6). The dialog closes + // itself now, but the apply still has to happen either way. try { - await AckLibraryWarning(this.current.libraryId); + await AckLibraryWarning(current.libraryId); } catch (err) { console.error('autotag: could not record the warning ack', err); this.errorMessage = describeError( err, 'That acknowledgement could not be saved.', ); - } finally { - this.dialog = 'none'; } await this.executeApply(); - }; - private onLeaveCancel = () => { this.dialog = 'none'; }; - private onLeaveConfirm = async () => { - if (!this.current) return; - - this.dialog = 'none'; - - try { - await LeaveAsIs(this.current.groupKey); - } catch (err) { - console.error('autotag: leave-as-is failed', err); - this.errorMessage = describeError( - err, - 'That folder could not be left as it is.', - ); - - return; - } - - await this.refreshAfterAction(); - }; + } /** Open the in-app MB search dialog, seeding the query fields from * the current folder's album/artist so the common case is one @@ -1444,6 +1430,10 @@ export class AutotagView extends ViewLifecycleMixin(LitElement) { this.searchArtist = this.current?.albumArtist ?? ''; this.searchResults = []; this.searchError = ''; + // The query is seeded from the folder, so "no results" was true of + // a search nobody had run yet — visible the moment the dialog + // opens, under the fields still being filled in. + this.searchRan = false; this.dialog = 'search'; } @@ -1454,6 +1444,7 @@ export class AutotagView extends ViewLifecycleMixin(LitElement) { if (!query) return; this.searchLoading = true; this.searchError = ''; + this.searchRan = true; try { this.searchResults = await SearchCandidates( this.searchKind, query, this.searchArtist.trim(), @@ -1734,9 +1725,11 @@ export class AutotagView extends ViewLifecycleMixin(LitElement) { private async onApply(): Promise { if (!this.current || !this.score || this.score.candidates.length === 0) return; if (!this.hasLibraryWarningBeenAcked(this.current.libraryId)) { - this.dialog = 'warning'; + await this.confirmWarningThenApply(); + return; } + await this.executeApply(); } @@ -1803,14 +1796,27 @@ export class AutotagView extends ViewLifecycleMixin(LitElement) { private async onLeave(): Promise { if (!this.current) return; - if (this.topScore() < CONFIDENT_SCORE) { - this.dialog = 'leave'; + const groupKey = this.current.groupKey; - return; + if (this.topScore() < CONFIDENT_SCORE) { + const top = Math.round(this.topScore() * 100); + const ok = await confirmAction({ + title: 'Keep the current tags?', + message: `No candidate scored high enough to trust automatically (top is ${top}%). ` + + `“Leave as-is” marks the local tags on ${this.current.albumName} as correct ` + + 'and removes this folder from the review queue.', + impact: 'If the local tags are wrong, prefer Skip (S) instead — ' + + 'that leaves the folder pending for later review.', + confirmLabel: 'Mark as correct', + }); + + // The confirmation is modal, but it is still an await: the + // folder we asked about has to be the folder we act on. + if (!ok || this.current?.groupKey !== groupKey) return; } try { - await LeaveAsIs(this.current.groupKey); + await LeaveAsIs(groupKey); } catch (err) { console.error('autotag: leave-as-is failed', err); this.errorMessage = describeError( @@ -1858,18 +1864,6 @@ export class AutotagView extends ViewLifecycleMixin(LitElement) { } } - /** Escape closes this view's hand-rolled dialogs. Everything else - * the page binds is a registered shortcut in the `autotag` panel - * scope, so the shortcut service is the only thing deciding what a - * key means — including on the pages this view is not on. */ - private onKeydown = (e: KeyboardEvent): void => { - if (e.key !== 'Escape' || this.dialog === 'none') return; - - e.preventDefault(); - this.dialog = 'none'; - this.pasteURL = ''; - }; - /* ── Version clustering ── */ /** @@ -2948,112 +2942,41 @@ export class AutotagView extends ViewLifecycleMixin(LitElement) { private renderPasteDialog() { return html` -
{ - if (e.target === e.currentTarget) this.onPasteCancel(); - }}> -
-
-

Paste MusicBrainz URL

- -
-

- Paste a MusicBrainz release or release-group URL. - Tracks are aligned automatically when it loads. -

- -
- - -
+ +

+ Paste a MusicBrainz release or release-group URL. + Tracks are aligned automatically when it loads. +

+ +
+ +
-
- `; - } - - private renderWarningDialog() { - if (!this.current) return nothing; - return html` -
{ - if (e.target === e.currentTarget) this.onWarningCancel(); - }}> -
-
-

Heads up: this rewrites audio files

- -
-

- Applying autotag writes new metadata directly to every - track in ${this.current.albumName}. - The change lands on disk and is not automatically - reversible. Files outside this library are not - touched. -

-

- This warning shows once per library. Click Continue to - acknowledge for - ${this.current.libraryName || `library #${this.current.libraryId}`}. -

-
- - -
-
-
- `; - } - - private renderLeaveDialog() { - if (!this.current) return nothing; - const top = this.score?.candidates[0]; - const topPct = top ? Math.round(top.score * 100) : 0; - return html` -
{ - if (e.target === e.currentTarget) this.onLeaveCancel(); - }}> -
-
-

Keep the current tags?

- -
-

- No candidate scored high enough to trust automatically - (top is ${topPct}%). "Leave as-is" marks the local - tags on ${this.current.albumName} as - correct and removes this folder from the review queue. -

-

- If the local tags are wrong, prefer S (Skip) - instead — that leaves the folder pending for later - review. -

-
- - -
-
-
+ `; } private renderSearchDialog() { return html` -
{ - if (e.target === e.currentTarget) this.onSearchCancel(); - }}> -
-
-

Search MusicBrainz

- -
+
{ this.searchQuery = (e.target as HTMLInputElement).value; }} @keydown=${(e: KeyboardEvent) => { if (e.key === 'Enter') void this.runSearch(); }} autofocus> { this.searchArtist = (e.target as HTMLInputElement).value; }} @keydown=${(e: KeyboardEvent) => { if (e.key === 'Enter') void this.runSearch(); }}> -
- - -
${this.searchError ? html`
${this.searchError}
` : nothing} ${this.searchResults.length > 0 ? html` -
+
${this.searchResults.map((hit) => html` -
{ void this.pickSearchResult(hit); }}>
${hit.title}
${hit.artist || '—'} ${hit.detail ? html`${hit.detail}` : nothing}
-
+ `)}
- ` : this.searchLoading || this.searchError || this.searchQuery.trim() === '' - ? nothing - : html`
No results — try dropping the artist or switching Album/Track.
`} -
-
+ ` : this.searchRan && !this.searchLoading && !this.searchError + ? html`
No results — try dropping the artist or switching Album/Track.
` + : nothing} +
+ + +
+ `; } + // Both dialogs render unconditionally so `wa-dialog` owns opening and + // closing (and therefore the focus trap and the focus restore); the + // `open` property is what says which — mounting one on demand would + // put the element and its `showModal()` in the same update. private renderDialog() { - switch (this.dialog) { - case 'paste': return this.renderPasteDialog(); - case 'warning': return this.renderWarningDialog(); - case 'leave': return this.renderLeaveDialog(); - case 'search': return this.renderSearchDialog(); - default: return nothing; - } + return html`${this.renderPasteDialog()}${this.renderSearchDialog()}`; } override render() { diff --git a/frontend/src/components/config-page/config-page.ts b/frontend/src/components/config-page/config-page.ts index ac4cbdf..f36169e 100644 --- a/frontend/src/components/config-page/config-page.ts +++ b/frontend/src/components/config-page/config-page.ts @@ -38,6 +38,7 @@ import './config-field'; import './config-section'; import './download-clients'; import './shortcut-capture'; +import { confirmAction } from '../confirm-dialog/confirm-dialog'; import { shortcutsStore } from '../../store/shortcuts-store'; import { ShortcutsController } from '../../store/controllers/shortcuts-controller'; @@ -222,9 +223,10 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) { @state() private libraries: library.Info[] = []; @state() private editingLibraryId: number | null = null; @state() private editingName = ''; + /** The library a removal is in flight for. The impact is computed + * before the confirmation rather than held here, so this is now + * "which row is busy" and nothing else. */ @state() private removingLibraryId: number | null = null; - @state() private removalImpact: library.RemovalImpact | null = null; - @state() private isRemoving = false; @state() private activeMenuId: number | null = null; @state() private concurrencyMode = 'auto'; @state() private indexStatus: explore.IndexStatus | null = null; @@ -629,51 +631,6 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) { ); } - /* Cancel confirmation dialog */ - .cancel-dialog-overlay { - position: fixed; - inset: 0; - background: rgba(0, 0, 0, 0.6); - display: flex; - align-items: center; - justify-content: center; - z-index: 1000; - } - - .cancel-dialog { - background: var( - --yj-bg-surface, - #2a2a2a - ); - border: 1px solid - var(--yj-border, #444); - border-radius: 8px; - padding: 24px; - max-width: 420px; - width: 90%; - } - - .cancel-dialog-title { - font-size: var(--yj-text-lg, 18px); - font-weight: 600; - margin-bottom: 12px; - } - - .cancel-dialog-message { - font-size: var(--yj-text-sm, 14px); - color: var( - --yj-text-secondary, - #aaa - ); - margin-bottom: 20px; - } - - .cancel-dialog-actions { - display: flex; - gap: 8px; - justify-content: flex-end; - } - .btn-primary { background: var( --yj-accent, @@ -1158,34 +1115,58 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) { this.editingName = (e.target as HTMLInputElement).value; }; + /** + * Say what will happen, then ask — through the one confirmation the + * app has, which is a `wa-dialog` and so brings the focus trap, the + * Escape handler and the focus restore the hand-rolled overlay this + * replaces had none of (a11y.16). + */ private handleRemoveClick = async (id: number): Promise => { this.activeMenuId = null; - try { - const impact = await GetRemovalImpact(id); - - this.removalImpact = impact; - this.removingLibraryId = id; - } catch (err) { - console.error('Failed to get removal impact:', err); - } - }; - - private handleConfirmRemove = async (): Promise => { - if (this.removingLibraryId === null) return; - - const id = this.removingLibraryId; const lib = this.libraries.find((l) => l.id === id); const libName = lib?.name ?? 'Library'; + let impact: library.RemovalImpact | null = null; - this.isRemoving = true; + try { + impact = await GetRemovalImpact(id); + } catch (err) { + // Asking without the impact is worse than not asking at all, + // so this is a failure the user has to see rather than a + // confirmation with a blank consequence. + console.error('Failed to get removal impact:', err); + notificationStore.persistent({ + key: 'library-remove', + title: 'Library not removed', + text: `Could not work out what removing “${libName}” would delete. ${describeError(err)}`, + detail: String(err), + }); + + return; + } + + const ok = await confirmAction({ + title: 'Remove library', + message: `Remove “${libName}”?`, + impact: `This deletes ${impact.trackCount} tracks, affects ` + + `${impact.playlistsAffected} playlists and removes ` + + `${impact.queueItemCount} queue items.`, + confirmLabel: 'Remove', + danger: true, + }); + + if (!ok) return; + + await this.removeLibrary(id, libName); + }; + + private async removeLibrary(id: number, libName: string): Promise { + this.removingLibraryId = id; try { const summary = await RemoveLibrary(id); this.removingLibraryId = null; - this.removalImpact = null; - this.isRemoving = false; notificationStore.transient({ tone: 'success', key: 'library-remove', @@ -1193,9 +1174,7 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) { }); void this.loadLibraries(); } catch (err) { - this.isRemoving = false; this.removingLibraryId = null; - this.removalImpact = null; console.error('Failed to remove library:', err); notificationStore.persistent({ key: 'library-remove', @@ -1204,13 +1183,7 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) { detail: String(err), }); } - }; - - private handleCancelRemove = (): void => { - this.removingLibraryId = null; - this.removalImpact = null; - this.isRemoving = false; - }; + } private toggleOverflowMenu = (id: number, e: Event): void => { e.stopPropagation(); @@ -2057,10 +2030,6 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) { // --- Library section --- private renderLibrarySection() { - const removingLib = this.libraries.find( - (l) => l.id === this.removingLibraryId, - ); - return html` ${lib.path} - ${lib.trackCount} tracks + ${this.removingLibraryId === lib.id + ? 'Removing…' + : html`${lib.trackCount} tracks`}
- -
-
-
- ` - : nothing} `; }