Compare commits

...
Author SHA1 Message Date
logan 7ba5d321f6 test(queue): pin the breakpoint listener the scrim rule rests on
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m30s
CI / e2e (pull_request) Successful in 9m18s
The scrim's existence comes from matchMedia rather than a stylesheet, which only holds if the query is listened to — and the stub's addEventListener was a no-op, so deleting the listener left all 986 tests green. The stub records its listeners now and the new case carries a panel across the breakpoint in both directions. Watched failing with the listener removed.
2026-08-21 16:24:04 +00:00
logan f126dd7397 fix(queue): draw the scrim only where it can be tapped
Below 600px `.panel-content` is `width: 100%`, so the scrim sat
entirely underneath an opaque panel -- measured at 424x439, host,
panel and scrim all 424x318. It dimmed nothing and dismissed nothing
there while wearing `cursor: pointer`, so #24's tap-outside-to-close
did not exist on the device it was drawn for.

Of the issue's two directions this takes the second. A gutter is the
drawer pattern and buys the affordance by taking width off a
full-screen surface on a 424px viewport; #55 already made the queue a
*screen* at that width, whose ways out are back and a 44px close
button. So there is no scrim there rather than an unreachable one.

Existence is `matchMedia` rather than `display: none`, on `job-band`'s
rule: a hidden scrim is still an element carrying the handler. The
600-899 band, where the panel is a 320px column of a wider content
area and the scrim has real uncovered pixels, is untouched.

The e2e half asserts *absence* at 424x439 rather than clicking,
because a phone-width case that clicks the scrim's centre hits the
panel and passes on the broken build -- which the issue anticipates.

Closes #171
2026-08-21 16:24:04 +00:00
logan 510d3470f9 Merge pull request 'fix(ui): keep a touch-only affordance reachable, or absent' (#181) from fix/137-touch-only-affordances into main
CI / check (push) Successful in 2m28s
CI / e2e (push) Successful in 9m28s
2026-08-21 16:23:43 +00:00
logan d78830aa52 fix(ui): make the touch pen a corner chip, not a scrim over the art
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m28s
CI / e2e (pull_request) Successful in 9m19s
Always-visible is not the same as always-in-the-way: the overlay is inset:0 at 50% black, so gating it on hover left every touch device with the artwork it is editing permanently darkened. It is only a hint — .cover-art-edit carries the click, so tapping the art always worked — while the × really is the only route to its action and stays. The chip borrows the remove button's size, disc and alpha.

Also corrects the claim that no tier can render as a touch device: no committed one does, which is a choice about projects rather than a limit.
2026-08-21 15:59:45 +00:00
logan a72d1f68ed fix(ui): keep a touch-only affordance reachable, or absent
Three controls are revealed by :hover and are the only route to their
action on a device that has none. #68 hid the home card's play button on
touch, which was right because tapping the card does the same thing;
these are the opposite case, so hiding them removes the action outright
and leaving them costs the same long-press flash #68 was filed for --
they are visibility:hidden / opacity:0, so on touch they are invisible
controls that still take taps.

track-details' cover-art overlay and remove, and shortcut-capture's
reset, are always visible under `@media not all and (hover: hover)`.

The queue row's remove is the third case the report names and takes the
other treatment, because #60 has since landed: the row's context menu is
a bottom sheet carrying "Remove from Queue", so the action is one
long-press away and an always-visible X would spend part of a 424px row
on something already reachable. It is display:none outside
`(hover: hover) and (pointer: fine)` rather than visibility:hidden,
which would leave a button holding its hit area and its place in the
accessibility tree -- the trap this issue is about.

The rule is not extracted into styles/ yet: that leaves two call sites
of the always-visible form, under the four the report names.

No tier here can render as a touch device, so the tests read the parsed
stylesheet the way #68's does and say so; the touch and hover renderings
were measured against the running app in a hasTouch context instead.

Closes #137
2026-08-21 15:59:45 +00:00
logan 60f1c5a6b2 Merge pull request 'build(frontend): fail css-check on a nested rule the phone drops' (#180) from fix/154-nested-css-check into main
CI / check (push) Successful in 2m33s
CI / e2e (push) Successful in 9m31s
2026-08-21 15:59:25 +00:00
7 changed files with 432 additions and 24 deletions
+56 -3
View File
@@ -1462,6 +1462,42 @@ vary) wins, ours being told from theirs by **identity** rather than
that ends the gesture is swallowed, keyed on the gesture rather than on
a time window so the first tap on the menu it opened is not eaten too.
**A control revealed by `:hover` is gated on the device having hover,
and which way round depends on whether it is the only route to its
action.** The gate itself is not optional: a touch long-press
synthesises a hover state in the WebView, so every one of these flashed
into view during the 500 ms hold above — a control appearing because
the user was reaching for a different one. Where the action is reachable
another way the control is **absent** on a touch device (the home card's
play button, #68; the queue row's remove, which the row's bottom-sheet
menu carries since #60), and that is `display: none` outside
`(hover: hover) and (pointer: fine)` rather than `opacity: 0` or
`visibility: hidden`, both of which leave a button holding its hit area
and its place in the accessibility tree. Where the control is the
**only** route it is instead always visible under
`@media not all and (hover: hover)``track-details`'s cover-art
overlay and remove, `shortcut-capture`'s reset (#137) — because hiding
it takes the action away entirely.
**Always-visible is not the same as always-in-the-way.** The cover-art
overlay is `inset: 0` at 50% black, which is fine as a hover state and
is not fine as the permanent appearance of the artwork being edited —
and it is only a *hint*, since `.cover-art-edit` carries the click and
tapping the art always worked. Off hover it becomes a corner chip in
the remove button's own language. The × beside it stays full-size,
because that one really is the only route to its action.
One thing to know before checking either: **no *committed* tier renders
as a touch device.** CDP's `Emulation.setEmulatedMedia` does not reach
the component tier's iframe, and the e2e projects are Desktop Chrome
and Desktop Safari, neither of which has touch — a Playwright project
using a mobile descriptor would report `hover: none`, so this is a
choice not to carry one rather than a thing that cannot be done. So
`hover-affordance.test.ts` asserts the *parsed stylesheet* — which rule
sits inside which media query — and says so; the regression it exists
for is someone hoisting a rule out of its query as a tidy-up, which
nothing on a desktop renders differently.
Three lists had no focused row to open a menu *from* — the queue panel
and both playlist detail views — and gained a roving tab stop through
`utils/roving-rows.ts`. **`track-list` deliberately does not use it**:
@@ -2033,9 +2069,26 @@ one that closes the queue — the reported defect moved one press later,
which looks exactly like a press that did nothing.
**And the way out is 44px on a phone.** With the panel spanning the
whole width the scrim has no uncovered pixels at all, so the close
button is the only pointer route out of a full-screen surface; it was
**25×21px**.
whole width there is no scrim there at all, so the close button is the
only pointer route out of a full-screen surface; it was **25×21px**.
**The scrim is drawn only where it can be tapped** (#171). Below 600px
`.panel-content` is `width: 100%`, so the scrim sat entirely underneath
an opaque panel — measured at 424×439, host, panel and scrim all
424×318 — dimming nothing and dismissing nothing while wearing
`cursor: pointer`. #24's tap-outside-to-close cannot exist on a surface
with no outside, and the screen above is what answers it instead: back,
and a 44px close button. The alternative — a gutter, which is the
drawer pattern — was declined, because it buys the affordance by taking
width off a full-screen surface on a 424px viewport. Two things about
it are load-bearing. Its **existence** is `matchMedia`, not
`display: none`, on `job-band`'s rule: a hidden scrim is still an
element carrying the dismissal handler. And **the 600899 band is
untouched**, where the panel is a 320px column of a wider content area
and the scrim has real uncovered pixels — which is why the e2e half
asserts *absence* at 424×439 rather than clicking, since a phone-width
case that clicks the scrim's centre hits the panel and passes on the
broken build.
What this does **not** fix is `page-header` overflowing on its own:
at 900×600 "New Smart Playlist" is still clipped to 114 of 162px with
+29 -2
View File
@@ -179,8 +179,8 @@ test.describe('the queue is a screen where it covers the content', () => {
*/
/**
* With the panel spanning the whole width the scrim has no uncovered
* pixels, so the close button is the only pointer route out of a
* With the panel spanning the whole width there is no scrim here at
* all (#171), so the close button is the only pointer route out of a
* full-screen surface. Measured at 424×439 before #55: **25×21px**.
*/
test('offers a way out a thumb can hit', async ({ app }) => {
@@ -194,6 +194,33 @@ test.describe('the queue is a screen where it covers the content', () => {
expect(box!.width).toBeGreaterThanOrEqual(44);
expect(box!.height).toBeGreaterThanOrEqual(44);
});
/**
* #171 — and it draws no scrim, because there is nowhere to tap.
*
* `.panel-content` is `width: 100%` here, so the scrim sat entirely
* underneath it: measured at 424×439, host, panel and scrim all
* 424×318. #24's tap-outside-to-close cannot exist on a surface with
* no outside, and a `cursor: pointer` layer nobody can reach is a
* claim the component cannot keep.
*
* Asserted as absence rather than by clicking, for the reason the
* issue gives: a naive phone case clicks the scrim's centre and hits
* the panel, so it passes on the build this exists to fail. The scrim
* is still real between 600 and 899px, which `queue-overlay.spec.ts`
* asserts at 900×600 by clicking it.
*/
test('draws no scrim, because a screen has no outside to tap', async ({
app,
}) => {
await openTheQueue(app);
const scrim = await queue(app).evaluate(
(el) => el.shadowRoot!.querySelector('.scrim') !== null,
);
expect(scrim).toBe(false);
});
});
/**
@@ -79,6 +79,17 @@ export class ShortcutCapture extends LitElement {
.reset-btn:hover {
color: var(--yj-accent-text, #ffd43b);
}
/*
* Reset is the only way to put a rebound shortcut back, so where
* the device has no hover it is always visible rather than an
* invisible button holding its hit area. The inverse of #68's
* rule, which applies where the hover control is redundant.
*/
@media not all and (hover: hover) {
.reset-btn {
opacity: 1;
}
}
`;
private handleClick = () => {
@@ -13,6 +13,7 @@ import type { MenuSurface } from '../menu-surface/menu-surface';
import '../menu-surface/menu-surface';
import '@awesome.me/webawesome/dist/components/dropdown-item/dropdown-item.js';
import { QueueController } from '@store/controllers/queue-controller';
import { PHONE_QUERY } from '@utils/breakpoints';
import { creditStore } from '@store/credit-store';
import {
describeQueueSource,
@@ -120,6 +121,25 @@ export class QueuePanel
@property({ type: Boolean, reflect: true })
overlay = false;
/**
* Phone width, from `matchMedia` rather than from a media query,
* because it decides whether the scrim *exists* (#171) —
* `job-band`'s rule, and a stylesheet cannot express it: a
* `display: none` scrim is still an element with a click handler.
*
* Below 600px the panel spans the whole content area, so the scrim
* has no uncovered pixels: measured at 424x439, host, panel and
* scrim are all 424x318 with the scrim entirely underneath. It dims
* nothing and dismisses nothing there, and the queue is a *screen*
* at that width anyway (#55) — back and a 44px close button are its
* ways out. Between 600 and 899 the panel is a 320px column of a
* wider content area, the scrim is reachable, and #24's
* tap-outside-to-close is real; that band is untouched.
*/
@state() private phone = false;
private phoneQuery?: MediaQueryList;
@state()
private isDragging = false;
@@ -391,6 +411,8 @@ export class QueuePanel
display: none;
}
/* Overlay only, and above 600px only -- see the phone field,
which is where that half is decided (#171). */
.scrim {
position: absolute;
inset: 0;
@@ -414,10 +436,10 @@ export class QueuePanel
/* A screen's way out has to be hittable with a thumb.
Measured at 424x439 before #55: these were **25x21px**,
and with the panel spanning the whole width the scrim
underneath has no uncovered pixels at all -- so it was
the only pointer route out of a full-screen surface.
Back answers it now as well, which is the other half.
and with the panel spanning the whole width there is no
scrim here at all (#171) -- so this is the only pointer
route out of a full-screen surface. Back answers it now
as well, which is the other half.
Sized only in overlay mode: inline these sit in a 320px
column beside the content, where a mouse is what reaches
@@ -639,6 +661,24 @@ export class QueuePanel
text-overflow: ellipsis;
}
/*
* The per-row remove is a hover affordance, and on a device
* without hover it is redundant rather than missing: the row's
* context menu is a bottom sheet since #60 and carries "Remove
* from Queue", so the action is one long-press away. An
* always-visible X would instead spend part of a 424px row on
* something already reachable. #68's treatment, for #68's reason.
*
* display:none outside the query rather than visibility:hidden:
* a hidden button still occupies its hit area and is still in
* the accessibility tree, so a phone would keep a target for a
* control it can never see.
*/
.remove-button {
display: none;
}
@media (hover: hover) and (pointer: fine) {
.remove-button {
background: none;
border: none;
@@ -657,6 +697,7 @@ export class QueuePanel
.remove-button:hover {
color: var(--yj-error-text, #ff8787);
}
}
.list-area.drag-over {
outline: 2px dashed var(--yj-accent, #ffd43b);
@@ -828,6 +869,12 @@ export class QueuePanel
// desktop width rather than the minimum.
this.updateOverlayMode();
// Read here rather than in a field initialiser, so a test can
// install its own matchMedia before the element is created.
this.phoneQuery = window.matchMedia?.(PHONE_QUERY);
this.phone = this.phoneQuery?.matches ?? false;
this.phoneQuery?.addEventListener('change', this.onPhoneMedia);
if (this.parentElement) {
this.spaceObserver = new ResizeObserver(() =>
this.updateOverlayMode(),
@@ -870,6 +917,8 @@ export class QueuePanel
this.creditsUnsub = undefined;
this.spaceObserver?.disconnect();
this.spaceObserver = undefined;
this.phoneQuery?.removeEventListener('change', this.onPhoneMedia);
this.phoneQuery = undefined;
document.removeEventListener('keydown', this.onOverlayKeydown);
document.removeEventListener(
'mousemove',
@@ -936,6 +985,10 @@ export class QueuePanel
this.overlay = available - this.panelWidth < MAIN_PANEL_FLOOR;
};
private onPhoneMedia = (e: MediaQueryListEvent): void => {
this.phone = e.matches;
};
/**
* Escape closes a scrimmed overlay, which is the one keyboard rule
* every dialog in this app already follows.
@@ -1972,7 +2025,7 @@ export class QueuePanel
const tracks = this.queue.tracks;
return html`
${this.overlay
${this.overlay && !this.phone
? html`<div
class="scrim"
part="scrim"
@@ -529,6 +529,44 @@ export class TrackDetails extends LitElement {
background: var(--yj-error, #e03131);
}
/*
* Both are the *only* route to changing or removing a track's
* cover art, so where the device has no hover they are always
* visible rather than hidden — the inverse of #68's rule, which
* applies where the hover control is redundant. Revealed by
* opacity, so what is on screen is what the desktop reveal shows
* and nothing about the layout moves.
*/
@media not all and (hover: hover) {
/* The × is genuinely the only route to removing the art, so
on a device that cannot hover it is simply always there.
The pen is not: .cover-art-edit carries the click that
opens the file picker, so tapping the artwork already
worked while the overlay was invisible. It is a discovery
hint — and paying for discovery by covering the artwork
being edited in 50% black, permanently, on every touch
device, is heavier than the hint is worth. It becomes a
corner chip in the remove button's own visual language
instead: same size, same disc, same alpha. */
.cover-art-remove {
opacity: 1;
}
.cover-art-overlay {
opacity: 1;
inset: auto 4px 4px auto;
width: 24px;
height: 24px;
border-radius: 50%;
background: rgba(0, 0, 0, 0.7);
}
.cover-art-overlay wa-icon {
font-size: 14px;
}
}
/* Error message */
.error-message {
flex: 1;
@@ -1,5 +1,6 @@
/**
* A hover affordance is gated on the device having hover.
* A hover affordance is gated on the device having hover — in whichever
* direction keeps the action reachable.
*
* The home page's cover cards reveal a play button on :hover. A touch
* long-press synthesises a hover state in the WebView, so on a phone
@@ -7,6 +8,15 @@
* utils/long-press.ts is measuring for a context menu — a control
* appearing because the user was reaching for a different one.
*
* #137 is the same sweep with the opposite answer for two of its three
* cases. Where the revealed control is the *only* route to its action,
* hiding it removes the action, so it is always visible where there is
* no hover: `track-details`'s cover-art overlay and remove, and
* `shortcut-capture`'s reset. The queue's per-row remove is the third,
* and is the redundant kind — since #60 the row's context menu is a
* bottom sheet carrying "Remove from Queue" — so it takes #68's
* treatment here.
*
* This is asserted against the *parsed stylesheet* rather than by
* emulating a touch device, and that is a limitation worth stating
* rather than hiding. CDP's Emulation.setEmulatedMedia does not reach
@@ -24,6 +34,9 @@
import { describe, expect, it } from 'vitest';
import '@components/home-view/home-view';
import '@components/queue-panel/queue-panel';
import '@components/track-details/track-details';
import '@components/config-page/shortcut-capture';
import { fixture } from '@test/support/render';
/** Every rule in the element's own adopted stylesheets, flattened. */
@@ -83,3 +96,91 @@ describe('the home card play button', () => {
expect(unconditional.some((r) => /display:\s*none/.test(r.text))).toBe(true);
});
});
describe("the queue row's remove button", () => {
it('is absent where the device has no hover, the menu carrying the action', async () => {
const el = await fixture('queue-panel', {});
const rules = rulesOf(el);
expect(rules.length).toBeGreaterThan(0);
// visibility:hidden alone would leave an invisible button holding
// its hit area on a phone, which is the trap #68's commit names.
const unconditional = rules.filter(
(r) => r.condition === null && r.text.startsWith('.remove-button'),
);
expect(unconditional.length).toBeGreaterThan(0);
expect(unconditional.some((r) => /display:\s*none/.test(r.text))).toBe(true);
const reveals = rules.filter(
(r) =>
r.text.includes('.remove-button') && /visibility:\s*visible/.test(r.text),
);
expect(reveals.length).toBeGreaterThan(0);
for (const rule of reveals) {
expect(rule.condition).toMatch(/hover:\s*hover/);
expect(rule.condition).toMatch(/pointer:\s*fine/);
}
});
});
/**
* The two affordances that are the only route to their action.
*
* Asserted as "there is a rule showing it, and its condition is a
* *negated* hover query" — the same stylesheet reading as above, for
* the same reason: this tier's iframe cannot be emulated as a touch
* device, and the regression worth catching is someone folding the rule
* away as redundant on the desktop it does nothing on.
*/
describe('an affordance with no other route', () => {
const cases: Array<[string, string, string[]]> = [
['track-details', 'track-details', ['.cover-art-overlay', '.cover-art-remove']],
['shortcut-capture', 'shortcut-capture', ['.reset-btn']],
];
for (const [name, tag, selectors] of cases) {
it(`${name} shows it where the device has no hover`, async () => {
const el = await fixture(tag, {});
const rules = rulesOf(el);
expect(rules.length).toBeGreaterThan(0);
for (const selector of selectors) {
const shown = rules.filter(
(r) =>
r.condition !== null &&
r.text.includes(selector) &&
/opacity:\s*1/.test(r.text),
);
const touch = shown.filter((r) => /not[\s\S]*hover:\s*hover/.test(r.condition!));
expect(touch.length).toBeGreaterThan(0);
}
});
}
// The one half this tier can measure rather than read: the query is
// negated, so on the hover-capable browser running these tests the
// control must still be revealed by hover and by nothing else. A rule
// written without the `not` would show it here, permanently, on every
// desktop.
it('leaves the desktop reveal alone, where the device does have hover', async () => {
expect(matchMedia('(hover: hover)').matches).toBe(true);
const el = await fixture('shortcut-capture', {
action: 'player.next',
label: 'Next Track',
currentKey: 'X',
defaultKey: 'N',
});
const btn = el.shadowRoot?.querySelector('.reset-btn');
expect(btn).not.toBeNull();
expect(getComputedStyle(btn!).opacity).toBe('0');
});
});
@@ -29,8 +29,61 @@ import { shadow } from '@test/support/render';
const wrappers: HTMLElement[] = [];
let restoreMedia: (() => void) | null = null;
/**
* Answer the shell's phone query with `phone` until restored.
*
* Stubbed rather than emulated, for the reason `search-dialog.test.ts`
* gives: the runner's viewport is fixed at 1280x800, and the panel
* reads `matchMedia` in `connectedCallback` precisely so a test can
* answer it first.
*/
/**
* Answering the phone query is not enough on its own: what decides
* whether the scrim exists is a `change` listener, and a stub whose
* `addEventListener` is a no-op leaves that listener untested — the
* whole suite stays green with it deleted. So the stub records the
* listeners and hands back a way to fire them.
*/
function stubPhone(phone: boolean): (next: boolean) => void {
const real = window.matchMedia.bind(window);
const listeners = new Set<(e: MediaQueryListEvent) => void>();
let matches = phone;
window.matchMedia = ((q: string) =>
q.includes('max-width: 599px')
? {
get matches() {
return matches;
},
media: q,
addEventListener(_: string, fn: (e: MediaQueryListEvent) => void) {
listeners.add(fn);
},
removeEventListener(_: string, fn: (e: MediaQueryListEvent) => void) {
listeners.delete(fn);
},
}
: real(q)) as typeof window.matchMedia;
restoreMedia = () => {
window.matchMedia = real;
};
return (next: boolean) => {
matches = next;
for (const fn of listeners) {
fn({ matches: next } as MediaQueryListEvent);
}
};
}
afterEach(() => {
for (const w of wrappers.splice(0)) w.remove();
restoreMedia?.();
restoreMedia = null;
});
/**
@@ -157,6 +210,78 @@ describe('the queue panel decides whether it can be a column', () => {
}
});
/**
* #171 — the scrim is a dismissal target, so it exists only where it
* has pixels to be tapped.
*
* Below 600px `.panel-content` is `width: 100%`, so the scrim is
* entirely underneath an opaque panel: measured at 424x439, host,
* panel and scrim all 424x318. Drawing it there is a `cursor:
* pointer` click target nobody can reach, and the queue is a screen
* at that width anyway (#55) — back and the close button are its ways
* out. Existence rather than `display: none`, because a hidden scrim
* is still an element carrying the handler.
*/
it('draws no scrim at phone width, where it would have no reachable pixels', async () => {
stubPhone(true);
const el = await panelIn(424);
expect(el.overlay).toBe(true);
expect(el.shadowRoot?.querySelector('.scrim')).toBeNull();
// The way out a thumb can hit is still there.
expect(
shadow(el, '[data-testid="queue-close"]')?.getAttribute('aria-label'),
).toBe('Close queue');
});
/**
* The 600899 band is where the panel is a 320px column of a wider
* content area, so the scrim has uncovered pixels and #24's
* tap-outside-to-close is real. Same width as the overlay tests
* above, with the phone query explicitly answered `false`, so this
* fails if the scrim is ever dropped for every overlay.
*/
it('keeps the scrim above phone width, where it can be tapped', async () => {
stubPhone(false);
const el = await panelIn(700);
expect(el.overlay).toBe(true);
shadow<HTMLElement>(el, '.scrim')?.click();
await el.updateComplete;
expect(el.open).toBe(false);
});
/**
* The scrim's existence comes from `matchMedia` rather than a
* stylesheet, which only holds up if the query is *listened* to — a
* panel opened on a desktop and carried across the breakpoint (a
* resized window, an unfolded phone) has to lose its scrim without
* being reopened. Nothing else in this file fires `change`, so
* deleting the listener leaves the whole suite green.
*/
it('drops the scrim when the viewport crosses the breakpoint', async () => {
const setPhone = stubPhone(false);
const el = await panelIn(700);
expect(el.shadowRoot?.querySelector('.scrim')).not.toBeNull();
setPhone(true);
await el.updateComplete;
expect(el.shadowRoot?.querySelector('.scrim')).toBeNull();
setPhone(false);
await el.updateComplete;
expect(el.shadowRoot?.querySelector('.scrim')).not.toBeNull();
});
/**
* Escape belongs to the overlay, not to the queue. An inline panel is
* beside the content rather than over it, so there is nothing to