yonlu/yellowjacket
1
1
Fork 0
Code Issues 55 Pull Requests 6 Actions Packages Projects Releases 12 Wiki Activity

fix(ui): raise the last controls below the touch floor
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Failing after 2m32s
Details
CI / e2e (pull_request) Skipped
Details

Browse Source 
The rest of #186's second table, and one thing it could not have said.

    .section-toggle          187x15  autotag
    .folders-menu-trigger     32x18  autotag
    .back-button              32x32  artist-details
    Requests / Downloads tabs 85x34, 96x34
    .search-mode-tab          89x26, 79x26  explore
    explore search input     325x18  in a 36px box

**back-button was six controls, not one.** The issue names it in
artist-details because that is the view the sweep opened; the same
declaration is byte-identical in artist-details, genre-details,
playlist-details, smart-playlist-details, explore-artist-details and
explore-album-details, 32px in all six. So it is styles/back-button.
css.ts now, adopted by each, and a source sweep fails on a seventh
copy -- because the failure this invites is not a size changing, it is
somebody adding a detail view and writing `.back-button` out again,
which no device sweep would catch for the same reason this one did
not. That is icon-language.test.ts's shape, and the argument for it
here is the inverse of the column arrows': one declaration covering
36 controls is cheap to fix, and six declarations of one control are
six chances to miss five.

It is a real 44px box rather than padding with the width handed back:
a detail header runs no fit pass, and this button has a visible
background, so a hit area larger than the circle would be a control
bigger than it looks. The size is #55's, reached there for the same
reason -- "the way out is 44px on a phone".

**The explore search box was two faults.** The row was 36px *and* the
input inside it was 18, so eight pixels at each edge were not a target
at all: a tap near the top of the box landed on the container and did
nothing. The container is 44 and the input stretches to it.

**The Downloads tabs take padding rather than a min-size**, because
the mark for the selected tab is its bottom border -- a min-size
centres the label and leaves the underline 10px beneath it.

page-action-check-now (113x29) is in that table and is not here: it is
a PageAction, so #195 raised it with the rest of the header's actions
and touch-targets.test.ts already covers it.

Verified on the device, sweeping each view the way the issue was
filed: explore, downloads, autotag and artist-details now report
**one** control under the floor apiece, and it is the skip link, which
#186 already ruled out as keyboard-only. .search-mode-tab 89x44 and
79x44, the search input 325x44, the Downloads tabs 85x44 and 96x44,
.section-toggle 174x44, .folders-menu-trigger 44x44, .back-button
44x44.

All 12 new tests fail on main, the source sweep naming all six copies.
make ui-test 1041 pass; make e2e 236 pass on chromium, which is half
an answer -- CI has the other half.

Closes #186
This commit is contained in:
logan
2026-08-21 23:03:18 -04:00
parent 2100f0022f
commit 7b09d467dd
11 changed files with 337 additions and 143 deletions
Download Patch File Download Diff File Expand all files Collapse all files
frontend/src/components/artist-details/artist-details.ts
+2 -26
View File
@@ -14,6 +14,7 @@ import {
import '@awesome.me/webawesome/dist/components/icon/icon.js';
import '@components/cover-grid/cover-grid.js';
import { designTokens } from '../../styles/tokens.css';
import { backButton } from '../../styles/back-button.css';
@customElement('artist-details')
export class ArtistDetails extends LitElement {
@@ -40,7 +41,7 @@ export class ArtistDetails extends LitElement {
/** Tracks the store's cached array reference to detect refreshes. */
private lastAlbumsRef: library.Album[] | null = null;
static override styles = [designTokens, css`
static override styles = [designTokens, backButton, css`
:host {
display: flex;
flex-direction: column;
@@ -66,31 +67,6 @@ export class ArtistDetails extends LitElement {
);
}
.back-button {
display: flex;
align-items: center;
justify-content: center;
width: 32px;
height: 32px;
border: none;
border-radius: 50%;
background: var(
--yj-bg-overlay,
rgba(255, 255, 255, 0.06)
);
color: var(--yj-text-primary, #fff);
cursor: pointer;
flex-shrink: 0;
transition: background-color 0.15s ease;
}
.back-button:hover {
background: var(
--yj-bg-hover,
rgba(255, 255, 255, 0.12)
);
}
.back-button wa-icon {
font-size: 16px; /* back button — outside type scale */
}
frontend/src/components/autotag-view/autotag-view.ts
+15 -1
View File
@@ -250,13 +250,20 @@ export class AutotagView extends ViewLifecycleMixin(LitElement) {
}
/* Collapsible-section toggle used in the Pending header —
transparent button that inherits the header's type. */
transparent button that inherits the header's type.
187x**15** before this (#186), which was the smallest
control measured anywhere in the app until the column
arrows were counted. It is transparent and full-width
already, so the floor costs it a height and nothing
else. */
.section-toggle {
display: flex;
align-items: center;
gap: 0.35rem;
flex: 1;
min-width: 0;
min-block-size: 44px;
padding: 0;
background: transparent;
border: 0;
@@ -274,6 +281,8 @@ export class AutotagView extends ViewLifecycleMixin(LitElement) {
color: var(--yj-text-tertiary, #888);
}
/* 32x18, and it has no background until hover -- so the
padding out to a square target is invisible (#186). */
.folders-menu-trigger {
background: transparent;
border: 0;
@@ -281,6 +290,8 @@ export class AutotagView extends ViewLifecycleMixin(LitElement) {
font-size: 1.1rem;
line-height: 1;
padding: 0.1rem 0.4rem;
min-inline-size: 44px;
min-block-size: 44px;
border-radius: 3px;
cursor: pointer;
}
@@ -293,7 +304,10 @@ export class AutotagView extends ViewLifecycleMixin(LitElement) {
.folders-refresh-trigger {
display: flex;
align-items: center;
justify-content: center;
font-size: 0.95rem;
min-inline-size: 44px;
min-block-size: 44px;
}
.folders-refresh-trigger:disabled {
frontend/src/components/downloads-view/downloads-view.ts
+9 -1
View File
@@ -93,8 +93,16 @@ export class DownloadsView extends ViewLifecycleMixin(LitElement) {
border-bottom: 1px solid var(--yj-bg-overlay, rgba(255, 255, 255, 0.08));
}
/* 85x34 and 96x34 before this (#186). A tab is the only
route to the panel it names, so it is the last control
that should be hard to hit -- and the underline that
marks the active one is drawn on the bottom border,
which a taller box moves further from the label. That
is why the height goes on padding rather than on a
min-size: the border follows the padding down and the
label stays where the underline is. */
.tab {
padding: 8px 14px;
padding: 13px 14px;
font-size: 13px;
font-weight: 600;
color: var(--yj-text-secondary, #b3b3b3);
frontend/src/components/explore-album-details/explore-album-details.ts
+2 -19
View File
@@ -2,6 +2,7 @@ import { LitElement, html, css, nothing } from 'lit';
import { customElement, property, state, query } from 'lit/decorators.js';
import { classMap } from 'lit/directives/class-map.js';
import { designTokens } from '../../styles/tokens.css';
import { backButton } from '../../styles/back-button.css';
import { srOnly } from '../../styles/sr-only.css';
import { unownedLabel, unownedStyles } from '@utils/ownership';
import {
@@ -355,6 +356,7 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
static override styles = [
designTokens,
backButton,
exploreLinkStyles,
contextMenuStyles,
srOnly,
@@ -379,25 +381,6 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
var(--yj-border-subtle, rgba(255, 255, 255, 0.06));
}
.back-button {
display: flex;
align-items: center;
justify-content: center;
width: 32px;
height: 32px;
border: none;
border-radius: 50%;
background: var(--yj-bg-overlay, rgba(255, 255, 255, 0.06));
color: var(--yj-text-primary, #fff);
cursor: pointer;
flex-shrink: 0;
transition: background-color 0.15s ease;
}
.back-button:hover {
background: var(--yj-bg-hover, rgba(255, 255, 255, 0.12));
}
.back-button wa-icon {
font-size: 16px;
}
frontend/src/components/explore-artist-details/explore-artist-details.ts
+2 -19
View File
@@ -3,6 +3,7 @@ import { LitElement, html, css, nothing } from 'lit';
import { customElement, property, state, query } from 'lit/decorators.js';
import { classMap } from 'lit/directives/class-map.js';
import { designTokens } from '../../styles/tokens.css';
import { backButton } from '../../styles/back-button.css';
import {
LookupArtist,
BrowseReleaseGroups,
@@ -266,6 +267,7 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
static override styles = [
designTokens,
backButton,
exploreLinkStyles,
contextMenuStyles,
unownedStyles,
@@ -289,25 +291,6 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
var(--yj-border-subtle, rgba(255, 255, 255, 0.06));
}
.back-button {
display: flex;
align-items: center;
justify-content: center;
width: 32px;
height: 32px;
border: none;
border-radius: 50%;
background: var(--yj-bg-overlay, rgba(255, 255, 255, 0.06));
color: var(--yj-text-primary, #fff);
cursor: pointer;
flex-shrink: 0;
transition: background-color 0.15s ease;
}
.back-button:hover {
background: var(--yj-bg-hover, rgba(255, 255, 255, 0.12));
}
.back-button wa-icon {
font-size: 16px;
}
frontend/src/components/explore-view/explore-view.ts
+17 -1
View File
@@ -256,15 +256,18 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
margin-bottom: 10px;
}
/* 89x26 and 79x26 before this (#186). */
.search-mode-tab {
display: inline-flex;
align-items: center;
justify-content: center;
gap: 6px;
background: none;
border: 1px solid transparent;
border-radius: 6px;
color: var(--yj-text-tertiary, #888);
cursor: pointer;
min-block-size: 44px;
padding: 5px 12px;
font-size: var(--yj-text-sm);
font-family: inherit;
@@ -289,7 +292,7 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
border-radius: 6px;
padding: 0 12px;
gap: 8px;
height: 36px;
min-height: 44px;
max-width: 520px;
transition: border-color 0.15s ease;
}
@@ -364,8 +367,15 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
flex-shrink: 0;
}
/* The input measured 325x**18** and the box around it 36,
which is two faults rather than one (#186): the row was
under the floor, and the input did not fill it, so eight
of those pixels were not a target at all. The container
is 44 and the input stretches to it -- a tap anywhere in
the box now lands on the input rather than beside it. */
input {
flex: 1;
align-self: stretch;
background: none;
border: none;
outline: none;
@@ -379,15 +389,21 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
color: var(--yj-text-tertiary, #888);
}
/* No background until hover, so the target grows and the
glyph does not. It is inside a 44px box already, hence
the width alone. */
.clear-button {
display: flex;
align-items: center;
justify-content: center;
align-self: stretch;
background: none;
border: none;
color: var(--yj-text-tertiary, #888);
cursor: pointer;
padding: 0;
min-inline-size: 44px;
margin-inline-end: -12px;
font-size: var(--yj-text-sm);
flex-shrink: 0;
}
frontend/src/components/genre-details/genre-details.ts
+2 -26
View File
@@ -15,6 +15,7 @@ import { describeError } from '@utils/describe-error';
import '@awesome.me/webawesome/dist/components/icon/icon.js';
import '@components/track-list/track-list.js';
import { designTokens } from '../../styles/tokens.css';
import { backButton } from '../../styles/back-button.css';
import { list } from '@utils/binding';
@customElement('genre-details')
@@ -37,7 +38,7 @@ export class GenreDetails extends LitElement {
private scanCompleteCleanup: (() => void) | null =
null;
static override styles = [designTokens, css`
static override styles = [designTokens, backButton, css`
:host {
display: flex;
flex-direction: column;
@@ -77,31 +78,6 @@ export class GenreDetails extends LitElement {
);
}
.back-button {
display: flex;
align-items: center;
justify-content: center;
width: 32px;
height: 32px;
border: none;
border-radius: 50%;
background: var(
--yj-bg-overlay,
rgba(255, 255, 255, 0.06)
);
color: var(--yj-text-primary, #fff);
cursor: pointer;
flex-shrink: 0;
transition: background-color 0.15s ease;
}
.back-button:hover {
background: var(
--yj-bg-hover,
rgba(255, 255, 255, 0.12)
);
}
.back-button wa-icon {
font-size: 16px; /* back button — outside type scale */
}
frontend/src/components/playlist-details/playlist-details.ts
+2 -25
View File
@@ -71,6 +71,7 @@ import {
exploreLinkStyles,
} from '@utils/explore-link';
import { designTokens } from '../../styles/tokens.css';
import { backButton } from '../../styles/back-button.css';
import { list } from '@utils/binding';
import {
ICON_PLAYLIST,
@@ -954,6 +955,7 @@ export class PlaylistDetails
static override styles = [
designTokens,
backButton,
contextMenuStyles,
exploreLinkStyles,
css`
@@ -981,31 +983,6 @@ export class PlaylistDetails
);
}
.back-button {
display: flex;
align-items: center;
justify-content: center;
width: 32px;
height: 32px;
border: none;
border-radius: 50%;
background: var(
--yj-bg-overlay,
rgba(255, 255, 255, 0.06)
);
color: var(--yj-text-primary, #fff);
cursor: pointer;
flex-shrink: 0;
transition: background-color 0.15s ease;
}
.back-button:hover {
background: var(
--yj-bg-hover,
rgba(255, 255, 255, 0.12)
);
}
.back-button wa-icon {
font-size: 16px;
}
frontend/src/components/smart-playlist-details/smart-playlist-details.ts
+2 -25
View File
@@ -62,6 +62,7 @@ import {
} from '@utils/explore-link';
import '@components/smart-playlist-editor/smart-playlist-editor.js';
import { designTokens } from '../../styles/tokens.css';
import { backButton } from '../../styles/back-button.css';
import { list } from '@utils/binding';
import {
ICON_PLAYLIST,
@@ -242,6 +243,7 @@ export class SmartPlaylistDetails
static override styles = [
designTokens,
backButton,
contextMenuStyles,
exploreLinkStyles,
css`
@@ -269,31 +271,6 @@ export class SmartPlaylistDetails
);
}
.back-button {
display: flex;
align-items: center;
justify-content: center;
width: 32px;
height: 32px;
border: none;
border-radius: 50%;
background: var(
--yj-bg-overlay,
rgba(255, 255, 255, 0.06)
);
color: var(--yj-text-primary, #fff);
cursor: pointer;
flex-shrink: 0;
transition: background-color 0.15s ease;
}
.back-button:hover {
background: var(
--yj-bg-hover,
rgba(255, 255, 255, 0.12)
);
}
.back-button wa-icon {
font-size: 16px;
}
frontend/src/styles/back-button.css.ts
+54
View File
@@ -0,0 +1,54 @@
import { css } from 'lit';
/**
* The way out of a detail view, at the app's 44px touch floor.
*
* #186's second table names `artist-details`' back button at
* **32x32**. It is the same declaration in **six** components —
* `artist-details`, `genre-details`, `playlist-details`,
* `smart-playlist-details`, `explore-artist-details` and
* `explore-album-details` — byte-identical, 32px in all six, and the
* sweep that filed the issue visited one of them.
*
* That is the argument for this file rather than six edits. A device
* sweep walks the views somebody thought to open, so six copies of a
* control is six chances for the next pass to miss five; the arrows
* and the toggles were each one declaration covering 36 and 29
* controls, and this is the same shape stated the other way round.
*
* **It is a real 44px box, not padding with the width handed back.**
* The header pass had to grow a hit area past its own layout box
* because `page-header` measures itself for #69's overflow fit; a
* detail view's header does not, so the control can simply be the
* target. It also *should* be — this button has a visible background,
* so a hit area larger than the circle would be a control that is
* bigger than it looks, which is the thing #187 accepts only where a
* thin painted track is the point.
*
* The size is #55's, arrived at for the same reason one component
* over: "the way out is 44px on a phone", when the queue panel's close
* button was 25x21 and, at phone width, the only pointer route off a
* full-screen surface. A detail view has the platform's back gesture
* as well, so this is less severe than the queue was — it is the same
* control wearing the same mistake.
*/
export const backButton = css`
.back-button {
display: flex;
align-items: center;
justify-content: center;
width: 44px;
height: 44px;
border: none;
border-radius: 50%;
background: var(--yj-bg-overlay, rgba(255, 255, 255, 0.06));
color: var(--yj-text-primary, #fff);
cursor: pointer;
flex-shrink: 0;
transition: background-color 0.15s ease;
}
.back-button:hover {
background: var(--yj-bg-hover, rgba(255, 255, 255, 0.12));
}
`;
frontend/test/components/detail-touch-targets.test.ts
+230
View File
@@ -0,0 +1,230 @@
/**
* The controls #186's second table found, outside Settings.
*
* Six one-off controls across four surfaces, and the reason they are a
* test rather than six stylesheet edits is `back-button`. The issue
* names it in `artist-details` at **32x32**; it is the same
* declaration, byte-identical, in *six* components — because a device
* sweep walks the views somebody thought to open, and five of them
* were not opened.
*
* So the assertion is over the whole set rather than over the one that
* was measured. That is `icon-language.test.ts`'s shape and it is here
* for the same reason: checking one call site checks one call site.
*
* | control | before | where |
* |---|---|---|
* | `.folders-menu-trigger` | **32x18** | autotag |
* | `.section-toggle` | 187x**15** | autotag |
* | `.back-button` | 32x32 | six detail views |
* | Requests / Downloads tabs | 85x**34**, 96x**34** | downloads |
* | `.search-mode-tab` | 89x**26**, 79x**26** | explore |
* | explore search input | 325x**18** in a 36px box | explore |
*
* `page-action-check-now` (113x29) is in that table and is **not**
* here: it is a `PageAction`, so #195 raised it with the rest of the
* page header's actions, and `touch-targets.test.ts` already covers
* it. Re-asserting it here would be a second statement of one rule.
*/
import { beforeEach, describe, expect, it } from 'vitest';
import '@components/artist-details/artist-details';
import '@components/autotag-view/autotag-view';
import '@components/downloads-view/downloads-view';
import '@components/explore-album-details/explore-album-details';
import '@components/explore-artist-details/explore-artist-details';
import '@components/explore-view/explore-view';
import '@components/genre-details/genre-details';
import '@components/playlist-details/playlist-details';
import '@components/smart-playlist-details/smart-playlist-details';
import { flush, stub } from '@test/support/harness';
import { fixture, shadow, shadowAll } from '@test/support/render';
/** The app's touch floor, from #56. */
const FLOOR = 44;
/**
* Every component that draws a back button.
*
* The list is here rather than derived because deriving it means
* reading the source, and this tier renders instead — but it is
* checked against the source by `the back button is one declaration`
* below, so a seventh view cannot join quietly.
*/
const BACK_BUTTON_VIEWS = [
'artist-details',
'genre-details',
'playlist-details',
'smart-playlist-details',
'explore-artist-details',
'explore-album-details',
] as const;
function boxOf(el: Element | null | undefined): { w: number; h: number } {
if (!el) return { w: 0, h: 0 };
const box = el.getBoundingClientRect();
return { w: Math.round(box.width), h: Math.round(box.height) };
}
describe('the way out of a detail view', () => {
beforeEach(() => {
for (const path of [
'library.Library.GetTracks',
'library.Library.GetAlbums',
'library.Library.GetArtists',
'library.Library.GetGenres',
'playlist.Service.GetAllPlaylists',
'playlist.Service.GetAllPlaylistsWithTracks',
]) {
stub(path, []);
}
});
it.each(BACK_BUTTON_VIEWS)('is 44px in <%s>', async (tag) => {
// #55 settled this one component over, when the queue panel's
// close button was 25x21 and, at phone width, the only pointer
// route off a full-screen surface: "the way out is 44px". A detail
// view has the platform's back gesture as well, so it is less
// severe -- and it is the same control wearing the same mistake.
const el = await fixture(tag);
await flush();
const back = shadow(el, '.back-button');
expect(back, `${tag} draws a back button`).toBeTruthy();
expect(boxOf(back)).toEqual({ w: FLOOR, h: FLOOR });
});
it('is one declaration, so a seventh view cannot miss it', async () => {
// The regression this exists for is not a size changing -- it is
// somebody adding a detail view and writing `.back-button` out
// again at 32px, which is exactly how there came to be six copies.
// A sweep of the running app would not catch it either, because a
// sweep visits the views you think to open.
const sources = import.meta.glob('../../src/components/**/*.ts', {
query: '?raw',
import: 'default',
eager: true,
}) as Record<string, string>;
expect(Object.keys(sources).length, 'the glob read something').toBeGreaterThan(0);
const redeclared = Object.entries(sources)
.filter(([, src]) => /^\s*\.back-button\s*(?::[a-z-]+\s*)?\{/m.test(src))
.map(([path]) => path);
expect(redeclared).toEqual([]);
});
});
describe('autotag', () => {
it('raises the two smallest controls the sweep found', async () => {
// 187x15 and 32x18. The section toggle was the smallest control
// measured anywhere in the app until the column arrows were
// counted, and autotag is off by default (#25), which is
// presumably why nobody had met either.
const el = await fixture('autotag-view');
await flush();
for (const selector of ['.section-toggle', '.folders-menu-trigger']) {
const control = shadowAll<HTMLElement>(el, selector).find(
(c) => c.getBoundingClientRect().height > 0,
);
if (!control) continue;
expect(boxOf(control).h, `${selector} height`).toBeGreaterThanOrEqual(FLOOR);
}
// The stylesheet is the assertion for whichever of the two this
// fixture does not render -- both are behind state a bare mount
// does not reach, and a test that silently checked nothing is the
// trap icon-language.test.ts's first assertion exists for.
const sheet = (el.constructor as typeof HTMLElement & { styles?: unknown })
.styles;
expect(String(sheet)).toContain('min-block-size: 44px');
});
});
describe('the Downloads tabs', () => {
beforeEach(() => {
stub('download.Service.ListDownloads', []);
stub('download.Service.ListRequests', []);
stub('download.Service.ListProviders', []);
});
it('are the only route to their panels, and are 44px', async () => {
const el = await fixture('downloads-view');
await flush();
const tabs = shadowAll<HTMLElement>(el, '[role="tab"]');
expect(tabs).toHaveLength(2);
for (const tab of tabs) {
expect(boxOf(tab).h, tab.textContent?.trim()).toBeGreaterThanOrEqual(FLOOR);
}
});
it('keeps the active underline against the label', async () => {
// The height is padding rather than a min-size, because the mark
// for the selected tab is the bottom border -- a min-size would
// centre the label and leave the underline 10px below it.
const el = await fixture('downloads-view');
await flush();
const tab = shadowAll<HTMLElement>(el, '[role="tab"]')[0]!;
const style = getComputedStyle(tab);
expect(parseFloat(style.paddingBlockStart)).toBeGreaterThan(8);
expect(style.paddingBlockStart).toBe(style.paddingBlockEnd);
});
});
describe("Explore's own search row", () => {
beforeEach(() => {
stub('explore.Service.GetShelves', { State: 'ready', Shelves: [] });
stub('explore.Service.GetIndexStatus', {});
});
it('raises the mode tabs', async () => {
const el = await fixture('explore-view');
await flush();
const tabs = shadowAll<HTMLElement>(el, '.search-mode-tab');
expect(tabs.length).toBeGreaterThan(0);
for (const tab of tabs) {
expect(boxOf(tab).h, tab.textContent?.trim()).toBeGreaterThanOrEqual(FLOOR);
}
});
it('makes the whole search box the input, not the middle 18px of it', async () => {
// Two faults, not one: the row was 36px and the input inside it
// was **18**, so half the box was not a target at all -- a tap
// near the top or bottom edge landed on the container and did
// nothing. The container is 44 and the input stretches to fill it.
const el = await fixture('explore-view');
await flush();
const box = shadow(el, '.search-container');
const input = shadow(el, '.search-container input');
expect(box, 'the search row renders').toBeTruthy();
expect(input, 'it holds an input').toBeTruthy();
expect(boxOf(box).h).toBeGreaterThanOrEqual(FLOOR);
expect(boxOf(input).h).toBeGreaterThanOrEqual(FLOOR);
});
});