diff --git a/frontend/src/components/artist-details/artist-details.ts b/frontend/src/components/artist-details/artist-details.ts index f360224..12e82d2 100644 --- a/frontend/src/components/artist-details/artist-details.ts +++ b/frontend/src/components/artist-details/artist-details.ts @@ -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 */ } diff --git a/frontend/src/components/autotag-view/autotag-view.ts b/frontend/src/components/autotag-view/autotag-view.ts index 56e2fc4..feed404 100644 --- a/frontend/src/components/autotag-view/autotag-view.ts +++ b/frontend/src/components/autotag-view/autotag-view.ts @@ -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 { diff --git a/frontend/src/components/config-page/config-field.ts b/frontend/src/components/config-page/config-field.ts index 032c784..5bbd352 100644 --- a/frontend/src/components/config-page/config-field.ts +++ b/frontend/src/components/config-page/config-field.ts @@ -85,6 +85,21 @@ export class ConfigField extends LitElement { gap: 0.5em; } + /* Every control here meets the app's 44px touch floor (#186). + + This is the shape every row in Settings uses, so it is the + one rule that covers the most controls -- and it is the + *cheapest* place to reach the floor, because there is no + overflow fit on this page. The page header's had one (#69), + which is why that pass had to grow padding and hand the + width back with a negative margin; here the control is a + block in a column and a taller box costs nothing but the + height it takes. + + Measured on the reference device before this: the select + 335x30, the text and number inputs the same, the browse + button 30 tall, the colour swatch 33x33 and the toggle + **34x19**. */ input[type='text'], input[type='number'] { background: var(--yj-bg-elevated, #343a40); @@ -95,6 +110,7 @@ export class ConfigField extends LitElement { font-size: 0.85em; font-family: inherit; min-width: 0; + min-block-size: 44px; flex: 1; } @@ -117,6 +133,7 @@ export class ConfigField extends LitElement { font-size: 0.85em; font-family: inherit; cursor: pointer; + min-block-size: 44px; flex: 1; } @@ -139,6 +156,7 @@ export class ConfigField extends LitElement { font-size: 0.85em; cursor: pointer; white-space: nowrap; + min-block-size: 44px; } button:hover { @@ -158,8 +176,12 @@ export class ConfigField extends LitElement { } input[type='color'] { - width: 2.5em; - height: 2.5em; + /* border-box, or the 2px border makes this 48 and the + assertion below reads as passing by four pixels of + border rather than by the rule. */ + box-sizing: border-box; + width: 44px; + height: 44px; border: 2px solid var(--yj-border, #444); border-radius: 4px; padding: 0; @@ -187,12 +209,32 @@ export class ConfigField extends LitElement { display: flex; align-items: center; justify-content: space-between; + min-block-size: 44px; } + /* The toggle is the one control here whose target and paint + must differ, and it is also the one no sweep can see. + + Its is opacity: 0; width: 0; height: 0, so a + walk of every input on the page skips it as a zero-sized + node -- the thing a finger actually hits is this diff --git a/frontend/src/components/config-page/download-clients.ts b/frontend/src/components/config-page/download-clients.ts index 6aef7a3..d1bceb8 100644 --- a/frontend/src/components/config-page/download-clients.ts +++ b/frontend/src/components/config-page/download-clients.ts @@ -8,6 +8,7 @@ import '@awesome.me/webawesome/dist/components/switch/switch.js'; import '@awesome.me/webawesome/dist/components/spinner/spinner.js'; import '@awesome.me/webawesome/dist/components/callout/callout.js'; import { designTokens } from '../../styles/tokens.css'; +import { waTouchFloor } from '../../styles/wa-touch-floor.css'; import type { DownloadDescriptor, DownloadProvider, @@ -136,6 +137,7 @@ export class DownloadClients extends LitElement { static override styles = [ designTokens, + waTouchFloor, css` :host { display: block; @@ -239,12 +241,17 @@ export class DownloadClients extends LitElement { margin-top: 0.4em; } + /* The checkbox is 16x16 and cannot grow without becoming + a 44px checkbox, but it is already wrapped in the label + that names it -- so the label is the target and only + needs the height (#186). Eight of them. */ .format-option { display: flex; align-items: center; gap: 0.4em; font-size: 0.9em; cursor: pointer; + min-block-size: 44px; } `, ]; diff --git a/frontend/src/components/config-page/shortcut-capture.ts b/frontend/src/components/config-page/shortcut-capture.ts index 96774fe..b072fd9 100644 --- a/frontend/src/components/config-page/shortcut-capture.ts +++ b/frontend/src/components/config-page/shortcut-capture.ts @@ -25,6 +25,11 @@ export class ShortcutCapture extends LitElement { :host { display: inline-block; } + /* 80x25, twenty-six of them -- the most numerous control on + the Settings page after the column lists (#186). The floor + is a height here and nothing else: the width was already + past it, and the type stays where it is so a shortcut still + reads as a key rather than as a button. */ button { font-family: inherit; font-size: var(--yj-text-sm, 13px); @@ -35,6 +40,7 @@ export class ShortcutCapture extends LitElement { color: var(--yj-text-primary, #eee); cursor: pointer; min-width: 80px; + min-height: 44px; text-align: center; transition: border-color 0.15s, @@ -61,6 +67,11 @@ export class ShortcutCapture extends LitElement { opacity: 0.7; } } + /* Reset renders only for a rebound shortcut, so a sweep of a + freshly-installed app never sees it -- it is not in #186's + tables for that reason, and it is a touch target the moment + anybody uses the feature. It also has no background, so the + padding out to 44px is invisible. */ .reset-btn { font-size: var(--yj-text-xs, 11px); padding: 2px 6px; @@ -69,7 +80,8 @@ export class ShortcutCapture extends LitElement { background: transparent; color: var(--yj-text-tertiary, #888); cursor: pointer; - min-width: auto; + min-width: 44px; + min-height: 44px; opacity: 0; transition: opacity 0.15s; } diff --git a/frontend/src/components/downloads-view/downloads-view.ts b/frontend/src/components/downloads-view/downloads-view.ts index 626e9a0..928a9e7 100644 --- a/frontend/src/components/downloads-view/downloads-view.ts +++ b/frontend/src/components/downloads-view/downloads-view.ts @@ -93,8 +93,26 @@ 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. So the + height goes on *padding*, keeping the border against + the label rather than 10px below a centred one. + + The min-size is the floor and is not redundant: padding + alone made this 44px here and **43px in CI**, because + the total is 13 + 13 + 2 + whatever line box the font + gives 13px text, and ubuntu:24.04's is a pixel shorter + than this machine's. A height computed from a font's + line box is not a height you control -- the same + mistake #195 made about a layout property measured on + one engine, one layer down, and caught here by the test + rather than by a person. */ .tab { - padding: 8px 14px; + min-block-size: 44px; + padding: 13px 14px; font-size: 13px; font-weight: 600; color: var(--yj-text-secondary, #b3b3b3); diff --git a/frontend/src/components/explore-album-details/explore-album-details.ts b/frontend/src/components/explore-album-details/explore-album-details.ts index e6e3758..15bd242 100644 --- a/frontend/src/components/explore-album-details/explore-album-details.ts +++ b/frontend/src/components/explore-album-details/explore-album-details.ts @@ -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; } diff --git a/frontend/src/components/explore-artist-details/explore-artist-details.ts b/frontend/src/components/explore-artist-details/explore-artist-details.ts index 5c42394..b750f67 100644 --- a/frontend/src/components/explore-artist-details/explore-artist-details.ts +++ b/frontend/src/components/explore-artist-details/explore-artist-details.ts @@ -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; } diff --git a/frontend/src/components/explore-view/explore-view.ts b/frontend/src/components/explore-view/explore-view.ts index a7bc195..dace829 100644 --- a/frontend/src/components/explore-view/explore-view.ts +++ b/frontend/src/components/explore-view/explore-view.ts @@ -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; } diff --git a/frontend/src/components/genre-details/genre-details.ts b/frontend/src/components/genre-details/genre-details.ts index 389be91..23322fe 100644 --- a/frontend/src/components/genre-details/genre-details.ts +++ b/frontend/src/components/genre-details/genre-details.ts @@ -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 */ } diff --git a/frontend/src/components/library-filter/library-filter.ts b/frontend/src/components/library-filter/library-filter.ts index f62085c..0a5dc13 100644 --- a/frontend/src/components/library-filter/library-filter.ts +++ b/frontend/src/components/library-filter/library-filter.ts @@ -23,8 +23,14 @@ export class LibraryFilter extends LitElement { align-items: center; } + /* 120x32 on the reference device (#186). This control has two + placements since #57 -- the desktop top bar and Settings -> + Libraries -- and it is the only route to setSelectedLibrary + in either, so it is one of the controls #148 argued must not + simply be taken away. It is one component, so it reaches the + floor in one place. */ select { - height: 32px; + min-height: 44px; padding: 0 8px; border-radius: 6px; border: 1px solid diff --git a/frontend/src/components/playlist-details/playlist-details.ts b/frontend/src/components/playlist-details/playlist-details.ts index 9db18c4..80c222c 100644 --- a/frontend/src/components/playlist-details/playlist-details.ts +++ b/frontend/src/components/playlist-details/playlist-details.ts @@ -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; } 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 4b9b112..a97f428 100644 --- a/frontend/src/components/smart-playlist-details/smart-playlist-details.ts +++ b/frontend/src/components/smart-playlist-details/smart-playlist-details.ts @@ -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; } diff --git a/frontend/src/styles/back-button.css.ts b/frontend/src/styles/back-button.css.ts new file mode 100644 index 0000000..73c11b0 --- /dev/null +++ b/frontend/src/styles/back-button.css.ts @@ -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)); + } +`; diff --git a/frontend/src/styles/wa-touch-floor.css.ts b/frontend/src/styles/wa-touch-floor.css.ts new file mode 100644 index 0000000..385c9fb --- /dev/null +++ b/frontend/src/styles/wa-touch-floor.css.ts @@ -0,0 +1,48 @@ +import { css } from 'lit'; + +/** + * A Web Awesome form control is at least the app's 44px touch floor. + * + * #56 named 44px and #186 found nothing but the transport had reached + * it. Web Awesome's form controls are the part of Settings this app + * does not draw: measured on the reference device (TLP301, 424x439), + * `wa-input`'s control is **204x20** and `wa-button` **185x21** — the + * shortest controls on the page, and the only ones whose height is + * decided inside somebody else's shadow root. + * + * `--wa-form-control-height` is that decision, and it is the library's + * own theming variable rather than a part or an internal — the default + * theme sets it at `:root` and every control that has a height reads + * it (button, input, select, radio). So this is `wa-slider-label`'s + * better half: the API first, and no reach into a shadow root at all. + * + * Three things about it are load-bearing. + * + * **A custom property inherits through a shadow boundary**, which is + * what lets a `:host` declaration reach a `wa-input` the host renders. + * That is also why it is a stylesheet a component adopts rather than a + * `:root` rule in `index.css`: a `:root` rule would cover every wa + * control in the app in one line and be invisible to the component + * tier, which renders a component and no page stylesheet. Here the + * floor is measurable where it is applied. + * + * **It is a flat 44px rather than a floor over the library's own + * expression.** The default is `round(calc(2 * padding-block + 1em * + * line-height), 1px)` — em-based, so `size="small"` is what produced + * the 20px above — and a `max(44px, …)` would have to restate that + * formula here, which is a copy of somebody else's arithmetic that + * goes stale silently. A flat value is safe because this app uses + * exactly two sizes, `small` and the default, and both are under the + * floor; a `size="large"` added later would be pinned down to 44 and + * should take that as the prompt to revisit this. + * + * **Only the height is pinned.** The font size still comes from + * `size="small"`, so a control grows its hit area without growing its + * visual weight — which is what #186's Direction asks for and what the + * page header's second pass had to be corrected to do. + */ +export const waTouchFloor = css` + :host { + --wa-form-control-height: 44px; + } +`; diff --git a/frontend/test/components/__screenshots__/page-header.test.ts/page-header-filtered-by-search-chromium-linux.png b/frontend/test/components/__screenshots__/page-header.test.ts/page-header-filtered-by-search-chromium-linux.png index 16ec379..2a43fb8 100644 Binary files a/frontend/test/components/__screenshots__/page-header.test.ts/page-header-filtered-by-search-chromium-linux.png and b/frontend/test/components/__screenshots__/page-header.test.ts/page-header-filtered-by-search-chromium-linux.png differ diff --git a/frontend/test/components/__screenshots__/page-header.test.ts/page-header-title-count-and-sort-chromium-linux.png b/frontend/test/components/__screenshots__/page-header.test.ts/page-header-title-count-and-sort-chromium-linux.png index f0e6892..fb47408 100644 Binary files a/frontend/test/components/__screenshots__/page-header.test.ts/page-header-title-count-and-sort-chromium-linux.png and b/frontend/test/components/__screenshots__/page-header.test.ts/page-header-title-count-and-sort-chromium-linux.png differ diff --git a/frontend/test/components/detail-touch-targets.test.ts b/frontend/test/components/detail-touch-targets.test.ts new file mode 100644 index 0000000..9cc5120 --- /dev/null +++ b/frontend/test/components/detail-touch-targets.test.ts @@ -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; + + 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(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(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(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(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); + }); +}); diff --git a/frontend/test/components/settings-touch-targets.test.ts b/frontend/test/components/settings-touch-targets.test.ts new file mode 100644 index 0000000..efa2e55 --- /dev/null +++ b/frontend/test/components/settings-touch-targets.test.ts @@ -0,0 +1,425 @@ +/** + * Every control in Settings is at least 44px (#186, second pass). + * + * The header pass covered the five controls a user meets on every + * screen. Settings is the other half and is much the larger one: swept + * on the reference device (TLP301, 424x439) with all eleven + * `config-section`s expanded, **120 controls** were under the floor, + * not the 93 the issue's first table implies, and `config-field` — the + * row shape the issue names — is eight of them. The bulk is behind the + * disclosures: + * + * | control | size | count | + * |---|---|---| + * | `.column-arrow-btn` | **16x14** | 36 | + * | `.column-toggle` | 16x16 | 29 | + * | `shortcut-capture` button | 80x**25** | 26 | + * | download format checkbox | 16x16 | 8 | + * | `config-field` select | 335x**30** | 7 | + * | `wa-input` / `wa-button` | 204x**20**, 185x**21** | 6 | + * | `library-filter` select | 120x**32** | 1 | + * | `.overflow-btn` | 31x31 | 1 | + * + * **This tier can measure it, unlike #187's seek bar**, for the reason + * the header pass gives: the controls are real elements and the rules + * are min-sizes, so a real Chromium rendering a real component gives + * the actual answer at any width. And unlike the header there is no + * overflow fit on this page, so nothing here needs the negative-margin + * treatment — height is free and the two square controls can simply be + * square. + * + * **What the sweep cannot see is written down here as a test rather + * than as a comment**, because it is the trap this whole issue keeps + * setting. Two controls are invisible to a walk of `button, select, + * input`: `config-field`'s toggle, whose `` is + * `opacity: 0; width: 0; height: 0` so the thing a finger hits is the + * `