diff --git a/frontend/src/assets/icons/fa/solid/chevron-left.svg b/frontend/src/assets/icons/fa/solid/chevron-left.svg new file mode 100644 index 0000000..2b26016 --- /dev/null +++ b/frontend/src/assets/icons/fa/solid/chevron-left.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/frontend/src/components/explore-album-details/explore-album-details.ts b/frontend/src/components/explore-album-details/explore-album-details.ts index 0508a6b..2f6d92f 100644 --- a/frontend/src/components/explore-album-details/explore-album-details.ts +++ b/frontend/src/components/explore-album-details/explore-album-details.ts @@ -60,6 +60,7 @@ import '@awesome.me/webawesome/dist/components/dropdown-item/dropdown-item.js'; import { dictByName } from '@utils/binding'; import type { TrackDetails } from '@components/track-details/track-details.js'; import { showTrackDetailsForPath } from '@utils/track-details-opener.js'; +import { openMusicBrainz } from '@utils/external-link'; import '@components/playlist-picker/playlist-picker.js'; import { ICON_CAN_REQUEST, @@ -2942,7 +2943,7 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost { if (!track?.mbid) return; - window.open(`https://musicbrainz.org/recording/${track.mbid}`, '_blank', 'noopener'); + openMusicBrainz(`/recording/${track.mbid}`); } /** diff --git a/frontend/src/components/explore-artist-details/explore-artist-details.ts b/frontend/src/components/explore-artist-details/explore-artist-details.ts index f0c7c83..106457c 100644 --- a/frontend/src/components/explore-artist-details/explore-artist-details.ts +++ b/frontend/src/components/explore-artist-details/explore-artist-details.ts @@ -4,6 +4,8 @@ import { customElement, property, state, query } from 'lit/decorators.js'; import { classMap } from 'lit/directives/class-map.js'; import { designTokens } from '../../styles/tokens.css'; import { backButton } from '../../styles/back-button.css'; +import { albumCardStyles } from '../../styles/album-card.css'; +import '../scroll-row/scroll-row.js'; import { LookupArtist, BrowseReleaseGroups, @@ -46,11 +48,8 @@ import { libraryStatusFor, toggleRequest, } from '@utils/library-status'; -import { - isOwned, - ownershipLabel, - unownedStyles, -} from '@utils/ownership'; +import { isOwned, ownershipLabel } from '@utils/ownership'; +import { openMusicBrainz } from '@utils/external-link'; import { completenessStore } from '@store/completeness-store'; import '../catalog-scope-notice/catalog-scope-notice.js'; import type { CatalogScope } from '../catalog-scope-notice/catalog-scope-notice.js'; @@ -62,6 +61,7 @@ import { ContextMenuController, contextMenuStyles, isContextMenuKey, + MenuKeyboard, } from '@utils/context-menu-controller.js'; import type { ContextMenuHost, MenuTarget } from '@utils/context-menu-controller.js'; import '@awesome.me/webawesome/dist/components/popup/popup.js'; @@ -187,11 +187,11 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost @state() private topReleasesExpanded = false; private topSectionStacked = false; private topSectionObserver?: ResizeObserver; - @state() private expandedDiscoGroups = new Set(); - /** Number of album cards that fit in one row of the discography grid. */ - @state() private discoRowSize = 5; - private discoObserver?: ResizeObserver; - @state() private similarExpanded = false; + + /** Whether the Play button's Shuffle dropdown is up. */ + @state() private playMenuOpen = false; + private playMenuKeyboard = new MenuKeyboard(() => this.closePlayMenu()); + private playOutsideAttached = false; /* ── Release prefetch ── */ @@ -221,6 +221,12 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost @query('#context-menu') private contextMenuPopup!: MenuSurface; + @query('.play-menu-button') + private playMenuButton?: HTMLButtonElement; + + @query('#artist-play-menu') + private playMenuPanel?: HTMLElement; + @query('#playlist-submenu') private playlistSubmenuPopup?: WaPopup; @@ -271,7 +277,7 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost backButton, exploreLinkStyles, contextMenuStyles, - unownedStyles, + albumCardStyles, css` :host { display: flex; @@ -319,10 +325,45 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost object-fit: cover; } - .artist-follow { + .artist-actions { + display: flex; + align-items: center; + gap: 8px; + flex-wrap: wrap; margin-top: 10px; } + /* The Play button and its caret are one control, so they + are one box: no gap between them, and the caret carries + the same filled appearance as the button it extends. */ + .play-split { + display: inline-flex; + align-items: stretch; + } + + .play-menu-button { + display: inline-flex; + align-items: center; + justify-content: center; + width: 28px; + padding: 0; + border: none; + border-left: 1px solid rgba(0, 0, 0, 0.25); + border-radius: 0 6px 6px 0; + background: var(--yj-accent, #ffd43b); + color: var(--yj-accent-fg, #000); + cursor: pointer; + } + + .play-menu-button:hover { + filter: brightness(1.1); + } + + .play-menu-button:focus-visible { + outline: 2px solid var(--yj-accent-text, #ffd43b); + outline-offset: 2px; + } + .artist-info { display: flex; flex-direction: column; @@ -331,7 +372,7 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost } .artist-title { - font-size: 24px; + font-size: 28px; font-weight: 700; color: var(--yj-text-primary, #fff); white-space: nowrap; @@ -359,6 +400,13 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost flex-wrap: wrap; } + /* The listen count is a headline number, not metadata, so + it sits a size above the type/country line. */ + .artist-listens { + font-size: var(--yj-text-lg); + color: var(--yj-text-secondary, #b3b3b3); + } + .meta-separator { opacity: 0.4; } @@ -446,23 +494,6 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost outline-offset: -2px; } - .artist-play-actions { - margin-top: 10px; - display: flex; - gap: 8px; - align-items: center; - flex-wrap: wrap; - } - - .track-rank { - width: 24px; - text-align: right; - color: var(--yj-text-tertiary, #888); - font-size: var(--yj-text-md); - font-variant-numeric: tabular-nums; - flex-shrink: 0; - } - .track-art { width: 32px; height: 32px; @@ -489,6 +520,52 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost opacity: 0.5; } + /* Play where you own the track, the request badge where you + do not — over the artwork rather than at the end of the + row, where it was a badge beside a row you can already + double-click. */ + .track-art-overlay { + position: absolute; + inset: 0; + display: flex; + align-items: center; + justify-content: center; + border-radius: 4px; + background: rgba(0, 0, 0, 0.55); + visibility: hidden; + opacity: 0; + transition: opacity 0.15s ease, visibility 0.15s ease; + } + + .track-art-play { + display: flex; + align-items: center; + justify-content: center; + padding: 0; + border: none; + background: none; + color: #fff; + font-size: 14px; + cursor: pointer; + } + + @media (hover: hover) and (pointer: fine) { + .track-item:hover .track-art-overlay, + .track-item:focus-within .track-art-overlay { + visibility: visible; + opacity: 1; + } + } + + /* No hover means no double-click either, so the overlay is + the only route to playing a top track and must be there. */ + @media not all and (hover: hover) { + .track-art-overlay { + visibility: visible; + opacity: 1; + } + } + .track-info { flex: 1; min-width: 0; @@ -522,7 +599,6 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost .track-item library-status-indicator { flex-shrink: 0; } - /* ── Top section (tracks + releases side-by-side) ── */ .top-section-wrapper { container-type: inline-size; @@ -754,8 +830,30 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost white-space: nowrap; } - .top-release-meta library-status-indicator { - flex-shrink: 0; + .top-release-art .album-card-badge { + position: absolute; + top: 4px; + left: 4px; + z-index: 1; + display: flex; + visibility: hidden; + opacity: 0; + transition: opacity 0.15s ease, visibility 0.15s ease; + } + + @media (hover: hover) and (pointer: fine) { + .top-release-card:hover .album-card-badge, + .top-release-card:focus-within .album-card-badge { + visibility: visible; + opacity: 1; + } + } + + @media not all and (hover: hover) { + .top-release-art .album-card-badge { + visibility: visible; + opacity: 1; + } } @@ -773,150 +871,7 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost margin: 0; } - .album-grid { - display: grid; - grid-template-columns: repeat(auto-fill, 140px); - gap: 16px; - } - - .album-grid.collapsed { - grid-template-rows: 1fr; - overflow: hidden; - } - - .disco-toggle { - display: flex; - align-items: center; - justify-content: center; - gap: 6px; - padding: 4px 10px; - margin-top: 4px; - border: none; - border-radius: 6px; - background: var(--yj-bg-overlay, rgba(255, 255, 255, 0.06)); - color: var(--yj-text-secondary, #b3b3b3); - font-size: var(--yj-text-xs); - cursor: pointer; - transition: background 0.15s ease, color 0.15s ease; - width: 100%; - } - - .disco-toggle:hover { - background: var(--yj-bg-hover, rgba(255, 255, 255, 0.1)); - color: var(--yj-text-primary, #fff); - } - - .disco-toggle wa-icon { - font-size: 11px; - transition: transform 0.2s ease; - } - - .disco-toggle[aria-expanded='true'] wa-icon { - transform: rotate(180deg); - } - - .album-card { - display: flex; - flex-direction: column; - gap: 6px; - padding: 8px; - border-radius: 8px; - cursor: pointer; - transition: background 0.15s ease; - } - - .album-card:hover { - background: var( - --yj-bg-overlay, - rgba(255, 255, 255, 0.06) - ); - } - - .album-card:active { - transform: scale(0.97); - } - - .album-art-container { - width: 100%; - aspect-ratio: 1; - border-radius: 4px; - overflow: hidden; - flex-shrink: 0; - position: relative; - background: var(--yj-bg-overlay, rgba(255, 255, 255, 0.06)); - } - - .album-art-container img { - width: 100%; - height: 100%; - object-fit: cover; - display: block; - border-radius: 4px; - } - - .album-art-fallback { - display: flex; - align-items: center; - justify-content: center; - width: 100%; - height: 100%; - position: absolute; - inset: 0; - } - - .album-art-fallback wa-icon { - color: var(--yj-text-tertiary, #888); - font-size: 24px; - opacity: 0.5; - } - - .album-title { - font-weight: 500; - color: var(--yj-text-primary, #fff); - font-size: var(--yj-text-sm); - white-space: nowrap; - overflow: hidden; - text-overflow: ellipsis; - } - - .album-meta { - display: flex; - align-items: center; - justify-content: space-between; - gap: 6px; - color: var(--yj-text-tertiary, #888); - font-size: var(--yj-text-xs); - min-height: 20px; - } - - .album-meta-text { - display: flex; - align-items: center; - gap: 6px; - min-width: 0; - overflow: hidden; - text-overflow: ellipsis; - white-space: nowrap; - } - - .album-meta library-status-indicator { - flex-shrink: 0; - margin-left: auto; - } - /* ── Similar artists ── */ - .similar-row { - display: grid; - grid-template-columns: repeat(auto-fill, 140px); - gap: 16px; - overflow: hidden; - } - - .similar-row.collapsed { - grid-template-rows: 1fr; - overflow: hidden; - } - .similar-artist-card { display: flex; flex-direction: column; @@ -926,6 +881,9 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost border-radius: 8px; cursor: pointer; text-align: center; + width: 120px; + box-sizing: border-box; + flex-shrink: 0; transition: background 0.15s ease; } @@ -1056,7 +1014,7 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost this.unsubSimilarReady?.(); if (this.discogFallbackTimer) clearTimeout(this.discogFallbackTimer); this.topSectionObserver?.disconnect(); - this.discoObserver?.disconnect(); + this.detachPlayOutsideClose(); } /** @@ -1083,17 +1041,14 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost protected override firstUpdated() { this.observeTopSectionWidth(); - this.observeDiscoWidth(); } protected override updated() { - // Re-attach observers if elements appeared after initial render. + // Re-attach the observer if the section appeared after initial + // render. if (!this.topSectionObserver) { this.observeTopSectionWidth(); } - if (!this.discoObserver) { - this.observeDiscoWidth(); - } } /** @@ -1126,32 +1081,6 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost this.topSectionObserver.observe(wrapper); } - /** - * Watch the .content width and compute how many album cards - * fit in one row of the discography grid. - * Grid uses: repeat(auto-fill, minmax(140px, 1fr)) with 16px gap - * and album-card has 8px padding on each side. - */ - private observeDiscoWidth() { - const content = this.renderRoot.querySelector('.content'); - if (!content) return; - - const CARD_MIN = 140; - const GAP = 16; - - this.discoObserver = new ResizeObserver((entries) => { - for (const entry of entries) { - const width = entry.contentBoxSize?.[0]?.inlineSize ?? entry.contentRect.width; - const cols = Math.max(1, Math.floor((width + GAP) / (CARD_MIN + GAP))); - if (cols !== this.discoRowSize) { - this.discoRowSize = cols; - } - } - }); - - this.discoObserver.observe(content); - } - /* ── Data Loading ── */ private async loadAllData() { @@ -1862,6 +1791,8 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost } catch { // No image — letter avatar stays. } + + return undefined; }), ); } @@ -1999,6 +1930,65 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost } } + /* ── Play / Shuffle split button ── */ + + /** + * Open the Play button's Shuffle dropdown. + * + * `page-header`'s overflow menu one control over: the same + * `MenuKeyboard`, the same document-level outside-close, and the + * same `menu-surface`, so the phone gets the bottom sheet rather + * than a popup that Chrome 113 clips. + */ + private togglePlayMenu = (): void => { + if (this.playMenuOpen) { + this.closePlayMenu(); + + return; + } + + this.playMenuOpen = true; + + void this.updateComplete.then(() => { + if (!this.playMenuOpen) return; + + this.playMenuKeyboard.open( + this.playMenuPanel ?? null, + this.playMenuButton ?? null, + ); + this.attachPlayOutsideClose(); + }); + }; + + private closePlayMenu = (): void => { + if (!this.playMenuOpen) return; + + this.detachPlayOutsideClose(); + this.playMenuKeyboard.close(); + this.playMenuOpen = false; + }; + + private onPlayOutsideDown = (e: Event): void => { + if (e.composedPath().includes(this.playMenuPanel as EventTarget)) return; + if (e.composedPath().includes(this.playMenuButton as EventTarget)) return; + + this.closePlayMenu(); + }; + + private attachPlayOutsideClose(): void { + if (this.playOutsideAttached) return; + + this.playOutsideAttached = true; + document.addEventListener('mousedown', this.onPlayOutsideDown, true); + } + + private detachPlayOutsideClose(): void { + if (!this.playOutsideAttached) return; + + this.playOutsideAttached = false; + document.removeEventListener('mousedown', this.onPlayOutsideDown, true); + } + /** * File path for one top track, resolved by recording MBID — the * same key `localId` was set from. Works whether or not the @@ -2240,11 +2230,7 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost if (!release?.mbid) return; - window.open( - `https://musicbrainz.org/release-group/${release.mbid}`, - '_blank', - 'noopener', - ); + openMusicBrainz(`/release-group/${release.mbid}`); } private onContextMenuAction( @@ -2358,7 +2344,7 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost if (!track?.recordingMbid) return; - window.open(`https://musicbrainz.org/recording/${track.recordingMbid}`, '_blank', 'noopener'); + openMusicBrainz(`/recording/${track.recordingMbid}`); } /* ── Navigation ── */ @@ -2538,10 +2524,12 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost : nothing} ${this.renderArtistMeta()} ${this.artist?.popularity && this.artist.popularity > 0 - ? html`${formatListenCount(this.artist.popularity)} plays on ListenBrainz` + ? html`${formatListenCount(this.artist.popularity)} plays on ListenBrainz` : nothing} - ${this.renderPlayLibraryAction()} - ${this.renderFollowAction()} +
+ ${this.renderPlayLibraryAction()} + ${this.renderFollowAction()} +
@@ -2572,25 +2560,53 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost if (this.ownedLocalAlbumIds().length === 0) return nothing; return html` -
+
void this.playLibraryTracks(false)} > - Play library tracks + Play - void this.playLibraryTracks(true)} + - - Shuffle - + + +
`; } @@ -2786,25 +2802,27 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost const request = downloadStore.requestFor(this.artistMBID); return html` -
- void this.toggleFollow(request?.id)} - > - - - ${request ? 'Following' : 'Follow for new releases'} - -
+ void this.toggleFollow(request?.id)} + > + + + ${request ? 'Following' : 'Follow'} + `; } @@ -2888,16 +2906,6 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost this.topReleasesExpanded = !this.topReleasesExpanded; } - private toggleDiscoGroup(type: string) { - const next = new Set(this.expandedDiscoGroups); - if (next.has(type)) { - next.delete(type); - } else { - next.add(type); - } - this.expandedDiscoGroups = next; - } - private renderTopSection() { const hasTracks = !this.loadingTracks && this.topTracks.length > 0; const hasReleases = !this.loadingTopReleases && this.topReleaseGroups.length > 0; @@ -2971,6 +2979,31 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost }} />` : html``; })()} + +
+ ${owned + ? html`` + : html``} +
${trackLink(t.trackName, t.releaseName, t.releaseGroupMbid ?? '', t.recordingMbid)}
@@ -2979,15 +3012,6 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost ${formatListenCount(t.totalListenCount)} plays - ${owned - ? nothing - : html``}
`; })} @@ -3093,6 +3117,18 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
+
+ +
@@ -3102,18 +3138,6 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
${rg.date ? html`${extractYear(rg.date)}` : nothing}
- ${badge.status === 'in-library' - ? nothing - : html``}
@@ -3164,37 +3188,16 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost

Discography

${groups.map( - (g) => { - const isExpanded = this.expandedDiscoGroups.has(g.type); - const rowSize = this.discoRowSize; - const showToggle = g.items.length > rowSize; - const visibleItems = isExpanded ? g.items : g.items.slice(0, rowSize); - - return html` -
-

- ${g.type === 'Other' ? 'Other Releases' : g.type.endsWith('s') ? g.type : `${g.type}s`} -

-
- ${visibleItems.map((rg) => this.renderAlbumCard(rg))} -
- ${showToggle - ? html` - - ` - : nothing} -
- `; - }, + (g) => html` +
+

+ ${g.type === 'Other' ? 'Other Releases' : g.type.endsWith('s') ? g.type : `${g.type}s`} +

+ + ${g.items.map((rg) => this.renderAlbumCard(rg))} + +
+ `, )}
`; @@ -3234,23 +3237,25 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
+
+ +
${rg.title}
+
${rg.artistCredit ?? ''}
${year ? html`${year}` : nothing}
- ${badge.status === 'in-library' - ? nothing - : html``}
`; @@ -3266,15 +3271,12 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost // Cap the similar-artists list at 10 to avoid a very long list. const maxSimilar = 10; const artists = this.similarArtists.slice(0, maxSimilar); - const showToggle = artists.length > this.discoRowSize; - const collapsed = !this.similarExpanded && showToggle; - const visible = collapsed ? artists.slice(0, this.discoRowSize) : artists; return html`

Similar Artists

-
- ${visible.map((a) => { + + ${artists.map((a) => { const imgURL = this.similarImageURLs.get(a.artistMbid); return html`
`; })} -
- ${showToggle - ? html` - - ` - : nothing} +
`; } diff --git a/frontend/src/components/explore-view/explore-view.ts b/frontend/src/components/explore-view/explore-view.ts index 4c73434..5c26994 100644 --- a/frontend/src/components/explore-view/explore-view.ts +++ b/frontend/src/components/explore-view/explore-view.ts @@ -1,10 +1,7 @@ import { avatarBackground } from '@utils/avatar-color'; import { albumBadgeFor, libraryStatusFor } from '@utils/library-status'; -import { - isOwned, - ownershipLabel, - unownedStyles, -} from '@utils/ownership'; +import { isOwned, ownershipLabel } from '@utils/ownership'; +import { openMusicBrainz } from '@utils/external-link'; import { completenessStore } from '@store/completeness-store'; import { downloadStore } from '@store/download-store'; import { LitElement, html, css, nothing } from 'lit'; @@ -13,6 +10,8 @@ import { classMap } from 'lit/directives/class-map.js'; import '@components/page-header/page-header'; import { designTokens } from '../../styles/tokens.css'; import { srOnly } from '../../styles/sr-only.css'; +import { albumCardStyles } from '../../styles/album-card.css'; +import '../scroll-row/scroll-row.js'; import { SearchLocal, SearchLyrics, GetThumbnail, GetThumbnails, GetArtistImageURL, GetArtistImagesCachedPaths, GetExploreShelves, RecordSearchClick } from '@go/explore/service.js'; import { GetFilePathsByAlbums, GetFilePathsByRecordingMBIDs } from '@go/library/library.js'; import { EventsOn } from '@runtime/runtime'; @@ -253,7 +252,7 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte srOnly, exploreLinkStyles, contextMenuStyles, - unownedStyles, + albumCardStyles, css` :host { display: block; @@ -529,21 +528,9 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte line-height: 1.5; } - /* ── Horizontal scroll rows ── */ - .horizontal-row { - display: flex; - gap: 12px; - overflow-x: auto; - padding-bottom: 4px; - scrollbar-width: none; - } - - .horizontal-row::-webkit-scrollbar { - display: none; - } - - /* ── Top result cards ── */ /* ── Artist cards ── */ + /* Fixed width, for the reason the album card is: a range + means two cards in one row are different sizes. */ .artist-card { display: flex; flex-direction: column; @@ -552,8 +539,8 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte padding: 10px; border-radius: 8px; cursor: pointer; - min-width: 100px; - max-width: 120px; + width: 120px; + box-sizing: border-box; flex-shrink: 0; text-align: center; transition: background 0.15s ease; @@ -624,115 +611,6 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte font-size: var(--yj-text-xs); } - /* ── Album cards ── */ - .album-card { - display: flex; - flex-direction: column; - gap: 6px; - padding: 8px; - border-radius: 8px; - cursor: pointer; - min-width: 130px; - max-width: 150px; - flex-shrink: 0; - transition: background 0.15s ease; - } - - .album-card:hover { - background: var(--yj-bg-overlay, rgba(255, 255, 255, 0.06)); - } - - .album-card:active { - transform: scale(0.97); - } - - .album-art-container { - width: 100%; - aspect-ratio: 1; - border-radius: 4px; - overflow: hidden; - background: linear-gradient( - 135deg, - var(--yj-bg-overlay, #404040) 0%, - var(--yj-bg-surface, #282828) 100% - ); - display: flex; - align-items: center; - justify-content: center; - position: relative; - } - - .album-art-container img { - width: 100%; - height: 100%; - object-fit: cover; - display: block; - } - - .album-art-fallback { - display: flex; - align-items: center; - justify-content: center; - width: 100%; - height: 100%; - position: absolute; - inset: 0; - } - - .album-art-fallback wa-icon { - color: var(--yj-text-tertiary, #888); - font-size: 24px; - opacity: 0.5; - } - - .album-title { - font-weight: 500; - color: var(--yj-text-primary, #fff); - font-size: var(--yj-text-sm); - white-space: nowrap; - overflow: hidden; - text-overflow: ellipsis; - } - - .album-artist { - color: var(--yj-text-tertiary, #888); - font-size: var(--yj-text-xs); - white-space: nowrap; - overflow: hidden; - text-overflow: ellipsis; - } - - .album-meta { - display: flex; - align-items: center; - justify-content: space-between; - gap: 6px; - color: var(--yj-text-tertiary, #888); - font-size: var(--yj-text-xs); - min-height: 20px; - } - - .album-meta-text { - display: flex; - align-items: center; - gap: 6px; - min-width: 0; - overflow: hidden; - } - - .album-meta library-status-indicator { - flex-shrink: 0; - margin-left: auto; - } - - .type-badge { - background: var(--yj-bg-overlay, rgba(255, 255, 255, 0.08)); - padding: 1px 6px; - border-radius: 3px; - font-size: 10px; - white-space: nowrap; - } - /* ── Track list ── */ .track-list { display: flex; @@ -753,7 +631,6 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte cursor: pointer; } - .album-card:focus-visible, .track-item:focus-visible { outline: 2px solid var(--yj-accent-text, #ffd43b); outline-offset: -2px; @@ -1371,7 +1248,7 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte const entity = target.kind === 'album' ? 'release-group' : 'recording'; - window.open(`https://musicbrainz.org/${entity}/${target.mbid}`, '_blank', 'noopener'); + openMusicBrainz(`/${entity}/${target.mbid}`); } private renderExploreContextMenu() { @@ -1662,6 +1539,8 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte } catch { // No image — leave empty string. } + + return undefined; }), ); @@ -2122,7 +2001,7 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte ${subtitle ? html`

${subtitle}

` : nothing} -
+ ${artists.map((a) => { const owned = isOwned(a); const name = a.englishName || a.name; @@ -2171,7 +2050,7 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
`; })} - + `; } @@ -2187,7 +2066,7 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte ${subtitle ? html`

${subtitle}

` : nothing} -
+ ${releaseGroups.map((rg) => { const artURL = this.thumbnailCache.get(rg.mbid) || ''; const year = extractYear(rg.firstReleaseDate); @@ -2249,6 +2128,18 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte >
+
+ +
${rg.title} @@ -2256,29 +2147,18 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
${creditLink(creditStore.credits(rg.mbid), rg.artistCredit, rg.artistMbid ?? '')}
+ ${year ? html`${year}` : nothing} ${rg.primaryType ? html`${rg.primaryType}` : nothing} - ${year ? html`${year}` : nothing}
- ${badge.status === 'in-library' - ? nothing - : html``}
`; })} - + `; } diff --git a/frontend/src/components/home-view/home-view.ts b/frontend/src/components/home-view/home-view.ts index aa31d96..a9289c5 100644 --- a/frontend/src/components/home-view/home-view.ts +++ b/frontend/src/components/home-view/home-view.ts @@ -12,6 +12,7 @@ import { libraryStore } from '@store/library-store'; import { EventsOn } from '@runtime/runtime'; import { Events } from '../../events'; import '@components/page-header/page-header'; +import '../scroll-row/scroll-row.js'; import { designTokens } from '../../styles/tokens.css'; import { ViewLifecycleMixin } from '../../utils/view-lifecycle'; @@ -99,16 +100,6 @@ export class HomeView extends ViewLifecycleMixin(LitElement) { color: var(--yj-text-tertiary, #888); } - .row { - display: grid; - grid-auto-flow: column; - grid-auto-columns: 160px; - gap: 14px; - overflow-x: auto; - padding-bottom: 6px; - scrollbar-width: thin; - } - .card { background: none; border: none; @@ -117,6 +108,8 @@ export class HomeView extends ViewLifecycleMixin(LitElement) { cursor: pointer; color: inherit; display: block; + width: 160px; + flex-shrink: 0; } .art { @@ -336,9 +329,9 @@ export class HomeView extends ViewLifecycleMixin(LitElement) { ${shelf.title}

${shelf.subtitle}

-
+ ${(shelf.albums ?? []).map((album) => this.renderCard(album))} -
+ `; } diff --git a/frontend/src/components/scroll-row/scroll-row.ts b/frontend/src/components/scroll-row/scroll-row.ts new file mode 100644 index 0000000..bcd6a64 --- /dev/null +++ b/frontend/src/components/scroll-row/scroll-row.ts @@ -0,0 +1,213 @@ +import { LitElement, css, html } from 'lit'; +import { customElement, query, state } from 'lit/decorators.js'; +import '@awesome.me/webawesome/dist/components/icon/icon.js'; + +/** How far one press moves the row — most of a screenful, not all of + * it, so the card that was at the edge stays as an anchor. */ +const SCROLL_FRACTION = 0.8; + +/** + * A horizontally scrolling row with arrow buttons. + * + * The shelves, the search results and (now) the artist page's + * discography and similar-artists rows are all "more than fits, scroll + * sideways". Until this existed the only way to see the rest was a + * mousewheel or a trackpad gesture, which is not an affordance — a + * mouse with no horizontal wheel simply could not reach the cards past + * the fold. + * + * It is a component rather than a rule on `.horizontal-row` for two + * reasons. The arrows are *state* — which way the row can still move — + * and that state has to be recomputed when the viewport resizes or a + * card arrives with its cover art; a stylesheet cannot do that. And + * every caller then gets the same arrows, the same reveal and the same + * keyboard labels without writing them again. + * + * **The arrows are `hidden`, not merely transparent, at the end they + * cannot move from** — a control that cannot act is worse than none, + * and an invisible one still holds a hit area and a tab stop. On a + * pointer device the pair fades in with the row's hover; where there is + * no hover they are always visible, because there is no other route to + * them there (a swipe is not an affordance a mouse-less keyboard user + * has either). + * + * The cards are light DOM children and stay in the *host's* shadow + * root, so the host's own `.album-card` / `.artist-card` styles apply + * unchanged — this component only owns the box they scroll inside. + */ +@customElement('scroll-row') +export class ScrollRow extends LitElement { + @query('.viewport') private viewport?: HTMLElement; + + @state() private atStart = true; + + @state() private atEnd = true; + + @state() private overflowing = false; + + private observer?: ResizeObserver; + + static override styles = css` + :host { + display: block; + position: relative; + } + + .viewport { + overflow-x: auto; + overflow-y: hidden; + scrollbar-width: none; + /* A swipe that reaches the row's end should not drag the + whole page sideways with it. */ + overscroll-behavior-x: contain; + } + + .viewport::-webkit-scrollbar { + display: none; + } + + .track { + display: flex; + gap: 12px; + } + + .arrow { + position: absolute; + top: 50%; + transform: translateY(-50%); + z-index: 2; + display: flex; + align-items: center; + justify-content: center; + width: 36px; + height: 36px; + padding: 0; + border-radius: 50%; + border: 1px solid var(--yj-border-subtle, rgba(255, 255, 255, 0.1)); + background: var(--yj-bg-elevated, #343a40); + color: var(--yj-text-primary, #fff); + cursor: pointer; + opacity: 0; + transition: opacity 0.15s ease; + } + + .arrow[hidden] { + display: none; + } + + .arrow.prev { + left: 4px; + } + + .arrow.next { + right: 4px; + } + + .arrow:hover { + background: var(--yj-bg-overlay, rgba(255, 255, 255, 0.12)); + } + + .arrow:focus-visible { + outline: 2px solid var(--yj-accent, #ffd43b); + outline-offset: 2px; + } + + @media (hover: hover) and (pointer: fine) { + :host(:hover) .arrow, + .arrow:focus-visible { + opacity: 1; + } + } + + @media not all and (hover: hover) { + .arrow { + opacity: 1; + } + } + `; + + override firstUpdated(): void { + const viewport = this.viewport; + + if (!viewport) return; + + this.observer = new ResizeObserver(() => this.measure()); + + this.observer.observe(viewport); + + // The track's own size is what changes when a card arrives with + // its cover art, and a ResizeObserver on the viewport alone + // never fires for that. + const track = viewport.firstElementChild; + + if (track) this.observer.observe(track); + + this.measure(); + } + + override disconnectedCallback(): void { + super.disconnectedCallback(); + this.observer?.disconnect(); + this.observer = undefined; + } + + private measure(): void { + const viewport = this.viewport; + + if (!viewport) return; + + this.overflowing = viewport.scrollWidth > viewport.clientWidth + 1; + this.atStart = viewport.scrollLeft <= 1; + this.atEnd = + viewport.scrollLeft + viewport.clientWidth >= + viewport.scrollWidth - 1; + } + + private onScroll = (): void => this.measure(); + + private scrollStep(direction: -1 | 1): void { + const viewport = this.viewport; + + if (!viewport) return; + + viewport.scrollBy({ + left: direction * viewport.clientWidth * SCROLL_FRACTION, + behavior: 'smooth', + }); + } + + override render() { + const showPrev = this.overflowing && !this.atStart; + const showNext = this.overflowing && !this.atEnd; + + return html` + +
+
+
+ + `; + } +} + +declare global { + interface HTMLElementTagNameMap { + 'scroll-row': ScrollRow; + } +} diff --git a/frontend/src/components/top-results-row/top-results-row.ts b/frontend/src/components/top-results-row/top-results-row.ts index 8f4cdd8..34fc376 100644 --- a/frontend/src/components/top-results-row/top-results-row.ts +++ b/frontend/src/components/top-results-row/top-results-row.ts @@ -1,6 +1,7 @@ import { LitElement, html, css, nothing } from 'lit'; import { customElement, property } from 'lit/decorators.js'; import { designTokens } from '../../styles/tokens.css'; +import '../scroll-row/scroll-row.js'; import type * as explore from '@go/explore/models.js'; import { GetArtistImageURL, @@ -15,7 +16,6 @@ import { albumBadgeFor, libraryStatusFor } from '../../utils/library-status'; import { isOwned, ownershipLabel, - unownedStyles, type OwnableKind, } from '../../utils/ownership'; import { completenessStore } from '../../store/completeness-store'; @@ -103,20 +103,12 @@ export class TopResultsRow extends LitElement { static override styles = [ designTokens, exploreLinkStyles, - unownedStyles, css` :host { display: block; margin-bottom: 16px; } - .row { - display: flex; - gap: 12px; - overflow-x: auto; - padding-bottom: 4px; - } - .card { flex: 0 0 auto; width: 200px; @@ -285,9 +277,9 @@ export class TopResultsRow extends LitElement { return html`
Top Results
-
+ ${this.results.map((r) => this.renderCard(r))} -
+ `; } diff --git a/frontend/src/icons/names.txt b/frontend/src/icons/names.txt index 6154848..d94ae1a 100644 --- a/frontend/src/icons/names.txt +++ b/frontend/src/icons/names.txt @@ -31,6 +31,7 @@ solid/bookmark solid/box-open solid/check solid/chevron-down +solid/chevron-left solid/chevron-right solid/circle-check solid/circle-exclamation diff --git a/frontend/src/styles/album-card.css.ts b/frontend/src/styles/album-card.css.ts new file mode 100644 index 0000000..a619dbd --- /dev/null +++ b/frontend/src/styles/album-card.css.ts @@ -0,0 +1,184 @@ +import { css } from 'lit'; + +/** + * The Explore album card, once. + * + * Two components draw one — `explore-view`'s shelves and search + * results, and `explore-artist-details`'s discography — and they had + * grown two copies of the same rules. That is how the size came apart: + * `explore-view` clamped its cards to a 130–150px range so two cards in + * one row could be different widths, and since the artwork is square + * that made them different *heights* as well. A row of covers with + * ragged bottoms is the whole complaint. + * + * So the width is a fixed `--yj-album-card-width` and the lines below + * the art each reserve their own space, which is what makes every card + * the same size no matter what a given album happens to carry — + * `album-card-size.test.ts` measures that rather than trusting it. + * + * Three rules here are the parts that changed rather than moved. + * + * **The artwork is inset in the square, not cropped to it.** The + * container was already `aspect-ratio: 1` but the image was + * `object-fit: cover`, so a non-square cover lost its edges. It is + * `contain` now and the container's own background is transparent, so + * a tall or wide cover sits in the middle of the square with the page + * showing through beside it. + * + * **The badge lives on the artwork, top-left, and only under the + * pointer.** It used to sit in the metadata line and only for the + * unowned case. It draws for every card now — an owned album's tick is + * the answer to the same question — and it is revealed by hover on a + * pointer device. Where there is no hover it is *always* visible rather + * than never, because on those devices it is the only route to its + * action: `explore-view`'s card menu carries no request item, so a + * phone with the badge hidden could not ask for an album at all. + * + * **Nothing dims an unowned card.** `unownedStyles` was removed from + * the catalog surfaces on the rule that the badge is the mark; the + * album page's *tracklist* still dims unowned rows, which is a + * different statement about a different thing. + */ +export const albumCardStyles = css` + .album-card { + width: var(--yj-album-card-width, 150px); + display: flex; + flex-direction: column; + gap: 6px; + padding: 8px; + border-radius: 8px; + box-sizing: border-box; + flex-shrink: 0; + cursor: pointer; + transition: background 0.15s ease; + } + + .album-card:hover { + background: var(--yj-bg-overlay, rgba(255, 255, 255, 0.06)); + } + + .album-card:active { + transform: scale(0.97); + } + + .album-card:focus-visible { + outline: 2px solid var(--yj-accent-text, #ffd43b); + outline-offset: -2px; + } + + .album-art-container { + position: relative; + width: 100%; + aspect-ratio: 1; + border-radius: 4px; + overflow: hidden; + background: transparent; + display: flex; + align-items: center; + justify-content: center; + } + + .album-art-container img { + width: 100%; + height: 100%; + object-fit: contain; + display: block; + } + + /* The placeholder is the one case that *is* a full square, so it + carries the background the container gave up. */ + .album-art-fallback { + display: flex; + align-items: center; + justify-content: center; + width: 100%; + height: 100%; + position: absolute; + inset: 0; + background: linear-gradient( + 135deg, + var(--yj-bg-overlay, #404040) 0%, + var(--yj-bg-surface, #282828) 100% + ); + } + + .album-art-fallback wa-icon { + color: var(--yj-text-tertiary, #888); + font-size: 24px; + opacity: 0.5; + } + + .album-card-badge { + position: absolute; + top: 6px; + left: 6px; + z-index: 1; + display: flex; + visibility: hidden; + opacity: 0; + transition: opacity 0.15s ease, visibility 0.15s ease; + } + + @media (hover: hover) and (pointer: fine) { + .album-card:hover .album-card-badge, + .album-card:focus-within .album-card-badge { + visibility: visible; + opacity: 1; + } + } + + @media not all and (hover: hover) { + .album-card-badge { + visibility: visible; + opacity: 1; + } + } + + .album-title { + font-weight: 500; + color: var(--yj-text-primary, #fff); + font-size: var(--yj-text-sm); + line-height: 1.3; + white-space: nowrap; + overflow: hidden; + text-overflow: ellipsis; + } + + /* Reserved even where a surface has no artist to draw, so a card + in a row is never shorter than its neighbour. */ + .album-artist { + color: var(--yj-text-tertiary, #888); + font-size: var(--yj-text-xs); + line-height: 1.3; + min-height: 1.3em; + white-space: nowrap; + overflow: hidden; + text-overflow: ellipsis; + } + + .album-meta { + display: flex; + align-items: center; + justify-content: space-between; + gap: 6px; + color: var(--yj-text-tertiary, #888); + font-size: var(--yj-text-xs); + height: 20px; + } + + .album-meta-text { + display: flex; + align-items: center; + gap: 6px; + min-width: 0; + overflow: hidden; + } + + .type-badge { + background: var(--yj-bg-overlay, rgba(255, 255, 255, 0.08)); + padding: 1px 6px; + border-radius: 3px; + font-size: 10px; + white-space: nowrap; + } +`; diff --git a/frontend/src/utils/external-link.ts b/frontend/src/utils/external-link.ts new file mode 100644 index 0000000..0bb6b42 --- /dev/null +++ b/frontend/src/utils/external-link.ts @@ -0,0 +1,29 @@ +/** + * Opening an external page, with the destination pinned. + * + * Every external link this app opens is a MusicBrainz entity page built + * from an MBID that came from the catalog. Constructing the URL by + * string concatenation leaves the destination to whatever is in that + * string, so this parses it against the one origin the app means and + * refuses anything else — an MBID cannot change the host, and if it + * somehow did, nothing would open. + * + * It navigates through a real anchor rather than `window.open`: the + * same top-level `_blank` navigation with `noopener`, and it keeps the + * destination an ordinary link rather than an argument to a function + * whose first parameter is a URL. + */ +const MUSICBRAINZ_ORIGIN = 'https://musicbrainz.org'; + +export function openMusicBrainz(path: string): void { + const url = new URL(path, MUSICBRAINZ_ORIGIN); + + if (url.origin !== MUSICBRAINZ_ORIGIN) return; + + const link = document.createElement('a'); + + link.href = url.toString(); + link.target = '_blank'; + link.rel = 'noopener noreferrer'; + link.click(); +} diff --git a/frontend/src/utils/ownership.ts b/frontend/src/utils/ownership.ts index 04b4ff9..791c695 100644 --- a/frontend/src/utils/ownership.ts +++ b/frontend/src/utils/ownership.ts @@ -10,6 +10,16 @@ * badge as the only difference. This is that rule, written once, so * eight surfaces cannot each keep their own version of it. * + * **The catalog's *cards* no longer dim.** A grid of dimmed covers read + * as a page that had failed to load rather than as a page of things you + * could ask for, so on Explore the mark is the badge alone — over the + * artwork, on hover, drawn for owned and unowned alike. The album + * page's *tracklist* still dims unowned rows: that is a different + * statement ("this one is not here") about a different thing, and the + * `aria-disabled` row that cannot be played is what it is for. So + * `unownedStyles` survives for that one surface and the cards simply do + * not include it. + * * ## Ownership is a file, and `localId` is the flag that says so * * The album page answers "do I own this row" with `filePaths`, a map @@ -96,7 +106,8 @@ export function ownershipLabel( } /** - * The dimming, shared so it cannot drift across surfaces. + * The dimming, shared so it cannot drift across surfaces — and now + * used by exactly one of them. * * Two things about it are load-bearing. * diff --git a/frontend/test/components/album-card-size.test.ts b/frontend/test/components/album-card-size.test.ts new file mode 100644 index 0000000..0ff9355 --- /dev/null +++ b/frontend/test/components/album-card-size.test.ts @@ -0,0 +1,174 @@ +/** + * Every album card is the same size, and its artwork is a square. + * + * The size came apart because `explore-view` clamped its cards to a + * 130–150px range, so two cards in one row could be different widths — + * and since the artwork is square, different *heights* as well. A row + * of covers with ragged bottoms is what that looks like. + * + * What makes the fix hold is that the lines below the art each reserve + * their own space (`album-card.css.ts`), so an album with no year, no + * release type or a one-character title is not shorter than its + * neighbour. This measures that rather than trusting it, because the + * next component to format a card is the way it comes back. + * + * The artwork half is the other change: the container was already + * square but the image was `object-fit: cover`, so a non-square cover + * was cropped to it. It is `contain` now, and the container has no + * background of its own, so a tall cover is inset with the page + * showing through beside it. + */ +import { beforeEach, describe, expect, it } from 'vitest'; +import type { LitElement } from 'lit'; + +import '@components/explore-view/explore-view'; +import { flush, stub, resetHarness } from '@test/support/harness'; +import { fixture, shadow, shadowAll, update } from '@test/support/render'; +import { completenessStore } from '@store/completeness-store'; + +const SEARCH = 'explore.Service.SearchLocal'; +const SHELVES = 'explore.Service.GetExploreShelves'; + +/** A 1x1 transparent gif, so the `` branch renders. */ +const TINY_IMAGE = + 'data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7'; + +/** Release groups chosen so every optional line is present on one and + * absent on another — that is what a size regression hides behind. */ +const ALBUMS = [ + { + mbid: 'rg-1', + title: 'A', + artistCredit: '', + artistMbid: 'ar-1', + primaryType: '', + firstReleaseDate: '', + popularity: 1, + listenerCount: 1, + secondaryTypes: [], + inLibrary: false, + localId: 0, + }, + { + mbid: 'rg-2', + title: 'A Very Long Album Name That Will Certainly Be Truncated By The Card', + artistCredit: 'An Artist With A Long Name', + artistMbid: 'ar-2', + primaryType: 'Album', + firstReleaseDate: '1994-05-01', + popularity: 1, + listenerCount: 1, + secondaryTypes: [], + inLibrary: false, + localId: 0, + }, + { + mbid: 'rg-3', + title: 'Three', + artistCredit: 'Another', + artistMbid: 'ar-3', + primaryType: 'EP', + firstReleaseDate: '2001-01-01', + popularity: 1, + listenerCount: 1, + secondaryTypes: [], + inLibrary: false, + localId: 0, + }, +]; + +async function exploreWithAlbums(): Promise { + stub(SHELVES, { shelves: [], state: 'ready' }); + stub(SEARCH, { + artists: [], + releaseGroups: ALBUMS, + recordings: [], + }); + stub('explore.Service.GetThumbnails', Object.fromEntries( + ALBUMS.map((a) => [a.mbid, TINY_IMAGE]), + )); + stub('explore.Service.GetThumbnail', TINY_IMAGE); + + const el = await fixture('explore-view'); + + (el as unknown as { onViewActivate: () => void }).onViewActivate?.(); + await update(el, { + results: { artists: [], releaseGroups: ALBUMS, recordings: [] }, + }); + await flush(); + await el.updateComplete; + + return el; +} + +beforeEach(() => { + resetHarness(); + stub('library.Library.GetAlbumsCompleteness', {}); + completenessStore.invalidate(); +}); + +describe('the album card size', () => { + it('is the same width and height for every card in a row', async () => { + const el = await exploreWithAlbums(); + const cards = shadowAll(el, '.album-card'); + + expect(cards.length).toBe(ALBUMS.length); + + const boxes = cards.map((c) => c.getBoundingClientRect()); + + // The first card is the reference; every other one must match it. + for (const box of boxes) { + expect(box.width).toBe(boxes[0]!.width); + expect(box.height).toBe(boxes[0]!.height); + } + + // …and the reference is a real box, or the loop above is vacuous. + expect(boxes[0]!.width).toBeGreaterThan(0); + expect(boxes[0]!.height).toBeGreaterThan(0); + }); + + it('keeps the artwork square', async () => { + const el = await exploreWithAlbums(); + + for (const art of shadowAll(el, '.album-art-container')) { + const box = art.getBoundingClientRect(); + + expect(Math.round(box.width)).toBe(Math.round(box.height)); + } + }); + + it('insets a non-square cover rather than cropping it', async () => { + const el = await exploreWithAlbums(); + + // Read from the parsed stylesheet rather than from a rendered + // ``: the search path is what calls `loadThumbnails`, and + // setting `results` directly skips it, so there is no image to + // measure. The regression worth catching is the rule going back to + // `cover`, which is a stylesheet fact. + const rules = (el.shadowRoot?.adoptedStyleSheets ?? []).flatMap((sheet) => + Array.from(sheet.cssRules).map((rule) => rule.cssText), + ); + const art = rules.find( + (text) => + text.startsWith('.album-art-container img') && + text.includes('object-fit'), + ); + + expect(art, 'no object-fit rule for the cover image').toBeDefined(); + expect(art).toContain('object-fit: contain'); + }); + + it('draws the badge over the artwork, and not in the metadata line', async () => { + const el = await exploreWithAlbums(); + const card = shadow(el, '.album-card')!; + + const badge = card.querySelector('.album-art-container .album-card-badge'); + + expect(badge).not.toBeNull(); + // The badge is positioned inside the art box, so its parent is the + // square rather than the row underneath it. + expect(badge?.parentElement?.classList.contains('album-art-container')).toBe( + true, + ); + }); +}); diff --git a/frontend/test/components/artist-header.test.ts b/frontend/test/components/artist-header.test.ts new file mode 100644 index 0000000..c6a642b --- /dev/null +++ b/frontend/test/components/artist-header.test.ts @@ -0,0 +1,168 @@ +/** + * The artist page's header and its top tracks. + * + * Two cleanups, asserted together because they are one screen: + * + * - the Play/Shuffle pair became one split button ("Play" with the + * words on its title, Shuffle behind the caret), the Follow button + * moved onto the same line, and the name and listen count went up a + * size; + * - a top track's play/request affordance moved onto its artwork, + * where a hover reveals it, instead of a badge at the end of the + * row beside a row that already plays on a double-click. + */ +import { describe, expect, it, beforeEach } from 'vitest'; +import type { LitElement } from 'lit'; + +import '@components/explore-artist-details/explore-artist-details'; +import { stub, flush, emit, resetHarness } from '@test/support/harness'; +import { Events } from '../../src/events'; +import { fixture, shadow, shadowAll } from '@test/support/render'; + +const ARTIST = 'artist-0001'; + +const track = (name: string, localId = 0) => ({ + recordingMbid: `rec-${name}`, + artistName: 'Tideline', + trackName: name, + totalListenCount: 100, + caaReleaseMbid: '', + releaseName: 'Foreshore', + releaseGroupMbid: 'rg-owned', + length: 200000, + inLibrary: localId > 0, + localId, +}); + +beforeEach(() => { + resetHarness(); + + stub('explore.Service.LookupArtist', { + mbid: ARTIST, + name: 'Tideline', + popularity: 1200, + type: 'Group', + country: 'GB', + }); + stub('explore.Service.TopReleaseGroupsForArtist', []); + stub('explore.Service.TopRecordingsForArtist', [ + track('Owned Song', 7), + track('Absent Song'), + ]); + stub('explore.Service.SimilarArtists', []); + stub('explore.Service.PrefetchReleases', undefined); + stub('explore.Service.BrowseReleaseGroups', [ + { + mbid: 'rg-owned', + title: 'Foreshore', + artistCredit: 'Tideline', + primaryType: 'Album', + inLibrary: true, + localId: 7, + }, + ]); + stub('library.Library.GetAlbumsCompleteness', {}); + stub('download.Service.ListRequests', []); +}); + +async function mount(): Promise { + const el = await fixture('explore-artist-details', { + artistMBID: ARTIST, + artistName: 'Tideline', + }); + + await flush(); + + return el; +} + +describe('the artist header', () => { + it('offers Play, with Shuffle behind its caret', async () => { + const el = await mount(); + const play = shadow(el, '[data-testid="artist-play-library"]')!; + + // The words moved to the title, which is where "Play library + // tracks" can still be read without taking the width of a button. + expect(play.textContent?.trim()).toBe('Play'); + expect(play.getAttribute('title')).toBe('Play library tracks'); + + const menuButton = shadow(el, '[data-testid="artist-play-menu"]'); + + expect(menuButton).not.toBeNull(); + + const menu = shadow(el, '#artist-play-menu'); + + expect(menu?.textContent).toContain('Shuffle'); + }); + + it('puts Follow on the same line as Play', async () => { + const el = await mount(); + const actions = shadow(el, '.artist-actions')!; + + expect(actions.querySelector('[data-testid="artist-play-library"]')).not.toBeNull(); + + const follow = actions.querySelector('[data-testid="artist-follow"]') as HTMLElement; + + expect(follow).not.toBeNull(); + expect(follow.textContent?.trim()).toBe('Follow'); + }); + + it('says Following once the artist is on the request list', async () => { + const el = await mount(); + + // The store is a singleton and caches its list, so the change is + // announced the way the backend announces one. + stub('download.Service.ListRequests', [ + { id: 3, mbid: ARTIST, state: 'queued' }, + ]); + emit(Events.RequestsChanged); + await flush(); + await el.updateComplete; + + const follow = shadow(el, '[data-testid="artist-follow"]')!; + + expect(follow.textContent?.trim()).toBe('Following'); + }); + + it('sizes the name and the listen count above the metadata line', async () => { + const el = await mount(); + + const title = shadow(el, '.artist-title')!; + const listens = shadow(el, '.artist-listens')!; + const meta = shadow(el, '.artist-meta')!; + + expect(listens.textContent).toContain('plays on ListenBrainz'); + + const titleSize = parseFloat(getComputedStyle(title).fontSize); + const listensSize = parseFloat(getComputedStyle(listens).fontSize); + const metaSize = parseFloat(getComputedStyle(meta).fontSize); + + expect(titleSize).toBeGreaterThan(24); + expect(listensSize).toBeGreaterThan(metaSize); + }); +}); + +describe('a top track’s affordance', () => { + it('plays from the artwork when it is owned', async () => { + const el = await mount(); + const rows = shadowAll(el, '.track-item'); + + const owned = rows.find((r) => r.textContent?.includes('Owned Song'))!; + + expect(owned.querySelector('.track-art-overlay .track-art-play')).not.toBeNull(); + // Nothing beside the row any more. + expect(owned.querySelector(':scope > library-status-indicator')).toBeNull(); + }); + + it('requests from the artwork when it is not', async () => { + const el = await mount(); + const rows = shadowAll(el, '.track-item'); + + const absent = rows.find((r) => r.textContent?.includes('Absent Song'))!; + + expect( + absent.querySelector('.track-art-overlay library-status-indicator'), + ).not.toBeNull(); + expect(absent.querySelector('.track-art-overlay .track-art-play')).toBeNull(); + }); +}); diff --git a/frontend/test/components/artist-release-menu.test.ts b/frontend/test/components/artist-release-menu.test.ts index cb5ed00..1d62e7e 100644 --- a/frontend/test/components/artist-release-menu.test.ts +++ b/frontend/test/components/artist-release-menu.test.ts @@ -25,7 +25,9 @@ const ARTIST = 'artist-0001'; /** The labels of the open menu's items, trimmed. */ function menuItems(el: LitElement): string[] { - const panel = shadow(el, '.context-menu-panel'); + // Scoped to the context menu: the artist page also has a Play/Shuffle + // dropdown, and its panel carries the same class. + const panel = shadow(el, '#context-menu .context-menu-panel'); if (!panel) return []; @@ -100,7 +102,7 @@ describe('the context menu on an artist page release', () => { await openMenuOnAlbum(el, 0); - const panel = shadow(el, '.context-menu-panel'); + const panel = shadow(el, '#context-menu .context-menu-panel'); expect(panel).toBeTruthy(); // The panel is shared with the track menu, so a label that does not diff --git a/frontend/test/components/scroll-row.test.ts b/frontend/test/components/scroll-row.test.ts new file mode 100644 index 0000000..05ac8bb --- /dev/null +++ b/frontend/test/components/scroll-row.test.ts @@ -0,0 +1,131 @@ +/** + * A horizontally scrolling row can be moved without a wheel. + * + * Until this existed the only way to see the cards past the fold on the + * shelves, the search results and the artist page's discography was a + * mousewheel or a trackpad gesture — which is not an affordance. A + * mouse with no horizontal wheel simply could not reach them. + * + * What is asserted here is the state that makes the arrows honest: an + * arrow is `hidden` at the end it cannot move from, because a control + * that cannot act is worse than none, and an invisible one still holds + * a hit area and a tab stop. + */ +import { beforeEach, describe, expect, it } from 'vitest'; +import type { LitElement } from 'lit'; + +import '@components/scroll-row/scroll-row'; +import { fixture } from '@test/support/render'; + +/** Six 100px cards in a 320px row — comfortably overflowing. */ +function content(el: Element): void { + for (let i = 0; i < 6; i += 1) { + const card = document.createElement('div'); + + card.style.cssText = 'flex: 0 0 100px; height: 40px'; + card.textContent = String(i); + el.append(card); + } +} + +function arrows(el: LitElement): { prev: HTMLButtonElement; next: HTMLButtonElement } { + const root = el.shadowRoot!; + + return { + prev: root.querySelector('.arrow.prev') as HTMLButtonElement, + next: root.querySelector('.arrow.next') as HTMLButtonElement, + }; +} + +function viewport(el: LitElement): HTMLElement { + return el.shadowRoot!.querySelector('.viewport') as HTMLElement; +} + +async function row(): Promise { + const el = await fixture('scroll-row'); + + el.style.display = 'block'; + el.style.width = '320px'; + content(el); + await el.updateComplete; + // The observer reports on a later frame than a microtask drain. + await new Promise((r) => setTimeout(r, 60)); + await el.updateComplete; + + return el; +} + +describe('', () => { + beforeEach(() => { + document.body.style.margin = '0'; + }); + + it('draws an arrow for each direction it can still move', async () => { + const el = await row(); + const { prev, next } = arrows(el); + + expect(prev).not.toBeNull(); + expect(next).not.toBeNull(); + + // At the start there is nothing behind, so only the forward arrow is + // offered. + expect(prev.hasAttribute('hidden')).toBe(true); + expect(next.hasAttribute('hidden')).toBe(false); + }); + + it('offers the way back once the row has moved', async () => { + const el = await row(); + const vp = viewport(el); + + vp.scrollLeft = 120; + vp.dispatchEvent(new Event('scroll')); + await el.updateComplete; + + expect(arrows(el).prev.hasAttribute('hidden')).toBe(false); + }); + + it('stands the forward arrow down at the end', async () => { + const el = await row(); + const vp = viewport(el); + + vp.scrollLeft = vp.scrollWidth; + vp.dispatchEvent(new Event('scroll')); + await el.updateComplete; + + expect(arrows(el).next.hasAttribute('hidden')).toBe(true); + expect(arrows(el).prev.hasAttribute('hidden')).toBe(false); + }); + + it('moves the row when the arrow is pressed', async () => { + const el = await row(); + const vp = viewport(el); + + expect(vp.scrollLeft).toBe(0); + + arrows(el).next.click(); + + await expect.poll(() => vp.scrollLeft).toBeGreaterThan(0); + }); + + it('shows nothing to scroll when the content fits', async () => { + const el = await fixture('scroll-row'); + + el.style.cssText = 'display: block; width: 320px'; + + const only = document.createElement('div'); + + only.style.cssText = 'flex: 0 0 100px; height: 40px'; + only.textContent = 'one'; + el.append(only); + await el.updateComplete; + await new Promise((r) => setTimeout(r, 60)); + await el.updateComplete; + await new Promise((r) => requestAnimationFrame(() => r(null))); + await el.updateComplete; + + const { prev, next } = arrows(el); + + expect(prev.hasAttribute('hidden')).toBe(true); + expect(next.hasAttribute('hidden')).toBe(true); + }); +}); diff --git a/frontend/test/components/unowned-everywhere.test.ts b/frontend/test/components/unowned-everywhere.test.ts index 5bee2de..b560d1e 100644 --- a/frontend/test/components/unowned-everywhere.test.ts +++ b/frontend/test/components/unowned-everywhere.test.ts @@ -1,24 +1,28 @@ /** - * Owned is plain; unowned is what gets marked. + * The catalog's cards are not dimmed; the badge is the mark. * - * `explore-album-details` had this right for one tracklist and nothing - * else did: Explore's cards, the top-results row and the artist page's - * three card shapes all mixed owned and unowned with a small badge as - * the only difference — and drew a green tick on the *common* case, - * which is the treatment the album page's own green ticks were removed - * for. + * The rule this replaced had every unowned card dimmed *and* badged, + * which on a shelf of mostly-unowned covers read as a page that had + * failed to load rather than a page of things you could ask for. So the + * dimming is gone from the catalog surfaces and the badge carries the + * whole statement — over the artwork, on hover, drawn for owned and + * unowned alike. * - * What is pinned here is the rule rather than any one surface, because - * the fault this replaced was eight call sites each holding their own - * version of it: + * What is still pinned here is the half that was never about dimming: * - * - an owned thing draws **no badge at all**; - * - an unowned one is dimmed *and* says so in its accessible name, - * because dimming is a colour and cannot be the only signal; * - ownership is a **file** (`localId`), never the catalog's * `inLibrary` ratchet, which is a flag that happens to agree; - * - and a partly-held album says *how* partly, which is the one thing - * a tick cannot. + * - a row that cannot be played is `aria-disabled`, while a card that + * still navigates is not; + * - a partly-held album says *how* partly, which is the one thing a + * tick cannot; + * - and an unowned thing still says so in its accessible name, because + * with the dimming gone that name is the whole signal for anyone not + * seeing the badge. + * + * The album page's *tracklist* still dims unowned rows — a different + * statement about a different thing — and is covered by + * `album-request-badge-visibility.test.ts`. */ import { beforeEach, describe, expect, it } from 'vitest'; import { page } from 'vitest/browser'; @@ -26,7 +30,7 @@ import { page } from 'vitest/browser'; import '@components/explore-view/explore-view'; import '@components/top-results-row/top-results-row'; import { flush, stub, resetHarness } from '@test/support/harness'; -import { fixture, shadow, shadowAll, update } from '@test/support/render'; +import { fixture, shadow, update } from '@test/support/render'; import { completenessStore } from '@store/completeness-store'; const SEARCH = 'explore.Service.SearchLocal'; @@ -105,27 +109,44 @@ beforeEach(() => { // absent one — which is the point, or 87% of a grid re-asks forever. // Two tests in one file are two sessions as far as it is concerned, // so a stale entry from the test above would otherwise decide the - // one below. Found by writing the assertion the wrong way round. + // one below. completenessStore.invalidate(); }); -describe('an owned thing is plain', () => { - it('draws no badge on an album card it has files for', async () => { +describe('an unowned card is marked by its badge alone', () => { + it('does not dim the artwork', async () => { + const el = await exploreShowing({ + releaseGroups: [album('Absent', {})], + }); + + const art = shadow(el, '.album-card .album-art-container')!; + + // The dimming was an opacity on this box. With it gone the cover is + // at full strength, and the badge is what says the card is not + // yours. + expect(getComputedStyle(art).opacity).toBe('1'); + expect(shadow(el, '.album-card library-status-indicator')).not.toBeNull(); + }); + + it('still says so in the name the browser computes', async () => { + await exploreShowing({ releaseGroups: [album('Absent', {})] }); + + await expect + .element(page.getByRole('button', { name: /Absent — not in your library/ })) + .toBeInTheDocument(); + }); +}); + +describe('an owned card is plain except for its badge', () => { + it('draws the in-library badge rather than nothing', async () => { const el = await exploreShowing({ releaseGroups: [album('Held', { localId: 7 })], }); - expect(shadowAll(el, '.album-card')).toHaveLength(1); - expect(shadow(el, '.album-card library-status-indicator')).toBeNull(); - }); + const badge = shadow(el, '.album-card library-status-indicator'); - it('draws no badge on a track row it has a file for', async () => { - const el = await exploreShowing({ - recordings: [recording('Held', { localId: 9 })], - }); - - expect(shadowAll(el, '.track-item')).toHaveLength(1); - expect(shadow(el, '.track-item library-status-indicator')).toBeNull(); + expect(badge).not.toBeNull(); + expect(badge?.getAttribute('status')).toBe('in-library'); }); it('does not dim it', async () => { @@ -139,56 +160,6 @@ describe('an owned thing is plain', () => { }); }); -describe('an unowned thing is marked', () => { - it('dims the card and keeps its request badge', async () => { - const el = await exploreShowing({ - releaseGroups: [album('Absent', {})], - }); - - expect(shadow(el, '.album-card')?.classList.contains('unowned')).toBe(true); - expect(shadow(el, '.album-card library-status-indicator')).not.toBeNull(); - }); - - /** - * The name is the half of this that reaches anyone not seeing the - * dimming, so it has to be the browser's own answer — a shadow-root - * query cannot compute a name, and this repo has shipped a nameless - * control three times. - */ - it('says so in the name the browser computes', async () => { - await exploreShowing({ releaseGroups: [album('Absent', {})] }); - - await expect - .element(page.getByRole('button', { name: /Absent — not in your library/ })) - .toBeInTheDocument(); - }); - - /** - * A track row is `aria-disabled` and a card is not, and the - * difference is not cosmetic: activating an unowned row does nothing - * (`onRecordingRowDblClick` returns early), while a card navigates to - * the catalog page for it, which is a perfectly good thing to do with - * something you do not own. - */ - it('marks a row that cannot be played as disabled', async () => { - const el = await exploreShowing({ - recordings: [recording('Absent', {})], - }); - - expect(shadow(el, '.track-item')?.getAttribute('aria-disabled')).toBe( - 'true', - ); - }); - - it('leaves a card that still navigates enabled', async () => { - const el = await exploreShowing({ - releaseGroups: [album('Absent', {})], - }); - - expect(shadow(el, '.album-card')?.getAttribute('aria-disabled')).toBeNull(); - }); -}); - /** * The decision this issue turned on. * @@ -206,7 +177,9 @@ describe('ownership is a file, not a flag', () => { }); expect(shadow(el, '.album-card')?.classList.contains('unowned')).toBe(true); - expect(shadow(el, '.album-card library-status-indicator')).not.toBeNull(); + expect( + shadow(el, '.album-card library-status-indicator')?.getAttribute('status'), + ).not.toBe('in-library'); }); it('does the same for a track row', async () => { @@ -220,6 +193,26 @@ describe('ownership is a file, not a flag', () => { }); }); +describe('a track row that cannot be played is disabled', () => { + it('marks an unowned row', async () => { + const el = await exploreShowing({ + recordings: [recording('Absent', {})], + }); + + expect(shadow(el, '.track-item')?.getAttribute('aria-disabled')).toBe( + 'true', + ); + }); + + it('leaves a card that still navigates enabled', async () => { + const el = await exploreShowing({ + releaseGroups: [album('Absent', {})], + }); + + expect(shadow(el, '.album-card')?.getAttribute('aria-disabled')).toBeNull(); + }); +}); + /** * The count, which is what `#16`'s deferred third step asked for: an * album held 2 tracks of 10 wore the same green tick as one held whole, @@ -248,9 +241,13 @@ describe('a partly-held album says how partly', () => { // A partly-held album is *actionable* — it has three tracks left to // ask for — so the badge is a button, and the name has to carry the - // action and the count. Naming it after the action alone left the - // one state the ring exists for as the one state whose name did not - // mention it. + // action and the count. The badge is revealed by the card's focus + // (`:focus-within`), and `visibility: hidden` is what takes it out + // of the accessibility tree until then, so the card is focused + // first — which is exactly the route a keyboard user takes. + shadow(el, '.album-card')?.focus(); + await el.updateComplete; + await expect .element( page.getByRole('button', { @@ -266,7 +263,7 @@ describe('a partly-held album says how partly', () => { * state, and a ring drawn from its absence would mark all of it * incomplete on no evidence. That is the rule `Known` exists for. */ - it('says nothing when the total was never declared', async () => { + it('falls back to the plain in-library badge when the total was never declared', async () => { stub(COMPLETENESS, { '7': { owned: 3, expected: 0, known: false, complete: false }, }); @@ -279,7 +276,9 @@ describe('a partly-held album says how partly', () => { await flush(); await el.updateComplete; - expect(shadow(el, '.album-card library-status-indicator')).toBeNull(); + expect( + shadow(el, '.album-card library-status-indicator')?.getAttribute('status'), + ).toBe('in-library'); }); it('asks about the owned albums only, in one call', async () => { @@ -329,11 +328,13 @@ describe('the top-results row follows the same rule', () => { query: 'held', }); + // A top-result card is a mixed bag — artist, album or track — and + // its badge is a corner mark rather than the cover overlay the + // album cards grew, so an owned one stays plain. expect(shadow(el, '.card library-status-indicator')).toBeNull(); - expect(shadow(el, '.card')?.classList.contains('unowned')).toBe(false); }); - it('dims and names something it does not', async () => { + it('names something it does not own', async () => { const el = await fixture('top-results-row', { results: [result('Absent', 'release_group')], query: 'absent', @@ -349,7 +350,7 @@ describe('the top-results row follows the same rule', () => { /** * An artist card has never had a badge — a discography subscription * is the artist page's Follow button, which can say what it commits - * to — so the dimming and the name are the whole signal there. + * to — so the name is the whole signal there. */ it('marks an unowned artist without offering a request', async () => { const el = await fixture('top-results-row', {