From b801fa533ad872198aa3eed2181b9ab4a40dc19c Mon Sep 17 00:00:00 2001 From: Logan Date: Thu, 20 Aug 2026 20:02:50 -0400 Subject: [PATCH 1/5] feat(shell): make search a button and a modal where searching applies The phone's top bar is about to go, and the search box is the one thing in it that is an action rather than chrome. It becomes a button in the row that already says which page you are on, opening a wa-dialog with the real search box in it. Three decisions worth the words. **A wa-dialog, and that is a mechanism rather than a taste.** wa-popup renders `
` and feature-detects the Popover API, falling back to `strategy: "fixed"` where there is none -- which is Chrome 113, the reference device, since `popover` is Chrome 114. And `position: fixed` escapes ancestor overflow but not `contain: paint`, which `.main-panel` carries, so a popup-shaped search panel opened from a view's header is structurally clipped on that device. `` / `showModal()` is Chrome 37 and uses the real top layer. No tier here can see the difference -- CI's Chromium and WebKit both have the Popover API -- so the component test asserts the *mechanism*, a native `` in the tree, rather than the symptom. **An element, not a PageAction.** Two of the seven searchable views are detail views with no page-header; they filter on the term and say so in their own headers. Declaring search as an action would mean seven hosts each writing it out, which is a second list of searchable views, and it would put a phone mode for actions inside page-header, which that component documents its refusal to grow. search-store's own map is the condition, asked by one component placed three times. **The modal carries the real search-bar**, so there is still one debounce, one clear button and one view-scoped placeholder. Escape closes it and *keeps* the term -- the input treats Escape as "clear the search", which is right in a header where the box stays on screen and wrong in a surface whose dismissal would then discard the search. --- frontend/index.html | 13 + frontend/index.ts | 5 + .../src/components/page-header/page-header.ts | 15 ++ .../playlist-details/playlist-details.ts | 14 + .../src/components/search-bar/search-bar.ts | 5 +- .../components/search-dialog/search-dialog.ts | 210 +++++++++++++++ .../search-dialog/search-trigger.ts | 147 +++++++++++ .../smart-playlist-details.ts | 13 + .../src/services/keyboard-shortcut-service.ts | 15 +- frontend/src/utils/icon-language.ts | 13 + .../test/components/search-dialog.test.ts | 248 ++++++++++++++++++ 11 files changed, 695 insertions(+), 3 deletions(-) create mode 100644 frontend/src/components/search-dialog/search-dialog.ts create mode 100644 frontend/src/components/search-dialog/search-trigger.ts create mode 100644 frontend/test/components/search-dialog.test.ts diff --git a/frontend/index.html b/frontend/index.html index 61688df..991def6 100644 --- a/frontend/index.html +++ b/frontend/index.html @@ -14,6 +14,13 @@ user is not walked through the header, the library filter, the search box and eleven nav items on every navigation. --> +

YellowJacket

@@ -84,6 +91,12 @@ + + diff --git a/frontend/index.ts b/frontend/index.ts index a31f1c5..cdaf1c9 100644 --- a/frontend/index.ts +++ b/frontend/index.ts @@ -28,6 +28,11 @@ import '@components/bottom-nav/bottom-nav.ts'; import '@components/queue-panel/queue-panel.ts'; import '@components/nav-history/nav-history.ts'; import '@components/search-bar/search-bar.ts'; +// The phone's search surface (#57). Eager, because below 600px it is +// the *only* way to search and a modal that has to fetch a chunk before +// it can take a keystroke is late by exactly the interval it exists to +// remove. It renders nothing until asked. +import '@components/search-dialog/search-dialog.ts'; import '@components/library-filter/library-filter.ts'; import '@components/first-run-wizard/first-run-wizard.ts'; import '@components/notifications/notification-host.ts'; diff --git a/frontend/src/components/page-header/page-header.ts b/frontend/src/components/page-header/page-header.ts index 58a0da0..a441b19 100644 --- a/frontend/src/components/page-header/page-header.ts +++ b/frontend/src/components/page-header/page-header.ts @@ -11,6 +11,7 @@ import { contextMenuStyles, } from '../../utils/context-menu-controller'; import { ICON_MORE_ACTIONS } from '../../utils/icon-language'; +import '../search-dialog/search-trigger'; /** * The one arrangement every primary view uses to say what it is. @@ -457,6 +458,20 @@ export class PageHeader extends LitElement { ${this.renderCount()}
${this.renderScope()} ${this.renderSort()} + + ${this.renderActions()}
diff --git a/frontend/src/components/playlist-details/playlist-details.ts b/frontend/src/components/playlist-details/playlist-details.ts index c2e5d75..c90e7ed 100644 --- a/frontend/src/components/playlist-details/playlist-details.ts +++ b/frontend/src/components/playlist-details/playlist-details.ts @@ -27,6 +27,7 @@ import { queueStore } from '@store/queue-store'; import { creditStore } from '@store/credit-store'; import { PlayerController } from '@store/controllers/player-controller'; import { SearchController } from '@store/controllers/search-controller'; +import '../search-dialog/search-trigger'; import { SelectionController } from '@utils/selection-controller'; import type { SelectionHost } from '@utils/selection-controller'; import { @@ -1039,6 +1040,16 @@ export class PlaylistDetails min-width: 0; } + /* #57. This view is in search-store's map and filters on the + term, but it is a detail view and so has no page-header to + carry the phone's search button. Pushed to the end of the + header row, which is where page-header puts it too. */ + .header-end { + margin-left: auto; + display: flex; + align-items: center; + } + .playlist-title { font-size: 24px; font-weight: 700; @@ -1383,6 +1394,9 @@ export class PlaylistDetails ` : ''}
+
+ +
${searchBar}
` and feature-detects the Popover API, falling + * back to `strategy: "fixed"` where there is none — which is the + * reference device, Chrome 113, since `popover` is Chrome 114. And + * `position: fixed` escapes ancestor *overflow* but not `contain: + * paint`, which makes an element a containing block for fixed + * descendants **and clips them**; `index.css` puts `contain: layout + * style paint` on `.main-panel`, which is the ancestor of every view. + * A popup-shaped search panel opened from a view's header would + * therefore be structurally clipped on the one device this issue is + * about, and **no tier here could see it** — CI's Chromium and WebKit + * both have the Popover API, so the popup is top-layered and correct. + * ``/`showModal()` is Chrome 37 and uses the real top layer, so + * this is immune by construction. + * + * **It carries the real ``**, not a second input. That is + * what keeps one debounce, one clear button, one accessible name and + * one view-scoped placeholder — and it is why `store/search-store.ts` + * is still the only statement of which views can search and what they + * search. The modal is a presentation of the control, not a copy of it. + * + * **The results are the view, not a list in here.** The Direction says + * "the box and live results"; the live results already exist, because + * the term is view-scoped and the page behind this dialog filters on it + * and says so in `page-header`'s "Showing albums matching …" line. + * Rendering results in the dialog would be a second implementation of + * every view's own filtering, and a worse one — it could not offer the + * row actions the view does. So Enter closes and hands the screen back. + * + * A singleton in `index.html` for the reason `shortcuts-overlay` is: + * one instance, one `data-testid`, one document listener, and no + * `data-testid="search-input"` resolving to two elements while it is + * shut. + */ +import { LitElement, css, html, nothing } from 'lit'; +import { customElement, query, state } from 'lit/decorators.js'; +import '@awesome.me/webawesome/dist/components/dialog/dialog.js'; + +import { designTokens } from '../../styles/tokens.css'; +import { nameDialogsIn } from '@utils/name-dialog'; +import { SearchController } from '@store/controllers/search-controller'; +import type { SearchBar } from '../search-bar/search-bar'; +import '../search-bar/search-bar'; + +/** The event any trigger dispatches to open this. */ +export const OPEN_SEARCH_EVENT = 'open-search'; + +@customElement('search-dialog') +export class SearchDialog extends LitElement { + private searchCtrl = new SearchController(this); + + @query('wa-dialog') private dialog?: HTMLElement & { open: boolean }; + + @query('search-bar') private bar?: SearchBar; + + @state() private isOpen = false; + + static override styles = [ + designTokens, + css` + :host { + display: contents; + } + + wa-dialog::part(dialog) { + background: var(--yj-bg-surface, #212529); + color: var(--yj-text-primary, #fff); + } + + /* The box is the whole content, so it gets the whole width + rather than the 360px cap it wears in a header. */ + search-bar { + display: block; + width: 100%; + --yj-search-max-width: none; + } + + .hint { + margin: 0.75em 0 0; + font-size: var(--yj-text-sm, 0.8125rem); + color: var(--yj-text-secondary, #b3b3b3); + } + `, + ]; + + override connectedCallback(): void { + super.connectedCallback(); + document.addEventListener(OPEN_SEARCH_EVENT, this.open); + // Capture, on the host: the path runs document -> host -> + // shadow root -> the input inside `search-bar`, so a capture + // listener here is the only one that gets the key *before* the + // input's own handler. A `@keydown` in the template is a + // bubbling listener and would run after the term was cleared, + // and there is nowhere to put a `firstUpdated` hook -- the + // first render of this element produces no content at all. + this.addEventListener('keydown', this.onKeydown, true); + } + + override disconnectedCallback(): void { + super.disconnectedCallback(); + document.removeEventListener(OPEN_SEARCH_EVENT, this.open); + this.removeEventListener('keydown', this.onKeydown, true); + } + + /** + * Not a toggle, for `shortcuts-overlay`'s reason: a dialog owns + * every unmodified key while it is up, so a second press of the + * shortcut that opened it never reaches the shortcut service. + */ + private open = (): void => { + if (this.isOpen) return; + + // Nothing to search here is not an error; it is the state the + // trigger already declines to render in. Guarding here too is + // what makes the keyboard route (Ctrl+F on a phone) agree with + // the button. + if (!this.searchCtrl.isSearchableView) return; + + this.isOpen = true; + + void this.updateComplete.then(() => { + if (this.dialog) this.dialog.open = true; + + // `wa-dialog` positions and shows in its own update, and + // `search-bar` populates its own shadow root in one more — + // the same lifecycle trap `name-dialog.ts` documents. One + // more frame, and the box has an input to focus. + requestAnimationFrame(() => this.bar?.focusInput()); + }); + }; + + private close(): void { + if (this.dialog) this.dialog.open = false; + + this.isOpen = false; + } + + /** + * Escape closes and **keeps the term**; Enter closes and shows the + * results. + * + * Escape is the one worth stating. `search-bar`'s input treats it + * as *clear the search*, which is right in a header — the box is on + * screen either way, so clearing is the only thing left for the key + * to mean. Here it would make dismissing the search surface + * silently discard the search, and discarding is what the clear + * button inside it is for. So this runs first and closes; the term + * survives, and the page behind is still filtered by it. + */ + private onKeydown = (e: KeyboardEvent): void => { + if (!this.isOpen) return; + + if (e.key === 'Escape') { + e.stopPropagation(); + this.close(); + + return; + } + + if (e.key === 'Enter') { + e.stopPropagation(); + e.preventDefault(); + this.close(); + } + }; + + /** + * Web Awesome renders `label` into a heading it never points the + * `` at. See `utils/name-dialog.ts`. + */ + override updated(): void { + nameDialogsIn(this.shadowRoot); + } + + override render() { + if (!this.isOpen) return nothing; + + const scope = this.searchCtrl.scopeLabel; + + return html` + this.close()} + > + +

+ Results appear on the page behind this. Press Enter + or close to see them. +

+ + `; + } +} + +declare global { + interface HTMLElementTagNameMap { + 'search-dialog': SearchDialog; + } +} diff --git a/frontend/src/components/search-dialog/search-trigger.ts b/frontend/src/components/search-dialog/search-trigger.ts new file mode 100644 index 0000000..16d0b71 --- /dev/null +++ b/frontend/src/components/search-dialog/search-trigger.ts @@ -0,0 +1,147 @@ +/** + * The phone's way into search (#57): one button, in the row that + * already says which page you are on. + * + * **Which views show it is not a decision this component makes.** + * `store/search-store.ts` has held the map of what each view searches + * since plan 007, and #57's own Findings say so — "that is exactly the + * condition for showing the button". So this asks `isSearchableView` + * and renders nothing otherwise, and no second list of searchable views + * exists to fall out of step with the first. + * + * **It is an element rather than a `PageAction`**, and that is the + * whole reason it is a component at all. Two of the seven searchable + * views — `playlist-details` and `smart-playlist-details` — have no + * `page-header`; they filter on the term and say so in their own + * headers. Declaring search as an action would mean seven hosts each + * writing it out, which is the second list again, and it would put a + * *phone mode for actions* inside `page-header`, which that component + * documents its refusal to grow. An element three headers place is one + * statement of the rule, placed three times. + * + * It does not participate in `page-header`'s overflow measurement, for + * the reason the count and the sort control do not: it is 32px, it is + * `flex-shrink: 0`, and the header's `fits()` sees its width like any + * other child. What it must never do is collapse into the overflow + * menu — on a phone that menu is the only home for the page's actions + * already, and search would be two taps behind an ellipsis. + */ +import { LitElement, css, html, nothing } from 'lit'; +import { customElement, state } from 'lit/decorators.js'; +import '@awesome.me/webawesome/dist/components/icon/icon.js'; + +import { designTokens } from '../../styles/tokens.css'; +import { PHONE_QUERY } from '@utils/breakpoints'; +import { SearchController } from '@store/controllers/search-controller'; +import { ICON_SEARCH } from '@utils/icon-language'; +import { OPEN_SEARCH_EVENT } from './search-dialog'; + +@customElement('search-trigger') +export class SearchTrigger extends LitElement { + private searchCtrl = new SearchController(this); + + /** + * From `matchMedia` rather than a media query, because this decides + * whether the button *exists* — `job-band`'s rule, and for the same + * consequence: a header that renders it at every width puts a + * second search affordance beside the desktop's own box. + */ + @state() private phone = false; + + private media?: MediaQueryList; + + static override styles = [ + designTokens, + css` + :host { + display: contents; + } + + button { + display: inline-flex; + align-items: center; + justify-content: center; + /* The smallest a touch target should be. The header's + own action buttons are smaller because they carry a + label; this one is a glyph. */ + min-width: 40px; + min-height: 40px; + padding: 0; + background: none; + border: 1px solid var(--yj-border-subtle, #555); + border-radius: 4px; + color: var(--yj-text-primary, #fff); + cursor: pointer; + flex-shrink: 0; + } + + button:focus-visible { + outline: 2px solid var(--yj-accent, #ffd43b); + outline-offset: -1px; + } + + /* A search that is *on* says so without a second control: + the page already carries "Showing albums matching ...", + and this is the button that reopens the box to change or + clear it. */ + button.filtering { + border-color: var(--yj-accent, #ffd43b); + color: var(--yj-accent-text, #ffd43b); + } + `, + ]; + + override connectedCallback(): void { + super.connectedCallback(); + + this.media = window.matchMedia(PHONE_QUERY); + this.phone = this.media.matches; + this.media.addEventListener('change', this.onMedia); + } + + override disconnectedCallback(): void { + super.disconnectedCallback(); + this.media?.removeEventListener('change', this.onMedia); + } + + private onMedia = (e: MediaQueryListEvent): void => { + this.phone = e.matches; + }; + + private onClick = (): void => { + document.dispatchEvent(new CustomEvent(OPEN_SEARCH_EVENT)); + }; + + override render() { + if (!this.phone || !this.searchCtrl.isSearchableView) return nothing; + + const scope = this.searchCtrl.scopeLabel; + const term = this.searchCtrl.term; + + // The name carries the state, because the colour cannot: a + // control that is a different colour and the same word is a + // control that says nothing to anyone not seeing it. Same rule + // `library-status.ts` states for a partial badge. + const label = term + ? `Search ${scope}, showing matches for ${term}` + : `Search ${scope}`; + + return html` + + `; + } +} + +declare global { + interface HTMLElementTagNameMap { + 'search-trigger': SearchTrigger; + } +} diff --git a/frontend/src/components/smart-playlist-details/smart-playlist-details.ts b/frontend/src/components/smart-playlist-details/smart-playlist-details.ts index d86101b..83adde0 100644 --- a/frontend/src/components/smart-playlist-details/smart-playlist-details.ts +++ b/frontend/src/components/smart-playlist-details/smart-playlist-details.ts @@ -18,6 +18,7 @@ import { queueStore } from '@store/queue-store'; import { creditStore } from '@store/credit-store'; import { PlayerController } from '@store/controllers/player-controller'; import { SearchController } from '@store/controllers/search-controller'; +import '../search-dialog/search-trigger'; import { SelectionController } from '@utils/selection-controller'; import type { SelectionHost } from '@utils/selection-controller'; import { @@ -359,6 +360,15 @@ export class SmartPlaylistDetails flex-shrink: 0; } + /* #57. Like playlist-details, this view filters on the search + term and has no page-header to carry the phone's search + button, so the action row does. */ + .actions-end { + margin-left: auto; + display: flex; + align-items: center; + } + .action-button { background: none; border: 1px solid var(--yj-border-subtle, #555); @@ -1300,6 +1310,9 @@ export class SmartPlaylistDetails Edit Rules `} +
+ +
${this.editing ? html` diff --git a/frontend/src/services/keyboard-shortcut-service.ts b/frontend/src/services/keyboard-shortcut-service.ts index 12689cf..43ac955 100644 --- a/frontend/src/services/keyboard-shortcut-service.ts +++ b/frontend/src/services/keyboard-shortcut-service.ts @@ -16,6 +16,7 @@ import { playerStore } from '@store/player-store'; import { queueStore } from '@store/queue-store'; import * as Player from '@go/player/player.js'; import type { SearchBar } from '@components/search-bar/search-bar'; +import { OPEN_SEARCH_EVENT } from '@components/search-dialog/search-dialog'; // =================================================================== // KEY STRING UTILITIES @@ -387,14 +388,24 @@ async function dispatch(action: string): Promise { break; // Navigation + // The key has one meaning -- *let me search this page* -- and + // two surfaces since #57. The header box is gone below 600px, + // so scoping the query to the bar is not tidiness: an unscoped + // `search-bar` also matches the one inside `search-dialog` + // while that is open, and would focus a box the user is + // already typing in while leaving the phone with nothing at + // all. The dialog declines to open on a view with nothing to + // search, which is the same condition the trigger renders on. case 'nav.search': case 'nav.searchAlt': { const bar = document.querySelector( - 'search-bar', + 'header.top-bar search-bar', ) as SearchBar | null; - if (bar && !bar.hasAttribute('hidden')) { + if (bar && bar.checkVisibility()) { bar.focusInput(); + } else { + document.dispatchEvent(new CustomEvent(OPEN_SEARCH_EVENT)); } break; diff --git a/frontend/src/utils/icon-language.ts b/frontend/src/utils/icon-language.ts index 6efe84d..9315b1f 100644 --- a/frontend/src/utils/icon-language.ts +++ b/frontend/src/utils/icon-language.ts @@ -124,6 +124,19 @@ export const ICON_DOWNLOADING = 'download'; */ export const ICON_MORE_ACTIONS = 'ellipsis'; +/** + * Look for something. + * + * Deliberately **not** governed by the sweep in + * `icon-language.test.ts`: `magnifying-glass` has only ever meant this, + * in the header box and in Explore's own catalog search alike, so + * governing it would force a rename on two call sites that are already + * right. It is written down because #57 gave the meaning a *button* as + * well as a box, and a second surface for the same verb is exactly the + * point at which two spellings start. + */ +export const ICON_SEARCH = 'magnifying-glass'; + /** * Take this away. * diff --git a/frontend/test/components/search-dialog.test.ts b/frontend/test/components/search-dialog.test.ts new file mode 100644 index 0000000..03f83de --- /dev/null +++ b/frontend/test/components/search-dialog.test.ts @@ -0,0 +1,248 @@ +/** + * The phone's search surface (#57). + * + * Two things are asserted here that the e2e tier cannot reach, and one + * that it deliberately must not be trusted with. + * + * **Which views show the trigger is `search-store`'s answer**, so this + * walks the map rather than sampling a view: the fault the issue guards + * against is a second list of searchable views, and a spec that checks + * Albums checks nothing about Playlists. + * + * **The dialog is a ``, not a popup.** #60 established from the + * Web Awesome source that `wa-popup` falls back to `position: fixed` + * without the Popover API — Chrome 113, the reference device — and that + * `.main-panel`'s `contain: paint` clips a fixed descendant. Every tier + * available here has the Popover API, so a popup renders perfectly in + * CI and is clipped on the device: **an assertion that the surface is + * not clipped passes on the broken build.** So the assertion is the + * *mechanism* — a real `` in the tree — which is the one form + * of this that a browser here can answer honestly. + * + * The breakpoint is stubbed rather than emulated, for the reason + * `now-playing-phone.test.ts` gives: the runner's viewport is fixed at + * 1280x800, and the component reads `matchMedia` in `connectedCallback` + * precisely so a test can answer it first. + */ +import { describe, expect, it, beforeEach, afterEach } from 'vitest'; + +import '@components/search-dialog/search-dialog'; +import '@components/search-dialog/search-trigger'; +import { searchStore } from '@store/search-store'; +import { fixture, shadow, deepShadow } from '@test/support/render'; +import { flush } from '@test/support/harness'; + +/** Views the store says can be searched, and what they search. */ +const SEARCHABLE: [string, string][] = [ + ['tracks', 'tracks'], + ['albums', 'albums'], + ['artists', 'artists'], + ['genres', 'genres'], + ['playlists', 'playlists'], + ['playlist-details', 'tracks in this playlist'], + ['smart-playlist-details', 'tracks in this smart playlist'], +]; + +/** Views with nothing of their own to search, or a search of their own. */ +const UNSEARCHABLE = ['home', 'explore', 'settings', 'downloads', 'autotag']; + +let restoreMedia: (() => void) | null = null; + +/** Answer the shell's phone query with `phone` until restored. */ +function stubPhone(phone: boolean): void { + const real = window.matchMedia.bind(window); + + window.matchMedia = ((q: string) => + q.includes('max-width: 599px') + ? { + matches: phone, + media: q, + addEventListener() {}, + removeEventListener() {}, + } + : real(q)) as typeof window.matchMedia; + + restoreMedia = () => { + window.matchMedia = real; + }; +} + +beforeEach(() => { + searchStore.setTerm(''); + searchStore.setCurrentView('tracks'); +}); + +afterEach(() => { + restoreMedia?.(); + restoreMedia = null; + searchStore.setTerm(''); + searchStore.setCurrentView('tracks'); +}); + +describe('', () => { + it('is offered on every view the store says can be searched', async () => { + stubPhone(true); + + // One element, walked across the views: the trigger reads the store + // on every render, so remounting per view would test mounting + // rather than the condition. + const el = await fixture('search-trigger'); + + for (const [view] of SEARCHABLE) { + searchStore.setCurrentView(view); + await el.updateComplete; + + expect( + shadow(el, '[data-testid="search-trigger"]'), + `no trigger on ${view}`, + ).not.toBeNull(); + } + }); + + it('names what the button will search', async () => { + stubPhone(true); + + const el = await fixture('search-trigger'); + + for (const [view, scope] of SEARCHABLE) { + searchStore.setCurrentView(view); + await el.updateComplete; + + expect( + shadow(el, '[data-testid="search-trigger"]')?.getAttribute( + 'aria-label', + ), + ).toBe(`Search ${scope}`); + } + }); + + it('is absent where there is nothing to search', async () => { + stubPhone(true); + + const el = await fixture('search-trigger'); + + for (const view of UNSEARCHABLE) { + searchStore.setCurrentView(view); + await el.updateComplete; + + expect( + shadow(el, '[data-testid="search-trigger"]'), + `a trigger appeared on ${view}`, + ).toBeNull(); + } + }); + + it('is absent above the phone breakpoint, where the header has a box', async () => { + stubPhone(false); + + const el = await fixture('search-trigger'); + + expect(shadow(el, '[data-testid="search-trigger"]')).toBeNull(); + }); + + /** + * A colour is not a signal on its own. The button is the only thing + * on screen that reopens a filtered search, so the state it is in has + * to reach someone who cannot see the accent border. + */ + it('says in its name that a search is applied', async () => { + stubPhone(true); + + const el = await fixture('search-trigger'); + + searchStore.setTerm('aurora'); + await el.updateComplete; + + const button = shadow(el, '[data-testid="search-trigger"]'); + + expect(button?.getAttribute('aria-label')).toContain('aurora'); + expect(button?.className).toContain('filtering'); + }); +}); + +describe('', () => { + it('opens on the event the trigger dispatches, as a real dialog', async () => { + stubPhone(true); + + const el = await fixture('search-dialog'); + const trigger = await fixture('search-trigger'); + + shadow(trigger, '[data-testid="search-trigger"]')?.click(); + await flush(); + await el.updateComplete; + + expect(shadow(el, '[data-testid="search-dialog"]')).not.toBeNull(); + + // The mechanism, not the appearance: a native is what + // reaches the top layer on Chrome 113, and a wa-popup would look + // identical in this browser while being clipped on the device. + expect(deepShadow(el, 'dialog')).not.toBeNull(); + }); + + /** + * It carries the real box rather than a second input, which is what + * keeps one debounce, one clear button and one view-scoped + * placeholder — and what keeps `search-store` the only statement of + * what a view searches. + */ + it('carries the header search box itself', async () => { + const el = await fixture('search-dialog'); + + document.dispatchEvent(new CustomEvent('open-search')); + await flush(); + await el.updateComplete; + + expect(shadow(el, 'search-bar')).not.toBeNull(); + }); + + /** + * The one place the shortcut route and the button could disagree. + * Ctrl+F on a view with nothing to search dispatches the same event + * the button would, and the button is not there to be pressed. + */ + it('declines to open where there is nothing to search', async () => { + const el = await fixture('search-dialog'); + + searchStore.setCurrentView('home'); + document.dispatchEvent(new CustomEvent('open-search')); + await flush(); + await el.updateComplete; + + expect(shadow(el, '[data-testid="search-dialog"]')).toBeNull(); + }); + + /** + * Escape closes and **keeps the term**. + * + * `search-bar`'s own input treats Escape as "clear the search", which + * is right in a header where the box stays on screen either way. Here + * it would mean dismissing the surface silently discarded the search, + * and the page behind would refill without being asked to. + */ + it('keeps the search when it is dismissed', async () => { + const el = await fixture('search-dialog'); + + document.dispatchEvent(new CustomEvent('open-search')); + await flush(); + await el.updateComplete; + + searchStore.setTerm('aurora'); + + const input = deepShadow(el, 'input'); + + expect(input).not.toBeNull(); + + input!.dispatchEvent( + new KeyboardEvent('keydown', { + key: 'Escape', + bubbles: true, + composed: true, + }), + ); + await flush(); + await el.updateComplete; + + expect(searchStore.getTerm()).toBe('aurora'); + expect(shadow(el, '[data-testid="search-dialog"]')).toBeNull(); + }); +}); -- 2.54.0 From 47bd9ef211352a17707342af840a1d57100d7f78 Mon Sep 17 00:00:00 2001 From: Logan Date: Thu, 20 Aug 2026 20:03:01 -0400 Subject: [PATCH 2/5] fix(header): let the count yield before an action is clipped Adding the phone's search button to this header is 43px more than the row has at 320px, which is a width the app promises and which header-action-overflow.spec.ts asks about. Measured on Playlists there, after the fit pass had already collapsed all three actions into "More actions" and truncated the title to nothing: title 0, count 50, sort 143, search 40, More 38, five 12px gaps and 32px of gutters -- 363 in 320, with the More button ending 27px past the edge. That is an action clipped, which is the exact defect this pass exists to prevent. The count is what yields, last, because it is the only item on that row that is neither an identity nor an action. The title yields first and may ellipsis away entirely, since the navigation also says which page you are on; the sort control and the buttons are each the only place they are said. An empty page says it is empty in its empty state and a full one is being looked at. With the count gone the header is 304 in 304, and the title comes back to 19px. It is rendered and hidden with an attribute rather than returned as `nothing`, for the reason the action buttons are: every pass starts from all-visible and needs a node to un-hide, or the first 320px window costs the count for the rest of the session. --- .../src/components/page-header/page-header.ts | 70 ++++++++++++++++--- 1 file changed, 61 insertions(+), 9 deletions(-) diff --git a/frontend/src/components/page-header/page-header.ts b/frontend/src/components/page-header/page-header.ts index a441b19..d67fed1 100644 --- a/frontend/src/components/page-header/page-header.ts +++ b/frontend/src/components/page-header/page-header.ts @@ -162,6 +162,15 @@ export class PageHeader extends LitElement { @state() private collapsed: ReadonlySet = new Set(); + /** + * Whether the count has been given up. Derived, like `collapsed`. + * + * It is the last thing to yield and the only thing here that is + * neither an identity nor an action — see `measureFit`. + */ + @state() + private countCollapsed = false; + @state() private menuOpen = false; @@ -531,12 +540,6 @@ export class PageHeader extends LitElement { if (!header) return; - if (this.actions.length === 0) { - this.commitCollapsed(new Set()); - - return; - } - const buttons = new Map(); for (const el of this.renderRoot.querySelectorAll( @@ -549,6 +552,7 @@ export class PageHeader extends LitElement { const more = this.moreButton; const title = this.renderRoot.querySelector('h1'); + const count = this.renderRoot.querySelector('.count'); /** * Nothing is clipped — which is not the same as the header not @@ -570,6 +574,8 @@ export class PageHeader extends LitElement { if (more) more.hidden = true; + if (count) count.hidden = false; + const collapsed = new Set(); if (!fits()) { @@ -586,7 +592,42 @@ export class PageHeader extends LitElement { } } - this.commitCollapsed(collapsed); + this.commitCollapsed(collapsed, this.collapseCount(count, fits)); + } + + /** + * The last thing to give way, after every action is in the menu and + * the title has already run out. + * + * There are four things competing for this row and three of them + * cannot go. The **title** yields first and is allowed to ellipsis + * away entirely at 320px, because the navigation also says which + * page you are on. The **sort** control and the **actions** are + * each the only place they are said, so an action collapses into + * the menu rather than disappearing and the sort control stays. + * That leaves the **count**, which is the one purely informational + * item on the row — an empty page says so in its empty state, and a + * full one is being looked at. + * + * It became reachable rather than theoretical with #57: below 600px + * the header also carries the phone's search button, and on + * Playlists at 320px that is 43px more than the row has. Measured + * there: title 0, count 50, sort 143, search 40, "More actions" 38, + * five 12px gaps and 32px of gutters — 363 in 320, with the More + * button ending 27px past the edge. Something has to go, and this + * is the only candidate that is not an action. + * + * @returns whether the count was given up. + */ + private collapseCount( + count: HTMLElement | null, + fits: () => boolean, + ): boolean { + if (count === null || fits()) return false; + + count.hidden = true; + + return true; } /** Lowest priority first; ties broken from the right. */ @@ -601,7 +642,9 @@ export class PageHeader extends LitElement { .map(({ action }) => action); } - private commitCollapsed(next: Set): void { + private commitCollapsed(next: Set, countHidden: boolean): void { + this.countCollapsed = countHidden; + const same = next.size === this.collapsed.size && [...next].every((id) => this.collapsed.has(id)); @@ -769,7 +812,16 @@ export class PageHeader extends LitElement { const noun = this.count === 1 ? this.countNoun : plural; - return html``; } -- 2.54.0 From ac8f86eb00a16ac641eaba4e7ba7591a7bfd1764 Mon Sep 17 00:00:00 2001 From: Logan Date: Thu, 20 Aug 2026 20:03:08 -0400 Subject: [PATCH 3/5] fix(settings): give the library selection a home that is not the top bar library-filter is the only control in the app that calls setSelectedLibrary, and the phone already hid it with a comment saying it was "reachable from the drawer's Settings". It was not: Settings adds, removes, renames and scans libraries, and does not set the view filter, which is a different thing -- it decides what Albums, Artists and Genres show. A phone therefore inherited whatever a desktop session last chose and could neither change nor see it, which is #24's sentence broken in the band it was written for. It is a second *placement* of the same component, not a second control, and it is at every width rather than below 600px. A phone-only copy is the cheaper answer and is the fault rather than the fix: "where do I change which library I am browsing" having two answers by viewport is exactly what one control in two places avoids. Closes #148 --- .../src/components/config-page/config-page.ts | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/frontend/src/components/config-page/config-page.ts b/frontend/src/components/config-page/config-page.ts index 37caa5a..6604419 100644 --- a/frontend/src/components/config-page/config-page.ts +++ b/frontend/src/components/config-page/config-page.ts @@ -56,6 +56,10 @@ import { import './config-field'; import './config-section'; +// The view filter's home (#148). The same component the top bar +// carries, placed a second time rather than reimplemented -- two +// definitions of "which library am I browsing" is what this is for. +import '@components/library-filter/library-filter'; import './download-clients'; import './shortcut-capture'; import { confirmAction } from '../confirm-dialog/confirm-dialog'; @@ -231,6 +235,42 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) { flex-wrap: wrap; } + /* #148, and the second half of #57. + + library-filter is the only control in the app that calls + setSelectedLibrary, and it lived in the top bar -- which + #57 takes out of the layout on a phone, and which #143 + already refused to hide as a fit step precisely because + hiding it takes away an action. So the selection gets a home + that does not depend on that bar existing. + + At every width, not below 600px: a phone-only copy would be + a second place the control lives, and "where do I change + which library I am browsing" having two answers by size is + the fault, not the fix. */ + .library-scope { + display: flex; + align-items: center; + justify-content: space-between; + gap: 1em; + flex-wrap: wrap; + margin-bottom: 1em; + } + + .library-scope .scope-label { + font-weight: 600; + font-size: 0.85em; + color: var(--yj-text-primary, #fff); + display: block; + } + + .library-scope .scope-description { + font-size: 0.75em; + color: var(--yj-text-tertiary, #888); + margin: 0.35em 0 0; + max-width: 40em; + } + .save-row { display: flex; gap: 0.5em; @@ -2370,6 +2410,21 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) { for new and changed files." .open=${true} > +
+
+ Showing +

+ Which library the Albums, Artists and Genres + views show. This is a view filter, not a + setting about the libraries themselves — the + list below is where they are added, renamed + and scanned. +

+
+ + +
+