fix(settings): raise every Settings control to the touch floor
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (push) Skipped
CI / e2e (push) Skipped
#56 named 44px and #195 took the page header there. Settings is the other half of #186 and much the larger one: swept on the reference device (TLP301, 424x439) with all eleven config-sections expanded, **120 controls** were under the floor -- not the 93 the issue's table implies, and config-field is eight of them. The bulk is behind the disclosures, which is why nobody had counted it: 36 .column-arrow-btn 16x14 <- smallest in the app 29 .column-toggle 16x16 26 shortcut-capture button 80x25 8 download format checkbox 16x16 7 config-field select 335x30 6 wa-input / wa-button 204x20, 185x21 **The density argument, measured rather than guessed, and it is smaller than it looks.** The rows were already near the floor -- .column-item is 335x36 and .shortcut-row 335x37; it is the controls *inside* them that were 14-25px. So a control grows into the row it already occupies and the row goes 36 to 44. Measured after: the two column lists went 373->447 and 690->850, +234px over the whole page. Half a screen of extra scroll on a page that already scrolls, against 36 targets of 16x14. **Settings is cheaper than the header was, and for a stated reason.** There is no overflow fit on this page, so the header's "only width is contested" rule does not bind at all and nothing here needs padding with a negative margin. Height is a min-size, and the two square controls can simply be square. Three shapes, because one rule does not fit three kinds of control: **A native checkbox is targeted through its label.** It cannot grow its hit area without growing its paint, and a 44px checkbox is not what anyone wants -- so .column-label is a real <label for> now and the column's *name* is the target, 70x44 rather than 16x16. That is the argument config-field already makes one file over ("a real label association also makes the label text a click target, which is behaviour, not annotation"), and here it is the whole fix. The download formats already had the label; they only needed the height. **The arrows take padding, which is invisible.** They carry background: none and a transparent border, so 16x14 -> 44x44 changes nothing anyone can see until hover -- #186's Direction exactly. **Web Awesome's controls come from the library's own API.** Their height is decided inside somebody else's shadow root, and --wa-form-control-height is the variable that decides it. A custom property inherits through a shadow boundary, so a :host declaration reaches them; styles/wa-touch-floor.css.ts is that, once, adopted rather than written at :root in index.css -- a :root rule would be invisible to the component tier, which renders a component and no page stylesheet. **Two controls no sweep can see are fixed by name**, and they are the trap this issue keeps setting. config-field's toggle has an <input> that is opacity: 0; width: 0; height: 0, so a walk of every input skips it as a zero-sized node -- what a finger hits is the <label>, which measured **34x19**, smaller than anything in either of #186's tables and absent from both. It is 44x44 with the pill still painted at 2.5em x 1.4em and negative inline margins keeping it flush with the inputs above. And shortcut-capture's reset button renders only for a shortcut somebody has rebound, so a sweep of a fresh install never meets it. Verified on the device, same method as the sweep that filed it: 120 controls under the floor before, 42 after. All 42 are accounted for -- 37 are checkboxes whose labels measure 70x44 and 57x44, four are wa-input's inner input at 204x**42**, which is the control measured *inside* its own 1px border (part=base is 238x44), and one is the skip link, which #186 already ruled out as keyboard-only. The e2e suite passes, top-bar-fit and header-action-overflow included -- but that is **chromium**, which is half an answer, and saying so is the whole of what #195's second commit was about. What can be argued rather than run: library-filter is the only thing here in a container that measures itself, and its width did not change. The fit measures inline size. Two page-header screenshots are refreshed because they are this issue's own debris -- #195's taller sort control, merged last session, with its references never re-recorded. app-sidebar's and now-playing's are deliberately left: they are unrelated drift, and blessing an unrelated screenshot is how the sidebar reference came to still list a destination #27 retired. That is #196. Closes #186
This commit is contained in:
@@ -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 <input> 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 <label>,
|
||||
which measured **34x19**. That is smaller than anything in
|
||||
#186's original table and it is absent from it for exactly
|
||||
that reason.
|
||||
|
||||
A 44px pill is not what a switch should look like, so the
|
||||
box is 44px and the paint is not: .toggle-slider is a
|
||||
2.5em x 1.4em child centred in it rather than an absolute
|
||||
fill. The negative inline margins hand the extra width back
|
||||
to the layout, so the pill stays flush with the right edge
|
||||
of the inputs in the rows above it -- the header pass's
|
||||
shape, used here for alignment rather than for a fit. */
|
||||
.toggle-switch {
|
||||
position: relative;
|
||||
width: 2.5em;
|
||||
height: 1.4em;
|
||||
display: grid;
|
||||
place-items: center;
|
||||
inline-size: 44px;
|
||||
block-size: 44px;
|
||||
margin-inline: calc((2.5em - 44px) / 2);
|
||||
}
|
||||
|
||||
.toggle-switch input {
|
||||
@@ -202,9 +244,10 @@ export class ConfigField extends LitElement {
|
||||
}
|
||||
|
||||
.toggle-slider {
|
||||
position: absolute;
|
||||
position: relative;
|
||||
cursor: pointer;
|
||||
inset: 0;
|
||||
inline-size: 2.5em;
|
||||
block-size: 1.4em;
|
||||
background: var(--yj-bg-overlay, #495057);
|
||||
border-radius: 1em;
|
||||
transition: background 0.2s;
|
||||
|
||||
@@ -176,6 +176,13 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) {
|
||||
cursor: pointer;
|
||||
transition: background-color 0.15s ease;
|
||||
white-space: nowrap;
|
||||
/* The app's 44px touch floor (#56, #186), stated once for
|
||||
all 41 buttons this page renders rather than per class.
|
||||
Height is free here: Settings has no overflow fit, so
|
||||
the header's "only width is contested" rule does not
|
||||
bind, and the two classes that need more than a height
|
||||
say so below. */
|
||||
min-block-size: 44px;
|
||||
}
|
||||
|
||||
button:disabled {
|
||||
@@ -509,11 +516,24 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) {
|
||||
margin: 0;
|
||||
}
|
||||
|
||||
/* The two column lists are the densest thing in the app, and
|
||||
the density argument is why they are shaped the way they
|
||||
are rather than simply grown (#186).
|
||||
|
||||
Measured on the reference device: the row was already
|
||||
335x36 -- it is the controls *inside* it that were 16x16 and
|
||||
**16x14**, the smallest anywhere in this app, 36 of them.
|
||||
So the fix grows the controls into the row they already
|
||||
occupy and only takes the row from 36 to 44, which over the
|
||||
two lists (10 and 19 items) is 232px of extra scroll on a
|
||||
439px screen. Growing each control to its own 44px row
|
||||
instead would have cost four screens. */
|
||||
.column-item {
|
||||
display: flex;
|
||||
align-items: center;
|
||||
align-items: stretch;
|
||||
gap: 0.5em;
|
||||
padding: 0.5em 0.75em;
|
||||
padding: 0 0.75em;
|
||||
min-block-size: 44px;
|
||||
border-bottom: 1px solid
|
||||
var(--yj-border-subtle, #333);
|
||||
font-size: 0.85em;
|
||||
@@ -531,8 +551,19 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) {
|
||||
color: var(--yj-text-tertiary, #888);
|
||||
}
|
||||
|
||||
/* A native checkbox cannot grow its hit area without growing
|
||||
its paint, and a 44px checkbox is not what anyone wants. So
|
||||
the target is the label instead: .column-label is a real
|
||||
<label for> now, which makes the column's *name* the thing
|
||||
you tap -- ~250x44 rather than 16x16.
|
||||
|
||||
That is the argument config-field already makes one file
|
||||
over for its own labels: "a real label association also
|
||||
makes the label text a click target for the control, which
|
||||
is behaviour, not annotation". Here it is the whole fix. */
|
||||
.column-toggle {
|
||||
cursor: pointer;
|
||||
align-self: center;
|
||||
accent-color: var(
|
||||
--yj-accent,
|
||||
#ffd43b
|
||||
@@ -541,20 +572,32 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) {
|
||||
|
||||
.column-label {
|
||||
flex: 1;
|
||||
display: flex;
|
||||
align-items: center;
|
||||
cursor: pointer;
|
||||
min-block-size: 44px;
|
||||
}
|
||||
|
||||
.view-note {
|
||||
color: var(--yj-text-tertiary, #888);
|
||||
font-size: var(--yj-font-size-sm, 0.85rem);
|
||||
margin-left: auto;
|
||||
/* The row stretches its children so the label can be a
|
||||
full-height target; this is text, not a target. */
|
||||
align-self: center;
|
||||
}
|
||||
|
||||
.column-arrows {
|
||||
display: flex;
|
||||
align-items: stretch;
|
||||
gap: 0.15em;
|
||||
margin-left: auto;
|
||||
}
|
||||
|
||||
/* 16x14 before this, and they carry background: none and a
|
||||
transparent border -- so padding out to 44px grows the
|
||||
target and changes nothing anyone can see until hover,
|
||||
which is precisely what #186's Direction asks for. */
|
||||
.column-arrow-btn {
|
||||
background: none;
|
||||
border: 1px solid transparent;
|
||||
@@ -564,6 +607,8 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) {
|
||||
font-size: 0.65em;
|
||||
line-height: 1;
|
||||
padding: 0.2em 0.35em;
|
||||
min-inline-size: 44px;
|
||||
min-block-size: 44px;
|
||||
transition:
|
||||
color 0.15s,
|
||||
border-color 0.15s;
|
||||
@@ -711,6 +756,11 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) {
|
||||
padding: 0.2em 0.4em;
|
||||
letter-spacing: 2px;
|
||||
border-radius: 4px;
|
||||
/* Square, so it needs the width too -- the shared rule
|
||||
above only gives it a height. It was 31x31, and it is
|
||||
the only route to "Remove library", which is the case
|
||||
#55 settled one component over: the way out is 44px. */
|
||||
min-inline-size: 44px;
|
||||
}
|
||||
|
||||
.overflow-btn:hover {
|
||||
@@ -2003,6 +2053,7 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) {
|
||||
class="column-item ${checked ? 'enabled' : 'disabled'}"
|
||||
>
|
||||
<input
|
||||
id="view-${v.id}"
|
||||
type="checkbox"
|
||||
class="column-toggle"
|
||||
aria-label="Show ${v.label} in the navigation"
|
||||
@@ -2014,9 +2065,9 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) {
|
||||
(e.target as HTMLInputElement).checked,
|
||||
)}
|
||||
/>
|
||||
<span class="column-label">
|
||||
<label class="column-label" for="view-${v.id}">
|
||||
${v.label}
|
||||
</span>
|
||||
</label>
|
||||
${note
|
||||
? html`<span class="view-note">${note}</span>`
|
||||
: nothing}
|
||||
@@ -2212,6 +2263,7 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) {
|
||||
class="column-item ${checked ? 'enabled' : 'disabled'}"
|
||||
>
|
||||
<input
|
||||
id="column-${id}"
|
||||
type="checkbox"
|
||||
class="column-toggle"
|
||||
aria-label="Show the ${columnLabel} column"
|
||||
@@ -2222,11 +2274,12 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) {
|
||||
id,
|
||||
)}
|
||||
/>
|
||||
<span
|
||||
<label
|
||||
class="column-label"
|
||||
for="column-${id}"
|
||||
>
|
||||
${columnLabel}
|
||||
</span>
|
||||
</label>
|
||||
<span
|
||||
class="column-arrows"
|
||||
>
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
`,
|
||||
];
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
`;
|
||||
BIN
Binary file not shown.
|
Before Width: | Height: | Size: 6.4 KiB After Width: | Height: | Size: 6.5 KiB |
BIN
Binary file not shown.
|
Before Width: | Height: | Size: 4.2 KiB After Width: | Height: | Size: 4.3 KiB |
@@ -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 `<input>` is
|
||||
* `opacity: 0; width: 0; height: 0` so the thing a finger hits is the
|
||||
* `<label>` around it (34x19, smaller than anything in either of the
|
||||
* issue's tables), and `shortcut-capture`'s reset button, which renders
|
||||
* only for a shortcut somebody has already rebound. Both are asserted
|
||||
* by name below.
|
||||
*/
|
||||
import { beforeEach, describe, expect, it } from 'vitest';
|
||||
|
||||
import '@components/config-page/config-field';
|
||||
import '@components/config-page/config-page';
|
||||
import '@components/config-page/download-clients';
|
||||
import '@components/config-page/shortcut-capture';
|
||||
import '@components/library-filter/library-filter';
|
||||
|
||||
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;
|
||||
|
||||
/**
|
||||
* Everything a finger can hit, through every shadow root under `root`.
|
||||
*
|
||||
* It descends rather than querying one root because Settings is a tree
|
||||
* of components — `config-page` renders `config-section`s holding
|
||||
* `config-field`s and `shortcut-capture`s — and the defect was
|
||||
* distributed across all of them. This is the device sweep, run here.
|
||||
*/
|
||||
function controlsUnder(root: Document | ShadowRoot | Element): { name: string; el: HTMLElement }[] {
|
||||
const SELECTOR = 'button, select, input, [role="button"], [role="tab"], [role="switch"]';
|
||||
const found: { name: string; el: HTMLElement }[] = [];
|
||||
const seen = new Set<Element>();
|
||||
|
||||
const walk = (node: ParentNode, depth: number): void => {
|
||||
if (depth > 20) return;
|
||||
|
||||
for (const el of Array.from(node.querySelectorAll('*'))) {
|
||||
if (el.matches(SELECTOR) && !seen.has(el)) {
|
||||
seen.add(el);
|
||||
|
||||
const box = el.getBoundingClientRect();
|
||||
const style = getComputedStyle(el);
|
||||
const rendered =
|
||||
(box.width > 0 || box.height > 0) &&
|
||||
style.visibility !== 'hidden' &&
|
||||
style.display !== 'none';
|
||||
|
||||
if (rendered) {
|
||||
const host = (el.getRootNode() as ShadowRoot).host;
|
||||
|
||||
found.push({
|
||||
name: `${host ? host.tagName.toLowerCase() : 'root'} ${
|
||||
(typeof el.className === 'string' && el.className) || el.tagName.toLowerCase()
|
||||
}`,
|
||||
el: el as HTMLElement,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
if (el.shadowRoot) walk(el.shadowRoot, depth + 1);
|
||||
}
|
||||
};
|
||||
|
||||
walk(root as ParentNode, 0);
|
||||
|
||||
return found;
|
||||
}
|
||||
|
||||
/**
|
||||
* What a finger actually hits for `el`.
|
||||
*
|
||||
* For everything in this app that is one element, that is the element.
|
||||
* A native checkbox is the exception and is why this function exists:
|
||||
* it cannot grow its hit area without growing its paint, and a 44px
|
||||
* checkbox is not what anyone wants — so a checkbox that has a label
|
||||
* is targeted *through* the label, which is the fix the column lists
|
||||
* and the download formats both use.
|
||||
*
|
||||
* The fallback is the checkbox itself, deliberately: a checkbox with
|
||||
* no label is a 16px target and this must still say so.
|
||||
*/
|
||||
function hitTarget(el: HTMLElement): HTMLElement {
|
||||
const input = el as HTMLInputElement;
|
||||
|
||||
if (input.type !== 'checkbox' && input.type !== 'radio') return el;
|
||||
|
||||
const wrapping = el.closest('label');
|
||||
const root = el.getRootNode() as ShadowRoot | Document;
|
||||
const associated = input.id
|
||||
? root.querySelector<HTMLLabelElement>(`label[for="${CSS.escape(input.id)}"]`)
|
||||
: null;
|
||||
|
||||
return wrapping ?? associated ?? el;
|
||||
}
|
||||
|
||||
/** The ones that miss the floor, reported with the numbers. */
|
||||
function tooSmall(controls: { name: string; el: HTMLElement }[]): string[] {
|
||||
return controls
|
||||
.map(({ name, el }) => {
|
||||
const box = hitTarget(el).getBoundingClientRect();
|
||||
|
||||
return { name, w: Math.round(box.width), h: Math.round(box.height) };
|
||||
})
|
||||
.filter((c) => c.w < FLOOR || c.h < FLOOR)
|
||||
.map((c) => `${c.name} ${c.w}x${c.h}`);
|
||||
}
|
||||
|
||||
/**
|
||||
* Open every disclosure under `host`, so the sweep can see the page.
|
||||
*
|
||||
* A collapsed `config-section` renders its body with `hidden`, so its
|
||||
* controls measure 0x0 — which is exactly why the issue's first table
|
||||
* lists seven Settings controls and the real count is 120.
|
||||
*/
|
||||
async function expandEverySection(host: HTMLElement & { updateComplete: Promise<unknown> }) {
|
||||
const sections = shadowAll<HTMLElement>(host, 'config-section');
|
||||
|
||||
expect(sections.length, 'the page renders disclosures to open').toBeGreaterThan(0);
|
||||
|
||||
for (const section of sections) {
|
||||
section.shadowRoot
|
||||
?.querySelector<HTMLButtonElement>('button[aria-expanded="false"]')
|
||||
?.click();
|
||||
}
|
||||
|
||||
await flush();
|
||||
await host.updateComplete;
|
||||
|
||||
for (const section of sections) {
|
||||
await (section as HTMLElement & { updateComplete?: Promise<unknown> }).updateComplete;
|
||||
}
|
||||
|
||||
return sections;
|
||||
}
|
||||
|
||||
/** A control's own box, named so a failure says which and how small. */
|
||||
function boxOf(el: Element | null | undefined): string {
|
||||
if (!el) return 'missing';
|
||||
|
||||
const box = el.getBoundingClientRect();
|
||||
|
||||
return `${Math.round(box.width)}x${Math.round(box.height)}`;
|
||||
}
|
||||
|
||||
function meetsFloor(el: Element | null | undefined): boolean {
|
||||
if (!el) return false;
|
||||
|
||||
const box = el.getBoundingClientRect();
|
||||
|
||||
return Math.round(box.width) >= FLOOR && Math.round(box.height) >= FLOOR;
|
||||
}
|
||||
|
||||
describe('a config field is the shape every Settings row uses', () => {
|
||||
it.each(['text', 'number', 'select', 'directory', 'color'] as const)(
|
||||
'a %s field meets the floor',
|
||||
async (type) => {
|
||||
const el = await fixture('config-field', {
|
||||
schema: {
|
||||
key: 'k',
|
||||
label: 'Music folder',
|
||||
type,
|
||||
options: [{ value: 'dark', label: 'Dark' }],
|
||||
},
|
||||
value: type === 'color' ? '#ffd43b' : '',
|
||||
});
|
||||
|
||||
const controls = controlsUnder(el.shadowRoot!);
|
||||
|
||||
// A sweep that found nothing passes vacuously.
|
||||
expect(controls.length).toBeGreaterThan(0);
|
||||
expect(tooSmall(controls)).toEqual([]);
|
||||
},
|
||||
);
|
||||
|
||||
it('grows the toggle, which no sweep of inputs can see', async () => {
|
||||
// The `<input>` is opacity: 0; width: 0; height: 0, so the walk
|
||||
// above skips it as a zero-sized node -- and the thing a finger
|
||||
// hits is the styling <label> around it, which measured 34x19 on
|
||||
// the device. It is absent from #186's tables for exactly that
|
||||
// reason, and it is smaller than everything in them.
|
||||
const el = await fixture('config-field', {
|
||||
schema: { key: 'x', label: 'Scan on startup', type: 'toggle' },
|
||||
value: true,
|
||||
});
|
||||
|
||||
const target = shadow(el, '.toggle-switch');
|
||||
|
||||
expect(boxOf(target)).toBe('44x44');
|
||||
});
|
||||
|
||||
it('keeps the toggle painted at its old size, in its old place', async () => {
|
||||
// A 44px pill is not what a switch should look like. The box is
|
||||
// 44px and the paint is not: the slider is a child centred in it,
|
||||
// and negative inline margins hand the extra width back so the
|
||||
// pill stays flush with the inputs in the rows above.
|
||||
const el = await fixture('config-field', {
|
||||
schema: { key: 'x', label: 'Scan on startup', type: 'toggle' },
|
||||
value: true,
|
||||
});
|
||||
|
||||
const target = shadow(el, '.toggle-switch') as HTMLElement;
|
||||
const slider = shadow(el, '.toggle-slider');
|
||||
|
||||
expect(slider!.getBoundingClientRect().height).toBeLessThan(FLOOR);
|
||||
|
||||
const style = getComputedStyle(target);
|
||||
const handedBack =
|
||||
parseFloat(style.marginInlineStart) + parseFloat(style.marginInlineEnd);
|
||||
|
||||
expect(handedBack).toBeLessThan(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe('the shortcut editor', () => {
|
||||
it('meets the floor', async () => {
|
||||
const el = await fixture('shortcut-capture', {
|
||||
action: 'play.toggle',
|
||||
label: 'Play/pause',
|
||||
currentKey: 'Space',
|
||||
defaultKey: 'Space',
|
||||
});
|
||||
|
||||
expect(meetsFloor(shadow(el, 'button'))).toBe(true);
|
||||
});
|
||||
|
||||
it('grows the reset button, which only a rebound shortcut renders', async () => {
|
||||
// Not in either of #186's tables, and it cannot be: a sweep of a
|
||||
// freshly-installed app never sees it. It appears the moment
|
||||
// anybody uses the feature.
|
||||
const el = await fixture('shortcut-capture', {
|
||||
action: 'play.toggle',
|
||||
label: 'Play/pause',
|
||||
currentKey: 'K',
|
||||
defaultKey: 'Space',
|
||||
});
|
||||
|
||||
const reset = shadow(el, '.reset-btn');
|
||||
|
||||
expect(reset, 'a rebound shortcut renders a reset button').toBeTruthy();
|
||||
expect(meetsFloor(reset)).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('the library filter', () => {
|
||||
it('meets the floor in both of its placements', async () => {
|
||||
// One component, two mount points since #57 -- the desktop top bar
|
||||
// and Settings -> Libraries -- so it reaches the floor once.
|
||||
const el = await fixture('library-filter');
|
||||
|
||||
const select = shadow(el, 'select');
|
||||
|
||||
expect(select, 'the filter renders a select').toBeTruthy();
|
||||
expect(Math.round(select!.getBoundingClientRect().height)).toBeGreaterThanOrEqual(FLOOR);
|
||||
});
|
||||
});
|
||||
|
||||
describe('download clients', () => {
|
||||
beforeEach(() => {
|
||||
stub('download.Service.ListProviders', []);
|
||||
stub('download.Service.ProviderKinds', []);
|
||||
stub('config.Config.GetDownloadPreferences', {});
|
||||
});
|
||||
|
||||
it('gives Web Awesome form controls the floor through the library API', async () => {
|
||||
// wa-input's control is inside somebody else's shadow root, so the
|
||||
// height comes from --wa-form-control-height rather than from a
|
||||
// rule of ours reaching in. A custom property inherits through a
|
||||
// shadow boundary, which is what makes a :host declaration reach
|
||||
// it -- and what makes it measurable here.
|
||||
const el = await fixture('download-clients');
|
||||
|
||||
await flush();
|
||||
await expandEverySection(el);
|
||||
|
||||
expect(getComputedStyle(el).getPropertyValue('--wa-form-control-height').trim()).toBe(
|
||||
'44px',
|
||||
);
|
||||
|
||||
// And the outcome, not only the mechanism. A sweep of `input`
|
||||
// reports a wa-input at 204x**42** even when this is right,
|
||||
// because the inner input sits *inside* the control's own 1px
|
||||
// border -- Web Awesome sizes it
|
||||
// `calc(--wa-form-control-height - border-width * 2)`. What a
|
||||
// finger hits is `part=base`, measured at 238x44 on the device.
|
||||
//
|
||||
// This reaches into another library's shadow root, which
|
||||
// `name-dialog.ts` only permits where the failure is bounded. It
|
||||
// is bounded the other way here: this is a test, so a renamed
|
||||
// part fails loudly rather than silently passing, which is the
|
||||
// direction that costs nobody a device session.
|
||||
const input = shadowAll<HTMLElement>(el, 'wa-input').find(
|
||||
(w) => w.getBoundingClientRect().height > 0,
|
||||
);
|
||||
|
||||
expect(input, 'the add form renders a wa-input').toBeTruthy();
|
||||
|
||||
const base = input!.shadowRoot?.querySelector('[part~="base"]');
|
||||
|
||||
expect(base, 'wa-input still calls its control box "base"').toBeTruthy();
|
||||
expect(Math.round(base!.getBoundingClientRect().height)).toBeGreaterThanOrEqual(FLOOR);
|
||||
});
|
||||
|
||||
it('makes each allowed-format checkbox label a target', async () => {
|
||||
const el = await fixture('download-clients');
|
||||
|
||||
await flush();
|
||||
await el.updateComplete;
|
||||
|
||||
// Every section starts collapsed, and a collapsed body is `hidden`
|
||||
// — so its controls measure 0x0 and a sweep of an unexpanded page
|
||||
// reports them all as fine. That is how the issue's first table
|
||||
// came to list seven Settings controls when there are 120.
|
||||
await expandEverySection(el);
|
||||
|
||||
const options = shadowAll<HTMLElement>(el, '.format-option');
|
||||
|
||||
expect(options.length).toBeGreaterThan(0);
|
||||
|
||||
const short = options
|
||||
.map((o) => ({ label: o.textContent?.trim(), h: Math.round(o.getBoundingClientRect().height) }))
|
||||
.filter((o) => o.h < FLOOR);
|
||||
|
||||
expect(short).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('the whole Settings page', () => {
|
||||
beforeEach(() => {
|
||||
for (const path of [
|
||||
'library.Library.GetAllLibrariesWithTrackCounts',
|
||||
'jobs.Service.GetJobs',
|
||||
'download.Service.ListProviders',
|
||||
'download.Service.ProviderKinds',
|
||||
]) {
|
||||
stub(path, []);
|
||||
}
|
||||
|
||||
stub('config.Config.GetShortcuts', {});
|
||||
stub('config.Config.GetDownloadPreferences', {});
|
||||
stub('config.Config.GetThemeAccentColor', '#ffd43b');
|
||||
stub('config.Config.GetThemeBackgroundShade', 'dark');
|
||||
});
|
||||
|
||||
it('has no control under the floor with every section expanded', async () => {
|
||||
// The device sweep, run here: eleven collapsed sections is what
|
||||
// made the first table look like seven controls. The density is
|
||||
// behind the disclosures.
|
||||
const el = await fixture('config-page');
|
||||
|
||||
await flush();
|
||||
await el.updateComplete;
|
||||
await expandEverySection(el);
|
||||
|
||||
const controls = controlsUnder(el.shadowRoot!);
|
||||
|
||||
expect(controls.length).toBeGreaterThan(0);
|
||||
expect(tooSmall(controls)).toEqual([]);
|
||||
});
|
||||
|
||||
it('makes a column row a target by naming it, not by growing the checkbox', async () => {
|
||||
// A native checkbox cannot grow its hit area without growing its
|
||||
// paint. The label is the target instead -- which is also the
|
||||
// argument config-field already makes for its own labels, and it
|
||||
// is behaviour rather than annotation: the column's name is now a
|
||||
// click target for its checkbox.
|
||||
const el = await fixture('config-page');
|
||||
|
||||
await flush();
|
||||
await el.updateComplete;
|
||||
await expandEverySection(el);
|
||||
|
||||
const labels = shadowAll<HTMLLabelElement>(el, 'label.column-label');
|
||||
|
||||
expect(labels.length).toBeGreaterThan(0);
|
||||
|
||||
for (const label of labels) {
|
||||
const target = label.htmlFor
|
||||
? el.shadowRoot!.getElementById(label.htmlFor)
|
||||
: null;
|
||||
|
||||
expect(
|
||||
(target as HTMLInputElement | null)?.type,
|
||||
`${label.textContent?.trim()} names its checkbox`,
|
||||
).toBe('checkbox');
|
||||
expect(
|
||||
Math.round(label.getBoundingClientRect().height),
|
||||
`${label.textContent?.trim()} is a target`,
|
||||
).toBeGreaterThanOrEqual(FLOOR);
|
||||
}
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user