fix(ui): make every track, album and artist name navigate somewhere
A name linked only when the entity carried an MBID — and for tracks, only when it carried two. That rule is invisible, so a track list read as randomly broken: some titles were clickable, most were not, and nothing on screen said why. A name now always goes somewhere. Tagged entities open their MusicBrainz page as before; untagged ones open the *library* page for the same album or artist, which both detail views already support via a local id — they just had no caller passing one. An untagged track highlights by title, since a recording MBID is exactly what it lacks. Links now fire on a genuine single click only. Every list these appear in also plays a row on double-click, and the title is the widest thing in the row, so the first click of that gesture lands on the link: navigating immediately meant double-clicking a track title opened a page instead of playing it, which the e2e playback suite caught. The navigation is held for one double-click interval and dropped if the second click arrives, while the dblclick itself is left to bubble to the row — so rows do not need to know links exist.
This commit is contained in:
+11
-1
@@ -209,7 +209,14 @@ document.addEventListener('navigate', (e: Event) => {
|
||||
break;
|
||||
}
|
||||
case 'explore-album-details': {
|
||||
const { releaseGroupMBID, albumName, artistName, highlightTrackMBID, localAlbumId } = detail;
|
||||
const {
|
||||
releaseGroupMBID,
|
||||
albumName,
|
||||
artistName,
|
||||
highlightTrackMBID,
|
||||
highlightTrackTitle,
|
||||
localAlbumId,
|
||||
} = detail;
|
||||
const el = document.createElement('explore-album-details');
|
||||
|
||||
if (releaseGroupMBID) el.setAttribute('release-group-mbid', releaseGroupMBID);
|
||||
@@ -218,6 +225,9 @@ document.addEventListener('navigate', (e: Event) => {
|
||||
if (highlightTrackMBID) {
|
||||
el.setAttribute('highlight-track-mbid', highlightTrackMBID);
|
||||
}
|
||||
if (highlightTrackTitle) {
|
||||
el.setAttribute('highlight-track-title', highlightTrackTitle);
|
||||
}
|
||||
if (localAlbumId) el.setAttribute('local-album-id', String(localAlbumId));
|
||||
mainContent.appendChild(el);
|
||||
currentDetailEl = el;
|
||||
|
||||
@@ -97,6 +97,11 @@ export class ExploreAlbumDetails extends LitElement {
|
||||
@property({ type: String, attribute: 'highlight-track-mbid' })
|
||||
highlightTrackMBID = '';
|
||||
|
||||
/** Highlight target for a track with no recording MBID — the only
|
||||
* handle an untagged track has is its title. */
|
||||
@property({ type: String, attribute: 'highlight-track-title' })
|
||||
highlightTrackTitle = '';
|
||||
|
||||
@property({ type: Number, attribute: 'local-album-id' })
|
||||
localAlbumId = 0;
|
||||
|
||||
@@ -552,13 +557,15 @@ export class ExploreAlbumDetails extends LitElement {
|
||||
|
||||
override updated() {
|
||||
if (
|
||||
this.highlightTrackMBID &&
|
||||
(this.highlightTrackMBID || this.highlightTrackTitle) &&
|
||||
!this.hasScrolledToHighlight &&
|
||||
!this.loadingReleases
|
||||
) {
|
||||
const el = this.shadowRoot?.querySelector<HTMLElement>(
|
||||
`[data-track-mbid="${this.highlightTrackMBID}"]`,
|
||||
);
|
||||
const el = this.highlightTrackMBID
|
||||
? this.shadowRoot?.querySelector<HTMLElement>(
|
||||
`[data-track-mbid="${this.highlightTrackMBID}"]`,
|
||||
)
|
||||
: this.findRowByTitle(this.highlightTrackTitle);
|
||||
|
||||
if (el) {
|
||||
this.hasScrolledToHighlight = true;
|
||||
@@ -609,6 +616,26 @@ export class ExploreAlbumDetails extends LitElement {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Locate a track row by title, for tracks with no recording MBID.
|
||||
* Matched in JS rather than by attribute selector because titles
|
||||
* contain quotes and other things a selector cannot carry.
|
||||
*/
|
||||
private findRowByTitle(title: string): HTMLElement | null {
|
||||
const wanted = title.trim().toLowerCase();
|
||||
const rows = this.shadowRoot?.querySelectorAll<HTMLElement>(
|
||||
'.track-row[data-track-title]',
|
||||
);
|
||||
|
||||
for (const row of rows ?? []) {
|
||||
if ((row.dataset.trackTitle ?? '').toLowerCase() === wanted) {
|
||||
return row;
|
||||
}
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
/* ── Data Loading ── */
|
||||
|
||||
private async loadAllData() {
|
||||
@@ -1904,6 +1931,7 @@ export class ExploreAlbumDetails extends LitElement {
|
||||
<div
|
||||
class="track-row"
|
||||
data-track-mbid="${track.mbid}"
|
||||
data-track-title="${track.title}"
|
||||
>
|
||||
<span class="track-position"
|
||||
>${track.position}</span
|
||||
|
||||
@@ -1386,9 +1386,9 @@ export class PlaylistDetails
|
||||
? html`<img src="${track.CoverArtSmall || track.CoverArtMedium}" alt="" />`
|
||||
: nothing}
|
||||
</div>
|
||||
<span class="cell col-title" title="${track.Title || track.FilePath}">${trackLink(track.Title, track.Album, track.ReleaseGroupMBID, track.RecordingMBID) || track.FilePath}</span>
|
||||
<span class="cell col-title" title="${track.Title || track.FilePath}">${trackLink(track.Title, track.Album, track.ReleaseGroupMBID, track.RecordingMBID, undefined, track.Artist) || track.FilePath}</span>
|
||||
<span class="cell col-artist" title="${track.Artist}">${artistLink(track.Artist, track.ArtistMBID)}</span>
|
||||
<span class="cell col-album" title="${track.Album}">${albumLink(track.Album, track.ReleaseGroupMBID)}</span>
|
||||
<span class="cell col-album" title="${track.Album}">${albumLink(track.Album, track.ReleaseGroupMBID, undefined, track.Artist)}</span>
|
||||
<span class="cell col-duration">${formatMilliseconds(track.Duration)}</span>`}
|
||||
</div>
|
||||
`;
|
||||
|
||||
@@ -1422,7 +1422,7 @@ export class QueuePanel
|
||||
${artUrl ? html`<div class="track-art"><img src="${artUrl}" alt="" loading="lazy" /></div>` : nothing}
|
||||
<div class="track-details">
|
||||
<span class="track-title">
|
||||
${trackLink(this.getDisplayTitle(track), track.album, track.releaseGroupMbid, track.recordingMbid)}
|
||||
${trackLink(this.getDisplayTitle(track), track.album, track.releaseGroupMbid, track.recordingMbid, undefined, track.artist)}
|
||||
</span>
|
||||
<span class="track-artist">
|
||||
${artistLink(track.artist, track.artistMbid) || 'Unknown Artist'}
|
||||
|
||||
@@ -1245,9 +1245,9 @@ export class SmartPlaylistDetails
|
||||
? html`<img src="${track.CoverArtSmall || track.CoverArtMedium}" alt="" />`
|
||||
: nothing}
|
||||
</div>
|
||||
<span class="cell col-title" title="${track.Title || track.FilePath}">${trackLink(track.Title, track.Album, track.ReleaseGroupMBID, track.RecordingMBID) || track.FilePath}</span>
|
||||
<span class="cell col-title" title="${track.Title || track.FilePath}">${trackLink(track.Title, track.Album, track.ReleaseGroupMBID, track.RecordingMBID, undefined, track.Artist) || track.FilePath}</span>
|
||||
<span class="cell col-artist" title="${track.Artist}">${artistLink(track.Artist, track.ArtistMBID)}</span>
|
||||
<span class="cell col-album" title="${track.Album}">${albumLink(track.Album, track.ReleaseGroupMBID)}</span>
|
||||
<span class="cell col-album" title="${track.Album}">${albumLink(track.Album, track.ReleaseGroupMBID, undefined, track.Artist)}</span>
|
||||
<span class="cell col-duration">${formatMilliseconds(track.Duration)}</span>`}
|
||||
</div>
|
||||
`;
|
||||
|
||||
@@ -1766,11 +1766,11 @@ export class TrackList extends LitElement implements SelectionHost, ContextMenuH
|
||||
|
||||
// Wrap artist/album/track values in explore links.
|
||||
if (col.id === 'trackName') {
|
||||
display = trackLink(track.TrackName, track.Album, track.ReleaseGroupMBID, track.RecordingMBID, display as any);
|
||||
display = trackLink(track.TrackName, track.Album, track.ReleaseGroupMBID, track.RecordingMBID, display as any, track.ArtistName);
|
||||
} else if (col.id === 'artistName') {
|
||||
display = artistLink(track.ArtistName, track.ArtistMBID, display as any);
|
||||
} else if (col.id === 'album') {
|
||||
display = albumLink(track.Album, track.ReleaseGroupMBID, display as any);
|
||||
display = albumLink(track.Album, track.ReleaseGroupMBID, display as any, track.ArtistName);
|
||||
}
|
||||
|
||||
return html`
|
||||
|
||||
@@ -1,13 +1,23 @@
|
||||
/**
|
||||
* Utility for rendering artist/album names as clickable links
|
||||
* that navigate to their MusicBrainz explore detail pages.
|
||||
* Utility for rendering track/album/artist names as clickable links.
|
||||
*
|
||||
* Links are rendered only when an MBID is provided. If the MBID
|
||||
* is empty (entity not tagged), the name renders as plain text.
|
||||
* A name links to its MusicBrainz page when the entity is tagged, and
|
||||
* to the local library page for the same thing when it is not. Both
|
||||
* destinations are the same two components — `explore-album-details`
|
||||
* and `explore-artist-details` both accept a local id instead of an
|
||||
* MBID — so an untagged album is not a dead end, it is just a page with
|
||||
* less on it.
|
||||
*
|
||||
* Falling back rather than rendering plain text is deliberate: a list
|
||||
* where some rows are clickable and others silently are not reads as a
|
||||
* bug, not as a statement about metadata. The only case that still
|
||||
* renders as text is one we genuinely cannot route (no name at all, or
|
||||
* nothing in the library by that name).
|
||||
*/
|
||||
|
||||
import { html, css } from 'lit';
|
||||
import type { TemplateResult } from 'lit';
|
||||
import { libraryStore } from '../store/library-store';
|
||||
|
||||
/** Shared CSS for explore link styling. Import into component styles. */
|
||||
export const exploreLinkStyles = css`
|
||||
@@ -22,58 +32,125 @@ export const exploreLinkStyles = css`
|
||||
}
|
||||
`;
|
||||
|
||||
/**
|
||||
* Dispatch a navigate event to the explore-artist-details page.
|
||||
* The event bubbles through shadow DOM boundaries.
|
||||
*/
|
||||
function navigateToArtist(artistName: string, mbid: string, e: Event): void {
|
||||
e.stopPropagation();
|
||||
e.preventDefault();
|
||||
|
||||
const target = e.currentTarget as HTMLElement;
|
||||
|
||||
/** Fire a navigate event from the clicked element. */
|
||||
function navigate(target: EventTarget, detail: Record<string, unknown>): void {
|
||||
target.dispatchEvent(
|
||||
new CustomEvent('navigate', {
|
||||
bubbles: true,
|
||||
composed: true,
|
||||
detail: {
|
||||
view: 'explore-artist-details',
|
||||
artistMBID: mbid,
|
||||
artistName,
|
||||
},
|
||||
detail,
|
||||
}),
|
||||
);
|
||||
}
|
||||
|
||||
/** Case-insensitive compare that tolerates undefined. */
|
||||
function sameName(a: string | undefined, b: string | undefined): boolean {
|
||||
return (a ?? '').toLowerCase() === (b ?? '').toLowerCase();
|
||||
}
|
||||
|
||||
/**
|
||||
* Dispatch a navigate event to the explore-album-details page.
|
||||
* The event bubbles through shadow DOM boundaries.
|
||||
* Find the library album row for a name, loading the album cache first
|
||||
* if a view that populates it has not been opened yet.
|
||||
*/
|
||||
function navigateToAlbum(albumName: string, mbid: string, e: Event): void {
|
||||
e.stopPropagation();
|
||||
e.preventDefault();
|
||||
async function findLocalAlbum(
|
||||
albumName: string,
|
||||
artistName?: string,
|
||||
): Promise<{ ID: number; Name: string; ArtistName: string } | null> {
|
||||
let albums = libraryStore.cachedAlbums;
|
||||
|
||||
const target = e.currentTarget as HTMLElement;
|
||||
if (!albums) {
|
||||
try {
|
||||
albums = await libraryStore.getAlbums();
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
target.dispatchEvent(
|
||||
new CustomEvent('navigate', {
|
||||
bubbles: true,
|
||||
composed: true,
|
||||
detail: {
|
||||
view: 'explore-album-details',
|
||||
releaseGroupMBID: mbid,
|
||||
albumName,
|
||||
},
|
||||
}),
|
||||
);
|
||||
let fallback: (typeof albums)[0] | null = null;
|
||||
|
||||
for (const album of albums ?? []) {
|
||||
if (!sameName(album.Name, albumName)) continue;
|
||||
if (artistName && sameName(album.ArtistName, artistName)) return album;
|
||||
fallback ??= album;
|
||||
}
|
||||
|
||||
return fallback;
|
||||
}
|
||||
|
||||
/** Find the library artist row for a name, loading the cache if needed. */
|
||||
async function findLocalArtist(
|
||||
artistName: string,
|
||||
): Promise<{ ID: number; Name: string } | null> {
|
||||
let artists = libraryStore.cachedArtists;
|
||||
|
||||
if (!artists) {
|
||||
try {
|
||||
artists = await libraryStore.getArtists();
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
for (const artist of artists ?? []) {
|
||||
if (sameName(artist.Name, artistName)) return artist;
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Render an artist name as a clickable link if an MBID is provided,
|
||||
* or as plain text if not.
|
||||
* How long a link waits before navigating.
|
||||
*
|
||||
* Every list these links appear in also plays a row on double-click,
|
||||
* and the title is the widest thing in the row — so the same gesture
|
||||
* that plays a track starts with a click on its name. Navigating on
|
||||
* the first of those two clicks means double-clicking a track title
|
||||
* opens a page instead of playing it. Holding the navigation for one
|
||||
* double-click interval, and dropping it if the second click arrives,
|
||||
* lets one element serve both without the row having to know links
|
||||
* exist.
|
||||
*/
|
||||
const DOUBLE_CLICK_GRACE_MS = 250;
|
||||
|
||||
/**
|
||||
* Wrap a link action so it fires on a genuine single click only.
|
||||
*
|
||||
* The click's propagation is stopped (the row must not also treat it as
|
||||
* a selection) but the *double*-click is left alone, so it still
|
||||
* reaches the row and plays the track.
|
||||
*/
|
||||
function singleClick(
|
||||
run: (target: EventTarget) => void,
|
||||
): (e: MouseEvent) => void {
|
||||
return (e: MouseEvent) => {
|
||||
e.stopPropagation();
|
||||
e.preventDefault();
|
||||
|
||||
// detail > 1 is the second click of a double click; the first
|
||||
// one already scheduled and is about to be cancelled.
|
||||
if (e.detail > 1) return;
|
||||
|
||||
const target = (e.currentTarget ?? e.target) as EventTarget;
|
||||
|
||||
const timer = window.setTimeout(() => {
|
||||
target.removeEventListener('dblclick', cancel);
|
||||
run(target);
|
||||
}, DOUBLE_CLICK_GRACE_MS);
|
||||
|
||||
function cancel(): void {
|
||||
window.clearTimeout(timer);
|
||||
}
|
||||
|
||||
target.addEventListener('dblclick', cancel, { once: true });
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Render an artist name as a link to the artist page — the
|
||||
* MusicBrainz one when tagged, the library one when not.
|
||||
*
|
||||
* @param artistName - The artist name to display.
|
||||
* @param mbid - The MusicBrainz artist ID. Empty string = no link.
|
||||
* @param mbid - The MusicBrainz artist ID. Empty string = local only.
|
||||
* @param content - Optional custom content to render inside the link
|
||||
* (e.g. highlighted search result). Defaults to artistName.
|
||||
*/
|
||||
@@ -83,78 +160,75 @@ export function artistLink(
|
||||
content?: TemplateResult | string,
|
||||
): TemplateResult | string {
|
||||
if (!artistName) return artistName;
|
||||
if (!mbid) return content ?? artistName;
|
||||
|
||||
const onClick = singleClick((target) => {
|
||||
void (async () => {
|
||||
if (mbid) {
|
||||
navigate(target, {
|
||||
view: 'explore-artist-details',
|
||||
artistMBID: mbid,
|
||||
artistName,
|
||||
});
|
||||
|
||||
return;
|
||||
}
|
||||
|
||||
const local = await findLocalArtist(artistName);
|
||||
if (!local) return;
|
||||
|
||||
navigate(target, {
|
||||
view: 'explore-artist-details',
|
||||
artistMBID: '',
|
||||
artistName,
|
||||
localArtistId: local.ID,
|
||||
});
|
||||
})();
|
||||
});
|
||||
|
||||
return html`<a
|
||||
class="explore-link"
|
||||
@click=${(e: Event) => navigateToArtist(artistName, mbid, e)}
|
||||
title="View artist on Explore"
|
||||
@click=${onClick}
|
||||
title=${mbid ? 'View artist on Explore' : 'View artist in your library'}
|
||||
>${content ?? artistName}</a>`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Dispatch a navigate event to the explore-album-details page
|
||||
* with a highlight on a specific track.
|
||||
*/
|
||||
function navigateToTrack(
|
||||
albumName: string,
|
||||
releaseGroupMBID: string,
|
||||
recordingMBID: string,
|
||||
e: Event,
|
||||
): void {
|
||||
e.stopPropagation();
|
||||
e.preventDefault();
|
||||
|
||||
const target = e.currentTarget as HTMLElement;
|
||||
|
||||
target.dispatchEvent(
|
||||
new CustomEvent('navigate', {
|
||||
bubbles: true,
|
||||
composed: true,
|
||||
detail: {
|
||||
view: 'explore-album-details',
|
||||
releaseGroupMBID,
|
||||
albumName,
|
||||
highlightTrackMBID: recordingMBID,
|
||||
},
|
||||
}),
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Render an album name as a clickable link if an MBID is provided,
|
||||
* or as plain text if not.
|
||||
* Render an album name as a link to the album page — the MusicBrainz
|
||||
* one when tagged, the library one when not.
|
||||
*
|
||||
* @param albumName - The album name to display.
|
||||
* @param mbid - The MusicBrainz release group ID. Empty string = no link.
|
||||
* @param content - Optional custom content to render inside the link
|
||||
* (e.g. highlighted search result). Defaults to albumName.
|
||||
* @param mbid - The MusicBrainz release group ID. Empty = local only.
|
||||
* @param content - Optional custom content to render inside the link.
|
||||
* @param artistName - Disambiguates same-named albums in the library.
|
||||
*/
|
||||
export function albumLink(
|
||||
albumName: string,
|
||||
mbid: string,
|
||||
content?: TemplateResult | string,
|
||||
artistName?: string,
|
||||
): TemplateResult | string {
|
||||
if (!albumName) return albumName;
|
||||
if (!mbid) return content ?? albumName;
|
||||
|
||||
return html`<a
|
||||
class="explore-link"
|
||||
@click=${(e: Event) => navigateToAlbum(albumName, mbid, e)}
|
||||
title="View album on Explore"
|
||||
@click=${singleClick((target) => {
|
||||
void openAlbum(target, albumName, mbid, artistName);
|
||||
})}
|
||||
title=${mbid ? 'View album on Explore' : 'View album in your library'}
|
||||
>${content ?? albumName}</a>`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Render a track name as a clickable link that opens the album's
|
||||
* explore page with the track highlighted. Requires both a
|
||||
* release group MBID (album) and a recording MBID (track).
|
||||
* Render a track name as a link that opens the track's album with the
|
||||
* track highlighted. An untagged track highlights by title on the
|
||||
* library album page instead, so every row in a list behaves the same.
|
||||
*
|
||||
* @param trackName - The track name to display.
|
||||
* @param albumName - The album name (for the page title).
|
||||
* @param releaseGroupMBID - The album's MusicBrainz release group ID.
|
||||
* @param recordingMBID - The track's MusicBrainz recording ID.
|
||||
* @param content - Optional custom content (e.g. highlighted text).
|
||||
* @param artistName - Disambiguates same-named albums in the library.
|
||||
*/
|
||||
export function trackLink(
|
||||
trackName: string,
|
||||
@@ -162,13 +236,58 @@ export function trackLink(
|
||||
releaseGroupMBID: string,
|
||||
recordingMBID: string,
|
||||
content?: TemplateResult | string,
|
||||
artistName?: string,
|
||||
): TemplateResult | string {
|
||||
if (!trackName) return trackName;
|
||||
if (!releaseGroupMBID || !recordingMBID) return content ?? trackName;
|
||||
if (!albumName) return content ?? trackName;
|
||||
|
||||
return html`<a
|
||||
class="explore-link"
|
||||
@click=${(e: Event) => navigateToTrack(albumName, releaseGroupMBID, recordingMBID, e)}
|
||||
title="View track on album page"
|
||||
@click=${singleClick((target) => {
|
||||
void openAlbum(
|
||||
target,
|
||||
albumName,
|
||||
releaseGroupMBID,
|
||||
artistName,
|
||||
recordingMBID,
|
||||
trackName,
|
||||
);
|
||||
})}
|
||||
title=${releaseGroupMBID
|
||||
? 'View track on the album page'
|
||||
: 'View track on the album page in your library'}
|
||||
>${content ?? trackName}</a>`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Route to an album page, preferring the catalog and falling back to
|
||||
* the library copy. `highlight*` marks one track on arrival.
|
||||
*/
|
||||
async function openAlbum(
|
||||
target: EventTarget,
|
||||
albumName: string,
|
||||
releaseGroupMBID: string,
|
||||
artistName?: string,
|
||||
highlightTrackMBID?: string,
|
||||
highlightTrackTitle?: string,
|
||||
): Promise<void> {
|
||||
const detail: Record<string, unknown> = {
|
||||
view: 'explore-album-details',
|
||||
releaseGroupMBID,
|
||||
albumName,
|
||||
artistName: artistName ?? '',
|
||||
};
|
||||
|
||||
if (highlightTrackMBID) detail.highlightTrackMBID = highlightTrackMBID;
|
||||
if (highlightTrackTitle) detail.highlightTrackTitle = highlightTrackTitle;
|
||||
|
||||
if (!releaseGroupMBID) {
|
||||
const local = await findLocalAlbum(albumName, artistName);
|
||||
if (!local) return;
|
||||
|
||||
detail.localAlbumId = local.ID;
|
||||
detail.artistName = local.ArtistName;
|
||||
}
|
||||
|
||||
navigate(target, detail);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user