From 3aa2a434b4a0cf989087b5cf0eb46e1ffa3434ef Mon Sep 17 00:00:00 2001 From: Logan Date: Mon, 24 Aug 2026 03:51:12 -0400 Subject: [PATCH] feat(android): the tap highlight goes, a press state replaces it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The phone drew a grey box over the bounding rect of whatever was tapped, which is the web view saying what it is. It is gone in one declaration: `-webkit-tap-highlight-color` is inherited and an inherited property crosses a shadow boundary, so `html` in index.css reaches every shadow root in the app. Measured three roots deep, rgba(0, 0, 0, 0.18) before and rgba(0, 0, 0, 0) after. Removing it removes the only touch feedback several surfaces had, so the press state is part of the same change rather than a later polish item — with the highlight gone a held row measured the *hover* tint, which on a phone is synthesised by the hold itself and outlives it. The four lists' rows, the tab bar, the sidebar's destinations and the shared context-menu item take --yj-press-overlay on :active; the cards already had scale(0.97). The press selector carries a state class because a row is .track-row.selected.active, so a bare :active shows nothing on the row a phone is most likely to press. And those surfaces' hover tints move behind (hover: hover) and (pointer: fine), which is #68's gate applied to a tint rather than a revealed control. user-select, the other half of the Findings, was already done: the first rule in index.css covers the shadow roots for the same reason. touch-action: manipulation is declined — the 300ms delay it is offered for is already absent on a width=device-width viewport, and what it would really change is the gesture stack tuned by measurement on a device this session cannot measure. Closes #54 --- .planning/NOTES.md | 35 ++++ CLAUDE.md | 45 +++++ e2e/specs/native-touch-feel.spec.ts | 140 +++++++++++++ frontend/index.css | 20 ++ .../src/components/bottom-nav/bottom-nav.ts | 11 + .../playlist-details/playlist-details.ts | 35 +++- .../src/components/queue-panel/queue-panel.ts | 23 ++- .../src/components/sidebar/app-sidebar.ts | 19 +- .../smart-playlist-details.ts | 23 ++- .../src/components/track-list/track-list.ts | 29 ++- frontend/src/store/theme-store.ts | 15 ++ frontend/src/utils/context-menu-controller.ts | 31 ++- .../test/components/press-feedback.test.ts | 191 ++++++++++++++++++ 13 files changed, 599 insertions(+), 18 deletions(-) create mode 100644 e2e/specs/native-touch-feel.spec.ts create mode 100644 frontend/test/components/press-feedback.test.ts diff --git a/.planning/NOTES.md b/.planning/NOTES.md index 8283ae8..31f3767 100644 --- a/.planning/NOTES.md +++ b/.planning/NOTES.md @@ -4994,3 +4994,38 @@ ordinary track makes it 9. Filed as its own issue rather than fixed in #67's diff: it is a property of the shared sheet (`components/menu-surface/`), not of the items. + +## The tap highlight is one inherited declaration (measured 2026-08-24) + +`-webkit-tap-highlight-color` is an **inherited** property, and an +inherited property crosses a shadow boundary — so `html { … : +transparent }` in `index.css` reaches every shadow root in the app and +no component needs a rule of its own. Measured in the running app +(Chromium, `app-sidebar`'s `li button`, which is three shadow roots +from the document): `rgba(0, 0, 0, 0)` with the rule, and +`rgba(0, 0, 0, 0.18)` with it removed. That 0.18 grey over the bounding +rect of whatever was tapped is what #54 reported. + +The same argument was already spent once and is worth not +re-deriving: `index.css`'s first rule is `*, *::before, *::after { +user-select: none }`, which for the same reason already covers the +shadow roots — #54's Findings ask for `user-select` on interactive +surfaces and it has been done since before the issue was filed. + +**What the highlight was, on the surfaces that had nothing else, is the +press feedback.** Measured on a track row with the press rule removed +and the button held down: `rgba(255, 255, 255, 0.05)` — the *hover* +tint, arriving because the pointer is over the row, which is a +synthesised hover on a phone and outlives the press. With the rule: +0.12 while held, and the neighbouring row unchanged. So the press state +is part of removing the highlight rather than a separate polish item, +and the hover tints on those same surfaces moved behind +`(hover: hover) and (pointer: fine)`, which is #68's gate applied to a +tint rather than to a revealed control. + +**`touch-action: manipulation` was considered and not taken.** The +Findings offer it for the 300ms tap delay; this app's viewport is +`width=device-width`, which is what removes that delay in Chrome, so +the stated benefit is not there to win. What it would change is the +gesture stack #63 tuned by measurement on the device (`pan-y` plus a +non-passive `preventDefault`), and that is not measurable from here. diff --git a/CLAUDE.md b/CLAUDE.md index 5122044..763e8a2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1631,6 +1631,51 @@ 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. +**The web view's own tap highlight is gone, and what replaced it is a +press state** (#54). `-webkit-tap-highlight-color` is an *inherited* +property, so one declaration on `html` in `index.css` reaches every +shadow root in the app and takes away the grey box a phone drew over +the bounding rect of whatever was tapped — measured at +`rgba(0, 0, 0, 0.18)` with the rule removed. `user-select` is the same +argument and was already done: `index.css`'s first rule is `*, *::before, +*::after { user-select: none }`, which reaches the shadow roots for the +same reason. + +Three things about it are load-bearing. + +**Removing the highlight removes the only touch feedback several +surfaces had**, so the press state is part of the same change rather +than a later polish item: the four lists' rows, `bottom-nav`'s tabs, +`app-sidebar`'s destinations (which are also the phone's "More" sheet) +and the shared `contextMenuStyles` menu item all take +`--yj-press-overlay` on `:active`. The cards already had one +(`transform: scale(0.97)`) and are untouched. + +**A press selector carries a state class or it does nothing where it +matters.** A row is `.track-row.selected.active`, so a bare +`.track-row:active` is one class short of it and the press is invisible +on exactly the row a phone is most likely to press — the one it has +just selected. The rule is last and lists `.selected:active` / +`.active:active` beside the bare form. + +**And the hover tints on those same surfaces moved behind +`(hover: hover) and (pointer: fine)`**, which is #68's gate applied to +a tint rather than to a revealed control and for the same mechanism: a +hold synthesises a hover in the WebView, so an ungated tint arrives +because a finger touched the row and stays there after it has gone — +measured, since with the press rule removed a held row reads +`rgba(255, 255, 255, 0.05)`, the hover tint, rather than nothing. +`touch-action: manipulation` was considered and declined: the 300ms +delay it is offered for is already absent on a `width=device-width` +viewport, and what it would really change is the gesture stack #63 +tuned by measurement on a device this session cannot measure. + +The split of tiers is `hover-affordance.test.ts`'s: `press-feedback. +test.ts` reads the parsed stylesheet, because `:active` cannot be +forced there either, and `native-touch-feel.spec.ts` *measures* — it +holds the button down on a real row of the real list, and it is the +only tier that loads `index.css` at all. + 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**: diff --git a/e2e/specs/native-touch-feel.spec.ts b/e2e/specs/native-touch-feel.spec.ts new file mode 100644 index 0000000..acea470 --- /dev/null +++ b/e2e/specs/native-touch-feel.spec.ts @@ -0,0 +1,140 @@ +import { test, expect } from '../support/fixtures.js'; + +/** + * The web view's own tap highlight, and what replaced it (#54). + * + * Two halves, and each is here because no other tier can see it. + * + * **The highlight is killed by one declaration on `html`**, which + * reaches the app's shadow roots because `-webkit-tap-highlight-color` + * is inherited and inheritance crosses a shadow boundary. That is a + * property of `index.css`, and `index.css` is loaded by the real app + * and by nothing else — the component tier mounts a component with no + * page stylesheet at all, which is the same reason the theme's ramps + * are invisible to it. + * + * **The press state is measured rather than read.** The component tier + * asserts the shape of the stylesheet (which rule is inside which + * query, and that the press selector carries a state class), because + * `:active` cannot be forced there. Here there is a real pointer: hold + * the button down on a real row of the real list and read what the row + * became. That is the assertion that would fail if the rule were + * hoisted, renamed, or lost to `.selected`. + * + * What neither half is, is the device. Chrome 113's WebView is where + * the grey box was reported and where a finger is; the numbers from it + * are on the PR. + */ +type Page = import('@playwright/test').Page; + +/** The phone this work was measured against, in CSS pixels. */ +const DEVICE = { width: 424, height: 439 }; + +/** The computed tap-highlight colour of a node inside a shadow root. */ +const tapHighlight = (page: Page, host: string, inner: string) => + page.evaluate( + ([hostSel, innerSel]) => { + const el = document + .querySelector(hostSel!) + ?.shadowRoot?.querySelector(innerSel!); + + if (!el) return null; + + return getComputedStyle(el).getPropertyValue( + '-webkit-tap-highlight-color', + ); + }, + [host, inner], + ); + +test.describe('the tap highlight', () => { + test('is transparent inside a shadow root, from one rule on html', async ({ + app, + browserName, + }) => { + await app.getByTestId('nav-tracks').click(); + await expect(app.getByTestId('main-content')).toHaveAttribute( + 'data-active-view', + 'tracks', + ); + + const row = await tapHighlight(app, 'track-list', '.track-row'); + + expect(row).not.toBeNull(); + + // The property is a WebKit extension that only iOS honours, so an + // engine is free not to report one at all. Chromium always does — + // measured at rgba(0, 0, 0, 0.18) with the rule removed, which is + // the grey box the report describes — so the assertion is not + // skippable there, and nothing this app can do makes the property + // disappear on an engine that has it. + test.skip( + row === '', + `${browserName} reports no -webkit-tap-highlight-color to read`, + ); + + expect(row).toBe('rgba(0, 0, 0, 0)'); + }); +}); + +test.describe('the press state that replaced it', () => { + test.beforeEach(async ({ app }) => { + await app.setViewportSize(DEVICE); + await app.getByTestId('tab-tracks').click(); + await expect(app.getByTestId('main-content')).toHaveAttribute( + 'data-active-view', + 'tracks', + ); + await expect(app.locator('track-list').first()).toBeVisible(); + }); + + test.afterEach(async ({ app }) => { + await app.mouse.up(); + await app.setViewportSize({ width: 1440, height: 900 }); + }); + + test('shows on the row being pressed, and on that row only', async ({ + app, + }) => { + const rows = await app.evaluate(() => { + const found = document + .querySelector('track-list') + ?.shadowRoot?.querySelectorAll('.track-row'); + + if (!found || found.length < 2) return null; + + const rect = found[1]!.getBoundingClientRect(); + + return { + x: Math.round(rect.x + rect.width / 2), + y: Math.round(rect.y + rect.height / 2), + }; + }); + + expect(rows).not.toBeNull(); + + const backgrounds = () => + app.evaluate(() => { + const found = document + .querySelector('track-list')! + .shadowRoot!.querySelectorAll('.track-row'); + + return { + pressed: getComputedStyle(found[1]!).backgroundColor, + neighbour: getComputedStyle(found[2]!).backgroundColor, + }; + }); + + await app.mouse.move(rows!.x, rows!.y); + await app.mouse.down(); + + const held = await backgrounds(); + + // The press overlay, from the theme rather than from a literal in + // a component: rgba(255, 255, 255, 0.12) on both dark ramps. + expect(held.pressed).toBe('rgba(255, 255, 255, 0.12)'); + expect(held.neighbour).not.toBe(held.pressed); + + await app.mouse.up(); + }); +}); diff --git a/frontend/index.css b/frontend/index.css index a7e2438..ac8d257 100644 --- a/frontend/index.css +++ b/frontend/index.css @@ -7,6 +7,26 @@ html { height: 100%; + + /* #54. The web view's own tap highlight -- the grey box a phone + draws over the bounding rect of whatever was tapped -- gone in + one declaration, because `-webkit-tap-highlight-color` is an + *inherited* property and an inherited property crosses a shadow + boundary. So this reaches every one of the app's shadow roots + without a rule in any of them; before it, exactly one component + (`library-status-indicator`) set it and the box appeared + everywhere else. + + What it costs is the only touch feedback several surfaces had, + which is why the rows, the tab bar and the shared menu items + grew a `:active` state in the same change: removing the wrong + feedback and leaving none is not an improvement. The cards + already had one (`transform: scale(0.97)`). + + `user-select` is the same argument one rule up and was already + done: the `*` rule at the top of this file is inherited into the + shadow roots too. */ + -webkit-tap-highlight-color: transparent; } body { diff --git a/frontend/src/components/bottom-nav/bottom-nav.ts b/frontend/src/components/bottom-nav/bottom-nav.ts index c029664..4a4637b 100644 --- a/frontend/src/components/bottom-nav/bottom-nav.ts +++ b/frontend/src/components/bottom-nav/bottom-nav.ts @@ -89,6 +89,17 @@ export class BottomNav extends LitElement { color: var(--yj-accent, #ffd43b); } + /* The press state (#54). This bar is the phone's primary + navigation and had no feedback of its own at all -- what a + tap produced was the web view's tap highlight, a grey box + over the whole 48px cell, which index.css has now taken + away. The .active rule above is which tab you are *on*; this + is the tab being pressed, so they are a colour and a + background rather than two colours. */ + button:active { + background-color: var(--yj-press-overlay, rgba(255, 255, 255, 0.12)); + } + button:focus-visible { outline: 2px solid var(--yj-accent, #ffd43b); outline-offset: -2px; diff --git a/frontend/src/components/playlist-details/playlist-details.ts b/frontend/src/components/playlist-details/playlist-details.ts index 7f32a9e..e110353 100644 --- a/frontend/src/components/playlist-details/playlist-details.ts +++ b/frontend/src/components/playlist-details/playlist-details.ts @@ -1357,8 +1357,14 @@ export class PlaylistDetails user-select: none; } - .track-item:hover { - background-color: var(--yj-hover-overlay, rgba(255, 255, 255, 0.05)); + /* A hover tint is for a device that hovers (#54). A hold + synthesises a hover in the WebView, so ungated this arrives + because a finger touched the row and stays after it has + gone; the press state below is what a tap gets instead. */ + @media (hover: hover) and (pointer: fine) { + .track-item:hover { + background-color: var(--yj-hover-overlay, rgba(255, 255, 255, 0.05)); + } } .track-item.selected { @@ -1378,11 +1384,13 @@ export class PlaylistDetails cursor: pointer; } - .track-item.phantom:hover { - background-color: var( - --yj-hover-overlay, - rgba(255, 255, 255, 0.05) - ); + @media (hover: hover) and (pointer: fine) { + .track-item.phantom:hover { + background-color: var( + --yj-hover-overlay, + rgba(255, 255, 255, 0.05) + ); + } } .track-item.phantom.selected { @@ -1392,6 +1400,19 @@ export class PlaylistDetails ); } + /* The press state (#54): the feedback a tap has now that the + web view's own highlight box is gone (index.css). Last, and + carrying a class, because a selected or playing row is two + classes deep and a bare :active would lose to it. */ + .track-item.selected:active, + .track-item.active:active, + .track-item:active { + background-color: var( + --yj-press-overlay, + rgba(255, 255, 255, 0.12) + ); + } + .phantom-row { grid-column: 1 / -1; display: flex; diff --git a/frontend/src/components/queue-panel/queue-panel.ts b/frontend/src/components/queue-panel/queue-panel.ts index 016151f..97d8e57 100644 --- a/frontend/src/components/queue-panel/queue-panel.ts +++ b/frontend/src/components/queue-panel/queue-panel.ts @@ -581,8 +581,14 @@ export class QueuePanel contain: strict; } - .track-item:hover { - background-color: var(--yj-hover-overlay, rgba(255, 255, 255, 0.05)); + /* A hover tint is for a device that hovers (#54). A hold + synthesises a hover in the WebView, so ungated this arrives + because a finger touched the row and stays after it has + gone; the press state below is what a tap gets instead. */ + @media (hover: hover) and (pointer: fine) { + .track-item:hover { + background-color: var(--yj-hover-overlay, rgba(255, 255, 255, 0.05)); + } } .track-item.selected { @@ -597,6 +603,19 @@ export class QueuePanel background-color: var(--yj-selection-bg, rgba(100, 160, 255, 0.15)); } + /* The press state (#54): the feedback a tap has now that the + web view's own highlight box is gone (index.css). Last, and + carrying a class, because a selected or playing row is two + classes deep and a bare :active would lose to it. */ + .track-item.selected:active, + .track-item.active:active, + .track-item:active { + background-color: var( + --yj-press-overlay, + rgba(255, 255, 255, 0.12) + ); + } + .track-position { font-size: var(--yj-text-sm); color: var(--yj-text-tertiary, #888); diff --git a/frontend/src/components/sidebar/app-sidebar.ts b/frontend/src/components/sidebar/app-sidebar.ts index 5e4e8a1..f97b263 100644 --- a/frontend/src/components/sidebar/app-sidebar.ts +++ b/frontend/src/components/sidebar/app-sidebar.ts @@ -95,8 +95,15 @@ export class AppSidebar extends LitElement { text-align: center; } - li button:hover { - background-color: var(--yj-bg-elevated, #343a40); + /* A hover tint is for a device that hovers (#54), and this + component is on a phone too: below 600px it is what + bottom-nav's "More" sheet mounts, where a hold + synthesises a hover and leaves a destination looking picked + after the finger has gone. */ + @media (hover: hover) and (pointer: fine) { + li button:hover { + background-color: var(--yj-bg-elevated, #343a40); + } } li button:focus-visible { @@ -108,6 +115,14 @@ export class AppSidebar extends LitElement { background-color: var(--yj-bg-overlay, #495057); } + /* The press state (#54), after the .active rule and at the same + specificity, so pressing the destination you are already on + still says something. It is what a tap gets now that + index.css has taken the web view's own highlight box away. */ + li button:active { + background-color: var(--yj-press-overlay, rgba(255, 255, 255, 0.12)); + } + li button p { margin: 0; white-space: nowrap; 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 e918db1..d68706b 100644 --- a/frontend/src/components/smart-playlist-details/smart-playlist-details.ts +++ b/frontend/src/components/smart-playlist-details/smart-playlist-details.ts @@ -516,8 +516,14 @@ export class SmartPlaylistDetails user-select: none; } - .track-item:hover { - background-color: var(--yj-hover-overlay, rgba(255, 255, 255, 0.05)); + /* A hover tint is for a device that hovers (#54). A hold + synthesises a hover in the WebView, so ungated this arrives + because a finger touched the row and stays after it has + gone; the press state below is what a tap gets instead. */ + @media (hover: hover) and (pointer: fine) { + .track-item:hover { + background-color: var(--yj-hover-overlay, rgba(255, 255, 255, 0.05)); + } } .track-item.selected { @@ -533,6 +539,19 @@ export class SmartPlaylistDetails background-color: var(--yj-selection-bg, rgba(100, 160, 255, 0.15)); } + /* The press state (#54): the feedback a tap has now that the + web view's own highlight box is gone (index.css). Last, and + carrying a class, because a selected or playing row is two + classes deep and a bare :active would lose to it. */ + .track-item.selected:active, + .track-item.active:active, + .track-item:active { + background-color: var( + --yj-press-overlay, + rgba(255, 255, 255, 0.12) + ); + } + /* Phantom rows span the full grid */ .track-item.phantom { display: grid; diff --git a/frontend/src/components/track-list/track-list.ts b/frontend/src/components/track-list/track-list.ts index 226a6ec..9e1c31b 100644 --- a/frontend/src/components/track-list/track-list.ts +++ b/frontend/src/components/track-list/track-list.ts @@ -1201,8 +1201,16 @@ export class TrackList padding-left: 6px; } - .track-row:hover { - background-color: var(--yj-hover-overlay, rgba(255, 255, 255, 0.05)); + /* A hover tint is for a device that hovers (#54): a hold + synthesises a hover in the WebView, so ungated this is a + highlight that arrives because a finger touched the row and + then stays there after it has gone -- which reads as a + selection the user did not make. Same gate, and the same + mechanism, as #68's revealed controls. */ + @media (hover: hover) and (pointer: fine) { + .track-row:hover { + background-color: var(--yj-hover-overlay, rgba(255, 255, 255, 0.05)); + } } .track-row.selected { @@ -1239,6 +1247,23 @@ export class TrackList background-color: var(--yj-selection-bg, rgba(100, 160, 255, 0.15)); } + /* The press state (#54), and the only feedback a tap has now that + the web view's tap highlight is gone (index.css). + + **Last, and as specific as the state rules above**: a row that + is selected and playing is .track-row.selected.active, so a + bare .track-row:active is one class short of it and a press + on the row a phone is most likely to press -- the one it just + selected -- would show nothing. Instant rather than + transitioned, because the only measured statement here about + transitions on a list is that two card grids removed theirs + for software-rendering repaint cost. */ + .track-row.selected:active, + .track-row.active:active, + .track-row:active { + background-color: var(--yj-press-overlay, rgba(255, 255, 255, 0.12)); + } + .cell { overflow: hidden; diff --git a/frontend/src/store/theme-store.ts b/frontend/src/store/theme-store.ts index 770f728..0a981ea 100644 --- a/frontend/src/store/theme-store.ts +++ b/frontend/src/store/theme-store.ts @@ -46,6 +46,17 @@ export interface ShadePalette { border: string; borderSubtle: string; hoverOverlay: string; + /** + * The tint a surface takes while it is being pressed (#54). + * + * Separate from `hoverOverlay` because the two answer different + * questions and only one of them a phone can ask: a hover is a + * pointer resting somewhere, a press is a finger on the thing it + * is about to activate. It is deliberately the stronger of the + * two — a press that reads the same as a hover says nothing on a + * device where the hover is synthesised by the press itself. + */ + pressOverlay: string; selectionBg: string; } @@ -95,6 +106,7 @@ export const SHADE_PALETTES: Record = { border: '#333333', borderSubtle: '#222222', hoverOverlay: 'rgba(255, 255, 255, 0.05)', + pressOverlay: 'rgba(255, 255, 255, 0.12)', selectionBg: 'rgba(100, 160, 255, 0.15)', }, dark: { @@ -114,6 +126,7 @@ export const SHADE_PALETTES: Record = { border: '#444444', borderSubtle: '#333333', hoverOverlay: 'rgba(255, 255, 255, 0.05)', + pressOverlay: 'rgba(255, 255, 255, 0.12)', selectionBg: 'rgba(100, 160, 255, 0.15)', }, light: { @@ -133,6 +146,7 @@ export const SHADE_PALETTES: Record = { border: '#ced4da', borderSubtle: '#dee2e6', hoverOverlay: 'rgba(0, 0, 0, 0.05)', + pressOverlay: 'rgba(0, 0, 0, 0.12)', selectionBg: 'rgba(100, 160, 255, 0.15)', }, }; @@ -285,6 +299,7 @@ function deriveThemeVariables( // Interactive overlays '--yj-hover-overlay': palette.hoverOverlay, + '--yj-press-overlay': palette.pressOverlay, '--yj-selection-bg': palette.selectionBg, // Semantic *fills* — the background of a solid button or badge. diff --git a/frontend/src/utils/context-menu-controller.ts b/frontend/src/utils/context-menu-controller.ts index 53d51c5..bed0922 100644 --- a/frontend/src/utils/context-menu-controller.ts +++ b/frontend/src/utils/context-menu-controller.ts @@ -710,10 +710,35 @@ export const contextMenuStyles = css` font-size: 13px; } - .context-menu-panel wa-dropdown-item:hover { + /* A hover tint is for a device that hovers (#54). + + Below the query is a phone, where a hold *synthesises* a hover + in the WebView -- the same mechanism #68 gates the revealed + controls on -- so an ungated tint is a highlight that arrives + because a finger touched the row and then stays on it after the + finger has gone. Which is indistinguishable from the press + state below, and outlives it. */ + @media (hover: hover) and (pointer: fine) { + .context-menu-panel wa-dropdown-item:hover { + background-color: var( + --yj-hover-overlay, + rgba(255, 255, 255, 0.1) + ); + } + } + + /* And a press state is for every device, because it is the one + piece of feedback a tap has now that the web view's own + highlight box is gone (index.css). Stronger than the hover tint + on purpose, and instant rather than transitioned: the only + measured statement this repo has about transitions on these + surfaces is the two card grids that removed theirs because + software rendering repaints per frame, and the phone is not + something this session can measure. */ + .context-menu-panel wa-dropdown-item:active { background-color: var( - --yj-hover-overlay, - rgba(255, 255, 255, 0.1) + --yj-press-overlay, + rgba(255, 255, 255, 0.12) ); } diff --git a/frontend/test/components/press-feedback.test.ts b/frontend/test/components/press-feedback.test.ts new file mode 100644 index 0000000..4b67a71 --- /dev/null +++ b/frontend/test/components/press-feedback.test.ts @@ -0,0 +1,191 @@ +/** + * What a tap looks like now that the web view's own highlight is gone + * (#54). + * + * `index.css` sets `-webkit-tap-highlight-color: transparent` on + * `html`, which — the property being inherited — reaches every shadow + * root in the app. That takes away the grey box a phone drew over the + * bounding rect of whatever was tapped, and with it the only touch + * feedback the rows, the tab bar, the sidebar's destinations and the + * shared menu items had. So the press states below are not decoration: + * without them this change trades wrong feedback for none. + * + * **Asserted against the parsed stylesheet**, on `hover-affordance`'s + * precedent and with the same limitation stated out loud: CDP's + * `Emulation.setEmulatedMedia` does not reach this tier's iframe, so + * there is no way here to render a component as a phone would, and + * `:active` cannot be forced from a test either. What the browser will + * answer is the shape it built from the `css` literal — which rule sits + * inside which media query, and what the press selector actually is. + * + * Two regressions are worth catching that way, and both are silent on a + * desktop: + * + * - someone hoisting a hover tint back out of its query as a tidy-up, + * which on a phone is a highlight that arrives because a finger + * touched the row and stays after it has gone; + * - someone simplifying the press selector to a bare `:active`, which + * is one class short of `.selected` / `.active` and so does nothing + * on the row a phone is most likely to press — the one it has just + * selected. + * + * The pixels are the Android tier's, and the tap highlight itself is + * `e2e/specs/native-touch-feel.spec.ts`, since only the real app loads + * `index.css` at all. + */ +import { describe, expect, it } from 'vitest'; + +import '@components/track-list/track-list'; +import '@components/queue-panel/queue-panel'; +import '@components/playlist-details/playlist-details'; +import '@components/smart-playlist-details/smart-playlist-details'; +import '@components/bottom-nav/bottom-nav'; +import '@components/sidebar/app-sidebar'; +import { fixture } from '@test/support/render'; + +/** Every rule in the element's own adopted stylesheets, flattened. */ +function rulesOf(host: Element): { text: string; condition: string | null }[] { + const sheets = host.shadowRoot?.adoptedStyleSheets ?? []; + const out: { text: string; condition: string | null }[] = []; + + for (const sheet of sheets) { + for (const rule of Array.from(sheet.cssRules)) { + if (rule instanceof CSSMediaRule) { + for (const inner of Array.from(rule.cssRules)) { + out.push({ text: inner.cssText, condition: rule.conditionText }); + } + + continue; + } + + out.push({ text: rule.cssText, condition: null }); + } + } + + return out; +} + +/** The four lists, their row selector, and the tag that draws them. */ +const LISTS: Array<[string, string]> = [ + ['track-list', '.track-row'], + ['queue-panel', '.track-item'], + ['playlist-details', '.track-item'], + ['smart-playlist-details', '.track-item'], +]; + +describe('a row says it is being pressed', () => { + for (const [tag, row] of LISTS) { + it(`${tag} draws a press state that survives its state classes`, async () => { + const el = await fixture(tag, {}); + const rules = rulesOf(el); + + // Worth nothing if it read no rules at all — the first assertion + // icon-language.test.ts makes, for the same reason. + expect(rules.length).toBeGreaterThan(0); + + const press = rules.filter( + (r) => r.text.includes(`${row}:active`) && r.text.includes('background-color'), + ); + + expect(press.length).toBeGreaterThan(0); + + for (const rule of press) { + // A press is not a hover: it is the one thing a touch device + // can say, so it must not sit behind a pointer query. + expect(rule.condition).toBeNull(); + expect(rule.text).toContain('--yj-press-overlay'); + } + + // The load-bearing half: the selector carries a state class, or + // it loses to `.selected` / `.selected.active` and the press is + // invisible on a selected or playing row. + expect(press.some((r) => r.text.includes(`${row}.selected:active`))).toBe(true); + }); + + it(`${tag} keeps its hover tint for devices that hover`, async () => { + const el = await fixture(tag, {}); + const rules = rulesOf(el); + + expect(rules.length).toBeGreaterThan(0); + + const hover = rules.filter( + (r) => + r.text.includes(`${row}:hover`) && + r.text.includes('--yj-hover-overlay'), + ); + + expect(hover.length).toBeGreaterThan(0); + + for (const rule of hover) { + expect(rule.condition).toMatch(/hover:\s*hover/); + expect(rule.condition).toMatch(/pointer:\s*fine/); + } + }); + } +}); + +describe('the two navigations say they are being pressed', () => { + it('the phone tab bar, which had no state of its own at all', async () => { + const el = await fixture('bottom-nav', {}); + const rules = rulesOf(el); + + expect(rules.length).toBeGreaterThan(0); + + const press = rules.filter((r) => r.text.startsWith('button:active')); + + expect(press.length).toBe(1); + expect(press[0]!.condition).toBeNull(); + expect(press[0]!.text).toContain('--yj-press-overlay'); + }); + + it("the sidebar, which is also the phone's More sheet", async () => { + const el = await fixture('app-sidebar', {}); + const rules = rulesOf(el); + + expect(rules.length).toBeGreaterThan(0); + + const press = rules.filter((r) => r.text.startsWith('li button:active')); + + expect(press.length).toBe(1); + expect(press[0]!.condition).toBeNull(); + expect(press[0]!.text).toContain('--yj-press-overlay'); + + // Its hover tint is a destination looking picked, if it is left to + // a synthesised hover inside the More sheet. + const hover = rules.filter((r) => r.text.startsWith('li button:hover')); + + expect(hover.length).toBeGreaterThan(0); + + for (const rule of hover) { + expect(rule.condition).toMatch(/hover:\s*hover/); + } + }); +}); + +describe('the shared context menu', () => { + // One stylesheet, fourteen menus — the same reason the sheet's row + // height lives there rather than in each host. + it('presses its items, in the one place every menu includes', async () => { + const el = await fixture('queue-panel', {}); + const rules = rulesOf(el); + + const press = rules.filter((r) => + r.text.startsWith('.context-menu-panel wa-dropdown-item:active'), + ); + + expect(press.length).toBe(1); + expect(press[0]!.condition).toBeNull(); + expect(press[0]!.text).toContain('--yj-press-overlay'); + + const hover = rules.filter((r) => + r.text.startsWith('.context-menu-panel wa-dropdown-item:hover'), + ); + + expect(hover.length).toBeGreaterThan(0); + + for (const rule of hover) { + expect(rule.condition).toMatch(/hover:\s*hover/); + expect(rule.condition).toMatch(/pointer:\s*fine/); + } + }); +});