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(); + }); +});