fix(ui): keep a touch-only affordance reachable, or absent
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m26s
CI / e2e (pull_request) Successful in 9m10s

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
This commit is contained in:
2026-08-21 06:40:26 -04:00
parent 14e3ab574c
commit 5fbe1d9923
5 changed files with 186 additions and 14 deletions
+26
View File
@@ -1462,6 +1462,32 @@ 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.
One thing to know before checking either: **no tier here can render 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. 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**:
@@ -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 = () => {
@@ -639,23 +639,42 @@ 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 {
background: none;
border: none;
color: var(--yj-text-tertiary, #888);
cursor: pointer;
padding: 4px;
display: flex;
align-items: center;
visibility: hidden;
display: none;
}
.track-item:hover .remove-button {
visibility: visible;
}
@media (hover: hover) and (pointer: fine) {
.remove-button {
background: none;
border: none;
color: var(--yj-text-tertiary, #888);
cursor: pointer;
padding: 4px;
display: flex;
align-items: center;
visibility: hidden;
}
.remove-button:hover {
color: var(--yj-error-text, #ff8787);
.track-item:hover .remove-button {
visibility: visible;
}
.remove-button:hover {
color: var(--yj-error-text, #ff8787);
}
}
.list-area.drag-over {
@@ -529,6 +529,21 @@ 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) {
.cover-art-overlay,
.cover-art-remove {
opacity: 1;
}
}
/* 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');
});
});